Merge pull request #37429 from nextcloud/object-store-rmdir

improve objectstore rmdir handling

lib/private/Files/ObjectStore/ObjectStoreStorage.php

@@ -27,6 +27,7 @@
  * along with this program. If not, see <http://www.gnu.org/licenses/>
  *
  */
+
 namespace OC\Files\ObjectStore;
 
 use Aws\S3\Exception\S3Exception;
@@ -177,63 +178,65 @@ class ObjectStoreStorage extends \OC\Files\Storage\Common implements IChunkedFil
 
 	public function rmdir($path) {
 		$path = $this->normalizePath($path);
+		$entry = $this->getCache()->get($path);
 
-		if (!$this->is_dir($path)) {
-			return false;
-		}
-
-		if (!$this->rmObjects($path)) {
+		if (!$entry || $entry->getMimeType() !== ICacheEntry::DIRECTORY_MIMETYPE) {
 			return false;
 		}
 
-		$this->getCache()->remove($path);
-
-		return true;
+		return $this->rmObjects($entry);
 	}
 
-	private function rmObjects($path) {
-		$children = $this->getCache()->getFolderContents($path);
+	private function rmObjects(ICacheEntry $entry): bool {
+		$children = $this->getCache()->getFolderContentsById($entry->getId());
 		foreach ($children as $child) {
-			if ($child['mimetype'] === 'httpd/unix-directory') {
-				if (!$this->rmObjects($child['path'])) {
+			if ($child->getMimeType() === ICacheEntry::DIRECTORY_MIMETYPE) {
+				if (!$this->rmObjects($child)) {
 					return false;
 				}
 			} else {
-				if (!$this->unlink($child['path'])) {
+				if (!$this->rmObject($child)) {
 					return false;
 				}
 			}
 		}
 
+		$this->getCache()->remove($entry->getPath());
+
 		return true;
 	}
 
 	public function unlink($path) {
 		$path = $this->normalizePath($path);
-		$stat = $this->stat($path);
+		$entry = $this->getCache()->get($path);
 
-		if ($stat && isset($stat['fileid'])) {
-			if ($stat['mimetype'] === 'httpd/unix-directory') {
-				return $this->rmdir($path);
-			}
-			try {
-				$this->objectStore->deleteObject($this->getURN($stat['fileid']));
-			} catch (\Exception $ex) {
-				if ($ex->getCode() !== 404) {
-					$this->logger->logException($ex, [
-						'app' => 'objectstore',
-						'message' => 'Could not delete object ' . $this->getURN($stat['fileid']) . ' for ' . $path,
-					]);
-					return false;
-				}
-				//removing from cache is ok as it does not exist in the objectstore anyway
+		if ($entry instanceof ICacheEntry) {
+			if ($entry->getMimeType() === ICacheEntry::DIRECTORY_MIMETYPE) {
+				return $this->rmObjects($entry);
+			} else {
+				return $this->rmObject($entry);
 			}
-			$this->getCache()->remove($path);
-			return true;
 		}
 		return false;
 	}
 
+	public function rmObject(ICacheEntry $entry): bool {
+		try {
+			$this->objectStore->deleteObject($this->getURN($entry->getId()));
+		} catch (\Exception $ex) {
+			if ($ex->getCode() !== 404) {
+				$this->logger->logException($ex, [
+					'app' => 'objectstore',
+					'message' => 'Could not delete object ' . $this->getURN($entry->getId()) . ' for ' . $entry->getPath(),
+				]);
+				return false;
+			}
+			//removing from cache is ok as it does not exist in the objectstore anyway
+		}
+		$this->getCache()->remove($entry->getPath());
+		return true;
+	}
+
 	public function stat($path) {
 		$path = $this->normalizePath($path);
 		$cacheEntry = $this->getCache()->get($path);
@@ -557,7 +560,12 @@ class ObjectStoreStorage extends \OC\Files\Storage\Common implements IChunkedFil
 		return $this->objectStore;
 	}
 
-	public function copyFromStorage(IStorage $sourceStorage, $sourceInternalPath, $targetInternalPath, $preserveMtime = false) {
+	public function copyFromStorage(
+		IStorage $sourceStorage,
+		$sourceInternalPath,
+		$targetInternalPath,
+		$preserveMtime = false
+	) {
 		if ($sourceStorage->instanceOfStorage(ObjectStoreStorage::class)) {
 			/** @var ObjectStoreStorage $sourceStorage */
 			if ($sourceStorage->getObjectStore()->getStorageId() === $this->getObjectStore()->getStorageId()) {
@@ -645,7 +653,13 @@ class ObjectStoreStorage extends \OC\Files\Storage\Common implements IChunkedFil
 	 *
 	 * @throws GenericFileException
 	 */
-	public function putChunkedWritePart(string $targetPath, string $writeToken, string $chunkId, $data, $size = null): ?array {
+	public function putChunkedWritePart(
+		string $targetPath,
+		string $writeToken,
+		string $chunkId,
+		$data,
+		$size = null
+	): ?array {
 		if (!$this->objectStore instanceof IObjectStoreMultiPartUpload) {
 			throw new GenericFileException('Object store does not support multipart upload');
 		}
@@ -656,7 +670,7 @@ class ObjectStoreStorage extends \OC\Files\Storage\Common implements IChunkedFil
 
 		$parts[$chunkId] = [
 			'PartNumber' => $chunkId,
-			'ETag' => trim($result->get('ETag'), '"')
+			'ETag' => trim($result->get('ETag'), '"'),
 		];
 		return $parts[$chunkId];
 	}
@@ -680,11 +694,11 @@ class ObjectStoreStorage extends \OC\Files\Storage\Common implements IChunkedFil
 				$stat['mimetype'] = $this->getMimeType($targetPath);
 				$this->getCache()->update($stat['fileid'], $stat);
 			}
-		} catch (S3MultipartUploadException | S3Exception $e) {
+		} catch (S3MultipartUploadException|S3Exception $e) {
 			$this->objectStore->abortMultipartUpload($urn, $writeToken);
 			$this->logger->logException($e, [
 				'app' => 'objectstore',
-				'message' => 'Could not compete multipart upload ' . $urn. ' with uploadId ' . $writeToken
+				'message' => 'Could not compete multipart upload ' . $urn . ' with uploadId ' . $writeToken,
 			]);
 			throw new GenericFileException('Could not write chunked file');
 		}