diff --git a/lib/Classifiers/Images/ClusteringFaceClassifier.php b/lib/Classifiers/Images/ClusteringFaceClassifier.php index 81eff74a6..a47f1c037 100644 --- a/lib/Classifiers/Images/ClusteringFaceClassifier.php +++ b/lib/Classifiers/Images/ClusteringFaceClassifier.php @@ -54,6 +54,7 @@ public function __construct( * @throws NotFoundException */ private function getUsersWithFileAccess(Node $node): array { + $this->userMountCache->clear(); $mountInfos = $this->userMountCache->getMountsForFileId($node->getId()); $userIds = array_map(static function (ICachedMountInfo $mountInfo) { return $mountInfo->getUser()->getUID(); diff --git a/lib/Db/FsActionMapper.php b/lib/Db/FsActionMapper.php index 56d46585c..1a4abadf8 100644 --- a/lib/Db/FsActionMapper.php +++ b/lib/Db/FsActionMapper.php @@ -54,12 +54,8 @@ public function findByStorageId(string $className, int $storageId, int $limit = * @param class-string $className * @return list * @throws \OCP\DB\Exception - * @throws \Exception */ public function find(string $className, int $limit = 0): array { - if (!in_array('storage_id', $className::$columns, true)) { - throw new \Exception('entity does not have a storage_id column'); - } $qb = $this->db->getQueryBuilder(); $qb->selectDistinct($className::$columns) ->from($className::$tableName); @@ -243,7 +239,7 @@ public function insertMove(int $nodeId, string $owner, array $addedUsers, array $move->setAddedUsers($addedUsers); $move->setTargetUsers($targetUsers); $this->insert($move); - $arguments = [ 'type' => FsDeletion::class ]; + $arguments = [ 'type' => FsMove::class ]; if (!$this->jobList->has(ProcessFsActionsJob::class, $arguments)) { $this->jobList->add(ProcessFsActionsJob::class, $arguments); } diff --git a/lib/Db/FsMove.php b/lib/Db/FsMove.php index 198994b24..8aa351bcc 100644 --- a/lib/Db/FsMove.php +++ b/lib/Db/FsMove.php @@ -48,14 +48,27 @@ public function __construct() { * @return list */ public function getAddedUsers(): array { - return explode(',', $this->addedUsers ?? ''); + return self::explodeUsers($this->addedUsers); } /** * @return list */ public function getTargetUsers(): array { - return explode(',', $this->targetUsers ?? ''); + return self::explodeUsers($this->targetUsers); + } + + /** + * explode() on an empty string yields [''], which would be treated as a user + * with an empty user ID, so map the empty column to an empty list instead. + * + * @return list + */ + private static function explodeUsers(?string $users): array { + if ($users === null || $users === '') { + return []; + } + return explode(',', $users); } /** diff --git a/lib/Hooks/FileListener.php b/lib/Hooks/FileListener.php index 1426b6384..1ebee83f1 100644 --- a/lib/Hooks/FileListener.php +++ b/lib/Hooks/FileListener.php @@ -283,18 +283,18 @@ public function postInsert(Node $node, bool $recurse = true, ?array $mimeTypes = * @throws Exception */ public function postRename(Node $source, Node $target): void { - $targetUserIds = $this->getUsersWithFileAccess($target->getId()); - - $usersToAdd = array_values(array_diff($targetUserIds, $this->sourceUserIds)); - $existingUsers = array_diff($targetUserIds, $usersToAdd); - $sourceOwner = $source->getOwner(); - $targetOwner = $target->getOwner(); - $ownerId = $sourceOwner?->getUID() ?? $targetOwner?->getUID() ?? $existingUsers[0]; - if (preg_match('#^/[^/]*?/files/#', $target->getPath()) !== 1 && preg_match('#^/groupfolders/#', $target->getPath()) !== 1) { return; } + $targetUserIds = $this->getUsersWithFileAccess($target->getId()); + $usersToAdd = array_values(array_diff($targetUserIds, $this->sourceUserIds)); + + // Recorded for diagnostics only, and empty for group folder nodes, which have no + // owner: FsActionService picks the user to copy detections from by looking at who + // actually holds detections for the file. + $ownerId = $source->getOwner()?->getUID() ?? $target->getOwner()?->getUID() ?? ''; + $this->fsActionMapper->insertMove($target->getId(), $ownerId, $usersToAdd, $targetUserIds); } diff --git a/lib/Service/FsActionService.php b/lib/Service/FsActionService.php index 23bd2b643..a1122c694 100644 --- a/lib/Service/FsActionService.php +++ b/lib/Service/FsActionService.php @@ -125,7 +125,7 @@ public function processActions(array $actions): void { break; } try { - $this->onMove($action->getOwner(), $action->getAddedUsers(), $action->getTargetUsers(), $node); + $this->onMove($action->getAddedUsers(), $action->getTargetUsers(), $node); } catch (Exception|InvalidPathException|NotFoundException $e) { $this->logger->warning('Failed to process move action: ' . $e->getMessage() . ' Continuing.', ['exception' => $e]); } @@ -168,24 +168,17 @@ private function onAccessUpdate(int $storageId, int $rootId): void { $files = $this->storageService->getFilesInMount($storageId, $rootId, [ClusteringFaceClassifier::MODEL_NAME], 0, 0); $userIdsToScheduleClustering = []; foreach ($files as $fileInfo) { - $node = $this->rootFolder->getFirstNodeById($fileInfo['fileid']) ?: null; - $ownerId = $node?->getOwner()?->getUID(); - if ($ownerId === null) { + $detectionCountByUser = $this->getDetectionCountByUser($fileInfo['fileid']); + if (count($detectionCountByUser) === 0) { + // Nothing detected for this file (yet): nothing to copy, nothing to prune continue; } - $detectionsForFile = $this->faceDetectionMapper->findByFileId($fileInfo['fileid']); - $userHasDetectionForFile = []; - foreach ($detectionsForFile as $detection) { - $userHasDetectionForFile[$detection->getUserId()] = true; - } + $sourceUserId = (string)array_key_first($detectionCountByUser); foreach ($userIds as $userId) { - if ($userId === $ownerId) { - continue; - } - if ($userHasDetectionForFile[$userId] ?? false) { + if (isset($detectionCountByUser[(string)$userId])) { continue; } - $this->faceDetectionMapper->copyDetectionsForFileFromUserToUser($fileInfo['fileid'], $ownerId, $userId); + $this->faceDetectionMapper->copyDetectionsForFileFromUserToUser($fileInfo['fileid'], $sourceUserId, (string)$userId); $userIdsToScheduleClustering[$userId] = true; } $this->faceDetectionMapper->removeDetectionsForFileFromUsersNotInList($fileInfo['fileid'], $userIds); @@ -195,6 +188,28 @@ private function onAccessUpdate(int $storageId, int $rootId): void { } } + /** + * How many face detections each user holds for a file, most complete set first. + * + * The first key is the user to copy detections from when granting access to further + * users. The file system owner cannot be used for that: group folder nodes have no + * owner when they are resolved outside of a user session, which is the case in the + * background jobs that process fs actions. Whoever already holds detections for the + * file is both a more reliable and a more direct answer to the question. + * + * @return array + * @throws Exception + */ + private function getDetectionCountByUser(int $fileId): array { + $detectionCountByUser = []; + foreach ($this->faceDetectionMapper->findByFileId($fileId) as $detection) { + $userId = (string)$detection->getUserId(); + $detectionCountByUser[$userId] = ($detectionCountByUser[$userId] ?? 0) + 1; + } + arsort($detectionCountByUser); + return $detectionCountByUser; + } + /** * @throws \OCP\Files\InvalidPathException */ @@ -325,33 +340,38 @@ public function onDeletion(int $nodeId, ?array $mimeTypes = null): void { } /** - * @param string $ownerId * @param list $usersToAdd * @param list $targetUserIds * @param Node $node * @return void * @throws Exception|InvalidPathException|NotFoundException */ - private function onMove(string $ownerId, array $usersToAdd, array $targetUserIds, Node $node): void { + private function onMove(array $usersToAdd, array $targetUserIds, Node $node): void { if ($node instanceof Folder) { try { foreach ($node->getDirectoryListing() as $n) { if (!in_array($n->getMimetype(), Constants::IMAGE_FORMATS)) { continue; } - $this->onMove($ownerId, $usersToAdd, $targetUserIds, $n); + $this->onMove($usersToAdd, $targetUserIds, $n); } } catch (NotFoundException|Exception|InvalidPathException $e) { $this->logger->warning('Error in recognize file listener', ['exception' => $e]); } return; } + $detectionCountByUser = $this->getDetectionCountByUser($node->getId()); + if (count($detectionCountByUser) === 0) { + // Nothing detected for this file (yet): nothing to copy, nothing to prune + return; + } + $sourceUserId = (string)array_key_first($detectionCountByUser); foreach ($usersToAdd as $userId) { - if (count($this->faceDetectionMapper->findByFileIdAndUser($node->getId(), $userId)) > 0) { + if (isset($detectionCountByUser[(string)$userId])) { continue; } - $this->faceDetectionMapper->copyDetectionsForFileFromUserToUser($node->getId(), $ownerId, $userId); - $this->jobList->add(ClusterFacesJob::class, ['userId' => $userId]); + $this->faceDetectionMapper->copyDetectionsForFileFromUserToUser($node->getId(), $sourceUserId, (string)$userId); + $this->jobList->add(ClusterFacesJob::class, ['userId' => (string)$userId]); } $this->faceDetectionMapper->removeDetectionsForFileFromUsersNotInList($node->getId(), $targetUserIds); }