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 <[email protected]>
nextcloud · Aug 19, 2024 · 5cee0b7 · 5cee0b7
1 parent 37b355b
commit 5cee0b7
Show file tree

Hide file tree

Showing 6 changed files with 50 additions and 102 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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) {