Merge pull request #897 from nextcloud/fix/lockDeletion

Fix/lock deletion

Author: juliusknorr

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));
 	}
 }

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 {

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);
 	}
 }

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 [];

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

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)