From 2dcd7be703c354901385a28d4a186100cdc94f4b Mon Sep 17 00:00:00 2001 From: Salvatore Martire <4652631+salmart-dev@users.noreply.github.com> Date: Tue, 11 Nov 2025 18:38:36 +0100 Subject: [PATCH 1/2] test: add test that checks for locks deletion Signed-off-by: Salvatore Martire <4652631+salmart-dev@users.noreply.github.com> --- tests/Feature/LockFeatureTest.php | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/Feature/LockFeatureTest.php b/tests/Feature/LockFeatureTest.php index 0050e313..d1f50033 100644 --- a/tests/Feature/LockFeatureTest.php +++ b/tests/Feature/LockFeatureTest.php @@ -216,6 +216,27 @@ public function testExpiredLocksAreDeprecated() { self::assertNotEmpty($deprecated); } + public function testRemoveLocks(): void { + $service = \OCP\Server::get(LockService::class); + \OCP\Server::get(IConfig::class)->setAppValue(Application::APP_ID, ConfigService::LOCK_TIMEOUT, 30); + $file = $this->loginAndGetUserFolder(self::TEST_USER1)->newFile('test-expired-lock-is-deprecated', 'AAA'); + $lock1 = $this->lockManager->lock(new LockContext($file, ILock::TYPE_USER, self::TEST_USER1)); + $file2 = $this->loginAndGetUserFolder(self::TEST_USER1)->newFile('test-expired-lock-is-deprecated-2', 'AAA'); + $lock2 = $this->lockManager->lock(new LockContext($file2, ILock::TYPE_USER, self::TEST_USER1)); + $this->toTheFuture(3600); + $mapToTokens = fn (ILock $lock) => $lock->getToken(); + $deprecated = array_map($mapToTokens, $service->getDeprecatedLocks()); + + self::assertContains($lock1->getToken(), $deprecated); + self::assertContains($lock2->getToken(), $deprecated); + + $service->removeLocks([$lock1, $lock2]); + $deprecated = array_map($mapToTokens, $service->getDeprecatedLocks()); + + self::assertNotContains($lock1->getToken(), $deprecated); + self::assertNotContains($lock2->getToken(), $deprecated); + } + public function testLockUserInfinite() { \OCP\Server::get(IConfig::class)->setAppValue(Application::APP_ID, ConfigService::LOCK_TIMEOUT, 0); $file = $this->loginAndGetUserFolder(self::TEST_USER1) From 42a75c812337c932da256e4e76efb06539c33d06 Mon Sep 17 00:00:00 2001 From: Salvatore Martire <4652631+salmart-dev@users.noreply.github.com> Date: Tue, 11 Nov 2025 18:52:56 +0100 Subject: [PATCH 2/2] fix: issue in query which leads to no locks removed This commit fixes an issue with the query logic which prevented locks from being deleted as long as more than one lock is expired. To prevent a potential mass deletion of data due to the fix, the code has been adjusted to delete only 1000 locks at every run of the background job. Signed-off-by: Salvatore Martire <4652631+salmart-dev@users.noreply.github.com> --- lib/Cron/Unlock.php | 6 +++--- lib/Db/CoreQueryBuilder.php | 2 +- lib/Db/LocksRequest.php | 7 ++++++- lib/Service/LockService.php | 6 ++++-- lib/Tools/Db/ExtendedQueryBuilder.php | 2 +- 5 files changed, 15 insertions(+), 8 deletions(-) diff --git a/lib/Cron/Unlock.php b/lib/Cron/Unlock.php index 8b4a2537..b88c9494 100644 --- a/lib/Cron/Unlock.php +++ b/lib/Cron/Unlock.php @@ -25,10 +25,10 @@ public function __construct(ITimeFactory $timeFactory, LockService $lockService) } protected function run($argument): void { - $this->manageTimeoutLock(); + $this->deleteExpiredLocks(); } - private function manageTimeoutLock(): void { - $this->lockService->removeLocks($this->lockService->getDeprecatedLocks()); + private function deleteExpiredLocks(): void { + $this->lockService->removeLocks($this->lockService->getDeprecatedLocks(1000)); } } diff --git a/lib/Db/CoreQueryBuilder.php b/lib/Db/CoreQueryBuilder.php index 2b598912..7a71a0a6 100644 --- a/lib/Db/CoreQueryBuilder.php +++ b/lib/Db/CoreQueryBuilder.php @@ -29,7 +29,7 @@ public function limitToFileId(int $fileId): void { * @param array $ids */ public function limitToIds(array $ids): void { - $this->limitArray('id', $ids); + $this->limitInArray('id', $ids); } public function limitToFileIds(array $ids): void { diff --git a/lib/Db/LocksRequest.php b/lib/Db/LocksRequest.php index af10b139..f6765389 100644 --- a/lib/Db/LocksRequest.php +++ b/lib/Db/LocksRequest.php @@ -123,16 +123,21 @@ public function getAll(): array { /** * @param int $timeout in minutes + * @param int $limit how many locks to retrieve (0 for all, default) * * @return FileLock[] * @throws Exception */ - public function getLocksOlderThan(int $timeout): array { + public function getLocksOlderThan(int $timeout, int $limit = 0): array { $now = \OC::$server->get(ITimeFactory::class)->getTime(); $oldCreationTime = $now - $timeout * 60; $qb = $this->getLocksSelectSql(); $qb->andWhere($qb->expr()->lt('l.creation', $qb->createNamedParameter($oldCreationTime, IQueryBuilder::PARAM_INT))); + if ($limit !== 0) { + $qb->setMaxResults($limit); + } + return $this->getLocksFromRequest($qb); } } diff --git a/lib/Service/LockService.php b/lib/Service/LockService.php index 60ad0fc9..4de1a204 100644 --- a/lib/Service/LockService.php +++ b/lib/Service/LockService.php @@ -274,9 +274,11 @@ public function unlockFile(int $fileId, string $userId, bool $force = false, int /** + * @param int $limit how many locks to retrieve (0 for all, default) + * * @return FileLock[] */ - public function getDeprecatedLocks(): array { + public function getDeprecatedLocks(int $limit = 0): array { $timeout = (int)$this->configService->getAppValue(ConfigService::LOCK_TIMEOUT); if ($timeout === 0) { $this->logger->notice( @@ -286,7 +288,7 @@ public function getDeprecatedLocks(): array { } try { - $locks = $this->locksRequest->getLocksOlderThan($timeout); + $locks = $this->locksRequest->getLocksOlderThan($timeout, $limit); } catch (Exception $e) { $this->logger->warning('Failed to get locks older then timeout', ['exception' => $e]); return []; diff --git a/lib/Tools/Db/ExtendedQueryBuilder.php b/lib/Tools/Db/ExtendedQueryBuilder.php index 42be92b7..0fc942bf 100644 --- a/lib/Tools/Db/ExtendedQueryBuilder.php +++ b/lib/Tools/Db/ExtendedQueryBuilder.php @@ -101,7 +101,7 @@ public function limitToId(int $id): void { * @param array $ids */ public function limitToIds(array $ids): void { - $this->limitArray('id', $ids); + $this->limitInArray('id', $ids); } /**