From 6b798d5bc73988a6891f3a4bf118297824ed1b1a Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Wed, 4 Nov 2026 11:52:43 +0100 Subject: [PATCH 1/4] feat(settings): log setting changes to the audit log Changing a Recognize setting through SettingsService::setSetting (the admin settings page) now writes a `recognize.setting.changed` entry with the setting key, the new value and the previous value. The values are booleans, numbers and binary paths; none of them are secrets. The binary paths (node, ffmpeg) are executed by the app, so their new value is part of the entry. Only actual changes are logged: the admin page re-submits every setting on each save, so writing an unchanged value produces no entry. Progress state that the background jobs write through the same method (*.status, *.lastFile, clusterFaces.lastRun) is not configuration and is not logged. Settings changed with occ config:app:set bypass this service and are covered by the occ command line entry admin_audit writes itself. All entries are built by AuditEventFactory, which passes the operation only to servers whose CriticalActionPerformedEvent supports it. Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Marcel Klehr --- lib/AuditLog/AuditEventFactory.php | 25 ++++++ lib/AuditLog/Operation.php | 23 +++++ lib/Service/SettingsService.php | 36 +++++++- tests/AuditLog/AuditLogTestHelper.php | 19 +++++ tests/AuditLog/OperationTest.php | 32 +++++++ tests/Service/SettingsServiceAuditTest.php | 98 ++++++++++++++++++++++ tests/bootstrap.php | 2 + 7 files changed, 234 insertions(+), 1 deletion(-) create mode 100644 lib/AuditLog/AuditEventFactory.php create mode 100644 lib/AuditLog/Operation.php create mode 100644 tests/AuditLog/AuditLogTestHelper.php create mode 100644 tests/AuditLog/OperationTest.php create mode 100644 tests/Service/SettingsServiceAuditTest.php diff --git a/lib/AuditLog/AuditEventFactory.php b/lib/AuditLog/AuditEventFactory.php new file mode 100644 index 000000000..fdcec9624 --- /dev/null +++ b/lib/AuditLog/AuditEventFactory.php @@ -0,0 +1,25 @@ + $parameters values for the placeholders of $message, in order, optionally followed by more context + */ + public static function create(Operation $operation, string $message, array $parameters = []): CriticalActionPerformedEvent { + $arguments = [$message, $parameters, false]; + if (method_exists(CriticalActionPerformedEvent::class, 'getOperation')) { + $arguments[] = $operation->value; + } + return new CriticalActionPerformedEvent(...$arguments); + } +} diff --git a/lib/AuditLog/Operation.php b/lib/AuditLog/Operation.php new file mode 100644 index 000000000..a3be8a14b --- /dev/null +++ b/lib/AuditLog/Operation.php @@ -0,0 +1,23 @@ + 'An admin changed a Recognize setting to a different value.', + }; + } +} diff --git a/lib/Service/SettingsService.php b/lib/Service/SettingsService.php index adfeb1fd7..abb0bfeaf 100644 --- a/lib/Service/SettingsService.php +++ b/lib/Service/SettingsService.php @@ -9,6 +9,8 @@ namespace OCA\Recognize\Service; +use OCA\Recognize\AuditLog\AuditEventFactory; +use OCA\Recognize\AuditLog\Operation; use OCA\Recognize\BackgroundJobs\SchedulerJob; use OCA\Recognize\Classifiers\Audio\MusicnnClassifier; use OCA\Recognize\Classifiers\Images\ClusteringFaceClassifier; @@ -18,6 +20,7 @@ use OCA\Recognize\Exception\Exception; use OCP\AppFramework\Services\IAppConfig; use OCP\BackgroundJob\IJobList; +use OCP\EventDispatcher\IEventDispatcher; use OCP\Server; final class SettingsService { @@ -99,12 +102,32 @@ final class SettingsService { 'clusterFaces.concurrency', ]; + /** + * Progress state written by the background jobs, not configuration + */ + private const STATE_SETTINGS = [ + 'clusterFaces.status', + 'faces.status', + 'imagenet.status', + 'landmarks.status', + 'movinet.status', + 'musicnn.status', + 'clusterFaces.lastRun', + 'faces.lastFile', + 'imagenet.lastFile', + 'landmarks.lastFile', + 'movinet.lastFile', + 'musicnn.lastFile', + ]; + private IAppConfig $config; private IJobList $jobList; + private IEventDispatcher $eventDispatcher; - public function __construct(IAppConfig $config, IJobList $jobList) { + public function __construct(IAppConfig $config, IJobList $jobList, IEventDispatcher $eventDispatcher) { $this->config = $config; $this->jobList = $jobList; + $this->eventDispatcher = $eventDispatcher; } /** @@ -158,6 +181,8 @@ public function setSetting(string $key, string $value): void { if (!array_key_exists($key, self::DEFAULTS)) { throw new Exception('Unknown settings key '.$key); } + $isConfiguration = !in_array($key, self::STATE_SETTINGS, true); + $oldValue = $isConfiguration ? $this->getSetting($key) : null; if ($value === 'true' && $this->config->getAppValueString($key, 'false') === 'false') { // Additional model enabled: Schedule new crawl run for the affected mime types switch ($key) { @@ -185,6 +210,15 @@ public function setSetting(string $key, string $value): void { $lazy = true; } $this->config->setAppValueString($key, $value, lazy: $lazy); + + // The admin UI saves all settings at once, so only actual changes are logged + if ($isConfiguration && $oldValue !== $value) { + $this->eventDispatcher->dispatchTyped(AuditEventFactory::create( + Operation::SettingChanged, + 'Recognize setting "%s" was changed to "%s"', + ['key' => $key, 'value' => $value, 'oldValue' => $oldValue], + )); + } } /** diff --git a/tests/AuditLog/AuditLogTestHelper.php b/tests/AuditLog/AuditLogTestHelper.php new file mode 100644 index 000000000..b52f1018e --- /dev/null +++ b/tests/AuditLog/AuditLogTestHelper.php @@ -0,0 +1,19 @@ +getOperation() === $operation; + } +} diff --git a/tests/AuditLog/OperationTest.php b/tests/AuditLog/OperationTest.php new file mode 100644 index 000000000..e2c9b5f68 --- /dev/null +++ b/tests/AuditLog/OperationTest.php @@ -0,0 +1,32 @@ + [$operation], Operation::cases()); + } + + /** + * @dataProvider operationProvider + */ + public function testOperationFormat(Operation $operation): void { + $this->assertMatchesRegularExpression('/^recognize\.[a-z][a-z0-9_]*\.[a-z][a-z0-9_]*$/', $operation->value); + } + + /** + * @dataProvider operationProvider + */ + public function testOperationHasDescription(Operation $operation): void { + $this->assertNotSame('', trim($operation->description())); + } +} diff --git a/tests/Service/SettingsServiceAuditTest.php b/tests/Service/SettingsServiceAuditTest.php new file mode 100644 index 000000000..e210e0c7b --- /dev/null +++ b/tests/Service/SettingsServiceAuditTest.php @@ -0,0 +1,98 @@ + */ + private array $stored = []; + + protected function setUp(): void { + parent::setUp(); + $this->stored = []; + $this->config = $this->createMock(IAppConfig::class); + $this->config->method('getAppValueString') + ->willReturnCallback(fn (string $key, string $default = '', bool $lazy = false): string => $this->stored[$key] ?? $default); + $this->config->method('setAppValueString') + ->willReturnCallback(function (string $key, string $value, bool $lazy = false): bool { + $this->stored[$key] = $value; + return true; + }); + $this->jobList = $this->createMock(IJobList::class); + $this->eventDispatcher = $this->createMock(IEventDispatcher::class); + $this->service = new SettingsService($this->config, $this->jobList, $this->eventDispatcher); + } + + public function testChangedSettingIsLogged(): void { + $this->stored['node_binary'] = '/usr/bin/node'; + + $this->eventDispatcher->expects($this->once()) + ->method('dispatchTyped') + ->with($this->callback(function (CriticalActionPerformedEvent $event): bool { + $this->assertSame('Recognize setting "%s" was changed to "%s"', $event->getLogMessage()); + $this->assertSame(['key' => 'node_binary', 'value' => '/opt/node/bin/node', 'oldValue' => '/usr/bin/node'], $event->getParameters()); + $this->assertTrue(AuditLogTestHelper::hasOperation($event, 'recognize.setting.changed')); + return true; + })); + + $this->service->setSetting('node_binary', '/opt/node/bin/node'); + } + + public function testChangeFromDefaultIsLogged(): void { + $this->eventDispatcher->expects($this->once()) + ->method('dispatchTyped') + ->with($this->callback(function (CriticalActionPerformedEvent $event): bool { + $this->assertSame(['key' => 'tensorflow.cores', 'value' => '4', 'oldValue' => '0'], $event->getParameters()); + return true; + })); + + $this->service->setSetting('tensorflow.cores', '4'); + } + + public function testUnchangedSettingIsNotLogged(): void { + $this->stored['faces.enabled'] = 'true'; + $this->eventDispatcher->expects($this->never())->method('dispatchTyped'); + + $this->service->setSetting('faces.enabled', 'true'); + } + + public function testSettingDefaultValueIsNotLogged(): void { + $this->eventDispatcher->expects($this->never())->method('dispatchTyped'); + + $this->service->setSetting('imagenet.enabled', 'false'); + } + + public function testStateSettingsAreNotLogged(): void { + $this->eventDispatcher->expects($this->never())->method('dispatchTyped'); + + $this->service->setSetting('faces.status', 'true'); + $this->service->setSetting('faces.lastFile', '1700000000'); + $this->service->setSetting('clusterFaces.lastRun', '1700000000'); + } + + public function testUnknownSettingIsNotLogged(): void { + $this->eventDispatcher->expects($this->never())->method('dispatchTyped'); + $this->expectException(Exception::class); + + $this->service->setSetting('unknown', 'value'); + } +} diff --git a/tests/bootstrap.php b/tests/bootstrap.php index 20c82f957..616277aa4 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -16,3 +16,5 @@ \OCP\Server::get(\OC\Files\Node\HookConnector::class)->viewToNode(); // Do not clear hooks, we need them for our tests! // OC_Hook::clear(); + +require_once __DIR__ . '/AuditLog/AuditLogTestHelper.php'; From 940be595147aeb462dc42ccee01f3b0522d36dde Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Wed, 4 Nov 2026 11:53:52 +0100 Subject: [PATCH 2/4] feat(reset): log bulk resets of classifications and faces to the audit log Resetting data of all users now writes an audit entry: - recognize.classifications.reset: TagManager::resetClassifications removed the Recognize classification tags from all files (admin page "reset tags", occ recognize:reset-tags), with the number of files. - recognize.face_clusters.deleted_all: FaceClusterMapper::deleteAll deleted every face cluster (admin page "reset faces", occ recognize:reset-faces, occ recognize:reset-face-clusters), with the number of deleted rows. - recognize.face_detections.deleted_all: FaceDetectionMapper::deleteAll deleted every face detection (admin page "reset faces", occ recognize:reset-faces), with the number of deleted rows. The admin controller and the occ commands call the mappers directly, so the mappers are the one place that covers every entry point. Resetting faces therefore writes one entry per table. Nothing is logged when there was nothing to delete or when the deletion failed. Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Marcel Klehr --- lib/AuditLog/Operation.php | 6 ++ lib/Db/FaceClusterMapper.php | 17 +++- lib/Db/FaceDetectionMapper.php | 16 +++- lib/Service/TagManager.php | 14 +++- tests/Db/FaceMapperAuditTest.php | 107 ++++++++++++++++++++++++++ tests/Service/TagManagerAuditTest.php | 88 +++++++++++++++++++++ 6 files changed, 243 insertions(+), 5 deletions(-) create mode 100644 tests/Db/FaceMapperAuditTest.php create mode 100644 tests/Service/TagManagerAuditTest.php diff --git a/lib/AuditLog/Operation.php b/lib/AuditLog/Operation.php index a3be8a14b..da7b173f8 100644 --- a/lib/AuditLog/Operation.php +++ b/lib/AuditLog/Operation.php @@ -14,10 +14,16 @@ */ enum Operation: string { case SettingChanged = 'recognize.setting.changed'; + case ClassificationsReset = 'recognize.classifications.reset'; + case FaceClustersDeletedAll = 'recognize.face_clusters.deleted_all'; + case FaceDetectionsDeletedAll = 'recognize.face_detections.deleted_all'; public function description(): string { return match ($this) { self::SettingChanged => 'An admin changed a Recognize setting to a different value.', + self::ClassificationsReset => 'Recognize classification tags were removed from all files of all users.', + self::FaceClustersDeletedAll => 'All face clusters (persons) of all users were deleted.', + self::FaceDetectionsDeletedAll => 'All face detections of all users were deleted.', }; } } diff --git a/lib/Db/FaceClusterMapper.php b/lib/Db/FaceClusterMapper.php index 0212a12a3..99a15e6d7 100644 --- a/lib/Db/FaceClusterMapper.php +++ b/lib/Db/FaceClusterMapper.php @@ -7,16 +7,22 @@ declare(strict_types=1); namespace OCA\Recognize\Db; +use OCA\Recognize\AuditLog\AuditEventFactory; +use OCA\Recognize\AuditLog\Operation; use OCP\AppFramework\Db\Entity; use OCP\AppFramework\Db\QBMapper; use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\EventDispatcher\IEventDispatcher; use OCP\IDBConnection; /** * @template-extends QBMapper */ final class FaceClusterMapper extends QBMapper { - public function __construct(IDBConnection $db) { + public function __construct( + IDBConnection $db, + private IEventDispatcher $eventDispatcher, + ) { parent::__construct($db, 'recognize_face_clusters', FaceCluster::class); $this->db = $db; } @@ -104,7 +110,14 @@ public function delete(Entity $entity): Entity { */ public function deleteAll(): void { $qb = $this->db->getQueryBuilder(); - $qb->delete('recognize_face_clusters') + $count = $qb->delete('recognize_face_clusters') ->executeStatement(); + if ($count > 0) { + $this->eventDispatcher->dispatchTyped(AuditEventFactory::create( + Operation::FaceClustersDeletedAll, + 'Deleted all %s Recognize face clusters', + ['count' => $count], + )); + } } } diff --git a/lib/Db/FaceDetectionMapper.php b/lib/Db/FaceDetectionMapper.php index f5906e1fa..9722630fd 100644 --- a/lib/Db/FaceDetectionMapper.php +++ b/lib/Db/FaceDetectionMapper.php @@ -7,6 +7,8 @@ declare(strict_types=1); namespace OCA\Recognize\Db; +use OCA\Recognize\AuditLog\AuditEventFactory; +use OCA\Recognize\AuditLog\Operation; use OCA\Recognize\Classifiers\TaskProcessing\ImageFaceRecognitionClassifier; use OCA\Recognize\Service\FaceClusterAnalyzer; use OCA\Recognize\Service\SettingsService; @@ -15,6 +17,7 @@ use OCP\AppFramework\Db\QBMapper; use OCP\DB\Exception; use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\EventDispatcher\IEventDispatcher; use OCP\IConfig; use OCP\IDBConnection; @@ -24,12 +27,14 @@ final class FaceDetectionMapper extends QBMapper { private IConfig $config; private SettingsService $settingsService; + private IEventDispatcher $eventDispatcher; - public function __construct(IDBConnection $db, IConfig $config, SettingsService $settingsService) { + public function __construct(IDBConnection $db, IConfig $config, SettingsService $settingsService, IEventDispatcher $eventDispatcher) { parent::__construct($db, 'recognize_face_detections', FaceDetection::class); $this->db = $db; $this->config = $config; $this->settingsService = $settingsService; + $this->eventDispatcher = $eventDispatcher; } private function getMinDetectionSize(): float { @@ -91,8 +96,15 @@ public function insertWithoutDeduplication(Entity $entity): FaceDetection { */ public function deleteAll(): void { $qb = $this->db->getQueryBuilder(); - $qb->delete('recognize_face_detections') + $count = $qb->delete('recognize_face_detections') ->executeStatement(); + if ($count > 0) { + $this->eventDispatcher->dispatchTyped(AuditEventFactory::create( + Operation::FaceDetectionsDeletedAll, + 'Deleted all %s Recognize face detections', + ['count' => $count], + )); + } } /** diff --git a/lib/Service/TagManager.php b/lib/Service/TagManager.php index 99fd5abd4..14f8c0549 100644 --- a/lib/Service/TagManager.php +++ b/lib/Service/TagManager.php @@ -7,6 +7,9 @@ declare(strict_types=1); namespace OCA\Recognize\Service; +use OCA\Recognize\AuditLog\AuditEventFactory; +use OCA\Recognize\AuditLog\Operation; +use OCP\EventDispatcher\IEventDispatcher; use OCP\SystemTag\ISystemTag; use OCP\SystemTag\ISystemTagManager; use OCP\SystemTag\ISystemTagObjectMapper; @@ -17,10 +20,12 @@ final class TagManager { private ISystemTagManager $tagManager; private ISystemTagObjectMapper $objectMapper; + private IEventDispatcher $eventDispatcher; - public function __construct(ISystemTagManager $systemTagManager, ISystemTagObjectMapper $objectMapper) { + public function __construct(ISystemTagManager $systemTagManager, ISystemTagObjectMapper $objectMapper, IEventDispatcher $eventDispatcher) { $this->tagManager = $systemTagManager; $this->objectMapper = $objectMapper; + $this->eventDispatcher = $eventDispatcher; } /** @@ -138,6 +143,13 @@ public function resetClassifications(): void { foreach ($fileIds as $id) { $this->objectMapper->unassignTags($id, 'files', $tagIds); } + if (count($fileIds) > 0) { + $this->eventDispatcher->dispatchTyped(AuditEventFactory::create( + Operation::ClassificationsReset, + 'Removed Recognize classification tags from %s files', + ['fileCount' => count($fileIds)], + )); + } } /** diff --git a/tests/Db/FaceMapperAuditTest.php b/tests/Db/FaceMapperAuditTest.php new file mode 100644 index 000000000..73e653876 --- /dev/null +++ b/tests/Db/FaceMapperAuditTest.php @@ -0,0 +1,107 @@ +db = Server::get(IDBConnection::class); + $this->eventDispatcher = $this->createMock(IEventDispatcher::class); + $this->clusterMapper = new FaceClusterMapper($this->db, $this->eventDispatcher); + $this->detectionMapper = new FaceDetectionMapper($this->db, Server::get(IConfig::class), Server::get(SettingsService::class), $this->eventDispatcher); + $this->db->getQueryBuilder()->delete('recognize_face_clusters')->executeStatement(); + $this->db->getQueryBuilder()->delete('recognize_face_detections')->executeStatement(); + } + + protected function tearDown(): void { + $this->db->getQueryBuilder()->delete('recognize_face_clusters')->executeStatement(); + $this->db->getQueryBuilder()->delete('recognize_face_detections')->executeStatement(); + parent::tearDown(); + } + + public function testDeleteAllClustersIsLogged(): void { + foreach (['user1', 'user2'] as $userId) { + $cluster = new FaceCluster(); + $cluster->setUserId($userId); + $cluster->setTitle(''); + $this->clusterMapper->insert($cluster); + } + + $this->eventDispatcher->expects($this->once()) + ->method('dispatchTyped') + ->with($this->callback(function (CriticalActionPerformedEvent $event): bool { + $this->assertSame('Deleted all %s Recognize face clusters', $event->getLogMessage()); + $this->assertSame(['count' => 2], $event->getParameters()); + $this->assertTrue(AuditLogTestHelper::hasOperation($event, 'recognize.face_clusters.deleted_all')); + return true; + })); + + $this->clusterMapper->deleteAll(); + } + + public function testDeleteAllClustersWithoutClustersIsNotLogged(): void { + $this->eventDispatcher->expects($this->never())->method('dispatchTyped'); + + $this->clusterMapper->deleteAll(); + } + + public function testDeleteAllDetectionsIsLogged(): void { + for ($i = 0; $i < 3; $i++) { + $detection = new FaceDetection(); + $detection->setUserId('user' . $i); + $detection->setX(0.1 * $i); + $detection->setY(0.1 * $i); + $detection->setHeight(0.5); + $detection->setWidth(0.5); + $detection->setFileId($i); + $detection->setThreshold(0.5); + $detection->setVector([1, 2, 3, 4, 5, 6, 7, 8, 9, 0]); + $this->detectionMapper->insertWithoutDeduplication($detection); + } + + $this->eventDispatcher->expects($this->once()) + ->method('dispatchTyped') + ->with($this->callback(function (CriticalActionPerformedEvent $event): bool { + $this->assertSame('Deleted all %s Recognize face detections', $event->getLogMessage()); + $this->assertSame(['count' => 3], $event->getParameters()); + $this->assertTrue(AuditLogTestHelper::hasOperation($event, 'recognize.face_detections.deleted_all')); + return true; + })); + + $this->detectionMapper->deleteAll(); + } + + public function testDeleteAllDetectionsWithoutDetectionsIsNotLogged(): void { + $this->eventDispatcher->expects($this->never())->method('dispatchTyped'); + + $this->detectionMapper->deleteAll(); + } +} diff --git a/tests/Service/TagManagerAuditTest.php b/tests/Service/TagManagerAuditTest.php new file mode 100644 index 000000000..47caeb20b --- /dev/null +++ b/tests/Service/TagManagerAuditTest.php @@ -0,0 +1,88 @@ +systemTagManager = $this->createMock(ISystemTagManager::class); + $this->objectMapper = $this->createMock(ISystemTagObjectMapper::class); + $this->eventDispatcher = $this->createMock(IEventDispatcher::class); + $this->tagManager = new TagManager($this->systemTagManager, $this->objectMapper, $this->eventDispatcher); + } + + private function createTag(string $id, string $name = 'tag'): ISystemTag&MockObject { + $tag = $this->createMock(ISystemTag::class); + $tag->method('getId')->willReturn($id); + $tag->method('getName')->willReturn($name); + return $tag; + } + + private function mockProcessedTag(): void { + $processedTag = $this->createTag('1', TagManager::RECOGNIZED_TAG); + $this->systemTagManager->method('getTag') + ->willReturnCallback(function (string $name) use ($processedTag): ISystemTag { + if ($name === TagManager::RECOGNIZED_TAG) { + return $processedTag; + } + throw new TagNotFoundException(); + }); + } + + public function testResetClassificationsIsLogged(): void { + $this->mockProcessedTag(); + $this->objectMapper->method('getObjectIdsForTags')->willReturn(['10', '11', '12']); + $this->objectMapper->expects($this->exactly(3))->method('unassignTags'); + + $this->eventDispatcher->expects($this->once()) + ->method('dispatchTyped') + ->with($this->callback(function (CriticalActionPerformedEvent $event): bool { + $this->assertSame('Removed Recognize classification tags from %s files', $event->getLogMessage()); + $this->assertSame(['fileCount' => 3], $event->getParameters()); + $this->assertTrue(AuditLogTestHelper::hasOperation($event, 'recognize.classifications.reset')); + return true; + })); + + $this->tagManager->resetClassifications(); + } + + public function testResetClassificationsWithoutClassifiedFilesIsNotLogged(): void { + $this->mockProcessedTag(); + $this->objectMapper->method('getObjectIdsForTags')->willReturn([]); + $this->eventDispatcher->expects($this->never())->method('dispatchTyped'); + + $this->tagManager->resetClassifications(); + } + + public function testFailedResetClassificationsIsNotLogged(): void { + $this->mockProcessedTag(); + $this->objectMapper->method('getObjectIdsForTags')->willReturn(['10']); + $this->objectMapper->method('unassignTags')->willThrowException(new \RuntimeException('failed')); + $this->eventDispatcher->expects($this->never())->method('dispatchTyped'); + $this->expectException(\RuntimeException::class); + + $this->tagManager->resetClassifications(); + } +} From 657b6fd96c34678b993443a330d394906ddbead1 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Wed, 4 Nov 2026 11:56:39 +0100 Subject: [PATCH 3/4] feat(tags): log deletion of empty and legacy system tags to the audit log The occ commands that delete system tags now write an audit entry: - recognize.empty_tags.deleted: TagManager::removeEmptyTags (occ recognize:cleanup-tags) deleted every system tag that is assigned to no file. This includes tags created by admins or other apps, not only Recognize's. - recognize.legacy_tags.deleted: TagManager::removeTags (occ recognize:remove-legacy-tags) deleted the tags of old Recognize versions. Both entries carry the number of deleted tags and their IDs. Tag names are not logged. admin_audit logs the creation of system tags but not their deletion, and the occ command line alone does not show which tags were removed. One entry is written per command run, only after all deletions succeeded and only if at least one tag was deleted. Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Marcel Klehr --- lib/AuditLog/Operation.php | 4 ++ lib/Service/TagManager.php | 20 ++++++++ tests/Service/TagManagerAuditTest.php | 68 +++++++++++++++++++++++++++ 3 files changed, 92 insertions(+) diff --git a/lib/AuditLog/Operation.php b/lib/AuditLog/Operation.php index da7b173f8..569e5ab0e 100644 --- a/lib/AuditLog/Operation.php +++ b/lib/AuditLog/Operation.php @@ -17,6 +17,8 @@ enum Operation: string { case ClassificationsReset = 'recognize.classifications.reset'; case FaceClustersDeletedAll = 'recognize.face_clusters.deleted_all'; case FaceDetectionsDeletedAll = 'recognize.face_detections.deleted_all'; + case EmptyTagsDeleted = 'recognize.empty_tags.deleted'; + case LegacyTagsDeleted = 'recognize.legacy_tags.deleted'; public function description(): string { return match ($this) { @@ -24,6 +26,8 @@ public function description(): string { self::ClassificationsReset => 'Recognize classification tags were removed from all files of all users.', self::FaceClustersDeletedAll => 'All face clusters (persons) of all users were deleted.', self::FaceDetectionsDeletedAll => 'All face detections of all users were deleted.', + self::EmptyTagsDeleted => 'System tags that were assigned to no file were deleted, including tags not created by Recognize.', + self::LegacyTagsDeleted => 'System tags created by old Recognize versions were deleted.', }; } } diff --git a/lib/Service/TagManager.php b/lib/Service/TagManager.php index 14f8c0549..9dd610596 100644 --- a/lib/Service/TagManager.php +++ b/lib/Service/TagManager.php @@ -157,20 +157,40 @@ public function resetClassifications(): void { */ public function removeEmptyTags(): void { $tags = $this->tagManager->getAllTags(); + $deletedTagIds = []; foreach ($tags as $tag) { $files = $this->objectMapper->getObjectIdsForTags($tag->getId(), 'files', 1); if (empty($files)) { $this->tagManager->deleteTags($tag->getId()); + $deletedTagIds[] = $tag->getId(); } } + $this->logDeletedTags(Operation::EmptyTagsDeleted, 'Deleted %s empty system tags', $deletedTagIds); } public function removeTags(array $tagNames) :void { $tags = $this->tagManager->getAllTags(); + $deletedTagIds = []; foreach ($tags as $tag) { if (in_array($tag->getName(), $tagNames)) { $this->tagManager->deleteTags($tag->getId()); + $deletedTagIds[] = $tag->getId(); } } + $this->logDeletedTags(Operation::LegacyTagsDeleted, 'Deleted %s legacy Recognize system tags', $deletedTagIds); + } + + /** + * @param list $tagIds + */ + private function logDeletedTags(Operation $operation, string $message, array $tagIds): void { + if (count($tagIds) === 0) { + return; + } + $this->eventDispatcher->dispatchTyped(AuditEventFactory::create( + $operation, + $message, + ['count' => count($tagIds), 'tagIds' => $tagIds], + )); } } diff --git a/tests/Service/TagManagerAuditTest.php b/tests/Service/TagManagerAuditTest.php index 47caeb20b..9b5e58e53 100644 --- a/tests/Service/TagManagerAuditTest.php +++ b/tests/Service/TagManagerAuditTest.php @@ -85,4 +85,72 @@ public function testFailedResetClassificationsIsNotLogged(): void { $this->tagManager->resetClassifications(); } + + public function testRemoveEmptyTagsIsLogged(): void { + $this->systemTagManager->method('getAllTags')->willReturn([ + $this->createTag('5'), + $this->createTag('6'), + $this->createTag('7'), + ]); + $this->objectMapper->method('getObjectIdsForTags') + ->willReturnCallback(fn (string $tagId): array => $tagId === '6' ? ['42'] : []); + $this->systemTagManager->expects($this->exactly(2))->method('deleteTags'); + + $this->eventDispatcher->expects($this->once()) + ->method('dispatchTyped') + ->with($this->callback(function (CriticalActionPerformedEvent $event): bool { + $this->assertSame('Deleted %s empty system tags', $event->getLogMessage()); + $this->assertSame(['count' => 2, 'tagIds' => ['5', '7']], $event->getParameters()); + $this->assertTrue(AuditLogTestHelper::hasOperation($event, 'recognize.empty_tags.deleted')); + return true; + })); + + $this->tagManager->removeEmptyTags(); + } + + public function testRemoveEmptyTagsWithoutEmptyTagsIsNotLogged(): void { + $this->systemTagManager->method('getAllTags')->willReturn([$this->createTag('5')]); + $this->objectMapper->method('getObjectIdsForTags')->willReturn(['42']); + $this->systemTagManager->expects($this->never())->method('deleteTags'); + $this->eventDispatcher->expects($this->never())->method('dispatchTyped'); + + $this->tagManager->removeEmptyTags(); + } + + public function testFailedRemoveEmptyTagsIsNotLogged(): void { + $this->systemTagManager->method('getAllTags')->willReturn([$this->createTag('5')]); + $this->objectMapper->method('getObjectIdsForTags')->willReturn([]); + $this->systemTagManager->method('deleteTags')->willThrowException(new TagNotFoundException()); + $this->eventDispatcher->expects($this->never())->method('dispatchTyped'); + $this->expectException(TagNotFoundException::class); + + $this->tagManager->removeEmptyTags(); + } + + public function testRemoveLegacyTagsIsLogged(): void { + $this->systemTagManager->method('getAllTags')->willReturn([ + $this->createTag('5', 'cat'), + $this->createTag('6', 'holiday 2024'), + $this->createTag('7', 'dog'), + ]); + $this->systemTagManager->expects($this->exactly(2))->method('deleteTags'); + + $this->eventDispatcher->expects($this->once()) + ->method('dispatchTyped') + ->with($this->callback(function (CriticalActionPerformedEvent $event): bool { + $this->assertSame('Deleted %s legacy Recognize system tags', $event->getLogMessage()); + $this->assertSame(['count' => 2, 'tagIds' => ['5', '7']], $event->getParameters()); + $this->assertTrue(AuditLogTestHelper::hasOperation($event, 'recognize.legacy_tags.deleted')); + return true; + })); + + $this->tagManager->removeTags(['cat', 'dog']); + } + + public function testRemoveLegacyTagsWithoutMatchIsNotLogged(): void { + $this->systemTagManager->method('getAllTags')->willReturn([$this->createTag('6', 'holiday 2024')]); + $this->eventDispatcher->expects($this->never())->method('dispatchTyped'); + + $this->tagManager->removeTags(['cat', 'dog']); + } } From c99718db7b878ec9f43ec663f01d2c0cb2de36d8 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Wed, 4 Nov 2026 11:57:47 +0100 Subject: [PATCH 4/4] feat(queue): log clearing queues and scheduling a recrawl to the audit log Clearing all queues and scheduling a recrawl now writes an audit entry: - recognize.queues.cleared: QueueService::clearAllJobs emptied every queue and removed all scheduled Recognize background jobs (admin page, occ recognize:clear-background-jobs). - recognize.recrawl.scheduled: QueueService::scheduleRecrawl did the same and scheduled a new crawl of all files (admin page, occ recognize:recrawl). Both entries carry the number of removed queue entries. A recrawl writes only its own entry, not an additional one for the clearing. The admin controller and both occ commands repeated the same list of queues and jobs; they now call these two service methods, so every entry point writes the same entry once and only after all steps succeeded. This also makes occ recognize:clear-background-jobs remove the face clustering job, as the admin page and the recrawl already did. Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Marcel Klehr --- lib/AuditLog/Operation.php | 4 + lib/Command/ClearBackgroundJobs.php | 25 +----- lib/Command/Recrawl.php | 28 +------ lib/Controller/AdminController.php | 19 +---- lib/Db/QueueMapper.php | 8 +- lib/Service/QueueService.php | 57 ++++++++++++- tests/Service/QueueServiceAuditTest.php | 107 ++++++++++++++++++++++++ 7 files changed, 179 insertions(+), 69 deletions(-) create mode 100644 tests/Service/QueueServiceAuditTest.php diff --git a/lib/AuditLog/Operation.php b/lib/AuditLog/Operation.php index 569e5ab0e..92b5c8b06 100644 --- a/lib/AuditLog/Operation.php +++ b/lib/AuditLog/Operation.php @@ -19,6 +19,8 @@ enum Operation: string { case FaceDetectionsDeletedAll = 'recognize.face_detections.deleted_all'; case EmptyTagsDeleted = 'recognize.empty_tags.deleted'; case LegacyTagsDeleted = 'recognize.legacy_tags.deleted'; + case QueuesCleared = 'recognize.queues.cleared'; + case RecrawlScheduled = 'recognize.recrawl.scheduled'; public function description(): string { return match ($this) { @@ -28,6 +30,8 @@ public function description(): string { self::FaceDetectionsDeletedAll => 'All face detections of all users were deleted.', self::EmptyTagsDeleted => 'System tags that were assigned to no file were deleted, including tags not created by Recognize.', self::LegacyTagsDeleted => 'System tags created by old Recognize versions were deleted.', + self::QueuesCleared => 'All Recognize queues and scheduled background jobs were cleared.', + self::RecrawlScheduled => 'All Recognize queues and background jobs were cleared and a new crawl of all files was scheduled.', }; } } diff --git a/lib/Command/ClearBackgroundJobs.php b/lib/Command/ClearBackgroundJobs.php index e00bd575c..5b0f00cd2 100644 --- a/lib/Command/ClearBackgroundJobs.php +++ b/lib/Command/ClearBackgroundJobs.php @@ -7,29 +7,19 @@ declare(strict_types=1); namespace OCA\Recognize\Command; -use OCA\Recognize\BackgroundJobs\ClassifyFacesJob; -use OCA\Recognize\BackgroundJobs\ClassifyImagenetJob; -use OCA\Recognize\BackgroundJobs\ClassifyLandmarksJob; -use OCA\Recognize\BackgroundJobs\ClassifyMovinetJob; -use OCA\Recognize\BackgroundJobs\ClassifyMusicnnJob; -use OCA\Recognize\BackgroundJobs\SchedulerJob; -use OCA\Recognize\BackgroundJobs\StorageCrawlJob; use OCA\Recognize\Service\QueueService; -use OCP\BackgroundJob\IJobList; use Psr\Log\LoggerInterface; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; final class ClearBackgroundJobs extends Command { - private IJobList $jobList; private LoggerInterface $logger; private QueueService $queue; - public function __construct(IJobList $jobList, LoggerInterface $logger, QueueService $queue) { + public function __construct(LoggerInterface $logger, QueueService $queue) { parent::__construct(); - $this->jobList = $jobList; $this->logger = $logger; $this->queue = $queue; } @@ -54,18 +44,7 @@ protected function configure() { */ protected function execute(InputInterface $input, OutputInterface $output): int { try { - $this->queue->clearQueue('imagenet'); - $this->queue->clearQueue('faces'); - $this->queue->clearQueue('landmarks'); - $this->queue->clearQueue('movinet'); - $this->queue->clearQueue('musicnn'); - $this->jobList->remove(ClassifyFacesJob::class); - $this->jobList->remove(ClassifyImagenetJob::class); - $this->jobList->remove(ClassifyLandmarksJob::class); - $this->jobList->remove(ClassifyMusicnnJob::class); - $this->jobList->remove(ClassifyMovinetJob::class); - $this->jobList->remove(SchedulerJob::class); - $this->jobList->remove(StorageCrawlJob::class); + $this->queue->clearAllJobs(); } catch (\Exception $ex) { $output->writeln('Failed to clear background jobs'); $output->writeln($ex->getMessage()); diff --git a/lib/Command/Recrawl.php b/lib/Command/Recrawl.php index 1790972d4..68aba846e 100644 --- a/lib/Command/Recrawl.php +++ b/lib/Command/Recrawl.php @@ -7,29 +7,18 @@ declare(strict_types=1); namespace OCA\Recognize\Command; -use OCA\Recognize\BackgroundJobs\ClassifyFacesJob; -use OCA\Recognize\BackgroundJobs\ClassifyImagenetJob; -use OCA\Recognize\BackgroundJobs\ClassifyLandmarksJob; -use OCA\Recognize\BackgroundJobs\ClassifyMovinetJob; -use OCA\Recognize\BackgroundJobs\ClassifyMusicnnJob; -use OCA\Recognize\BackgroundJobs\ClusterFacesJob; -use OCA\Recognize\BackgroundJobs\SchedulerJob; -use OCA\Recognize\BackgroundJobs\StorageCrawlJob; use OCA\Recognize\Service\QueueService; -use OCP\BackgroundJob\IJobList; use Psr\Log\LoggerInterface; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; final class Recrawl extends Command { - private IJobList $jobList; private LoggerInterface $logger; private QueueService $queue; - public function __construct(IJobList $jobList, LoggerInterface $logger, QueueService $queue) { + public function __construct(LoggerInterface $logger, QueueService $queue) { parent::__construct(); - $this->jobList = $jobList; $this->logger = $logger; $this->queue = $queue; } @@ -54,20 +43,7 @@ protected function configure() { */ protected function execute(InputInterface $input, OutputInterface $output): int { try { - $this->queue->clearQueue('imagenet'); - $this->queue->clearQueue('faces'); - $this->queue->clearQueue('landmarks'); - $this->queue->clearQueue('movinet'); - $this->queue->clearQueue('musicnn'); - $this->jobList->remove(ClassifyFacesJob::class); - $this->jobList->remove(ClassifyImagenetJob::class); - $this->jobList->remove(ClassifyLandmarksJob::class); - $this->jobList->remove(ClassifyMusicnnJob::class); - $this->jobList->remove(ClassifyMovinetJob::class); - $this->jobList->remove(ClusterFacesJob::class); - $this->jobList->remove(SchedulerJob::class); - $this->jobList->remove(StorageCrawlJob::class); - $this->jobList->add(SchedulerJob::class); + $this->queue->scheduleRecrawl(); } catch (\Exception $ex) { $output->writeln('Failed to schedule recrawl'); $output->writeln($ex->getMessage()); diff --git a/lib/Controller/AdminController.php b/lib/Controller/AdminController.php index 0b1a149e8..0aee1ac11 100644 --- a/lib/Controller/AdminController.php +++ b/lib/Controller/AdminController.php @@ -15,8 +15,6 @@ use OCA\Recognize\BackgroundJobs\ClassifyMovinetJob; use OCA\Recognize\BackgroundJobs\ClassifyMusicnnJob; use OCA\Recognize\BackgroundJobs\ClusterFacesJob; -use OCA\Recognize\BackgroundJobs\SchedulerJob; -use OCA\Recognize\BackgroundJobs\StorageCrawlJob; use OCA\Recognize\Db\FaceClusterMapper; use OCA\Recognize\Db\FaceDetectionMapper; use OCA\Recognize\Service\QueueService; @@ -63,25 +61,12 @@ public function reset(): JSONResponse { } public function clearAllJobs(): JSONResponse { - $this->queue->clearQueue('imagenet'); - $this->queue->clearQueue('faces'); - $this->queue->clearQueue('landmarks'); - $this->queue->clearQueue('movinet'); - $this->queue->clearQueue('musicnn'); - $this->jobList->remove(ClassifyFacesJob::class); - $this->jobList->remove(ClassifyImagenetJob::class); - $this->jobList->remove(ClassifyLandmarksJob::class); - $this->jobList->remove(ClassifyMusicnnJob::class); - $this->jobList->remove(ClassifyMovinetJob::class); - $this->jobList->remove(ClusterFacesJob::class); - $this->jobList->remove(SchedulerJob::class); - $this->jobList->remove(StorageCrawlJob::class); + $this->queue->clearAllJobs(); return new JSONResponse([]); } public function recrawl(): JSONResponse { - $this->clearAllJobs(); - $this->jobList->add(SchedulerJob::class); + $this->queue->scheduleRecrawl(); return new JSONResponse([]); } diff --git a/lib/Db/QueueMapper.php b/lib/Db/QueueMapper.php index ffa93fa70..7b8a6be24 100644 --- a/lib/Db/QueueMapper.php +++ b/lib/Db/QueueMapper.php @@ -163,9 +163,13 @@ public function insertIntoQueue(string $model, QueueFile $file) : QueueFile { return $insertedFile; } - public function clearQueue(string $model): void { + /** + * @return int the number of removed queue entries + * @throws \OCP\DB\Exception + */ + public function clearQueue(string $model): int { $qb = $this->db->getQueryBuilder(); - $qb->delete($this->getQueueTable($model))->executeStatement(); + return $qb->delete($this->getQueueTable($model))->executeStatement(); } /** diff --git a/lib/Service/QueueService.php b/lib/Service/QueueService.php index bffee7e75..70b91e592 100644 --- a/lib/Service/QueueService.php +++ b/lib/Service/QueueService.php @@ -7,11 +7,16 @@ declare(strict_types=1); namespace OCA\Recognize\Service; +use OCA\Recognize\AuditLog\AuditEventFactory; +use OCA\Recognize\AuditLog\Operation; use OCA\Recognize\BackgroundJobs\ClassifyFacesJob; use OCA\Recognize\BackgroundJobs\ClassifyImagenetJob; use OCA\Recognize\BackgroundJobs\ClassifyLandmarksJob; use OCA\Recognize\BackgroundJobs\ClassifyMovinetJob; use OCA\Recognize\BackgroundJobs\ClassifyMusicnnJob; +use OCA\Recognize\BackgroundJobs\ClusterFacesJob; +use OCA\Recognize\BackgroundJobs\SchedulerJob; +use OCA\Recognize\BackgroundJobs\StorageCrawlJob; use OCA\Recognize\Classifiers\Audio\MusicnnClassifier; use OCA\Recognize\Classifiers\Images\ClusteringFaceClassifier; use OCA\Recognize\Classifiers\Images\ImagenetClassifier; @@ -21,6 +26,7 @@ use OCA\Recognize\Db\QueueMapper; use OCP\AppFramework\Services\IAppConfig; use OCP\BackgroundJob\IJobList; +use OCP\EventDispatcher\IEventDispatcher; final class QueueService { /** @@ -37,11 +43,13 @@ final class QueueService { private QueueMapper $queueMapper; private IJobList $jobList; private IAppConfig $config; + private IEventDispatcher $eventDispatcher; - public function __construct(QueueMapper $queueMapper, IJobList $jobList, IAppConfig $config) { + public function __construct(QueueMapper $queueMapper, IJobList $jobList, IAppConfig $config, IEventDispatcher $eventDispatcher) { $this->queueMapper = $queueMapper; $this->jobList = $jobList; $this->config = $config; + $this->eventDispatcher = $eventDispatcher; } /** @@ -129,6 +137,53 @@ public function clearQueue(string $model): void { $this->queueMapper->clearQueue($model); } + /** + * Empties all queues and removes all scheduled Recognize background jobs + * + * @throws \OCP\DB\Exception + */ + public function clearAllJobs(): void { + $removedFiles = $this->removeAllJobs(); + $this->eventDispatcher->dispatchTyped(AuditEventFactory::create( + Operation::QueuesCleared, + 'Cleared all Recognize queues and background jobs, removing %s queued files', + ['removedFiles' => $removedFiles], + )); + } + + /** + * Empties all queues, removes all scheduled Recognize background jobs and schedules a new crawl of all files + * + * @throws \OCP\DB\Exception + */ + public function scheduleRecrawl(): void { + $removedFiles = $this->removeAllJobs(); + $this->jobList->add(SchedulerJob::class); + $this->eventDispatcher->dispatchTyped(AuditEventFactory::create( + Operation::RecrawlScheduled, + 'Scheduled a Recognize recrawl of all files, removing %s queued files', + ['removedFiles' => $removedFiles], + )); + } + + /** + * @return int the number of removed queue entries + * @throws \OCP\DB\Exception + */ + private function removeAllJobs(): int { + $removedFiles = 0; + foreach (array_keys(self::JOB_CLASSES) as $model) { + $removedFiles += $this->queueMapper->clearQueue($model); + } + foreach (self::JOB_CLASSES as $jobClass) { + $this->jobList->remove($jobClass); + } + $this->jobList->remove(ClusterFacesJob::class); + $this->jobList->remove(SchedulerJob::class); + $this->jobList->remove(StorageCrawlJob::class); + return $removedFiles; + } + public function count(string $model): int { return $this->queueMapper->count($model); } diff --git a/tests/Service/QueueServiceAuditTest.php b/tests/Service/QueueServiceAuditTest.php new file mode 100644 index 000000000..0387a009e --- /dev/null +++ b/tests/Service/QueueServiceAuditTest.php @@ -0,0 +1,107 @@ +queueMapper = new QueueMapper(Server::get(IDBConnection::class)); + $this->jobList = $this->createMock(IJobList::class); + $this->eventDispatcher = $this->createMock(IEventDispatcher::class); + $this->service = new QueueService($this->queueMapper, $this->jobList, $this->createMock(IAppConfig::class), $this->eventDispatcher); + foreach (self::MODELS as $model) { + $this->queueMapper->clearQueue($model); + } + } + + protected function tearDown(): void { + foreach (self::MODELS as $model) { + $this->queueMapper->clearQueue($model); + } + parent::tearDown(); + } + + private function queueFile(string $model, int $fileId): void { + $file = new QueueFile(); + $file->setFileId($fileId); + $file->setStorageId(1); + $file->setRootId(1); + $file->setUpdate(false); + $this->queueMapper->insertIntoQueue($model, $file); + } + + public function testClearAllJobsIsLogged(): void { + $this->queueFile('imagenet', 1); + $this->queueFile('imagenet', 2); + $this->queueFile('faces', 1); + $this->jobList->expects($this->never())->method('add'); + + $this->eventDispatcher->expects($this->once()) + ->method('dispatchTyped') + ->with($this->callback(function (CriticalActionPerformedEvent $event): bool { + $this->assertSame('Cleared all Recognize queues and background jobs, removing %s queued files', $event->getLogMessage()); + $this->assertSame(['removedFiles' => 3], $event->getParameters()); + $this->assertTrue(AuditLogTestHelper::hasOperation($event, 'recognize.queues.cleared')); + return true; + })); + + $this->service->clearAllJobs(); + foreach (self::MODELS as $model) { + $this->assertSame(0, $this->queueMapper->count($model)); + } + } + + public function testScheduleRecrawlIsLogged(): void { + $this->queueFile('musicnn', 1); + $this->jobList->expects($this->once())->method('add')->with(SchedulerJob::class); + + $this->eventDispatcher->expects($this->once()) + ->method('dispatchTyped') + ->with($this->callback(function (CriticalActionPerformedEvent $event): bool { + $this->assertSame('Scheduled a Recognize recrawl of all files, removing %s queued files', $event->getLogMessage()); + $this->assertSame(['removedFiles' => 1], $event->getParameters()); + $this->assertTrue(AuditLogTestHelper::hasOperation($event, 'recognize.recrawl.scheduled')); + return true; + })); + + $this->service->scheduleRecrawl(); + } + + public function testFailedRecrawlIsNotLogged(): void { + $this->jobList->method('add')->willThrowException(new \RuntimeException('failed')); + $this->eventDispatcher->expects($this->never())->method('dispatchTyped'); + $this->expectException(\RuntimeException::class); + + $this->service->scheduleRecrawl(); + } +}