From 8a30eaebd0a7e4988217560fab367e4b8ac54bac Mon Sep 17 00:00:00 2001 From: David Dreschner Date: Fri, 3 Apr 2026 17:11:46 +0300 Subject: [PATCH] feat: Implement background job to remove unused models Signed-off-by: David Dreschner --- appinfo/info.xml | 1 + lib/BackgroundJob/CleanupModelsJob.php | 50 +++++ lib/Db/ModelMapper.php | 16 ++ lib/Service/ModelStore.php | 55 +++++ tests/Unit/Service/ModelStoreTest.php | 281 +++++++++++++++++++++++++ 5 files changed, 403 insertions(+) create mode 100644 lib/BackgroundJob/CleanupModelsJob.php create mode 100644 tests/Unit/Service/ModelStoreTest.php diff --git a/appinfo/info.xml b/appinfo/info.xml index 677350ca..fd79ba95 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -32,6 +32,7 @@ OCA\SuspiciousLogin\BackgroundJob\ETLJob OCA\SuspiciousLogin\BackgroundJob\TrainJobIpV4 OCA\SuspiciousLogin\BackgroundJob\TrainJobIpV6 + OCA\SuspiciousLogin\BackgroundJob\CleanupModelsJob diff --git a/lib/BackgroundJob/CleanupModelsJob.php b/lib/BackgroundJob/CleanupModelsJob.php new file mode 100644 index 00000000..cd508346 --- /dev/null +++ b/lib/BackgroundJob/CleanupModelsJob.php @@ -0,0 +1,50 @@ +setInterval(24 * 60 * 60); + $this->setTimeSensitivity(self::TIME_INSENSITIVE); + $this->allowParallelRuns = false; + } + + #[\Override] + protected function run($argument): void { + try { + $this->modelStore->cleanup(self::MODELS_TO_KEEP); + } catch (Throwable $e) { + $this->logger->error('Error during model cleanup: ' . $e->getMessage(), [ + 'exception' => $e, + ]); + } + } +} diff --git a/lib/Db/ModelMapper.php b/lib/Db/ModelMapper.php index d198e23c..278a5c0b 100644 --- a/lib/Db/ModelMapper.php +++ b/lib/Db/ModelMapper.php @@ -66,4 +66,20 @@ public function findMostRecent(int $max, string $addressType): array { return $this->findEntities($query); } + + /** + * Find all models older than `$numberOfModelsToKeep` per `$addressType` + * + * @return Model[] + */ + public function findOld(int $numberOfModelsToKeep, string $addressType): array { + $qb = $this->db->getQueryBuilder(); + $query = $qb->select('*') + ->from($this->getTableName()) + ->where($qb->expr()->eq('address_type', $qb->createNamedParameter($addressType))) + ->setFirstResult($numberOfModelsToKeep) + ->orderBy('created_at', 'desc'); + + return $this->findEntities($query); + } } diff --git a/lib/Service/ModelStore.php b/lib/Service/ModelStore.php index 1eb641b0..b4a39a02 100644 --- a/lib/Service/ModelStore.php +++ b/lib/Service/ModelStore.php @@ -165,4 +165,59 @@ public function persist(Learner $estimator, Model $model) { throw $e; } } + + /** + * Remove old models, keeping only the `$numberOfModelsToKeep` ones + * for each used address type (IPv4 / IPv6). + * + * @throws \OCP\DB\Exception + * @throws \OCP\Files\NotPermittedException + */ + public function cleanup(int $numberOfModelsToKeep): void { + $oldModels = array_merge( + $this->modelMapper->findOld($numberOfModelsToKeep, Ipv4Strategy::getTypeName()), + $this->modelMapper->findOld($numberOfModelsToKeep, IpV6Strategy::getTypeName()) + ); + + if (empty($oldModels)) { + $this->logger->debug('No old models to clean up'); + return; + } + + try { + $modelsFolder = $this->appData->getFolder(self::APPDATA_MODELS_FOLDER); + } catch (NotFoundException) { + $this->logger->debug('Models folder does not exist, skipping file deletion'); + } + + if ($this->cacheFactory->isLocalCacheAvailable()) { + $cache = $this->cacheFactory->createLocal(); + } + + foreach ($oldModels as $oldModel) { + $id = $oldModel->getId(); + $this->logger->debug("Cleaning up old model $id"); + + // Remove the model file from app data + if (isset($modelsFolder)) { + try { + $modelFile = $modelsFolder->getFile((string)$id); + $modelFile->delete(); + } catch (NotFoundException) { + $this->logger->debug("Model file $id not found in app data, skipping file deletion"); + } + } + + // Evict from cache + if (isset($cache)) { + $cache->remove($this->getCacheKey($id)); + } + + // Remove the DB record + $this->modelMapper->delete($oldModel); + } + + $numberOfOldModels = count($oldModels); + $this->logger->info("Cleaned up {$numberOfOldModels} old model(s)"); + } } diff --git a/tests/Unit/Service/ModelStoreTest.php b/tests/Unit/Service/ModelStoreTest.php new file mode 100644 index 00000000..a9ab44aa --- /dev/null +++ b/tests/Unit/Service/ModelStoreTest.php @@ -0,0 +1,281 @@ +modelMapper = $this->createMock(ModelMapper::class); + $this->appData = $this->createMock(IAppData::class); + $this->appManager = $this->createMock(IAppManager::class); + $this->tempManager = $this->createMock(ITempManager::class); + $this->cacheFactory = $this->createMock(ICacheFactory::class); + $this->logger = $this->createMock(LoggerInterface::class); + + $this->modelStore = new ModelStore( + $this->modelMapper, + $this->appData, + $this->appManager, + $this->tempManager, + $this->cacheFactory, + $this->logger, + ); + } + + private function createModelWithId(int $id): Model { + $model = new Model(); + $model->setId($id); + return $model; + } + + private function expectFindOldReturns(int $keep, array $ipv4Models, array $ipv6Models): void { + $this->modelMapper->expects(self::exactly(2)) + ->method('findOld') + ->willReturnMap([ + [$keep, Ipv4Strategy::getTypeName(), $ipv4Models], + [$keep, IpV6Strategy::getTypeName(), $ipv6Models], + ]); + } + + private static function expectedCacheKey(int $id): string { + return "suspicious_login_model_$id"; + } + + public function testCleanupDoesNothingWhenNoOldModels(): void { + $this->expectFindOldReturns(14, [], []); + + $this->appData->expects(self::never()) + ->method('getFolder'); + $this->modelMapper->expects(self::never()) + ->method('delete'); + + $this->modelStore->cleanup(14); + } + + public function testCleanupRemovesOldModelsFromBothStrategies(): void { + $model1 = $this->createModelWithId(1); + $model2 = $this->createModelWithId(2); + + $this->expectFindOldReturns(14, [$model1], [$model2]); + + $file1 = $this->createMock(ISimpleFile::class); + $file1->expects(self::once())->method('delete'); + $file2 = $this->createMock(ISimpleFile::class); + $file2->expects(self::once())->method('delete'); + + $modelsFolder = $this->createMock(ISimpleFolder::class); + $modelsFolder->expects(self::exactly(2)) + ->method('getFile') + ->willReturnMap([ + ['1', $file1], + ['2', $file2], + ]); + + $this->appData->expects(self::once()) + ->method('getFolder') + ->with(ModelStore::APPDATA_MODELS_FOLDER) + ->willReturn($modelsFolder); + + $this->cacheFactory->method('isLocalCacheAvailable')->willReturn(false); + + $deletedModels = []; + $this->modelMapper->expects(self::exactly(2)) + ->method('delete') + ->willReturnCallback(function (Model $model) use (&$deletedModels) { + $deletedModels[] = $model; + return $model; + }); + + $this->modelStore->cleanup(14); + + self::assertSame([$model1, $model2], $deletedModels); + } + + public function testCleanupDeletesDbRecordWhenModelFileNotFound(): void { + $model = $this->createModelWithId(42); + + $this->expectFindOldReturns(14, [$model], []); + + $modelsFolder = $this->createMock(ISimpleFolder::class); + $modelsFolder->expects(self::once()) + ->method('getFile') + ->with('42') + ->willThrowException(new NotFoundException()); + + $this->appData->expects(self::once()) + ->method('getFolder') + ->with(ModelStore::APPDATA_MODELS_FOLDER) + ->willReturn($modelsFolder); + + $this->cacheFactory->method('isLocalCacheAvailable')->willReturn(false); + + $this->modelMapper->expects(self::once()) + ->method('delete') + ->with($model); + + $this->modelStore->cleanup(14); + } + + public function testCleanupDeletesDbRecordWhenModelsFolderNotFound(): void { + $model = $this->createModelWithId(10); + + $this->expectFindOldReturns(14, [$model], []); + + $this->appData->expects(self::once()) + ->method('getFolder') + ->with(ModelStore::APPDATA_MODELS_FOLDER) + ->willThrowException(new NotFoundException()); + + $this->cacheFactory->method('isLocalCacheAvailable')->willReturn(false); + + $this->modelMapper->expects(self::once()) + ->method('delete') + ->with($model); + + $this->modelStore->cleanup(14); + } + + public function testCleanupRemovesOnlyIpv6Models(): void { + $model = $this->createModelWithId(5); + + $this->expectFindOldReturns(14, [], [$model]); + + $file = $this->createMock(ISimpleFile::class); + $file->expects(self::once())->method('delete'); + + $modelsFolder = $this->createMock(ISimpleFolder::class); + $modelsFolder->expects(self::once()) + ->method('getFile') + ->with('5') + ->willReturn($file); + + $this->appData->expects(self::once()) + ->method('getFolder') + ->with(ModelStore::APPDATA_MODELS_FOLDER) + ->willReturn($modelsFolder); + + $this->cacheFactory->method('isLocalCacheAvailable')->willReturn(false); + + $this->modelMapper->expects(self::once()) + ->method('delete') + ->with($model); + + $this->modelStore->cleanup(14); + } + + public function testCleanupAbortsWhenDeleteThrowsException(): void { + $model1 = $this->createModelWithId(1); + $model2 = $this->createModelWithId(2); + + $this->expectFindOldReturns(14, [$model1, $model2], []); + + $file1 = $this->createMock(ISimpleFile::class); + $file1->expects(self::once())->method('delete'); + + $modelsFolder = $this->createMock(ISimpleFolder::class); + $modelsFolder->expects(self::once()) + ->method('getFile') + ->with('1') + ->willReturn($file1); + + $this->appData->expects(self::once()) + ->method('getFolder') + ->with(ModelStore::APPDATA_MODELS_FOLDER) + ->willReturn($modelsFolder); + + $this->cacheFactory->method('isLocalCacheAvailable')->willReturn(false); + + $exception = new \Exception('DB error'); + $this->modelMapper->expects(self::once()) + ->method('delete') + ->with($model1) + ->willThrowException($exception); + + $this->expectException(\Exception::class); + $this->expectExceptionMessage('DB error'); + + $this->modelStore->cleanup(14); + } + + public function testCleanupPassesKeepValueToMapper(): void { + $this->expectFindOldReturns(3, [], []); + + $this->modelStore->cleanup(3); + } + + public static function cacheAvailabilityDataProvider(): array { + return [ + 'cache available' => [true], + 'cache unavailable' => [false], + ]; + } + + #[DataProvider('cacheAvailabilityDataProvider')] + public function testCleanupHandlesCacheEviction(bool $cacheAvailable): void { + $model = $this->createModelWithId(7); + + $this->expectFindOldReturns(14, [$model], []); + + $modelsFolder = $this->createMock(ISimpleFolder::class); + $file = $this->createMock(ISimpleFile::class); + $file->expects(self::once())->method('delete'); + $modelsFolder->method('getFile')->with('7')->willReturn($file); + + $this->appData->method('getFolder') + ->with(ModelStore::APPDATA_MODELS_FOLDER) + ->willReturn($modelsFolder); + + $this->cacheFactory->method('isLocalCacheAvailable')->willReturn($cacheAvailable); + + if ($cacheAvailable) { + $cache = $this->createMock(ICache::class); + $cache->expects(self::once()) + ->method('remove') + ->with(self::expectedCacheKey(7)); + $this->cacheFactory->method('createLocal')->willReturn($cache); + } else { + $this->cacheFactory->expects(self::never())->method('createLocal'); + } + + $this->modelMapper->expects(self::once()) + ->method('delete') + ->with($model); + + $this->modelStore->cleanup(14); + } +}