From 5cee0b7613f67a5eb3270b42c0a57e5af8c47fa1 Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Mon, 19 Aug 2024 13:48:49 +0200 Subject: [PATCH] refactor: Migrate away from deprecated `ILogger` interface to PSR-3 Mostly replace `ILogger` with `LoggerInterface` and some minor cleanup (constructor property promotion). Some places used the deprecated `logException` this is easy to migrate by simply use the appropriate loglevel on the logger and place the exception under the `exception` key in the context. Also the manual checking of the configured log level is not needed, as this is already done by the logger. Signed-off-by: Ferdinand Thiessen --- lib/Cron/ScheduledNotifications.php | 18 +++--------- lib/Cron/SessionsCleanup.php | 14 ++++----- lib/Middleware/DefaultBoardMiddleware.php | 28 ++++++------------ lib/Middleware/ExceptionMiddleware.php | 27 ++++++----------- lib/Service/CommentService.php | 30 +++++++++---------- lib/Service/FileService.php | 35 +++++++---------------- 6 files changed, 50 insertions(+), 102 deletions(-) diff --git a/lib/Cron/ScheduledNotifications.php b/lib/Cron/ScheduledNotifications.php index 44305cbe9..2c445399f 100644 --- a/lib/Cron/ScheduledNotifications.php +++ b/lib/Cron/ScheduledNotifications.php @@ -12,27 +12,17 @@ use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Utility\ITimeFactory; use OCP\BackgroundJob\Job; -use OCP\ILogger; +use Psr\Log\LoggerInterface; class ScheduledNotifications extends Job { - /** @var CardMapper */ - protected $cardMapper; - /** @var NotificationHelper */ - protected $notificationHelper; - /** @var ILogger */ - protected $logger; - public function __construct( ITimeFactory $time, - CardMapper $cardMapper, - NotificationHelper $notificationHelper, - ILogger $logger + protected CardMapper $cardMapper, + protected NotificationHelper $notificationHelper, + protected LoggerInterface $logger ) { parent::__construct($time); - $this->cardMapper = $cardMapper; - $this->notificationHelper = $notificationHelper; - $this->logger = $logger; } /** diff --git a/lib/Cron/SessionsCleanup.php b/lib/Cron/SessionsCleanup.php index 22bef290b..bbc4cfdce 100644 --- a/lib/Cron/SessionsCleanup.php +++ b/lib/Cron/SessionsCleanup.php @@ -14,21 +14,19 @@ use OCA\Deck\Service\SessionService; use OCP\AppFramework\Utility\ITimeFactory; use OCP\BackgroundJob\TimedJob; -use OCP\ILogger; +use Psr\Log\LoggerInterface; class SessionsCleanup extends TimedJob { - private $sessionService; private $documentService; - private $logger; private $imageService; - public function __construct(ITimeFactory $time, - SessionService $sessionService, - ILogger $logger) { + public function __construct( + ITimeFactory $time, + private SessionService $sessionService, + private LoggerInterface $logger, + ) { parent::__construct($time); - $this->sessionService = $sessionService; - $this->logger = $logger; $this->setInterval(SessionService::SESSION_VALID_TIME); } diff --git a/lib/Middleware/DefaultBoardMiddleware.php b/lib/Middleware/DefaultBoardMiddleware.php index ff0eb888d..344d463b6 100644 --- a/lib/Middleware/DefaultBoardMiddleware.php +++ b/lib/Middleware/DefaultBoardMiddleware.php @@ -10,27 +10,17 @@ use OCA\Deck\Service\PermissionService; use OCP\AppFramework\Middleware; use OCP\IL10N; -use OCP\ILogger; +use Psr\Log\LoggerInterface; class DefaultBoardMiddleware extends Middleware { - /** @var ILogger */ - private $logger; - /** @var IL10N */ - private $l10n; - /** @var DefaultBoardService */ - private $defaultBoardService; - /** @var PermissionService */ - private $permissionService; - /** @var string|null */ - private $userId; - - public function __construct(ILogger $logger, IL10N $l10n, DefaultBoardService $defaultBoardService, PermissionService $permissionService, $userId) { - $this->logger = $logger; - $this->l10n = $l10n; - $this->defaultBoardService = $defaultBoardService; - $this->permissionService = $permissionService; - $this->userId = $userId; + public function __construct( + private LoggerInterface $logger, + private IL10N $l10n, + private DefaultBoardService $defaultBoardService, + private PermissionService $permissionService, + private ?string $userId, + ) { } public function beforeController($controller, $methodName) { @@ -39,7 +29,7 @@ public function beforeController($controller, $methodName) { $this->defaultBoardService->createDefaultBoard($this->l10n->t('Personal'), $this->userId, '0087C5'); } } catch (\Throwable $e) { - $this->logger->logException($e); + $this->logger->error('Could not create default board', ['exception' => $e]); } } } diff --git a/lib/Middleware/ExceptionMiddleware.php b/lib/Middleware/ExceptionMiddleware.php index 9190dd24d..6da9dd5ca 100644 --- a/lib/Middleware/ExceptionMiddleware.php +++ b/lib/Middleware/ExceptionMiddleware.php @@ -15,28 +15,19 @@ use OCP\AppFramework\OCS\OCSException; use OCP\AppFramework\OCSController; use OCP\IConfig; -use OCP\ILogger; use OCP\IRequest; +use Psr\Log\LoggerInterface; class ExceptionMiddleware extends Middleware { - /** @var ILogger */ - private $logger; - /** @var IConfig */ - private $config; - /** @var IRequest */ - private $request; - /** * SharingMiddleware constructor. - * - * @param ILogger $logger - * @param IConfig $config */ - public function __construct(ILogger $logger, IConfig $config, IRequest $request) { - $this->logger = $logger; - $this->config = $config; - $this->request = $request; + public function __construct( + private LoggerInterface $logger, + private IConfig $config, + private IRequest $request, + ) { } /** @@ -69,9 +60,7 @@ public function afterException($controller, $methodName, \Exception $exception) } if ($exception instanceof StatusException) { - if ($this->config->getSystemValue('loglevel', ILogger::WARN) === ILogger::DEBUG) { - $this->logger->logException($exception); - } + $this->logger->debug($exception->getMessage(), ['exception' => $exception]); if ($exception instanceof ConflictException) { return new JSONResponse([ @@ -98,7 +87,7 @@ public function afterException($controller, $methodName, \Exception $exception) 'message' => $exceptionMessage, 'requestId' => $this->request->getId(), ]; - $this->logger->logException($exception); + $this->logger->error($exception->getMessage(), ['exception' => $exception]); if ($debugMode === true) { $response['exception'] = (array) $exception; } diff --git a/lib/Service/CommentService.php b/lib/Service/CommentService.php index d06c7cd93..47980a051 100644 --- a/lib/Service/CommentService.php +++ b/lib/Service/CommentService.php @@ -17,26 +17,22 @@ use OCP\Comments\ICommentsManager; use OCP\Comments\MessageTooLongException; use OCP\Comments\NotFoundException as CommentNotFoundException; -use OCP\ILogger; use OCP\IUserManager; use OutOfBoundsException; +use Psr\Log\LoggerInterface; + use function is_numeric; class CommentService { - private ICommentsManager $commentsManager; - private IUserManager $userManager; - private CardMapper $cardMapper; - private PermissionService $permissionService; - private ILogger $logger; - private ?string $userId; - - public function __construct(ICommentsManager $commentsManager, PermissionService $permissionService, CardMapper $cardMapper, IUserManager $userManager, ILogger $logger, ?string $userId) { - $this->commentsManager = $commentsManager; - $this->permissionService = $permissionService; - $this->cardMapper = $cardMapper; - $this->userManager = $userManager; - $this->logger = $logger; - $this->userId = $userId; + + public function __construct( + private ICommentsManager $commentsManager, + private PermissionService $permissionService, + private CardMapper $cardMapper, + private IUserManager $userManager, + private LoggerInterface $logger, + private ?string $userId, + ) { } public function list(string $cardId, int $limit = 20, int $offset = 0): DataResponse { @@ -187,8 +183,8 @@ private function formatComment(IComment $comment, $addReplyTo = false): array { try { $displayName = $this->commentsManager->resolveDisplayName($mention['type'], $mention['id']); } catch (OutOfBoundsException $e) { - $this->logger->logException($e); - // No displayname, upon client's discretion what to display. + $this->logger->warning('Mention type not registered, can not resolve display name.', ['exception' => $e, 'mention_type' => $mention['type']]); + // No display name, upon client's discretion what to display. $displayName = ''; } diff --git a/lib/Service/FileService.php b/lib/Service/FileService.php index 529562636..77466a8c0 100644 --- a/lib/Service/FileService.php +++ b/lib/Service/FileService.php @@ -20,37 +20,21 @@ use OCP\Files\SimpleFS\ISimpleFolder; use OCP\IConfig; use OCP\IL10N; -use OCP\ILogger; use OCP\IRequest; +use Psr\Log\LoggerInterface; class FileService implements IAttachmentService { - private $l10n; - private $appData; - private $request; - private $logger; - private $rootFolder; - private $config; - private $attachmentMapper; - private $mimeTypeDetector; public function __construct( - IL10N $l10n, - IAppData $appData, - IRequest $request, - ILogger $logger, - IRootFolder $rootFolder, - IConfig $config, - AttachmentMapper $attachmentMapper, - IMimeTypeDetector $mimeTypeDetector + private IL10N $l10n, + private IAppData $appData, + private IRequest $request, + private LoggerInterface $logger, + private IRootFolder $rootFolder, + private IConfig $config, + private AttachmentMapper $attachmentMapper, + private IMimeTypeDetector $mimeTypeDetector ) { - $this->l10n = $l10n; - $this->appData = $appData; - $this->request = $request; - $this->logger = $logger; - $this->rootFolder = $rootFolder; - $this->config = $config; - $this->attachmentMapper = $attachmentMapper; - $this->mimeTypeDetector = $mimeTypeDetector; } /** @@ -193,6 +177,7 @@ public function delete(Attachment $attachment) { /** * Workaround until ISimpleFile can be fetched as a resource * + * @return \OCP\Files\File * @throws \Exception */ private function getFileFromRootFolder(Attachment $attachment) {