diff --git a/lib/Controller/LockController.php b/lib/Controller/LockController.php index 4387749c..7967fb7b 100644 --- a/lib/Controller/LockController.php +++ b/lib/Controller/LockController.php @@ -16,7 +16,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; @@ -26,6 +25,7 @@ use OCP\IL10N; use OCP\IRequest; use OCP\IUserSession; +use Psr\Log\LoggerInterface; /** * Class LockController @@ -33,12 +33,13 @@ * @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, @@ -56,6 +57,7 @@ public function __construct( $this->registerResponder('xml', function ($data) { return $this->buildOCSResponse('xml', $data); }); + $this->logger = $logger; } @@ -171,7 +173,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); diff --git a/lib/Db/LocksRequest.php b/lib/Db/LocksRequest.php index 3948f0fc..50888ec0 100644 --- a/lib/Db/LocksRequest.php +++ b/lib/Db/LocksRequest.php @@ -13,6 +13,7 @@ use Exception; use OCA\FilesLock\Exceptions\LockNotFoundException; use OCA\FilesLock\Model\FileLock; +use OCP\AppFramework\Utility\ITimeFactory; use OCP\DB\QueryBuilder\IQueryBuilder; /** @@ -124,8 +125,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); } diff --git a/lib/Service/LockService.php b/lib/Service/LockService.php index ac806c9c..60ad0fc9 100644 --- a/lib/Service/LockService.php +++ b/lib/Service/LockService.php @@ -14,7 +14,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; @@ -27,14 +26,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; @@ -45,6 +43,7 @@ class LockService { private IEventDispatcher $eventDispatcher; private IUserSession $userSession; private IRequest $request; + private LoggerInterface $logger; private array $locks = []; @@ -64,6 +63,7 @@ public function __construct( IEventDispatcher $eventDispatcher, IUserSession $userSession, IRequest $request, + LoggerInterface $logger, ) { $this->l10n = $l10n; $this->userManager = $userManager; @@ -74,8 +74,7 @@ public function __construct( $this->eventDispatcher = $eventDispatcher; $this->userSession = $userSession; $this->request = $request; - - $this->setup('app', 'files_lock'); + $this->logger = $logger; } /** @@ -153,7 +152,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; @@ -165,7 +164,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); @@ -189,7 +188,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) { @@ -280,8 +279,8 @@ 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]; } @@ -289,6 +288,7 @@ public function getDeprecatedLocks(): array { try { $locks = $this->locksRequest->getLocksOlderThan($timeout); } catch (Exception $e) { + $this->logger->warning('Failed to get locks older then timeout', ['exception' => $e]); return []; } @@ -376,7 +376,7 @@ function (FileLock $lock) { }, $locks ); - $this->notice('removing locks', false, ['ids' => $ids]); + $this->logger->notice('removing locks', ['ids' => $ids]); $this->locksRequest->removeIds($ids); } diff --git a/lib/Tools/Traits/TLogger.php b/lib/Tools/Traits/TLogger.php deleted file mode 100644 index 4d74ba0f..00000000 --- a/lib/Tools/Traits/TLogger.php +++ /dev/null @@ -1,181 +0,0 @@ -throwable($t, self::$ERROR, $serializable); - } - - /** - * @param Throwable $t - * @param int $level - * @param array $serializable - */ - public function throwable(Throwable $t, int $level = 3, array $serializable = []): void { - $message = ''; - if (!empty($serializable)) { - $message = json_encode($serializable); - } - - $this->logger() - ->log( - $level, - $message, - [ - 'app' => $this->setup('app'), - 'exception' => $t - ] - ); - } - - - /** - * @param Exception $e - * @param array $serializable - */ - public function e(Exception $e, array $serializable = []): void { - $this->exception($e, self::$ERROR, $serializable); - } - - /** - * @param Exception $e - * @param int|array $level - * @param array $serializable - */ - public function exception(Exception $e, $level = 3, array $serializable = []): void { - if (is_array($level) && empty($serializable)) { - $serializable = $level; - $level = 3; - } - - $message = ''; - if (!empty($serializable)) { - $message = json_encode($serializable); - } - - if ($level === self::$DEBUG) { - $level = (int)$this->appConfig('debug_level'); - } - - $this->logger() - ->log( - $level, - $message, - [ - 'app' => $this->setup('app'), - 'exception' => $e - ] - ); - } - - - /** - * @param string $message - * @param bool $trace - * @param array $serializable - */ - public function emergency(string $message, bool $trace = false, array $serializable = []): void { - $this->log(self::$EMERGENCY, '[emergency] ' . $message, $trace, $serializable); - } - - /** - * @param string $message - * @param bool $trace - * @param array $serializable - */ - public function alert(string $message, bool $trace = false, array $serializable = []): void { - $this->log(self::$ALERT, '[alert] ' . $message, $trace, $serializable); - } - - /** - * @param string $message - * @param bool $trace - * @param array $serializable - */ - public function warning(string $message, bool $trace = false, array $serializable = []): void { - $this->log(self::$WARNING, '[warning] ' . $message, $trace, $serializable); - } - - /** - * @param string $message - * @param bool $trace - * @param array $serializable - */ - public function notice(string $message, bool $trace = false, array $serializable = []): void { - $this->log(self::$NOTICE, '[notice] ' . $message, $trace, $serializable); - } - - /** - * @param string $message - * @param array $serializable - */ - public function debug(string $message, array $serializable = []): void { - $message = '[debug] ' . $message; - $debugLevel = (int)$this->appConfig('debug_level'); - $this->log($debugLevel, $message, ($this->appConfig('debug_trace') === '1'), $serializable); - } - - - /** - * @param int $level - * @param string $message - * @param bool $trace - * @param array $serializable - */ - public function log(int $level, string $message, bool $trace = false, array $serializable = []): void { - $opts = ['app' => $this->setup('app')]; - if ($trace) { - $opts['exception'] = new HintException($message, json_encode($serializable)); - } elseif (!empty($serializable)) { - $message .= ' -- ' . json_encode($serializable); - } - - $this->logger() - ->log($level, $message, $opts); - } - - - /** - * @return LoggerInterface - */ - public function logger(): LoggerInterface { - if (isset($this->logger) && $this->logger instanceof LoggerInterface) { - return $this->logger; - } else { - return OC::$server->get(LoggerInterface::class); - } - } -} diff --git a/tests/Feature/LockFeatureTest.php b/tests/Feature/LockFeatureTest.php index 30ef120e..baae5d86 100644 --- a/tests/Feature/LockFeatureTest.php +++ b/tests/Feature/LockFeatureTest.php @@ -8,6 +8,7 @@ use OCA\FilesLock\AppInfo\Application; use OCA\FilesLock\Model\FileLock; use OCA\FilesLock\Service\ConfigService; +use OCA\FilesLock\Service\LockService; use OCP\AppFramework\Utility\ITimeFactory; use OCP\Files\File; use OCP\Files\IRootFolder; @@ -204,6 +205,16 @@ public function testLockUserExpire() { self::assertEquals('CCC', $file->getContent()); } + public function testExpiredLocksAreDeprecated() { + \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'); + $this->lockManager->lock(new LockContext($file, ILock::TYPE_USER, self::TEST_USER1)); + $this->toTheFuture(3600); + $deprecated = \OCP\Server::get(LockService::class)->getDeprecatedLocks(); + self::assertNotEmpty($deprecated); + } + public function testLockUserInfinite() { \OCP\Server::get(IConfig::class)->setAppValue(Application::APP_ID, ConfigService::LOCK_TIMEOUT, 0); $file = $this->loginAndGetUserFolder(self::TEST_USER1)