From d63fa2d9914b1629ce5f5cba0950ac5e27cebc00 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 27 Mar 2023 17:44:33 +0200 Subject: 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 --- .../Files/ObjectStore/ObjectStoreStorage.php | 86 +++++++++++++--------- 1 file 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 * */ + 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'); } -- cgit v1.2.3