From 99d29f8142a07a47afd7bb542507afdb02a36c1a 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 01e100da0c5f9babf29df698567eca26770139c9 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 50888ec0..72654ebe 100644 --- a/lib/Db/LocksRequest.php +++ b/lib/Db/LocksRequest.php @@ -120,16 +120,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 7f45f405..1e5a1e6a 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); } /**