improve objectstore rmdir handling

remove folder entries as they are cleared instead of in one go afterwards

otherwise they stick around if some of the child entries can't be removed

Signed-off-by: Robin Appelman <robin@icewind.nl>

1 files changed, 50 insertions, 36 deletions

diff --git a/lib/private/Files/ObjectStore/ObjectStoreStorage.php b/lib/private/Files/ObjectStore/ObjectStoreStorage.php
index 4ca00cf6a16..8ca5cf53d16 100644
--- a/lib/private/Files/ObjectStore/ObjectStoreStorage.php
+++ b/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');
 		}