From c47d1ad5d766fb63ab430e908d9dea589dd04a17 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Fri, 1 Aug 2025 12:47:57 +0200 Subject: [PATCH 1/6] fix(FsEventService#onDelete): Schedule deletion in chunks see #158 Signed-off-by: Marcel Klehr --- lib/Service/FsEventService.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/Service/FsEventService.php b/lib/Service/FsEventService.php index d4c7653d..af391523 100644 --- a/lib/Service/FsEventService.php +++ b/lib/Service/FsEventService.php @@ -76,18 +76,19 @@ public function onDelete(Node $node, bool $recurse = true): void { $files = [$node]; } + $fileRefs = []; foreach ($files as $file) { if (!$this->allowedMimeType($file)) { continue; } try { - $fileRef = ProviderConfigService::getSourceId($file->getId()); - $this->actionService->deleteSources($fileRef); + $fileRefs[] = ProviderConfigService::getSourceId($file->getId()); } catch (InvalidPathException|NotFoundException $e) { $this->logger->warning($e->getMessage(), ['exception' => $e]); } } + $this->actionService->deleteSources(...$fileRefs); } public function onInsert(Node $node, bool $recurse = true, bool $update = false): void { From 60638d278da8ed4b08e87daa63ef692eb3238441 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Fri, 1 Aug 2025 13:10:47 +0200 Subject: [PATCH 2/6] fix(StorageService#getAllFilesInFolder): Only return fileIds Do not return full Node instances of each file Signed-off-by: Marcel Klehr --- lib/Listener/ShareListener.php | 11 +++---- lib/Service/FsEventService.php | 55 ++++++++++------------------------ lib/Service/StorageService.php | 6 +--- 3 files changed, 21 insertions(+), 51 deletions(-) diff --git a/lib/Listener/ShareListener.php b/lib/Listener/ShareListener.php index 8a770148..c21335a8 100644 --- a/lib/Listener/ShareListener.php +++ b/lib/Listener/ShareListener.php @@ -65,14 +65,11 @@ public function handle(Event $event): void { if ($node->getType() === FileInfo::TYPE_FOLDER) { $files = $this->storageService->getAllFilesInFolder($node); - foreach ($files as $file) { - if (!$file instanceof File) { - continue; - } + foreach ($files as $fileId) { $this->actionService->updateAccess( UpdateAccessOp::ALLOW, $userIds, - ProviderConfigService::getSourceId($file->getId()), + ProviderConfigService::getSourceId($fileId), ); } } else { @@ -129,10 +126,10 @@ public function handle(Event $event): void { if ($node instanceof Folder) { $files = $this->storageService->getAllFilesInFolder($node); - foreach ($files as $file) { + foreach ($files as $fileId) { $this->actionService->updateAccessDeclSource( $userIds, - ProviderConfigService::getSourceId($file->getId()), + ProviderConfigService::getSourceId($fileId), ); } } else { diff --git a/lib/Service/FsEventService.php b/lib/Service/FsEventService.php index af391523..c2180876 100644 --- a/lib/Service/FsEventService.php +++ b/lib/Service/FsEventService.php @@ -11,8 +11,10 @@ use OCA\ContextChat\Db\QueueFile; use OCA\ContextChat\Logger; use OCP\DB\Exception; +use OCP\Files\File; use OCP\Files\Folder; use OCP\Files\InvalidPathException; +use OCP\Files\IRootFolder; use OCP\Files\Node; use OCP\Files\NotFoundException; @@ -24,6 +26,7 @@ public function __construct( private ActionScheduler $actionService, private StorageService $storageService, private \OCP\Share\IManager $shareManager, + private IRootFolder $rootFolder, ) { } @@ -36,33 +39,14 @@ public function onAccessUpdateDecl(Node $node, bool $recurse = true): void { $files = $this->storageService->getAllFilesInFolder($node); } else { - $files = [$node]; + $files = [$node->getId()]; } - foreach ($files as $file) { - if (!$this->allowedMimeType($file)) { - continue; - } - try { - $fileRef = ProviderConfigService::getSourceId($file->getId()); - $fileUserIds = $this->storageService->getUsersForFileId($file->getId()); - - if (class_exists('OCP\Files\Config\Event\UserMountAddedEvent')) { - $userIds = $fileUserIds; - } else { - // todo: Remove this once we no longer support Nextcloud 31 - $shareAccessList = $this->shareManager->getAccessList($file, true, true); - /** @var string[] $shareUserIds */ - $shareUserIds = array_keys($shareAccessList['users']); - $userIds = array_values(array_unique(array_merge($shareUserIds, $fileUserIds))); - } - - $this->actionService->updateAccessDeclSource($userIds, $fileRef); - } catch (InvalidPathException|NotFoundException $e) { - $this->logger->warning('Cannot get file id for declarative access update:' . $e->getMessage(), [ - 'exception' => $e - ]); - } + foreach ($files as $fileId) { + $fileRef = ProviderConfigService::getSourceId($fileId); + $fileUserIds = $this->storageService->getUsersForFileId($fileId); + + $this->actionService->updateAccessDeclSource($fileUserIds, $fileRef); } } @@ -73,17 +57,13 @@ public function onDelete(Node $node, bool $recurse = true): void { } $files = $this->storageService->getAllFilesInFolder($node); } else { - $files = [$node]; + $files = [$node->getId()]; } $fileRefs = []; - foreach ($files as $file) { - if (!$this->allowedMimeType($file)) { - continue; - } - + foreach ($files as $fileId) { try { - $fileRefs[] = ProviderConfigService::getSourceId($file->getId()); + $fileRefs[] = ProviderConfigService::getSourceId($fileId); } catch (InvalidPathException|NotFoundException $e) { $this->logger->warning($e->getMessage(), ['exception' => $e]); } @@ -101,17 +81,14 @@ public function onInsert(Node $node, bool $recurse = true, bool $update = false) } $files = $this->storageService->getAllFilesInFolder($node); } else { - $files = [$node]; + $files = [$node->getId()]; } - foreach ($files as $file) { - if (!$this->allowedMimeType($file)) { + foreach ($files as $fileId) { + $file = current($this->rootFolder->getById($fileId)); + if (!$file instanceof File) { continue; } - if (!$this->allowedPath($file)) { - continue; - } - $queueFile = new QueueFile(); if ($file->getMountPoint()->getNumericStorageId() === null) { return; diff --git a/lib/Service/StorageService.php b/lib/Service/StorageService.php index f295c812..50e50aa1 100644 --- a/lib/Service/StorageService.php +++ b/lib/Service/StorageService.php @@ -338,11 +338,7 @@ public function getAllFilesInFolder(Node $node): \Generator { $filesGen = $this->getFilesInMount($mount->getNumericStorageId(), $node->getId(), 0, 0); foreach ($filesGen as $fileId) { - $node = current($this->rootFolder->getById($fileId)); - if (!$node instanceof File) { - continue; - } - yield $node; + yield $fileId; } } From a666b5c4583bf57d9c702897492bab989adffb8d Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Fri, 1 Aug 2025 13:51:18 +0200 Subject: [PATCH 3/6] fix(FsEventService): Add back allowedPath check for good measure Signed-off-by: Marcel Klehr --- lib/Service/FsEventService.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/Service/FsEventService.php b/lib/Service/FsEventService.php index c2180876..293416be 100644 --- a/lib/Service/FsEventService.php +++ b/lib/Service/FsEventService.php @@ -89,6 +89,9 @@ public function onInsert(Node $node, bool $recurse = true, bool $update = false) if (!$file instanceof File) { continue; } + if (!$this->allowedPath($file)) { + continue; + } $queueFile = new QueueFile(); if ($file->getMountPoint()->getNumericStorageId() === null) { return; From 53277dddb8318dfb47e84b0d49fd7697bc6f0ddf Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Fri, 1 Aug 2025 13:55:20 +0200 Subject: [PATCH 4/6] fix: Catch errors from Node->getId() Signed-off-by: Marcel Klehr --- lib/Listener/ShareListener.php | 8 ++++---- lib/Service/FsEventService.php | 30 +++++++++++++++++++++--------- 2 files changed, 25 insertions(+), 13 deletions(-) diff --git a/lib/Listener/ShareListener.php b/lib/Listener/ShareListener.php index c21335a8..fe16be67 100644 --- a/lib/Listener/ShareListener.php +++ b/lib/Listener/ShareListener.php @@ -64,8 +64,8 @@ public function handle(Event $event): void { } if ($node->getType() === FileInfo::TYPE_FOLDER) { - $files = $this->storageService->getAllFilesInFolder($node); - foreach ($files as $fileId) { + $fileIds = $this->storageService->getAllFilesInFolder($node); + foreach ($fileIds as $fileId) { $this->actionService->updateAccess( UpdateAccessOp::ALLOW, $userIds, @@ -125,8 +125,8 @@ public function handle(Event $event): void { $userIds = array_values(array_unique(array_merge($realFileUserIds, $shareUserIds))); if ($node instanceof Folder) { - $files = $this->storageService->getAllFilesInFolder($node); - foreach ($files as $fileId) { + $fileIds = $this->storageService->getAllFilesInFolder($node); + foreach ($fileIds as $fileId) { $this->actionService->updateAccessDeclSource( $userIds, ProviderConfigService::getSourceId($fileId), diff --git a/lib/Service/FsEventService.php b/lib/Service/FsEventService.php index 293416be..f13a62e0 100644 --- a/lib/Service/FsEventService.php +++ b/lib/Service/FsEventService.php @@ -37,12 +37,16 @@ public function onAccessUpdateDecl(Node $node, bool $recurse = true): void { return; } - $files = $this->storageService->getAllFilesInFolder($node); + $fileIds = $this->storageService->getAllFilesInFolder($node); } else { - $files = [$node->getId()]; + try { + $fileIds = [$node->getId()]; + } catch (InvalidPathException|NotFoundException $e) { + return; + } } - foreach ($files as $fileId) { + foreach ($fileIds as $fileId) { $fileRef = ProviderConfigService::getSourceId($fileId); $fileUserIds = $this->storageService->getUsersForFileId($fileId); @@ -55,13 +59,17 @@ public function onDelete(Node $node, bool $recurse = true): void { if (!$recurse) { return; } - $files = $this->storageService->getAllFilesInFolder($node); + $fileIds = $this->storageService->getAllFilesInFolder($node); } else { - $files = [$node->getId()]; + try { + $fileIds = [$node->getId()]; + } catch (InvalidPathException|NotFoundException $e) { + return; + } } $fileRefs = []; - foreach ($files as $fileId) { + foreach ($fileIds as $fileId) { try { $fileRefs[] = ProviderConfigService::getSourceId($fileId); } catch (InvalidPathException|NotFoundException $e) { @@ -79,12 +87,16 @@ public function onInsert(Node $node, bool $recurse = true, bool $update = false) if (!$recurse) { return; } - $files = $this->storageService->getAllFilesInFolder($node); + $fileIds = $this->storageService->getAllFilesInFolder($node); } else { - $files = [$node->getId()]; + try { + $fileIds = [$node->getId()]; + } catch (InvalidPathException|NotFoundException $e) { + return; + } } - foreach ($files as $fileId) { + foreach ($fileIds as $fileId) { $file = current($this->rootFolder->getById($fileId)); if (!$file instanceof File) { continue; From 124402f5a4d5af4a51a5f0ea055d3ee9d5c951e4 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Fri, 1 Aug 2025 14:09:17 +0200 Subject: [PATCH 5/6] fix(FsEventService): Batch call to ActionScheduler#deleteSources Signed-off-by: Marcel Klehr --- lib/Public/ContentManager.php | 2 +- lib/Service/ActionScheduler.php | 4 ++-- lib/Service/FsEventService.php | 5 ++++- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/Public/ContentManager.php b/lib/Public/ContentManager.php index 9526e018..a70ee432 100644 --- a/lib/Public/ContentManager.php +++ b/lib/Public/ContentManager.php @@ -242,7 +242,7 @@ public function deleteContent(string $appId, string $providerId, array $itemIds) $this->collectAllContentProviders(); $providerKey = ProviderConfigService::getConfigKey($appId, $providerId); - $this->actionService->deleteSources(...array_map(function (string $itemId) use ($providerKey) { + $this->actionService->deleteSources(array_map(function (string $itemId) use ($providerKey) { return ProviderConfigService::getSourceId($itemId, $providerKey); }, $itemIds)); } diff --git a/lib/Service/ActionScheduler.php b/lib/Service/ActionScheduler.php index c99cafdb..dc91ad03 100644 --- a/lib/Service/ActionScheduler.php +++ b/lib/Service/ActionScheduler.php @@ -17,7 +17,7 @@ use OCP\BackgroundJob\IJobList; class ActionScheduler { - private const BATCH_SIZE = 500; + public const BATCH_SIZE = 500; public function __construct( private IJobList $jobList, @@ -46,7 +46,7 @@ private function scheduleAction(string $type, string $payload): void { * @param string[] $sourceIds * @return void */ - public function deleteSources(string ...$sourceIds): void { + public function deleteSources(array $sourceIds): void { // batch sourceIds into self::BATCH_SIZE chunks $batches = array_chunk($sourceIds, self::BATCH_SIZE); diff --git a/lib/Service/FsEventService.php b/lib/Service/FsEventService.php index f13a62e0..fbeba715 100644 --- a/lib/Service/FsEventService.php +++ b/lib/Service/FsEventService.php @@ -76,7 +76,10 @@ public function onDelete(Node $node, bool $recurse = true): void { $this->logger->warning($e->getMessage(), ['exception' => $e]); } } - $this->actionService->deleteSources(...$fileRefs); + $batches = array_chunk($fileRefs, ActionScheduler::BATCH_SIZE); + foreach ($batches as $batch) { + $this->actionService->deleteSources($batch); + } } public function onInsert(Node $node, bool $recurse = true, bool $update = false): void { From 67cabef5226ec756cd80fd3bda8d9b4ccade2b12 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Mon, 4 Aug 2025 07:32:10 +0200 Subject: [PATCH 6/6] fix(FsEventService): Add back mime type check for singular nodes Signed-off-by: Marcel Klehr --- lib/Service/FsEventService.php | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/Service/FsEventService.php b/lib/Service/FsEventService.php index fbeba715..faae1703 100644 --- a/lib/Service/FsEventService.php +++ b/lib/Service/FsEventService.php @@ -25,7 +25,6 @@ public function __construct( private QueueService $queue, private ActionScheduler $actionService, private StorageService $storageService, - private \OCP\Share\IManager $shareManager, private IRootFolder $rootFolder, ) { @@ -39,6 +38,9 @@ public function onAccessUpdateDecl(Node $node, bool $recurse = true): void { $fileIds = $this->storageService->getAllFilesInFolder($node); } else { + if (!$this->allowedMimeType($node)) { + return; + } try { $fileIds = [$node->getId()]; } catch (InvalidPathException|NotFoundException $e) { @@ -61,6 +63,9 @@ public function onDelete(Node $node, bool $recurse = true): void { } $fileIds = $this->storageService->getAllFilesInFolder($node); } else { + if (!$this->allowedMimeType($node)) { + return; + } try { $fileIds = [$node->getId()]; } catch (InvalidPathException|NotFoundException $e) { @@ -92,6 +97,9 @@ public function onInsert(Node $node, bool $recurse = true, bool $update = false) } $fileIds = $this->storageService->getAllFilesInFolder($node); } else { + if (!$this->allowedMimeType($node)) { + return; + } try { $fileIds = [$node->getId()]; } catch (InvalidPathException|NotFoundException $e) {