Skip to content

Commit

Permalink
fix(FileListener): Handle edge cases of moving folders between varyin…
Browse files Browse the repository at this point in the history 
…g stages of ignorance

Signed-off-by: Marcel Klehr <[email protected]>
  • Loading branch information
@marcelklehr
marcelklehr committed Oct 11, 2024
1 parent d34017f commit 52a86ae
Show file tree
Hide file tree
Showing 2 changed files with 125 additions and 58 deletions.
113 changes: 90 additions & 23 deletions lib/Hooks/FileListener.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
*/
class FileListener implements IEventListener {
private ?bool $movingFromIgnoredTerritory;
private ?array $movingDirFromIgnoredTerritory;
/** @var list<string> */
private array $sourceUserIds;
private ?Node $source = null;
Expand All @@ -64,6 +65,7 @@ public function __construct(
private IJobList $jobList,
) {
$this->movingFromIgnoredTerritory = null;
$this->movingDirFromIgnoredTerritory = null;
$this->sourceUserIds = [];
}

Expand Down Expand Up @@ -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();
Expand All @@ -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 (count($this->movingDirFromIgnoredTerritory) !== 0) {

Check failure on line 199 in lib/Hooks/FileListener.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis dev-master

PossiblyNullArgument 

lib/Hooks/FileListener.php:199:16: PossiblyNullArgument: Argument 1 of count cannot be null, possibly null value provided (see https://psalm.dev/078)
$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;
Expand Down Expand Up @@ -249,22 +279,26 @@ 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;
}
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]);
}
return;
}

if ($mimeTypes !== null && !in_array($node->getMimetype(), $mimeTypes)) {
return;
}

// Try Deleting possibly existing face detections
try {
/**
Expand Down Expand Up @@ -294,7 +328,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;
Expand All @@ -312,14 +346,18 @@ 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;
}
$queueFile->setStorageId($node->getMountPoint()->getNumericStorageId());
$queueFile->setRootId($node->getMountPoint()->getStorageRootId());

if ($this->isIgnored($node)) {
if ($this->isFileIgnored($node)) {
return;
}

Expand Down Expand Up @@ -404,7 +442,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();
Expand All @@ -422,24 +460,53 @@ 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;
}

$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) {
Expand Down
70 changes: 35 additions & 35 deletions tests/ClassifierTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 52a86ae

Please sign in to comment.