Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test(FileListener): More tests with folders #1205

Merged
merged 7 commits into from
Oct 11, 2024
Merged

test(FileListener): More tests with folders #1205

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
eeda7eb
test(FileListener): More tests with folders
marcelklehr Oct 10, 2024
3ef6109
fix(composer.json): Bump symfony/* to 6.4
marcelklehr Oct 10, 2024
3184d3c
fix: Drop php 8.0
marcelklehr Oct 10, 2024
43c26c1
fix(Logger): Adjust to new typings in symfony 6
marcelklehr Oct 10, 2024
5830005
Update composer.lock
marcelklehr Oct 10, 2024
d34017f
fix(FileListener): set recurse to true on BeforeNodeDeletedEvent
marcelklehr Oct 11, 2024
cd924c2
fix(FileListener): Handle edge cases of moving folders between varyin…
marcelklehr Oct 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion appinfo/info.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ The app does not send any sensitive data to cloud providers or similar services.
<screenshot>https://raw.githubusercontent.com/nextcloud/recognize/main/screenshots/Logo.png</screenshot>
<screenshot>https://raw.githubusercontent.com/nextcloud/recognize/main/screenshots/imagenet_examples.jpg</screenshot>
<dependencies>
<nextcloud min-version="30" max-version="31" />
<nextcloud min-version="31" max-version="31" />
</dependencies>
<background-jobs>
<job>OCA\Recognize\BackgroundJobs\MaintenanceJob</job>
Expand Down
8 changes: 4 additions & 4 deletions composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"require": {
"php": ">=8.0",
"php": ">=8.1",
"ext-json": "*",
"ext-pdo": "*",
"rubix/ml": "2.x",
Expand All @@ -13,8 +13,8 @@
},
"require-dev": {
"nextcloud/ocp": "dev-master",
"symfony/console": "^5.4",
"symfony/process": "^5.2"
"symfony/console": "^6.4",
"symfony/process": "^6.4"
},
"scripts": {
"lint": "find . -name \\*.php -not -path './vendor/*' -print0 | xargs -0 -n1 php -l",
Expand All @@ -37,7 +37,7 @@
},
"config": {
"platform": {
"php": "8.0.2"
"php": "8.1.0"
},
"allow-plugins": {
"bamarni/composer-bin-plugin": true,
Expand Down
115 changes: 91 additions & 24 deletions lib/Hooks/FileListener.php
View file
Open in desktop
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,23 +183,45 @@ 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;
}
if ($event instanceof BeforeNodeDeletedEvent) {
$this->postDelete($event->getNode(), false);
$this->postDelete($event->getNode());
return;
}
if ($event instanceof NodeDeletedEvent) {
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
19 changes: 10 additions & 9 deletions lib/Service/Logger.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

use Psr\Log\LoggerInterface;
use Symfony\Component\Console\Output\OutputInterface;
use \Stringable;

class Logger implements LoggerInterface {
private LoggerInterface $logger;
Expand All @@ -30,9 +31,9 @@
/**
* @inheritDoc
*/
public function emergency($message, array $context = array()) {
public function emergency(Stringable|string $message, array $context = array()): void {
if (isset($this->cliOutput)) {
$this->cliOutput->writeln($message);

Check failure on line 36 in lib/Service/Logger.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis dev-master

PossiblyInvalidArgument 

lib/Service/Logger.php:36:30: PossiblyInvalidArgument: Argument 1 of Symfony\Component\Console\Output\OutputInterface::writeln expects iterable<mixed, mixed>|string, but possibly different type Stringable|string provided (see https://psalm.dev/092)
}
$this->logger->emergency($message, $context);
}
Expand All @@ -40,9 +41,9 @@
/**
* @inheritDoc
*/
public function alert($message, array $context = array()) {
public function alert(Stringable|string $message, array $context = array()): void {
if (isset($this->cliOutput)) {
$this->cliOutput->writeln($message);

Check failure on line 46 in lib/Service/Logger.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis dev-master

PossiblyInvalidArgument 

lib/Service/Logger.php:46:30: PossiblyInvalidArgument: Argument 1 of Symfony\Component\Console\Output\OutputInterface::writeln expects iterable<mixed, mixed>|string, but possibly different type Stringable|string provided (see https://psalm.dev/092)
}
$this->logger->alert($message, $context);
}
Expand All @@ -50,9 +51,9 @@
/**
* @inheritDoc
*/
public function critical($message, array $context = array()) {
public function critical(Stringable|string $message, array $context = array()): void {
if (isset($this->cliOutput)) {
$this->cliOutput->writeln($message);

Check failure on line 56 in lib/Service/Logger.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis dev-master

PossiblyInvalidArgument 

lib/Service/Logger.php:56:30: PossiblyInvalidArgument: Argument 1 of Symfony\Component\Console\Output\OutputInterface::writeln expects iterable<mixed, mixed>|string, but possibly different type Stringable|string provided (see https://psalm.dev/092)
}
$this->logger->critical($message, $context);
}
Expand All @@ -60,9 +61,9 @@
/**
* @inheritDoc
*/
public function error($message, array $context = array()) {
public function error(Stringable|string $message, array $context = array()): void {
if (isset($this->cliOutput)) {
$this->cliOutput->writeln($message);

Check failure on line 66 in lib/Service/Logger.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis dev-master

PossiblyInvalidArgument 

lib/Service/Logger.php:66:30: PossiblyInvalidArgument: Argument 1 of Symfony\Component\Console\Output\OutputInterface::writeln expects iterable<mixed, mixed>|string, but possibly different type Stringable|string provided (see https://psalm.dev/092)
}
$this->logger->error($message, $context);
}
Expand All @@ -70,9 +71,9 @@
/**
* @inheritDoc
*/
public function warning($message, array $context = array()) {
public function warning(Stringable|string $message, array $context = array()): void {
if (isset($this->cliOutput)) {
$this->cliOutput->writeln($message);

Check failure on line 76 in lib/Service/Logger.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis dev-master

PossiblyInvalidArgument 

lib/Service/Logger.php:76:30: PossiblyInvalidArgument: Argument 1 of Symfony\Component\Console\Output\OutputInterface::writeln expects iterable<mixed, mixed>|string, but possibly different type Stringable|string provided (see https://psalm.dev/092)
}
$this->logger->warning($message, $context);
}
Expand All @@ -80,9 +81,9 @@
/**
* @inheritDoc
*/
public function notice($message, array $context = array()) {
public function notice(Stringable|string $message, array $context = array()): void {
if (isset($this->cliOutput)) {
$this->cliOutput->writeln($message);

Check failure on line 86 in lib/Service/Logger.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis dev-master

PossiblyInvalidArgument 

lib/Service/Logger.php:86:30: PossiblyInvalidArgument: Argument 1 of Symfony\Component\Console\Output\OutputInterface::writeln expects iterable<mixed, mixed>|string, but possibly different type Stringable|string provided (see https://psalm.dev/092)
}
$this->logger->notice($message, $context);
}
Expand All @@ -90,9 +91,9 @@
/**
* @inheritDoc
*/
public function info($message, array $context = array()) {
public function info(Stringable|string $message, array $context = array()): void {
if (isset($this->cliOutput) && !$this->cliOutput->isQuiet()) {
$this->cliOutput->writeln($message);

Check failure on line 96 in lib/Service/Logger.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis dev-master

PossiblyInvalidArgument 

lib/Service/Logger.php:96:30: PossiblyInvalidArgument: Argument 1 of Symfony\Component\Console\Output\OutputInterface::writeln expects iterable<mixed, mixed>|string, but possibly different type Stringable|string provided (see https://psalm.dev/092)
}
$this->logger->info($message, $context);
}
Expand All @@ -100,9 +101,9 @@
/**
* @inheritDoc
*/
public function debug($message, array $context = array()) {
public function debug(Stringable|string $message, array $context = array()): void {
if (isset($this->cliOutput) && !$this->cliOutput->isQuiet()) {
$this->cliOutput->writeln($message);

Check failure on line 106 in lib/Service/Logger.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis dev-master

PossiblyInvalidArgument 

lib/Service/Logger.php:106:30: PossiblyInvalidArgument: Argument 1 of Symfony\Component\Console\Output\OutputInterface::writeln expects iterable<mixed, mixed>|string, but possibly different type Stringable|string provided (see https://psalm.dev/092)
}
$this->logger->debug($message, $context);
}
Expand All @@ -110,9 +111,9 @@
/**
* @inheritDoc
*/
public function log($level, $message, array $context = array()) {
public function log($level, Stringable|string $message, array $context = array()): void {
if (isset($this->cliOutput)) {
$this->cliOutput->writeln($message);

Check failure on line 116 in lib/Service/Logger.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis dev-master

PossiblyInvalidArgument 

lib/Service/Logger.php:116:30: PossiblyInvalidArgument: Argument 1 of Symfony\Component\Console\Output\OutputInterface::writeln expects iterable<mixed, mixed>|string, but possibly different type Stringable|string provided (see https://psalm.dev/092)
}
$this->logger->log($level, $message, $context);
}
Expand Down
Loading
Loading