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 <opensource@fthiessen.de>

lib/Cron/ScheduledNotifications.php

@@ -12,27 +12,17 @@ use OCA\Deck\Notification\NotificationHelper;
 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,
+		protected CardMapper $cardMapper,
-		NotificationHelper $notificationHelper,
+		protected NotificationHelper $notificationHelper,
-		ILogger $logger
+		protected LoggerInterface $logger
 	) {
 		parent::__construct($time);
-		$this->cardMapper = $cardMapper;
-		$this->notificationHelper = $notificationHelper;
-		$this->logger = $logger;
 	}
 
 	/**

lib/Cron/SessionsCleanup.php

@@ -14,21 +14,19 @@ namespace OCA\Deck\Cron;
 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,
+	public function __construct(
-		SessionService $sessionService,
+		ITimeFactory $time,
-		ILogger $logger) {
+		private SessionService $sessionService,
+		private LoggerInterface $logger,
+	) {
 		parent::__construct($time);
-		$this->sessionService = $sessionService;
-		$this->logger = $logger;
 		$this->setInterval(SessionService::SESSION_VALID_TIME);
 	}
 

lib/Middleware/DefaultBoardMiddleware.php

@@ -10,27 +10,17 @@ use OCA\Deck\Service\DefaultBoardService;
 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 */
+	public function __construct(
-	private $logger;
+		private LoggerInterface $logger,
-	/** @var IL10N */
+		private IL10N $l10n,
-	private $l10n;
+		private DefaultBoardService $defaultBoardService,
-	/** @var DefaultBoardService */
+		private PermissionService $permissionService,
-	private $defaultBoardService;
+		private ?string $userId,
-	/** @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 beforeController($controller, $methodName) {
@@ -39,7 +29,7 @@ class DefaultBoardMiddleware extends Middleware {
 				$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]);
 		}
 	}
 }

lib/Middleware/ExceptionMiddleware.php

@@ -15,28 +15,19 @@ use OCP\AppFramework\Middleware;
 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) {
+	public function __construct(
-		$this->logger = $logger;
+		private LoggerInterface $logger,
-		$this->config = $config;
+		private IConfig $config,
-		$this->request = $request;
+		private IRequest $request,
+	) {
 	}
 
 	/**
@@ -69,9 +60,7 @@ class ExceptionMiddleware extends Middleware {
 		}
 
 		if ($exception instanceof StatusException) {
-			if ($this->config->getSystemValue('loglevel', ILogger::WARN) === ILogger::DEBUG) {
+			$this->logger->debug($exception->getMessage(), ['exception' => $exception]);
-				$this->logger->logException($exception);
-			}
 
 			if ($exception instanceof ConflictException) {
 				return new JSONResponse([
@@ -98,7 +87,7 @@ class ExceptionMiddleware extends Middleware {
 				'message' => $exceptionMessage,
 				'requestId' => $this->request->getId(),
 			];
-			$this->logger->logException($exception);
+			$this->logger->error($exception->getMessage(), ['exception' => $exception]);
 			if ($debugMode === true) {
 				$response['exception'] = (array) $exception;
 			}

lib/Service/CommentService.php

@@ -17,26 +17,22 @@ use OCP\Comments\IComment;
 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) {
+	public function __construct(
-		$this->commentsManager = $commentsManager;
+		private ICommentsManager $commentsManager,
-		$this->permissionService = $permissionService;
+		private PermissionService $permissionService,
-		$this->cardMapper = $cardMapper;
+		private CardMapper $cardMapper,
-		$this->userManager = $userManager;
+		private IUserManager $userManager,
-		$this->logger = $logger;
+		private LoggerInterface $logger,
-		$this->userId = $userId;
+		private ?string $userId,
+	) {
 	}
 
 	public function list(string $cardId, int $limit = 20, int $offset = 0): DataResponse {
@@ -187,8 +183,8 @@ class CommentService {
 				try {
 					$displayName = $this->commentsManager->resolveDisplayName($mention['type'], $mention['id']);
 				} catch (OutOfBoundsException $e) {
-					$this->logger->logException($e);
+					$this->logger->warning('Mention type not registered, can not resolve display name.', ['exception' => $e, 'mention_type' => $mention['type']]);
-					// No displayname, upon client's discretion what to display.
+					// No display name, upon client's discretion what to display.
 					$displayName = '';
 				}
 

lib/Service/FileService.php

@@ -20,37 +20,21 @@ use OCP\Files\SimpleFS\ISimpleFile;
 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,
+		private IL10N $l10n,
-		IAppData $appData,
+		private IAppData $appData,
-		IRequest $request,
+		private IRequest $request,
-		ILogger $logger,
+		private LoggerInterface $logger,
-		IRootFolder $rootFolder,
+		private IRootFolder $rootFolder,
-		IConfig $config,
+		private IConfig $config,
-		AttachmentMapper $attachmentMapper,
+		private AttachmentMapper $attachmentMapper,
-		IMimeTypeDetector $mimeTypeDetector
+		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 @@ class FileService implements IAttachmentService {
 	/**
 	 * Workaround until ISimpleFile can be fetched as a resource
 	 *
+	 * @return \OCP\Files\File
 	 * @throws \Exception
 	 */
 	private function getFileFromRootFolder(Attachment $attachment) {

tests/unit/Cron/ScheduledNoificationsTest.php

@@ -27,29 +27,24 @@ use OCA\Deck\Db\Card;
 use OCA\Deck\Db\CardMapper;
 use OCA\Deck\Notification\NotificationHelper;
 use OCP\AppFramework\Utility\ITimeFactory;
-use OCP\ILogger;
 use PHPUnit\Framework\MockObject\MockObject;
+use Psr\Log\LoggerInterface;
 use Test\TestCase;
 
 class ScheduledNoificationsTest extends TestCase {
 
-	/** @var ITimeFactory|MockObject */
+	protected ITimeFactory&MockObject $timeFactory;
-	protected $timeFactory;
+	protected CardMapper&MockObject $cardMapper;
-	/** @var CardMapper|MockObject */
+	protected NotificationHelper&MockObject $notificationHelper;
-	protected $cardMapper;
+	protected LoggerInterface&MockObject $logger;
-	/** @var NotificationHelper|MockObject */
+	protected ScheduledNotifications $scheduledNotifications;
-	protected $notificationHelper;
-	/** @var ILogger|MockObject */
-	protected $logger;
-	/** @var ScheduledNotifications */
-	protected $scheduledNotifications;
 
 	public function setUp(): void {
 		parent::setUp();
 		$this->timeFactory = $this->createMock(ITimeFactory::class);
 		$this->cardMapper = $this->createMock(CardMapper::class);
 		$this->notificationHelper = $this->createMock(NotificationHelper::class);
-		$this->logger = $this->createMock(ILogger::class);
+		$this->logger = $this->createMock(LoggerInterface::class);
 		$this->scheduledNotifications = new ScheduledNotifications($this->timeFactory, $this->cardMapper, $this->notificationHelper, $this->logger);
 	}
 

tests/unit/Middleware/ExceptionMiddlewareTest.php

@@ -32,21 +32,20 @@ use OCA\Deck\Service\PermissionService;
 use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http\JSONResponse;
 use OCP\IConfig;
-use OCP\ILogger;
 use OCP\IRequest;
+use PHPUnit\Framework\MockObject\MockObject;
+use Psr\Log\LoggerInterface;
 
 class ExceptionMiddlewareTest extends \Test\TestCase {
 
-	/** @var ILogger */
+	private LoggerInterface&MockObject $logger;
-	private $logger;
+	private IConfig&MockObject $config;
-	/** @var IConfig */
+	private IRequest&MockObject $request;
-	private $config;
+	private Controller&MockObject $controller;
-	private $request;
-	private $controller;
 	private $exceptionMiddleware;
 
 	public function setUp(): void {
-		$this->logger = $this->createMock(ILogger::class);
+		$this->logger = $this->createMock(LoggerInterface::class);
 		$this->config = $this->createMock(IConfig::class);
 		$this->request = $this->createMock(IRequest::class);
 		$this->controller = $this->createMock(Controller::class);

tests/unit/Service/FileServiceTest.php

@@ -34,38 +34,29 @@ use OCP\Files\SimpleFS\ISimpleFile;
 use OCP\Files\SimpleFS\ISimpleFolder;
 use OCP\IConfig;
 use OCP\IL10N;
-use OCP\ILogger;
 use OCP\IRequest;
 use PHPUnit\Framework\MockObject\MockObject;
+use Psr\Log\LoggerInterface;
 use Test\TestCase;
 
 class FileServiceTest extends TestCase {
 
-	/** @var IL10N|MockObject */
+	private IL10N&MockObject $l10n;
-	private $l10n;
+	private IAppData&MockObject $appData;
-	/** @var IAppData|MockObject */
+	private IRequest&MockObject $request;
-	private $appData;
+	private LoggerInterface&MockObject $logger;
-	/** @var IRequest|MockObject */
+	private IRootFolder&MockObject $rootFolder;
-	private $request;
+	private IConfig&MockObject $config;
-	/** @var ILogger|MockObject */
+	private AttachmentMapper&MockObject $attachmentMapper;
-	private $logger;
+	private IMimeTypeDetector&MockObject $mimeTypeDetector;
-	/** @var FileService */
+	private FileService $fileService;
-	private $fileService;
+	
-	/** @var IRootFolder */
-	private $rootFolder;
-	/** @var IConfig */
-	private $config;
-	/** @var AttachmentMapper|MockObject */
-	private $attachmentMapper;
-	/** @var IMimeTypeDetector|MockObject */
-	private $mimeTypeDetector;
-
 	public function setUp(): void {
 		parent::setUp();
 		$this->request = $this->createMock(IRequest::class);
 		$this->appData = $this->createMock(IAppData::class);
 		$this->l10n = $this->createMock(IL10N::class);
-		$this->logger = $this->createMock(ILogger::class);
+		$this->logger = $this->createMock(LoggerInterface::class);
 		$this->rootFolder = $this->createMock(IRootFolder::class);
 		$this->config = $this->createMock(IConfig::class);
 		$this->attachmentMapper = $this->createMock(AttachmentMapper::class);