Merge pull request #533 from nextcloud/backport/497/stable28

[stable28] fix(timeout): compare creation time to now - timeout

Author: max-nextcloud

lib/Controller/LockController.php

@@ -38,7 +38,6 @@
 use OCA\FilesLock\Model\FileLock;
 use OCA\FilesLock\Service\FileService;
 use OCA\FilesLock\Service\LockService;
-use OCA\FilesLock\Tools\Traits\TLogger;
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\DataResponse;
 use OCP\AppFramework\OCSController;
@@ -48,19 +47,21 @@
 use OCP\IL10N;
 use OCP\IRequest;
 use OCP\IUserSession;
+use Psr\Log\LoggerInterface;
 
 /**
  * Class LockController
  *
  * @package OCA\FilesLock\Controller
  */
 class LockController extends OCSController {
-	use TLogger;
 
 	private int $ocsVersion;
+	private LoggerInterface $logger;
 
 	public function __construct(
 		IRequest $request,
+		LoggerInterface $logger,
 		private IUserSession $userSession,
 		private FileService $fileService,
 		private LockService $lockService,
@@ -82,6 +83,7 @@ public function __construct(
 		$this->registerResponder('xml', function ($data) {
 			return $this->buildOCSResponse('xml', $data);
 		});
+		$this->logger = $logger;
 	}
 
 
@@ -197,7 +199,7 @@ protected function fail(
 		);
 
 		if ($log) {
-			$this->log(2, $status . ' - ' . json_encode($data));
+			$this->logger->warning('[warning] ' . $status . ' - ' . json_encode($data));
 		}
 
 		return new DataResponse($data, $status);

lib/Db/LocksRequest.php

@@ -35,6 +35,7 @@
 use Exception;
 use OCA\FilesLock\Exceptions\LockNotFoundException;
 use OCA\FilesLock\Model\FileLock;
+use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\DB\QueryBuilder\IQueryBuilder;
 
 /**
@@ -146,8 +147,10 @@ public function getAll(): array {
 	 * @throws Exception
 	 */
 	public function getLocksOlderThan(int $timeout): array {
+		$now = \OC::$server->get(ITimeFactory::class)->getTime();
+		$oldCreationTime = $now - $timeout * 60;
 		$qb = $this->getLocksSelectSql();
-		$qb->andWhere($qb->expr()->lt('l.creation', $qb->createNamedParameter($timeout * 60, IQueryBuilder::PARAM_INT)));
+		$qb->andWhere($qb->expr()->lt('l.creation', $qb->createNamedParameter($oldCreationTime, IQueryBuilder::PARAM_INT)));
 
 		return $this->getLocksFromRequest($qb);
 	}

lib/Service/LockService.php

@@ -36,7 +36,6 @@
 use OCA\FilesLock\Exceptions\LockNotFoundException;
 use OCA\FilesLock\Exceptions\UnauthorizedUnlockException;
 use OCA\FilesLock\Model\FileLock;
-use OCA\FilesLock\Tools\Traits\TLogger;
 use OCA\FilesLock\Tools\Traits\TStringTools;
 use OCP\App\IAppManager;
 use OCP\EventDispatcher\IEventDispatcher;
@@ -49,14 +48,13 @@
 use OCP\IRequest;
 use OCP\IUserManager;
 use OCP\IUserSession;
+use Psr\Log\LoggerInterface;
 
 class LockService {
 	public const PREFIX = 'files_lock';
 
 
 	use TStringTools;
-	use TLogger;
-
 
 	private IUserManager $userManager;
 	private IL10N $l10n;
@@ -67,6 +65,7 @@ class LockService {
 	private IEventDispatcher $eventDispatcher;
 	private IUserSession $userSession;
 	private IRequest $request;
+	private LoggerInterface $logger;
 
 
 	private array $locks = [];
@@ -86,6 +85,7 @@ public function __construct(
 		IEventDispatcher $eventDispatcher,
 		IUserSession $userSession,
 		IRequest $request,
+		LoggerInterface $logger,
 	) {
 		$this->l10n = $l10n;
 		$this->userManager = $userManager;
@@ -96,8 +96,7 @@ public function __construct(
 		$this->eventDispatcher = $eventDispatcher;
 		$this->userSession = $userSession;
 		$this->request = $request;
-
-		$this->setup('app', 'files_lock');
+		$this->logger = $logger;
 	}
 
 	/**
@@ -175,7 +174,7 @@ public function lock(LockContext $lockScope): FileLock {
 				$known->setTimeout(
 					$known->getETA() !== FileLock::ETA_INFINITE ? $known->getTimeout() - $known->getETA() + $this->configService->getTimeoutSeconds() : 0
 				);
-				$this->notice('extending existing lock', false, ['fileLock' => $known]);
+				$this->logger->notice('extending existing lock', ['fileLock' => $known]);
 				$this->locksRequest->update($known);
 				$this->injectMetadata($known);
 				return $known;
@@ -187,7 +186,7 @@ public function lock(LockContext $lockScope): FileLock {
 			$lock = FileLock::fromLockScope($lockScope, $this->configService->getTimeoutSeconds());
 			$this->generateToken($lock);
 			$lock->setCreation(time());
-			$this->notice('locking file', false, ['fileLock' => $lock]);
+			$this->logger->notice('locking file', ['fileLock' => $lock]);
 			$this->injectMetadata($lock);
 			$this->locksRequest->save($lock);
 			$this->propagateEtag($lockScope);
@@ -211,7 +210,7 @@ public function getAppName(string $appId): ?string {
 	 * @throws UnauthorizedUnlockException
 	 */
 	public function unlock(LockContext $lock, bool $force = false): FileLock {
-		$this->notice('unlocking file', false, ['fileLock' => $lock]);
+		$this->logger->notice('unlocking file', ['fileLock' => $lock]);
 
 		$known = $this->getLockFromFileId($lock->getNode()->getId());
 		if (!$force) {
@@ -302,15 +301,16 @@ public function unlockFile(int $fileId, string $userId, bool $force = false, int
 	public function getDeprecatedLocks(): array {
 		$timeout = (int)$this->configService->getAppValue(ConfigService::LOCK_TIMEOUT);
 		if ($timeout === 0) {
-			$this->notice(
-				'ConfigService::LOCK_TIMEOUT is not numerical, using default', true, ['current' => $timeout]
+			$this->logger->notice(
+				'ConfigService::LOCK_TIMEOUT is not numerical, using default', ['current' => $timeout, 'exception' => new \Exception()]
 			);
 			$timeout = (int)$this->configService->defaults[ConfigService::LOCK_TIMEOUT];
 		}
 
 		try {
 			$locks = $this->locksRequest->getLocksOlderThan($timeout);
 		} catch (Exception $e) {
+			$this->logger->warning('Failed to get locks older then timeout', ['exception' => $e]);
 			return [];
 		}
 
@@ -398,7 +398,7 @@ function (FileLock $lock) {
 			}, $locks
 		);
 
-		$this->notice('removing locks', false, ['ids' => $ids]);
+		$this->logger->notice('removing locks', ['ids' => $ids]);
 
 		$this->locksRequest->removeIds($ids);
 	}