Skip to content

Commit

Permalink
fix(FileListener)
Browse files Browse the repository at this point in the history
- all users should now get access when moving
- all new users should get a ClusterFacesJob scheduled when moving
- no new classifications should be run when moving unless external storage is concerned

fixes #1118

Signed-off-by: Marcel Klehr <[email protected]>
  • Loading branch information
marcelklehr committed Jun 20, 2024
1 parent 0228c37 commit 1d125d7
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 72 deletions.
6 changes: 1 addition & 5 deletions lib/Classifiers/Images/ClusteringFaceClassifier.php
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,8 @@ public function __construct(
* @throws NotFoundException
*/
private function getUsersWithFileAccess(Node $node): array {
/** @var array{users:array<string,array{node_id:int, node_path: string}>, remote: array<string,array{node_id:int, node_path: string}>, mail: array<string,array{node_id:int, node_path: string}>} $accessList */
$accessList = $this->shareManager->getAccessList($node, true, true);
$userIds = array_map(fn ($id) => strval($id), array_keys($accessList['users']));

$mountInfos = $this->userMountCache->getMountsForFileId($node->getId());
$userIds += array_map(static function (ICachedMountInfo $mountInfo) {
$userIds = array_map(static function (ICachedMountInfo $mountInfo) {
return $mountInfo->getUser()->getUID();
}, $mountInfos);

Expand Down
108 changes: 41 additions & 67 deletions lib/Hooks/FileListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
declare(strict_types=1);
namespace OCA\Recognize\Hooks;

use OCA\Recognize\BackgroundJobs\ClusterFacesJob;
use OCA\Recognize\Classifiers\Audio\MusicnnClassifier;
use OCA\Recognize\Classifiers\Images\ClusteringFaceClassifier;
use OCA\Recognize\Classifiers\Images\ImagenetClassifier;
Expand All @@ -16,6 +17,7 @@
use OCA\Recognize\Service\IgnoreService;
use OCA\Recognize\Service\QueueService;
use OCA\Recognize\Service\StorageService;
use OCP\BackgroundJob\IJobList;
use OCP\DB\Exception;
use OCP\EventDispatcher\Event;
use OCP\EventDispatcher\IEventListener;
Expand All @@ -35,13 +37,9 @@
use OCP\Files\Node;
use OCP\Files\NotFoundException;
use OCP\IGroupManager;
use OCP\IUser;
use OCP\Share\Events\ShareAcceptedEvent;
use OCP\Share\Events\ShareCreatedEvent;
use OCP\Share\Events\ShareDeletedEvent;
use OCP\Share\IManager;
use Psr\Container\ContainerExceptionInterface;
use Psr\Container\NotFoundExceptionInterface;
use Psr\Log\LoggerInterface;

/**
Expand All @@ -63,6 +61,7 @@ public function __construct(
private IUserMountCache $userMountCache,
private IManager $shareManager,
private IGroupManager $groupManager,
private IJobList $jobList,
) {
$this->movingFromIgnoredTerritory = null;
$this->sourceUserIds = [];
Expand All @@ -75,58 +74,18 @@ public function __construct(
* @throws NotFoundException
*/
private function getUsersWithFileAccess(Node $node): array {
/** @var array{users:array<string,array{node_id:int, node_path: string}>, remote: array<string,array{node_id:int, node_path: string}>, mail: array<string,array{node_id:int, node_path: string}>} $accessList */
$accessList = $this->shareManager->getAccessList($node, true, true);
$userIds = array_map(fn ($id) => strval($id), array_keys($accessList['users']));

$this->userMountCache->clear();
$mountInfos = $this->userMountCache->getMountsForFileId($node->getId());
$userIds += array_map(static function (ICachedMountInfo $mountInfo) {
$userIds = array_map(static function (ICachedMountInfo $mountInfo) {
return $mountInfo->getUser()->getUID();
}, $mountInfos);

$userIds += $this->getGroupFolderParticipants($node);

return array_values(array_unique($userIds));
}

private function getGroupFolderParticipants(Node $node) {
$filePath = $node->getPath();

// Now we need to find the group folder ID associated with this path
// Assuming the group folders are stored in a specific path, e.g., "/__groupfolders/<groupfolder_id>/..."
preg_match('/\/__groupfolders\/(\d+)\//', $filePath, $matches);

if (!isset($matches[1])) {
return [];
}

$groupFolderId = (int) $matches[1];

try {
$groupFolderManager = \OCP\Server::get(OCA\GroupFolders\Folder\FolderManager::class);
} catch (NotFoundExceptionInterface|ContainerExceptionInterface $e) {
return [];
}

$folder = $groupFolderManager->getFolder($groupFolderId);

$userIds = [];
// Fetch groups
if (!empty($folder['groups'])) {
foreach ($folder['groups'] as $groupId => $permissions) {
$group = $this->groupManager->get($groupId);
if ($group) {
$userIds += array_map(fn (IUser $user) => $user->getUID(), $group->getUsers());
}
}
}
return array_values(array_unique($userIds));
}

public function handle(Event $event): void {
try {
if ($event instanceof ShareAcceptedEvent || $event instanceof ShareCreatedEvent) {
if ($event instanceof ShareAcceptedEvent) {
$share = $event->getShare();
$ownerId = $share->getShareOwner();
$node = $share->getNode();
Expand Down Expand Up @@ -259,6 +218,9 @@ public function handle(Event $event): void {
if ($node instanceof Folder) {
return;
}
if (!str_contains('external', $node->getMountPoint()->getMountType())) {

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

View workflow job for this annotation

GitHub Actions / Nextcloud (8.1, dev-master)

InvalidLiteralArgument

lib/Hooks/FileListener.php:221:23: InvalidLiteralArgument: Argument 1 of str_contains expects a non-literal value, but 'external' provided (see https://psalm.dev/237)

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

View workflow job for this annotation

GitHub Actions / Nextcloud (8.1, dev-stable29)

InvalidLiteralArgument

lib/Hooks/FileListener.php:221:23: InvalidLiteralArgument: Argument 1 of str_contains expects a non-literal value, but 'external' provided (see https://psalm.dev/237)

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

View workflow job for this annotation

GitHub Actions / Nextcloud (8.2, dev-stable29)

InvalidLiteralArgument

lib/Hooks/FileListener.php:221:23: InvalidLiteralArgument: Argument 1 of str_contains expects a non-literal value, but 'external' provided (see https://psalm.dev/237)

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

View workflow job for this annotation

GitHub Actions / Nextcloud (8.3, dev-master)

InvalidLiteralArgument

lib/Hooks/FileListener.php:221:23: InvalidLiteralArgument: Argument 1 of str_contains expects a non-literal value, but 'external' provided (see https://psalm.dev/237)

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

View workflow job for this annotation

GitHub Actions / Nextcloud (8.2, dev-master)

InvalidLiteralArgument

lib/Hooks/FileListener.php:221:23: InvalidLiteralArgument: Argument 1 of str_contains expects a non-literal value, but 'external' provided (see https://psalm.dev/237)

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

View workflow job for this annotation

GitHub Actions / Nextcloud (8.3, dev-stable29)

InvalidLiteralArgument

lib/Hooks/FileListener.php:221:23: InvalidLiteralArgument: Argument 1 of str_contains expects a non-literal value, but 'external' provided (see https://psalm.dev/237)
return;
}
if (in_array($node->getName(), [...Constants::IGNORE_MARKERS_ALL, ...Constants::IGNORE_MARKERS_IMAGE, ...Constants::IGNORE_MARKERS_AUDIO, ...Constants::IGNORE_MARKERS_VIDEO], true)) {
$this->resetIgnoreCache($node);
$this->postDelete($node->getParent());
Expand Down Expand Up @@ -386,39 +348,51 @@ public function postInsert(Node $node, bool $recurse = true): void {
}
}

/**
* @throws InvalidPathException
* @throws NotFoundException
* @throws Exception
*/
public function postRename(Node $source, Node $target): void {
$targetUserIds = $this->getUsersWithFileAccess($target);

$usersToAdd = array_diff($targetUserIds, $this->sourceUserIds);
$existingUsers = array_diff($targetUserIds, $usersToAdd);
// *handwaving* I know this is a stretch but it's good enough
$ownerId = $existingUsers[0];
$ownerId = $source->getOwner() ? $source->getOwner()->getUID() : ($target->getOwner() ? $target->getOwner()->getUID() : $existingUsers[0]);

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

View workflow job for this annotation

GitHub Actions / Nextcloud (8.1, dev-master)

PossiblyNullReference

lib/Hooks/FileListener.php:361:57: PossiblyNullReference: Cannot call method getUID on possibly null value (see https://psalm.dev/083)

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

View workflow job for this annotation

GitHub Actions / Nextcloud (8.1, dev-master)

PossiblyNullReference

lib/Hooks/FileListener.php:361:112: PossiblyNullReference: Cannot call method getUID on possibly null value (see https://psalm.dev/083)

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

View workflow job for this annotation

GitHub Actions / Nextcloud (8.1, dev-stable29)

PossiblyNullReference

lib/Hooks/FileListener.php:361:57: PossiblyNullReference: Cannot call method getUID on possibly null value (see https://psalm.dev/083)

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

View workflow job for this annotation

GitHub Actions / Nextcloud (8.1, dev-stable29)

PossiblyNullReference

lib/Hooks/FileListener.php:361:112: PossiblyNullReference: Cannot call method getUID on possibly null value (see https://psalm.dev/083)

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

View workflow job for this annotation

GitHub Actions / Nextcloud (8.2, dev-stable29)

PossiblyNullReference

lib/Hooks/FileListener.php:361:57: PossiblyNullReference: Cannot call method getUID on possibly null value (see https://psalm.dev/083)

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

View workflow job for this annotation

GitHub Actions / Nextcloud (8.2, dev-stable29)

PossiblyNullReference

lib/Hooks/FileListener.php:361:112: PossiblyNullReference: Cannot call method getUID on possibly null value (see https://psalm.dev/083)

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

View workflow job for this annotation

GitHub Actions / Nextcloud (8.3, dev-master)

PossiblyNullReference

lib/Hooks/FileListener.php:361:57: PossiblyNullReference: Cannot call method getUID on possibly null value (see https://psalm.dev/083)

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

View workflow job for this annotation

GitHub Actions / Nextcloud (8.3, dev-master)

PossiblyNullReference

lib/Hooks/FileListener.php:361:112: PossiblyNullReference: Cannot call method getUID on possibly null value (see https://psalm.dev/083)

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

View workflow job for this annotation

GitHub Actions / Nextcloud (8.2, dev-master)

PossiblyNullReference

lib/Hooks/FileListener.php:361:57: PossiblyNullReference: Cannot call method getUID on possibly null value (see https://psalm.dev/083)

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

View workflow job for this annotation

GitHub Actions / Nextcloud (8.2, dev-master)

PossiblyNullReference

lib/Hooks/FileListener.php:361:112: PossiblyNullReference: Cannot call method getUID on possibly null value (see https://psalm.dev/083)

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

View workflow job for this annotation

GitHub Actions / Nextcloud (8.3, dev-stable29)

PossiblyNullReference

lib/Hooks/FileListener.php:361:57: PossiblyNullReference: Cannot call method getUID on possibly null value (see https://psalm.dev/083)

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

View workflow job for this annotation

GitHub Actions / Nextcloud (8.3, dev-stable29)

PossiblyNullReference

lib/Hooks/FileListener.php:361:112: PossiblyNullReference: Cannot call method getUID on possibly null value (see https://psalm.dev/083)

if ($target->getType() === FileInfo::TYPE_FOLDER) {
$mount = $target->getMountPoint();
$numericStorageId = $mount->getNumericStorageId();
if ($numericStorageId === null) {
return;
}
$files = $this->storageService->getFilesInMount($numericStorageId, $target->getId(), [ClusteringFaceClassifier::MODEL_NAME], 0, 0);
foreach ($files as $fileInfo) {
foreach ($usersToAdd as $userId) {
if (count($this->faceDetectionMapper->findByFileIdAndUser($target->getId(), $userId)) > 0) {
$this->copyFaceDetectionsForNode($ownerId, $usersToAdd, $targetUserIds, $target);

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

View workflow job for this annotation

GitHub Actions / Nextcloud (8.1, dev-master)

ArgumentTypeCoercion

lib/Hooks/FileListener.php:363:46: ArgumentTypeCoercion: Argument 2 of OCA\Recognize\Hooks\FileListener::copyFaceDetectionsForNode expects list<string>, but parent type array<int<0, max>, string> provided (see https://psalm.dev/193)

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

View workflow job for this annotation

GitHub Actions / Nextcloud (8.1, dev-stable29)

ArgumentTypeCoercion

lib/Hooks/FileListener.php:363:46: ArgumentTypeCoercion: Argument 2 of OCA\Recognize\Hooks\FileListener::copyFaceDetectionsForNode expects list<string>, but parent type array<int<0, max>, string> provided (see https://psalm.dev/193)

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

View workflow job for this annotation

GitHub Actions / Nextcloud (8.2, dev-stable29)

ArgumentTypeCoercion

lib/Hooks/FileListener.php:363:46: ArgumentTypeCoercion: Argument 2 of OCA\Recognize\Hooks\FileListener::copyFaceDetectionsForNode expects list<string>, but parent type array<int<0, max>, string> provided (see https://psalm.dev/193)

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

View workflow job for this annotation

GitHub Actions / Nextcloud (8.3, dev-master)

ArgumentTypeCoercion

lib/Hooks/FileListener.php:363:46: ArgumentTypeCoercion: Argument 2 of OCA\Recognize\Hooks\FileListener::copyFaceDetectionsForNode expects list<string>, but parent type array<int<0, max>, string> provided (see https://psalm.dev/193)

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

View workflow job for this annotation

GitHub Actions / Nextcloud (8.2, dev-master)

ArgumentTypeCoercion

lib/Hooks/FileListener.php:363:46: ArgumentTypeCoercion: Argument 2 of OCA\Recognize\Hooks\FileListener::copyFaceDetectionsForNode expects list<string>, but parent type array<int<0, max>, string> provided (see https://psalm.dev/193)

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

View workflow job for this annotation

GitHub Actions / Nextcloud (8.3, dev-stable29)

ArgumentTypeCoercion

lib/Hooks/FileListener.php:363:46: ArgumentTypeCoercion: Argument 2 of OCA\Recognize\Hooks\FileListener::copyFaceDetectionsForNode expects list<string>, but parent type array<int<0, max>, string> provided (see https://psalm.dev/193)
}

/**
* @param string $ownerId
* @param list<string> $usersToAdd
* @param list<string> $targetUserIds
* @param Node $node
* @return void
* @throws Exception|InvalidPathException|NotFoundException
*/
private function copyFaceDetectionsForNode(string $ownerId, array $usersToAdd, array $targetUserIds, Node $node): void {
if ($node instanceof Folder) {
try {
foreach ($node->getDirectoryListing() as $node) {
if (!in_array($node->getMimetype(), Constants::IMAGE_FORMATS)) {
continue;
}
$this->faceDetectionMapper->copyDetectionsForFileFromUserToUser($fileInfo['fileid'], $ownerId, $userId);
$this->copyFaceDetectionsForNode($ownerId, $usersToAdd, $targetUserIds, $node);
}
$this->faceDetectionMapper->removeDetectionsForFileFromUsersNotInList($fileInfo['fileid'], $targetUserIds);
} catch (NotFoundException|Exception|InvalidPathException $e) {
$this->logger->warning('Error in recognize file listener', ['exception' => $e]);
}
} else {
foreach ($usersToAdd as $userId) {
if (count($this->faceDetectionMapper->findByFileIdAndUser($target->getId(), $userId)) > 0) {
continue;
}
$this->faceDetectionMapper->copyDetectionsForFileFromUserToUser($target->getId(), $ownerId, $userId);
return;
}
foreach ($usersToAdd as $userId) {
if (count($this->faceDetectionMapper->findByFileIdAndUser($node->getId(), $userId)) > 0) {
continue;
}
$this->faceDetectionMapper->removeDetectionsForFileFromUsersNotInList($source->getId(), $targetUserIds);
$this->faceDetectionMapper->copyDetectionsForFileFromUserToUser($node->getId(), $ownerId, $userId);
$this->jobList->add(ClusterFacesJob::class, ['userId' => $userId]);
}
$this->faceDetectionMapper->removeDetectionsForFileFromUsersNotInList($node->getId(), $targetUserIds);
}

/**
Expand Down

0 comments on commit 1d125d7

Please sign in to comment.