From a90b8b5e3dc09b523adc4d05d8e5a37f378c8f7f 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 a183d75b7163bca7e0e5015e73a57199fb214c0b 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); } /**