Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
25 changes: 14 additions & 11 deletions lib/Db/FsActionMapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -227,21 +227,24 @@

/**
* @param int $nodeId
* @param string $owner
* @param list<string> $addedUsers
* @param list<string> $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 {

Check failure on line 240 in lib/Db/FsActionMapper.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis

ParseError

lib/Db/FsActionMapper.php:240:3: ParseError: Cannot use try without catch or finally on line 240 (see https://psalm.dev/173)
$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)) {
Expand Down
9 changes: 2 additions & 7 deletions lib/Db/FsMove.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,24 +15,19 @@
* @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';

Expand All @@ -48,14 +43,14 @@
* @return list<string>
*/
public function getAddedUsers(): array {
return explode(',', $this->addedUsers ?? '');

Check failure on line 46 in lib/Db/FsMove.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis

MixedArgument

lib/Db/FsMove.php:46:23: MixedArgument: Argument 2 of explode cannot be ''|mixed, expecting string (see https://psalm.dev/030)

Check failure on line 46 in lib/Db/FsMove.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis

UndefinedThisPropertyFetch

lib/Db/FsMove.php:46:23: UndefinedThisPropertyFetch: Instance property OCA\Recognize\Db\FsMove::$addedUsers is not defined (see https://psalm.dev/041)
}

/**
* @return list<string>
*/
public function getTargetUsers(): array {
return explode(',', $this->targetUsers ?? '');

Check failure on line 53 in lib/Db/FsMove.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis

MixedArgument

lib/Db/FsMove.php:53:23: MixedArgument: Argument 2 of explode cannot be ''|mixed, expecting string (see https://psalm.dev/030)

Check failure on line 53 in lib/Db/FsMove.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis

UndefinedThisPropertyFetch

lib/Db/FsMove.php:53:23: UndefinedThisPropertyFetch: Instance property OCA\Recognize\Db\FsMove::$targetUsers is not defined (see https://psalm.dev/041)
}

/**
Expand Down
27 changes: 1 addition & 26 deletions lib/Hooks/FileListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -37,9 +35,6 @@
final class FileListener implements IEventListener {
private ?bool $movingFromIgnoredTerritory;
private ?array $movingDirFromIgnoredTerritory;
/** @var list<string> */
private array $sourceUserIds;
private ?Node $source = null;

/** @var array<string, bool> */
private array $addedMounts = [];
Expand All @@ -48,28 +43,10 @@
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<string>
* @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 {
Expand Down Expand Up @@ -112,8 +89,6 @@
} else {
$this->movingDirFromIgnoredTerritory = $this->getDirIgnores($event->getSource());
}
$this->sourceUserIds = $this->getUsersWithFileAccess($event->getSource()->getId());
$this->source = $event->getSource();
return;
}
if ($event instanceof NodeRenamedEvent) {
Expand Down Expand Up @@ -171,7 +146,7 @@
return;
}
}
$this->postRename($this->source ?? $event->getSource(), $event->getTarget());
$this->postRename($event->getTarget());

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

View workflow job for this annotation

GitHub Actions / static-psalm-analysis

TooFewArguments

lib/Hooks/FileListener.php:149:12: TooFewArguments: Too few arguments for method OCA\Recognize\Hooks\FileListener::postrename saw 1 (see https://psalm.dev/025)

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

View workflow job for this annotation

GitHub Actions / static-psalm-analysis

TooFewArguments

lib/Hooks/FileListener.php:149:12: TooFewArguments: Too few arguments for OCA\Recognize\Hooks\FileListener::postRename - expecting target to be passed (see https://psalm.dev/025)
return;
}
if ($event instanceof BeforeNodeDeletedEvent) {
Expand Down Expand Up @@ -283,9 +258,9 @@
* @throws Exception
*/
public function postRename(Node $source, Node $target): void {
$targetUserIds = $this->getUsersWithFileAccess($target->getId());

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

View workflow job for this annotation

GitHub Actions / static-psalm-analysis

UndefinedMethod

lib/Hooks/FileListener.php:261:27: UndefinedMethod: Method OCA\Recognize\Hooks\FileListener::getUsersWithFileAccess does not exist (see https://psalm.dev/022)

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

View workflow job for this annotation

GitHub Actions / static-psalm-analysis

MixedAssignment

lib/Hooks/FileListener.php:261:3: MixedAssignment: Unable to determine the type that $targetUserIds is being assigned to (see https://psalm.dev/032)

$usersToAdd = array_values(array_diff($targetUserIds, $this->sourceUserIds));

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

View workflow job for this annotation

GitHub Actions / static-psalm-analysis

MixedArgument

lib/Hooks/FileListener.php:263:41: MixedArgument: Argument 1 of array_diff cannot be mixed, expecting array<array-key, mixed> (see https://psalm.dev/030)
$existingUsers = array_diff($targetUserIds, $usersToAdd);
$sourceOwner = $source->getOwner();
$targetOwner = $target->getOwner();
Expand Down
47 changes: 47 additions & 0 deletions lib/Migration/Version013002000Date20260924120000.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
<?php

/*
* Copyright (c) 2026 The Recognize contributors.
* This file is licensed under the Affero General Public License version 3 or later. See the COPYING file.
*/
declare(strict_types=1);
namespace OCA\Recognize\Migration;

use Closure;
use Doctrine\DBAL\Schema\SchemaException;
use OCP\DB\ISchemaWrapper;
use OCP\Migration\IOutput;
use OCP\Migration\SimpleMigrationStep;

/**
* Drops the move payload columns: users with access are resolved when a move is processed.
*/
final class Version013002000Date20260924120000 extends SimpleMigrationStep {

/**
* @param IOutput $output
* @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper`
* @param array $options
*
* @return ?ISchemaWrapper
* @throws SchemaException
*/
public function changeSchema(IOutput $output, Closure $schemaClosure, array $options) {
/** @var ISchemaWrapper $schema */
$schema = $schemaClosure();

if (!$schema->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;
}
}
12 changes: 7 additions & 5 deletions lib/Service/FsActionService.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@

final class FsActionService {
public const BATCH_SIZE = 1000;

public function __construct(
private FsActionMapper $fsActionMapper,
private LoggerInterface $logger,
Expand Down Expand Up @@ -75,6 +76,9 @@ public function processActionsByClass(string $className): void {
* @param array<FsCreation|FsDeletion|FsMove|FsAccessUpdate> $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) {
Expand Down Expand Up @@ -149,7 +153,6 @@ public function processActions(array $actions): void {
* @return list<string>
*/
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();
Expand All @@ -164,7 +167,6 @@ 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) {
Expand Down Expand Up @@ -336,13 +338,13 @@ private function onMove(string $ownerId, array $usersToAdd, array $targetUserIds
if ($node instanceof Folder) {
try {
foreach ($node->getDirectoryListing() as $n) {
if (!in_array($n->getMimetype(), Constants::IMAGE_FORMATS)) {
// 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;
}
$this->onMove($ownerId, $usersToAdd, $targetUserIds, $n);
}
} catch (NotFoundException|Exception|InvalidPathException $e) {
$this->logger->warning('Error in recognize file listener', ['exception' => $e]);
}
return;
}
Expand Down
5 changes: 5 additions & 0 deletions psalm-baseline.xml
Original file line number Diff line number Diff line change
Expand Up @@ -965,7 +965,12 @@
</PossiblyFalseArgument>
</file>
<file src="lib/Service/FsActionService.php">
<RedundantCast>
<code><![CDATA[(string)array_key_first($detectionCountByUser)]]></code>
</RedundantCast>
<RedundantCastGivenDocblockType>
<code><![CDATA[(string)$detection->getUserId()]]></code>
<code><![CDATA[(string)$userId]]></code>
<code><![CDATA[(string)$userId]]></code>
</RedundantCastGivenDocblockType>
<UndefinedClass>
Expand Down
Loading