diff --git a/appinfo/info.xml b/appinfo/info.xml
index e7e201b50..6ea182e38 100644
--- a/appinfo/info.xml
+++ b/appinfo/info.xml
@@ -75,7 +75,7 @@ Requirements:
The app does not send any sensitive data to cloud providers or similar services. All processing is done on your Nextcloud machine, using Tensorflow.js running in Node.js.
]]>
- 13.1.0
+ 13.2.0-dev
agpl
Marcel Klehr
diff --git a/lib/Classifiers/Images/ClusteringFaceClassifier.php b/lib/Classifiers/Images/ClusteringFaceClassifier.php
index 81eff74a6..dcdcad4c7 100644
--- a/lib/Classifiers/Images/ClusteringFaceClassifier.php
+++ b/lib/Classifiers/Images/ClusteringFaceClassifier.php
@@ -92,6 +92,10 @@ public function classify(array $queueFiles): void {
$classifierProcess = $this->classifyFiles(self::MODEL_NAME, $filteredQueueFiles, $timeout);
+ // The mount tables are read for every file in this batch, so refresh them once here
+ // rather than on every lookup.
+ $this->userMountCache->clear();
+
/**
* @var list $faces
*/
diff --git a/lib/Db/FsActionMapper.php b/lib/Db/FsActionMapper.php
index 56d46585c..2b92ec409 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);
@@ -227,26 +223,34 @@ public function insertDeletion(int $storageId, int $nodeId): FsCreation|FsDeleti
/**
* @param int $nodeId
- * @param string $owner
- * @param list $addedUsers
- * @param list $targetUsers
- * @return FsCreation|FsDeletion|FsMove|FsAccessUpdate
- * @throws Exception|MultipleObjectsReturnedException
+ * @return FsMove
+ * @throws Exception
*/
- public function insertMove(int $nodeId, string $owner, array $addedUsers, array $targetUsers): Entity {
+ public function insertMove(int $nodeId): FsMove {
+ // A move for this node may still be pending. Replace it with a fresh row rather than
+ // keeping it: the job deletes the rows it has processed by ID, so a row the job was
+ // already working on would be deleted without this move having been processed. A
+ // fresh row has a new ID, which the running job doesn't know about, so it survives
+ // until the next run.
+ $this->db->beginTransaction();
try {
- $move = $this->findByNodeId(FsMove::class, $nodeId);
- } catch (DoesNotExistException $e) {
+ $qb = $this->db->getQueryBuilder();
+ $qb->delete(FsMove::$tableName)
+ ->where($qb->expr()->eq('node_id', $qb->createPositionalParameter($nodeId, IQueryBuilder::PARAM_INT)));
+ $qb->executeStatement();
+
$move = new FsMove();
$move->setNodeId($nodeId);
- $move->setOwner($owner);
- $move->setAddedUsers($addedUsers);
- $move->setTargetUsers($targetUsers);
$this->insert($move);
- $arguments = [ 'type' => FsDeletion::class ];
- if (!$this->jobList->has(ProcessFsActionsJob::class, $arguments)) {
- $this->jobList->add(ProcessFsActionsJob::class, $arguments);
- }
+ $this->db->commit();
+ } catch (\Throwable $e) {
+ $this->db->rollBack();
+ throw $e;
+ }
+
+ $arguments = [ 'type' => FsMove::class ];
+ if (!$this->jobList->has(ProcessFsActionsJob::class, $arguments)) {
+ $this->jobList->add(ProcessFsActionsJob::class, $arguments);
}
return $move;
}
diff --git a/lib/Db/FsMove.php b/lib/Db/FsMove.php
index 198994b24..de0ea5427 100644
--- a/lib/Db/FsMove.php
+++ b/lib/Db/FsMove.php
@@ -15,60 +15,24 @@
* @package OCA\Recognize\Db
* @method int getNodeId()
* @method setNodeId(int $nodeId)
- * @method string getOwner()
- * @method setOwner(string $owner)
*/
final class FsMove extends Entity {
protected ?int $nodeId = null;
- protected ?string $owner = null;
- protected ?string $addedUsers = null;
- protected ?string $targetUsers = null;
/**
* @var string[]
*/
- public static array $columns = ['id', 'node_id', 'owner', 'added_users', 'target_users'];
+ public static array $columns = ['id', 'node_id'];
/**
* @var string[]
*/
- public static array $fields = ['id', 'nodeId', 'owner', 'addedUsers', 'targetUsers'];
+ public static array $fields = ['id', 'nodeId'];
public static string $tableName = 'recognize_fs_moves';
public function __construct() {
// add types in constructor
$this->addType('nodeId', 'integer');
- $this->addType('owner', 'string');
- $this->addType('addedUsers', 'string');
- $this->addType('targetUsers', 'string');
- }
-
- /**
- * @return list
- */
- public function getAddedUsers(): array {
- return explode(',', $this->addedUsers ?? '');
- }
-
- /**
- * @return list
- */
- public function getTargetUsers(): array {
- return explode(',', $this->targetUsers ?? '');
- }
-
- /**
- * @param list $users
- */
- public function setAddedUsers(array $users): void {
- $this->setter('addedUsers', [implode(',', $users)]);
- }
-
- /**
- * @param list $users
- */
- public function setTargetUsers(array $users): void {
- $this->setter('targetUsers', [implode(',', $users)]);
}
}
diff --git a/lib/Hooks/FileListener.php b/lib/Hooks/FileListener.php
index 1426b6384..17b8681d9 100644
--- a/lib/Hooks/FileListener.php
+++ b/lib/Hooks/FileListener.php
@@ -15,8 +15,6 @@
use OCP\EventDispatcher\Event;
use OCP\EventDispatcher\IEventListener;
use OCP\Files\Cache\CacheEntryInsertedEvent;
-use OCP\Files\Config\ICachedMountInfo;
-use OCP\Files\Config\IUserMountCache;
use OCP\Files\Events\Node\BeforeNodeDeletedEvent;
use OCP\Files\Events\Node\BeforeNodeRenamedEvent;
use OCP\Files\Events\Node\NodeCreatedEvent;
@@ -37,9 +35,6 @@
final class FileListener implements IEventListener {
private ?bool $movingFromIgnoredTerritory;
private ?array $movingDirFromIgnoredTerritory;
- /** @var list */
- private array $sourceUserIds;
- private ?Node $source = null;
/** @var array */
private array $addedMounts = [];
@@ -48,28 +43,10 @@ public function __construct(
private LoggerInterface $logger,
private IgnoreService $ignoreService,
private IRootFolder $rootFolder,
- private IUserMountCache $userMountCache,
private FsActionMapper $fsActionMapper,
) {
$this->movingFromIgnoredTerritory = null;
$this->movingDirFromIgnoredTerritory = null;
- $this->sourceUserIds = [];
- }
-
- /**
- * @param int $nodeId
- * @return list
- * @throws InvalidPathException
- * @throws NotFoundException
- */
- private function getUsersWithFileAccess(int $nodeId): array {
- $this->userMountCache->clear();
- $mountInfos = $this->userMountCache->getMountsForFileId($nodeId);
- $userIds = array_map(static function (ICachedMountInfo $mountInfo) {
- return $mountInfo->getUser()->getUID();
- }, $mountInfos);
-
- return array_values(array_unique($userIds));
}
public function handle(Event $event): void {
@@ -112,8 +89,6 @@ public function handle(Event $event): void {
} else {
$this->movingDirFromIgnoredTerritory = $this->getDirIgnores($event->getSource());
}
- $this->sourceUserIds = $this->getUsersWithFileAccess($event->getSource()->getId());
- $this->source = $event->getSource();
return;
}
if ($event instanceof NodeRenamedEvent) {
@@ -171,7 +146,7 @@ public function handle(Event $event): void {
return;
}
}
- $this->postRename($this->source ?? $event->getSource(), $event->getTarget());
+ $this->postRename($event->getTarget());
return;
}
if ($event instanceof BeforeNodeDeletedEvent) {
@@ -282,20 +257,14 @@ public function postInsert(Node $node, bool $recurse = true, ?array $mimeTypes =
* @throws NotFoundException
* @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];
-
+ public function postRename(Node $target): void {
if (preg_match('#^/[^/]*?/files/#', $target->getPath()) !== 1 && preg_match('#^/groupfolders/#', $target->getPath()) !== 1) {
return;
}
- $this->fsActionMapper->insertMove($target->getId(), $ownerId, $usersToAdd, $targetUserIds);
+ // Who has access to the moved node, and to each file below it, is resolved when
+ // the move is processed, so the node ID is all that needs to be recorded.
+ $this->fsActionMapper->insertMove($target->getId());
}
/**
diff --git a/lib/Migration/Version013002000Date20260924120000.php b/lib/Migration/Version013002000Date20260924120000.php
new file mode 100644
index 000000000..0f0bab26c
--- /dev/null
+++ b/lib/Migration/Version013002000Date20260924120000.php
@@ -0,0 +1,47 @@
+hasTable('recognize_fs_moves')) {
+ return null;
+ }
+
+ $changed = false;
+ $table = $schema->getTable('recognize_fs_moves');
+ foreach (['owner', 'added_users', 'target_users'] as $column) {
+ if ($table->hasColumn($column)) {
+ $table->dropColumn($column);
+ $changed = true;
+ }
+ }
+ return $changed ? $schema : null;
+ }
+}
diff --git a/lib/Service/FsActionService.php b/lib/Service/FsActionService.php
index 23bd2b643..0d7ec0bc2 100644
--- a/lib/Service/FsActionService.php
+++ b/lib/Service/FsActionService.php
@@ -32,6 +32,7 @@
final class FsActionService {
public const BATCH_SIZE = 1000;
+
public function __construct(
private FsActionMapper $fsActionMapper,
private LoggerInterface $logger,
@@ -75,6 +76,9 @@ public function processActionsByClass(string $className): void {
* @param array $actions
*/
public function processActions(array $actions): void {
+ // The mount tables are read repeatedly while processing a batch, so refresh them
+ // once here rather than on every lookup.
+ $this->userMountCache->clear();
$lastUserId = null;
foreach ($actions as $action) {
switch ($action::class) {
@@ -125,7 +129,7 @@ public function processActions(array $actions): void {
break;
}
try {
- $this->onMove($action->getOwner(), $action->getAddedUsers(), $action->getTargetUsers(), $node);
+ $this->onMove($node);
} catch (Exception|InvalidPathException|NotFoundException $e) {
$this->logger->warning('Failed to process move action: ' . $e->getMessage() . ' Continuing.', ['exception' => $e]);
}
@@ -149,7 +153,6 @@ public function processActions(array $actions): void {
* @return list
*/
private function getUsersWithFileAccess(int $nodeId): array {
- $this->userMountCache->clear();
$mountInfos = $this->userMountCache->getMountsForFileId($nodeId);
$userIds = array_map(static function (ICachedMountInfo $mountInfo) {
return $mountInfo->getUser()->getUID();
@@ -164,37 +167,73 @@ private function getUsersWithFileAccess(int $nodeId): array {
* @throws Exception
*/
private function onAccessUpdate(int $storageId, int $rootId): void {
- $userIds = $this->getUsersWithFileAccess($rootId);
$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) {
- continue;
- }
- $detectionsForFile = $this->faceDetectionMapper->findByFileId($fileInfo['fileid']);
- $userHasDetectionForFile = [];
- foreach ($detectionsForFile as $detection) {
- $userHasDetectionForFile[$detection->getUserId()] = true;
- }
- foreach ($userIds as $userId) {
- if ($userId === $ownerId) {
- continue;
- }
- if ($userHasDetectionForFile[$userId] ?? false) {
- continue;
- }
- $this->faceDetectionMapper->copyDetectionsForFileFromUserToUser($fileInfo['fileid'], $ownerId, $userId);
- $userIdsToScheduleClustering[$userId] = true;
- }
- $this->faceDetectionMapper->removeDetectionsForFileFromUsersNotInList($fileInfo['fileid'], $userIds);
+ $this->syncDetectionsForFile($fileInfo['fileid'], $userIdsToScheduleClustering);
}
foreach (array_keys($userIdsToScheduleClustering) as $userId) {
$this->jobList->add(ClusterFacesJob::class, ['userId' => (string)$userId]);
}
}
+ /**
+ * Gives every user with access to the file a copy of its face detections and removes
+ * the detections of users who lost access.
+ *
+ * Access is resolved per file rather than taken from an ancestor: a descendant can be
+ * shared directly, in which case it is reachable by users the ancestor is not shared
+ * with, and pruning against the ancestor's list would delete detections they still
+ * have access to.
+ *
+ * @param array $userIdsToScheduleClustering
+ * @throws Exception
+ */
+ private function syncDetectionsForFile(int $fileId, array &$userIdsToScheduleClustering): void {
+ $detectionCountByUser = $this->getDetectionCountByUser($fileId);
+ if (count($detectionCountByUser) === 0) {
+ // Nothing detected for this file (yet): nothing to copy, nothing to prune
+ return;
+ }
+ $targetUserIds = $this->getUsersWithFileAccess($fileId);
+ if (count($targetUserIds) === 0) {
+ // No mounts found, e.g. because the file vanished in the meantime: don't treat
+ // that as everyone having lost access
+ return;
+ }
+ $sourceUserId = (string)array_key_first($detectionCountByUser);
+ foreach ($targetUserIds as $userId) {
+ if (isset($detectionCountByUser[$userId])) {
+ continue;
+ }
+ $this->faceDetectionMapper->copyDetectionsForFileFromUserToUser($fileId, $sourceUserId, $userId);
+ $userIdsToScheduleClustering[$userId] = true;
+ }
+ $this->faceDetectionMapper->removeDetectionsForFileFromUsersNotInList($fileId, $targetUserIds);
+ }
+
+ /**
+ * 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,34 +364,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(Node $node): void {
+ $userIdsToScheduleClustering = [];
+ $this->moveNode($node, $userIdsToScheduleClustering);
+ foreach (array_keys($userIdsToScheduleClustering) as $userId) {
+ $this->jobList->add(ClusterFacesJob::class, ['userId' => (string)$userId]);
+ }
+ }
+
+ /**
+ * @param array $userIdsToScheduleClustering
+ * @throws Exception|InvalidPathException|NotFoundException
+ */
+ private function moveNode(Node $node, array &$userIdsToScheduleClustering): 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);
+ foreach ($node->getDirectoryListing() as $n) {
+ // Recurse into subfolders: we only get a rename event for the top node,
+ // so the whole subtree has to be walked here.
+ if ($n->getType() !== FileInfo::TYPE_FOLDER && !in_array($n->getMimetype(), Constants::IMAGE_FORMATS)) {
+ continue;
+ }
+ try {
+ $this->moveNode($n, $userIdsToScheduleClustering);
+ } catch (NotFoundException|Exception|InvalidPathException $e) {
+ // Per child, so one unreadable node doesn't abandon the rest of the subtree:
+ // the action row is deleted either way, so skipped files are never retried.
+ $this->logger->warning('Failed to process move for node ' . $n->getId(), ['exception' => $e]);
}
- } catch (NotFoundException|Exception|InvalidPathException $e) {
- $this->logger->warning('Error in recognize file listener', ['exception' => $e]);
}
return;
}
- foreach ($usersToAdd as $userId) {
- if (count($this->faceDetectionMapper->findByFileIdAndUser($node->getId(), $userId)) > 0) {
- continue;
- }
- $this->faceDetectionMapper->copyDetectionsForFileFromUserToUser($node->getId(), $ownerId, $userId);
- $this->jobList->add(ClusterFacesJob::class, ['userId' => $userId]);
- }
- $this->faceDetectionMapper->removeDetectionsForFileFromUsersNotInList($node->getId(), $targetUserIds);
+ $this->syncDetectionsForFile($node->getId(), $userIdsToScheduleClustering);
}
}
diff --git a/lib/TaskProcessing/TaskResultListener.php b/lib/TaskProcessing/TaskResultListener.php
index bfcad9f6f..97f5764e8 100644
--- a/lib/TaskProcessing/TaskResultListener.php
+++ b/lib/TaskProcessing/TaskResultListener.php
@@ -194,6 +194,9 @@ private function applyTagResults(array $fileIds, array $results, string $model,
private function applyFaceResults(array $fileIds, array $results): void {
$model = ClusteringFaceClassifier::MODEL_NAME;
$scheduledClusterJobsFor = [];
+ // The mount tables are read for every file in this batch, so refresh them once here
+ // rather than on every lookup.
+ $this->userMountCache->clear();
foreach ($fileIds as $i => $fileId) {
if (!isset($results[$i])) {
continue;
diff --git a/psalm-baseline.xml b/psalm-baseline.xml
index 190286a67..899e81642 100644
--- a/psalm-baseline.xml
+++ b/psalm-baseline.xml
@@ -1,10 +1,5 @@
-
-
- getContainer()->get(PropFindPlugin::class)]]>
-
-
getUserId()]]>
+