From 3966d91922404144613ea34da593571bb532a525 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Fri, 11 Oct 2024 10:56:18 +0200 Subject: [PATCH] fix(FileListener): Handle edge cases of moving folders between varying stages of ignorance Signed-off-by: Marcel Klehr --- lib/Hooks/FileListener.php | 113 +++++++++++++++++++++++++++++-------- tests/ClassifierTest.php | 70 +++++++++++------------ 2 files changed, 125 insertions(+), 58 deletions(-) diff --git a/lib/Hooks/FileListener.php b/lib/Hooks/FileListener.php index a33e07f2..459951c2 100644 --- a/lib/Hooks/FileListener.php +++ b/lib/Hooks/FileListener.php @@ -47,6 +47,7 @@ */ class FileListener implements IEventListener { private ?bool $movingFromIgnoredTerritory; + private ?array $movingDirFromIgnoredTerritory; /** @var list */ private array $sourceUserIds; private ?Node $source = null; @@ -64,6 +65,7 @@ public function __construct( private IJobList $jobList, ) { $this->movingFromIgnoredTerritory = null; + $this->movingDirFromIgnoredTerritory = null; $this->sourceUserIds = []; } @@ -139,16 +141,22 @@ public function handle(Event $event): void { } } if ($event instanceof BeforeNodeRenamedEvent) { + $this->movingFromIgnoredTerritory = null; + $this->movingDirFromIgnoredTerritory = []; if (in_array($event->getSource()->getName(), [...Constants::IGNORE_MARKERS_ALL, ...Constants::IGNORE_MARKERS_IMAGE, ...Constants::IGNORE_MARKERS_AUDIO, ...Constants::IGNORE_MARKERS_VIDEO], true)) { $this->resetIgnoreCache($event->getSource()); return; } // We try to remember whether the source node is in ignored territory // because after moving isIgnored doesn't work anymore :( - if ($this->isIgnored($event->getSource())) { - $this->movingFromIgnoredTerritory = true; + if ($event->getSource()->getType() !== FileInfo::TYPE_FOLDER) { + if ($this->isFileIgnored($event->getSource())) { + $this->movingFromIgnoredTerritory = true; + } else { + $this->movingFromIgnoredTerritory = false; + } } else { - $this->movingFromIgnoredTerritory = false; + $this->movingDirFromIgnoredTerritory = $this->getDirIgnores($event->getSource()); } $this->sourceUserIds = $this->getUsersWithFileAccess($event->getSource()); $this->source = $event->getSource(); @@ -175,17 +183,39 @@ public function handle(Event $event): void { $this->postDelete($event->getTarget()->getParent()); return; } - - if ($this->movingFromIgnoredTerritory) { - if ($this->isIgnored($event->getTarget())) { + if ($event->getTarget()->getType() !== FileInfo::TYPE_FOLDER) { + if ($this->movingFromIgnoredTerritory) { + if ($this->isFileIgnored($event->getTarget())) { + return; + } + $this->postInsert($event->getTarget()); + return; + } + if ($this->isFileIgnored($event->getTarget())) { + $this->postDelete($event->getTarget()); + return; + } + } else { + if ($this->movingDirFromIgnoredTerritory !== null && count($this->movingDirFromIgnoredTerritory) !== 0) { + $oldIgnores = $this->movingDirFromIgnoredTerritory; + $newIgnores = $this->getDirIgnores($event->getTarget()); + $diff1 = array_diff($newIgnores, $oldIgnores); + $diff2 = array_diff($oldIgnores, $newIgnores); + if (count($diff1) !== 0 || count($diff2) !== 0) { + if (count($diff1) !== 0) { + $this->postDelete($event->getTarget(), true, $diff1); + } + if (count($diff2) !== 0) { + $this->postInsert($event->getTarget(), true, $diff2); + } + } + return; + } + $ignoredMimeTypes = $this->getDirIgnores($event->getTarget()); + if (!empty($ignoredMimeTypes)) { + $this->postDelete($event->getTarget(), true, $ignoredMimeTypes); return; } - $this->postInsert($event->getTarget()); - return; - } - if ($this->isIgnored($event->getTarget())) { - $this->postDelete($event->getTarget()); - return; } $this->postRename($this->source ?? $event->getSource(), $event->getTarget()); return; @@ -246,7 +276,7 @@ public function handle(Event $event): void { } } - public function postDelete(Node $node, bool $recurse = true): void { + public function postDelete(Node $node, bool $recurse = true, ?array $mimeTypes = null): void { if ($node->getType() === FileInfo::TYPE_FOLDER) { if (!$recurse) { return; @@ -254,7 +284,7 @@ public function postDelete(Node $node, bool $recurse = true): void { try { /** @var Folder $node */ foreach ($node->getDirectoryListing() as $child) { - $this->postDelete($child); + $this->postDelete($child, true, $mimeTypes); } } catch (NotFoundException $e) { $this->logger->debug($e->getMessage(), ['exception' => $e]); @@ -262,6 +292,10 @@ public function postDelete(Node $node, bool $recurse = true): void { return; } + if ($mimeTypes !== null && !in_array($node->getMimetype(), $mimeTypes)) { + return; + } + // Try Deleting possibly existing face detections try { /** @@ -291,7 +325,7 @@ public function postDelete(Node $node, bool $recurse = true): void { /** * @throws \OCP\Files\InvalidPathException */ - public function postInsert(Node $node, bool $recurse = true): void { + public function postInsert(Node $node, bool $recurse = true, ?array $mimeTypes = null): void { if ($node->getType() === FileInfo::TYPE_FOLDER) { if (!$recurse) { return; @@ -309,6 +343,10 @@ public function postInsert(Node $node, bool $recurse = true): void { return; } + if ($mimeTypes !== null && !in_array($node->getMimetype(), $mimeTypes)) { + return; + } + $queueFile = new QueueFile(); if ($node->getMountPoint()->getNumericStorageId() === null) { return; @@ -316,7 +354,7 @@ public function postInsert(Node $node, bool $recurse = true): void { $queueFile->setStorageId($node->getMountPoint()->getNumericStorageId()); $queueFile->setRootId($node->getMountPoint()->getStorageRootId()); - if ($this->isIgnored($node)) { + if ($this->isFileIgnored($node)) { return; } @@ -401,7 +439,7 @@ private function copyFaceDetectionsForNode(string $ownerId, array $usersToAdd, a * @throws \OCP\Files\InvalidPathException * @throws \OCP\Files\NotFoundException */ - public function isIgnored(Node $node) : bool { + public function isFileIgnored(Node $node) : bool { $ignoreMarkers = []; $mimeType = $node->getMimetype(); $storageId = $node->getMountPoint()->getNumericStorageId(); @@ -419,6 +457,7 @@ public function isIgnored(Node $node) : bool { if (in_array($mimeType, Constants::AUDIO_FORMATS)) { $ignoreMarkers = array_merge($ignoreMarkers, Constants::IGNORE_MARKERS_AUDIO); } + if (count($ignoreMarkers) === 0) { return true; } @@ -426,17 +465,45 @@ public function isIgnored(Node $node) : bool { $ignoreMarkers = array_merge($ignoreMarkers, Constants::IGNORE_MARKERS_ALL); $ignoredPaths = $this->ignoreService->getIgnoredDirectories($storageId, $ignoreMarkers); - $relevantIgnorePaths = array_filter($ignoredPaths, static function (string $ignoredPath) use ($node) { - return stripos($node->getInternalPath(), $ignoredPath ? $ignoredPath . '/' : $ignoredPath) === 0; - }); - if (count($relevantIgnorePaths) > 0) { - return true; + foreach ($ignoredPaths as $ignoredPath) { + if (stripos($node->getInternalPath(), $ignoredPath ? $ignoredPath . '/' : $ignoredPath) === 0) { + return true; + } } - return false; } + /** + * @param \OCP\Files\Node $node + * @return array + * @throws Exception + */ + public function getDirIgnores(Node $node) : array { + $storageId = $node->getMountPoint()->getNumericStorageId(); + if ($storageId === null) { + return []; + } + + $ignoredMimeTypes = []; + foreach ([ + [Constants::IGNORE_MARKERS_IMAGE, Constants::IMAGE_FORMATS], + [Constants::IGNORE_MARKERS_VIDEO, Constants::VIDEO_FORMATS], + [Constants::IGNORE_MARKERS_AUDIO, Constants::AUDIO_FORMATS], + [Constants::IGNORE_MARKERS_ALL, array_merge(Constants::IMAGE_FORMATS, Constants::VIDEO_FORMATS, Constants::AUDIO_FORMATS)], + ] as $iteration) { + [$ignoreMarkers, $mimeTypes] = $iteration; + $ignoredPaths = $this->ignoreService->getIgnoredDirectories($storageId, $ignoreMarkers); + foreach ($ignoredPaths as $ignoredPath) { + if (stripos($node->getInternalPath(), $ignoredPath ? $ignoredPath . '/' : $ignoredPath) === 0) { + $ignoredMimeTypes = array_unique(array_merge($ignoredMimeTypes, $mimeTypes)); + } + } + } + + return $ignoredMimeTypes; + } + private function resetIgnoreCache(Node $node) : void { $storageId = $node->getMountPoint()->getNumericStorageId(); if ($storageId === null) { diff --git a/tests/ClassifierTest.php b/tests/ClassifierTest.php index c58e51be..72c65ba5 100644 --- a/tests/ClassifierTest.php +++ b/tests/ClassifierTest.php @@ -184,58 +184,58 @@ public function testFileListener(string $ignoreFileName) : void { self::assertCount(0, $this->queue->getFromQueue(ImagenetClassifier::MODEL_NAME, $storageId, $rootId, 100), 'entry should have been removed from imagenet queue after moving it into ignored territory'); } - /** - * @dataProvider ignoreImageFilesProvider - * @return void - * @throws \OCP\DB\Exception - * @throws \OCP\Files\InvalidPathException - * @throws \OCP\Files\NotFoundException - * @throws \OCP\Files\NotPermittedException - */ - public function testFileListenerWithFolder(string $ignoreFileName) : void { - $this->config->setAppValueString('imagenet.enabled', 'true'); - $this->queue->clearQueue(ImagenetClassifier::MODEL_NAME); + /** + * @dataProvider ignoreImageFilesProvider + * @return void + * @throws \OCP\DB\Exception + * @throws \OCP\Files\InvalidPathException + * @throws \OCP\Files\NotFoundException + * @throws \OCP\Files\NotPermittedException + */ + public function testFileListenerWithFolder(string $ignoreFileName) : void { + $this->config->setAppValueString('imagenet.enabled', 'true'); + $this->queue->clearQueue(ImagenetClassifier::MODEL_NAME); - $folder = $this->userFolder->newFolder('/folder/'); - $this->testFile = $folder->newFile('/alpine.jpg', file_get_contents(__DIR__.'/res/alpine.JPG')); - $ignoreFolder = $this->userFolder->newFolder('/test/ignore/'); - $ignoredFolder = $ignoreFolder->newFolder('/folder/'); - $ignoreFile = $this->userFolder->newFile('/test/' . $ignoreFileName, ''); - $this->ignoredFile = $ignoredFolder->newFile('/alpine-2.jpg', file_get_contents(__DIR__.'/res/alpine.JPG')); + $folder = $this->userFolder->newFolder('/folder/'); + $this->testFile = $folder->newFile('/alpine.jpg', file_get_contents(__DIR__.'/res/alpine.JPG')); + $ignoreFolder = $this->userFolder->newFolder('/test/ignore/'); + $ignoredFolder = $ignoreFolder->newFolder('/folder/'); + $ignoreFile = $this->userFolder->newFile('/test/' . $ignoreFileName, ''); + $this->ignoredFile = $ignoredFolder->newFile('/alpine-2.jpg', file_get_contents(__DIR__.'/res/alpine.JPG')); - $storageId = $this->testFile->getMountPoint()->getNumericStorageId(); - $rootId = $this->testFile->getMountPoint()->getStorageRootId(); + $storageId = $this->testFile->getMountPoint()->getNumericStorageId(); + $rootId = $this->testFile->getMountPoint()->getStorageRootId(); - self::assertCount(1, $this->queue->getFromQueue(ImagenetClassifier::MODEL_NAME, $storageId, $rootId, 100), 'one element should have been added to imagenet queue'); + self::assertCount(1, $this->queue->getFromQueue(ImagenetClassifier::MODEL_NAME, $storageId, $rootId, 100), 'one element should have been added to imagenet queue'); - $folder->delete(); + $folder->delete(); - self::assertCount(0, $this->queue->getFromQueue(ImagenetClassifier::MODEL_NAME, $storageId, $rootId, 100), 'entry should have been removed from imagenet queue'); + self::assertCount(0, $this->queue->getFromQueue(ImagenetClassifier::MODEL_NAME, $storageId, $rootId, 100), 'entry should have been removed from imagenet queue'); - $ignoreFile->delete(); + $ignoreFile->delete(); - self::assertCount(1, $this->queue->getFromQueue(ImagenetClassifier::MODEL_NAME, $storageId, $rootId, 100), 'one element should have been added to imagenet queue after deleting ignore file'); + self::assertCount(1, $this->queue->getFromQueue(ImagenetClassifier::MODEL_NAME, $storageId, $rootId, 100), 'one element should have been added to imagenet queue after deleting ignore file'); - $ignoreFile = $this->userFolder->newFile('/test/' . $ignoreFileName, ''); + $ignoreFile = $this->userFolder->newFile('/test/' . $ignoreFileName, ''); - self::assertCount(0, $this->queue->getFromQueue(ImagenetClassifier::MODEL_NAME, $storageId, $rootId, 100), 'entry should have been removed from imagenet queue after creating ignore file'); + self::assertCount(0, $this->queue->getFromQueue(ImagenetClassifier::MODEL_NAME, $storageId, $rootId, 100), 'entry should have been removed from imagenet queue after creating ignore file'); - $ignoredFolder->move($this->userFolder->getPath() . '/folder2'); + $ignoredFolder->move($this->userFolder->getPath() . '/folder2'); - self::assertCount(1, $this->queue->getFromQueue(ImagenetClassifier::MODEL_NAME, $storageId, $rootId, 100), 'one element should have been added to imagenet queue after moving it out of ignored territory'); + self::assertCount(1, $this->queue->getFromQueue(ImagenetClassifier::MODEL_NAME, $storageId, $rootId, 100), 'one element should have been added to imagenet queue after moving it out of ignored territory'); - $ignoreFile->move($this->userFolder->getPath() . '/' . $ignoreFileName); + $ignoreFile->move($this->userFolder->getPath() . '/' . $ignoreFileName); - self::assertCount(0, $this->queue->getFromQueue(ImagenetClassifier::MODEL_NAME, $storageId, $rootId, 100), 'entry should have been removed from imagenet queue after moving ignore file'); + self::assertCount(0, $this->queue->getFromQueue(ImagenetClassifier::MODEL_NAME, $storageId, $rootId, 100), 'entry should have been removed from imagenet queue after moving ignore file'); - $ignoreFile->move($this->userFolder->getPath() . '/test/' . $ignoreFileName); + $ignoreFile->move($this->userFolder->getPath() . '/test/' . $ignoreFileName); - self::assertCount(1, $this->queue->getFromQueue(ImagenetClassifier::MODEL_NAME, $storageId, $rootId, 100), 'one element should have been added to imagenet queue after moving ignore file'); + self::assertCount(1, $this->queue->getFromQueue(ImagenetClassifier::MODEL_NAME, $storageId, $rootId, 100), 'one element should have been added to imagenet queue after moving ignore file'); - $ignoredFolder->move($this->userFolder->getPath() . '/test/ignore/folder'); + $ignoredFolder->move($this->userFolder->getPath() . '/test/ignore/folder'); - self::assertCount(0, $this->queue->getFromQueue(ImagenetClassifier::MODEL_NAME, $storageId, $rootId, 100), 'entry should have been removed from imagenet queue after moving it into ignored territory'); - } + self::assertCount(0, $this->queue->getFromQueue(ImagenetClassifier::MODEL_NAME, $storageId, $rootId, 100), 'entry should have been removed from imagenet queue after moving it into ignored territory'); + } /** * @dataProvider ignoreImageFilesProvider