Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Migrate away from deprecated ILogger interface to PSR-3 #6230

Merged
merged 1 commit into from
Aug 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 4 additions & 14 deletions lib/Cron/ScheduledNotifications.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down
14 changes: 6 additions & 8 deletions lib/Cron/SessionsCleanup.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
28 changes: 9 additions & 19 deletions lib/Middleware/DefaultBoardMiddleware.php
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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]);
}
}
}
27 changes: 8 additions & 19 deletions lib/Middleware/ExceptionMiddleware.php
Original file line number Diff line number Diff line change
Expand Up @@ -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,
) {
}

/**
Expand Down Expand Up @@ -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([
Expand All @@ -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;
}
Expand Down
30 changes: 13 additions & 17 deletions lib/Service/CommentService.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 = '';
}

Expand Down
35 changes: 10 additions & 25 deletions lib/Service/FileService.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down Expand Up @@ -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) {
Expand Down
19 changes: 7 additions & 12 deletions tests/unit/Cron/ScheduledNoificationsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,29 +27,24 @@
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 $timeFactory;
/** @var CardMapper|MockObject */
protected $cardMapper;
/** @var NotificationHelper|MockObject */
protected $notificationHelper;
/** @var ILogger|MockObject */
protected $logger;
/** @var ScheduledNotifications */
protected $scheduledNotifications;
protected ITimeFactory&MockObject $timeFactory;
protected CardMapper&MockObject $cardMapper;
protected NotificationHelper&MockObject $notificationHelper;
protected LoggerInterface&MockObject $logger;
protected ScheduledNotifications $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);
}

Expand Down
15 changes: 7 additions & 8 deletions tests/unit/Middleware/ExceptionMiddlewareTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,21 +32,20 @@
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 $logger;
/** @var IConfig */
private $config;
private $request;
private $controller;
private LoggerInterface&MockObject $logger;
private IConfig&MockObject $config;
private IRequest&MockObject $request;
private Controller&MockObject $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);
Expand Down
33 changes: 12 additions & 21 deletions tests/unit/Service/FileServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,38 +34,29 @@
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 $l10n;
/** @var IAppData|MockObject */
private $appData;
/** @var IRequest|MockObject */
private $request;
/** @var ILogger|MockObject */
private $logger;
/** @var FileService */
private $fileService;
/** @var IRootFolder */
private $rootFolder;
/** @var IConfig */
private $config;
/** @var AttachmentMapper|MockObject */
private $attachmentMapper;
/** @var IMimeTypeDetector|MockObject */
private $mimeTypeDetector;

private IL10N&MockObject $l10n;
private IAppData&MockObject $appData;
private IRequest&MockObject $request;
private LoggerInterface&MockObject $logger;
private IRootFolder&MockObject $rootFolder;
private IConfig&MockObject $config;
private AttachmentMapper&MockObject $attachmentMapper;
private IMimeTypeDetector&MockObject $mimeTypeDetector;
private FileService $fileService;

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);
Expand Down