From: Benjamin Gaussorgues Date: Wed, 5 Jul 2023 14:28:22 +0000 (+0200) Subject: Migrate files sharing to PSR LoggerInterface X-Git-Tag: v28.0.0beta1~741^2 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=29c1848edaa4e0de3b20c54fdcc30d58d7409a1f;p=nextcloud-server.git Migrate files sharing to PSR LoggerInterface Signed-off-by: Benjamin Gaussorgues --- diff --git a/apps/files_sharing/lib/Controller/RemoteController.php b/apps/files_sharing/lib/Controller/RemoteController.php index 47523e08639..7618834e059 100644 --- a/apps/files_sharing/lib/Controller/RemoteController.php +++ b/apps/files_sharing/lib/Controller/RemoteController.php @@ -28,17 +28,10 @@ use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\OCS\OCSForbiddenException; use OCP\AppFramework\OCS\OCSNotFoundException; use OCP\AppFramework\OCSController; -use OCP\ILogger; use OCP\IRequest; +use Psr\Log\LoggerInterface; class RemoteController extends OCSController { - - /** @var Manager */ - private $externalManager; - - /** @var ILogger */ - private $logger; - /** * @NoAdminRequired * @@ -48,14 +41,13 @@ class RemoteController extends OCSController { * @param IRequest $request * @param Manager $externalManager */ - public function __construct($appName, - IRequest $request, - Manager $externalManager, - ILogger $logger) { + public function __construct( + $appName, + IRequest $request, + private Manager $externalManager, + private LoggerInterface $logger, + ) { parent::__construct($appName, $request); - - $this->externalManager = $externalManager; - $this->logger = $logger; } /** diff --git a/apps/files_sharing/lib/Controller/ShareController.php b/apps/files_sharing/lib/Controller/ShareController.php index ae21259e21e..f61665fe067 100644 --- a/apps/files_sharing/lib/Controller/ShareController.php +++ b/apps/files_sharing/lib/Controller/ShareController.php @@ -49,14 +49,9 @@ use OCA\FederatedFileSharing\FederatedShareProvider; use OCA\Files_Sharing\Activity\Providers\Downloads; use OCA\Files_Sharing\Event\BeforeTemplateRenderedEvent; use OCA\Files_Sharing\Event\ShareLinkAccessedEvent; -use OCA\Viewer\Event\LoadViewer; use OCP\Accounts\IAccountManager; use OCP\AppFramework\AuthPublicShareController; use OCP\AppFramework\Http\NotFoundResponse; -use OCP\AppFramework\Http\Template\ExternalShareMenuAction; -use OCP\AppFramework\Http\Template\LinkMenuAction; -use OCP\AppFramework\Http\Template\PublicTemplateResponse; -use OCP\AppFramework\Http\Template\SimpleMenuAction; use OCP\AppFramework\Http\TemplateResponse; use OCP\Defaults; use OCP\EventDispatcher\IEventDispatcher; @@ -65,12 +60,10 @@ use OCP\Files\IRootFolder; use OCP\Files\NotFoundException; use OCP\IConfig; use OCP\IL10N; -use OCP\ILogger; use OCP\IPreview; use OCP\IRequest; use OCP\ISession; use OCP\IURLGenerator; -use OCP\IUser; use OCP\IUserManager; use OCP\Security\ISecureRandom; use OCP\Share; @@ -86,64 +79,34 @@ use OCP\Template; * @package OCA\Files_Sharing\Controllers */ class ShareController extends AuthPublicShareController { - protected IConfig $config; - protected IUserManager $userManager; - protected ILogger $logger; - protected \OCP\Activity\IManager $activityManager; - protected IPreview $previewManager; - protected IRootFolder $rootFolder; - protected FederatedShareProvider $federatedShareProvider; - protected IAccountManager $accountManager; - protected IEventDispatcher $eventDispatcher; - protected IL10N $l10n; - protected Defaults $defaults; - protected ShareManager $shareManager; - protected ISecureRandom $secureRandom; protected ?Share\IShare $share = null; - private IPublicShareTemplateFactory $publicShareTemplateFactory; + + public const SHARE_ACCESS = 'access'; + public const SHARE_AUTH = 'auth'; + public const SHARE_DOWNLOAD = 'download'; public function __construct( string $appName, IRequest $request, - IConfig $config, + protected IConfig $config, IURLGenerator $urlGenerator, - IUserManager $userManager, - ILogger $logger, - \OCP\Activity\IManager $activityManager, - ShareManager $shareManager, + protected IUserManager $userManager, + protected \OCP\Activity\IManager $activityManager, + protected ShareManager $shareManager, ISession $session, - IPreview $previewManager, - IRootFolder $rootFolder, - FederatedShareProvider $federatedShareProvider, - IAccountManager $accountManager, - IEventDispatcher $eventDispatcher, - IL10N $l10n, - ISecureRandom $secureRandom, - Defaults $defaults, - IPublicShareTemplateFactory $publicShareTemplateFactory + protected IPreview $previewManager, + protected IRootFolder $rootFolder, + protected FederatedShareProvider $federatedShareProvider, + protected IAccountManager $accountManager, + protected IEventDispatcher $eventDispatcher, + protected IL10N $l10n, + protected ISecureRandom $secureRandom, + protected Defaults $defaults, + private IPublicShareTemplateFactory $publicShareTemplateFactory, ) { parent::__construct($appName, $request, $session, $urlGenerator); - - $this->config = $config; - $this->userManager = $userManager; - $this->logger = $logger; - $this->activityManager = $activityManager; - $this->previewManager = $previewManager; - $this->rootFolder = $rootFolder; - $this->federatedShareProvider = $federatedShareProvider; - $this->accountManager = $accountManager; - $this->eventDispatcher = $eventDispatcher; - $this->l10n = $l10n; - $this->secureRandom = $secureRandom; - $this->defaults = $defaults; - $this->shareManager = $shareManager; - $this->publicShareTemplateFactory = $publicShareTemplateFactory; } - public const SHARE_ACCESS = 'access'; - public const SHARE_AUTH = 'auth'; - public const SHARE_DOWNLOAD = 'download'; - /** * @PublicPage * @NoCSRFRequired @@ -212,7 +175,6 @@ class ShareController extends AuthPublicShareController { * @return bool */ protected function validateIdentity(?string $identityToken = null): bool { - if ($this->share->getShareType() !== IShare::TYPE_EMAIL) { return false; } @@ -496,7 +458,6 @@ class ShareController extends AuthPublicShareController { if (!empty($downloadStartSecret) && !isset($downloadStartSecret[32]) && preg_match('!^[a-zA-Z0-9]+$!', $downloadStartSecret) === 1) { - // FIXME: set on the response once we use an actual app framework response setcookie('ocDownloadStarted', $downloadStartSecret, time() + 20, '/'); } diff --git a/apps/files_sharing/lib/Listener/ShareInteractionListener.php b/apps/files_sharing/lib/Listener/ShareInteractionListener.php index 65ad555f5bd..9eb69de7e83 100644 --- a/apps/files_sharing/lib/Listener/ShareInteractionListener.php +++ b/apps/files_sharing/lib/Listener/ShareInteractionListener.php @@ -30,10 +30,10 @@ use OCP\Contacts\Events\ContactInteractedWithEvent; use OCP\EventDispatcher\Event; use OCP\EventDispatcher\IEventDispatcher; use OCP\EventDispatcher\IEventListener; -use OCP\ILogger; use OCP\IUserManager; use OCP\Share\Events\ShareCreatedEvent; use OCP\Share\IShare; +use Psr\Log\LoggerInterface; use function in_array; class ShareInteractionListener implements IEventListener { @@ -43,21 +43,11 @@ class ShareInteractionListener implements IEventListener { IShare::TYPE_REMOTE, ]; - /** @var IEventDispatcher */ - private $dispatcher; - - /** @var IUserManager */ - private $userManager; - - /** @var ILogger */ - private $logger; - - public function __construct(IEventDispatcher $dispatcher, - IUserManager $userManager, - ILogger $logger) { - $this->dispatcher = $dispatcher; - $this->userManager = $userManager; - $this->logger = $logger; + public function __construct( + private IEventDispatcher $dispatcher, + private IUserManager $userManager, + private LoggerInterface $logger, + ) { } public function handle(Event $event): void { diff --git a/apps/files_sharing/lib/MountProvider.php b/apps/files_sharing/lib/MountProvider.php index 2ad7ede8e40..d4cc13842a3 100644 --- a/apps/files_sharing/lib/MountProvider.php +++ b/apps/files_sharing/lib/MountProvider.php @@ -36,54 +36,26 @@ use OCP\Files\Config\IMountProvider; use OCP\Files\Storage\IStorageFactory; use OCP\ICacheFactory; use OCP\IConfig; -use OCP\ILogger; use OCP\IUser; -use OCP\Share\IAttributes; use OCP\Share\IManager; use OCP\Share\IShare; +use Psr\Log\LoggerInterface; class MountProvider implements IMountProvider { - /** - * @var \OCP\IConfig - */ - protected $config; - - /** - * @var IManager - */ - protected $shareManager; - - /** - * @var ILogger - */ - protected $logger; - - /** @var IEventDispatcher */ - protected $eventDispatcher; - - /** @var ICacheFactory */ - protected $cacheFactory; - /** * @param \OCP\IConfig $config * @param IManager $shareManager - * @param ILogger $logger + * @param LoggerInterface $logger */ public function __construct( - IConfig $config, - IManager $shareManager, - ILogger $logger, - IEventDispatcher $eventDispatcher, - ICacheFactory $cacheFactory + protected IConfig $config, + protected IManager $shareManager, + protected LoggerInterface $logger, + protected IEventDispatcher $eventDispatcher, + protected ICacheFactory $cacheFactory ) { - $this->config = $config; - $this->shareManager = $shareManager; - $this->logger = $logger; - $this->eventDispatcher = $eventDispatcher; - $this->cacheFactory = $cacheFactory; } - /** * Get all mountpoints applicable for the user and check for shares where we need to update the etags * @@ -157,8 +129,13 @@ class MountProvider implements IMountProvider { $mounts[$additionalMount->getMountPoint()] = $additionalMount; } } catch (\Exception $e) { - $this->logger->logException($e); - $this->logger->error('Error while trying to create shared mount'); + $this->logger->error( + 'Error while trying to create shared mount', + [ + 'app' => 'files_sharing', + 'exception' => $e, + ], + ); } } @@ -234,7 +211,9 @@ class MountProvider implements IMountProvider { // Since these are readly available here, storing them // enables the DAV FilesPlugin to avoid executing many // DB queries to retrieve the same information. - $allNotes = implode("\n", array_map(function ($sh) { return $sh->getNote(); }, $shares)); + $allNotes = implode("\n", array_map(function ($sh) { + return $sh->getNote(); + }, $shares)); $superShare->setNote($allNotes); // use most permissive permissions diff --git a/apps/files_sharing/lib/SharedStorage.php b/apps/files_sharing/lib/SharedStorage.php index 5243c8d1594..759394f22e6 100644 --- a/apps/files_sharing/lib/SharedStorage.php +++ b/apps/files_sharing/lib/SharedStorage.php @@ -54,6 +54,7 @@ use OCP\Files\Storage\IDisableEncryptionStorage; use OCP\Files\Storage\IStorage; use OCP\Lock\ILockingProvider; use OCP\Share\IShare; +use Psr\Log\LoggerInterface; /** * Convert target path to source path and pass the function call to the correct storage provider @@ -80,10 +81,7 @@ class SharedStorage extends \OC\Files\Storage\Wrapper\Jail implements ISharedSto /** @var string */ private $user; - /** - * @var \OCP\ILogger - */ - private $logger; + private LoggerInterface $logger; /** @var IStorage */ private $nonMaskedStorage; @@ -100,7 +98,7 @@ class SharedStorage extends \OC\Files\Storage\Wrapper\Jail implements ISharedSto public function __construct($arguments) { $this->ownerView = $arguments['ownerView']; - $this->logger = \OC::$server->getLogger(); + $this->logger = \OC::$server->get(LoggerInterface::class); $this->superShare = $arguments['superShare']; $this->groupedShares = $arguments['groupedShares']; @@ -173,7 +171,7 @@ class SharedStorage extends \OC\Files\Storage\Wrapper\Jail implements ISharedSto $this->storage = new FailedStorage(['exception' => $e]); $this->cache = new FailedCache(); $this->rootPath = ''; - $this->logger->logException($e); + $this->logger->error($e->getMessage(), ['exception' => $e]); } if (!$this->nonMaskedStorage) { diff --git a/apps/files_sharing/tests/Controller/ShareControllerTest.php b/apps/files_sharing/tests/Controller/ShareControllerTest.php index 7445fa1d7ab..2a9ab1a3b08 100644 --- a/apps/files_sharing/tests/Controller/ShareControllerTest.php +++ b/apps/files_sharing/tests/Controller/ShareControllerTest.php @@ -54,7 +54,6 @@ use OCP\Files\NotFoundException; use OCP\Files\Storage; use OCP\IConfig; use OCP\IL10N; -use OCP\ILogger; use OCP\IPreview; use OCP\IRequest; use OCP\ISession; @@ -78,7 +77,6 @@ use OCP\Share\IPublicShareTemplateFactory; * @package OCA\Files_Sharing\Controllers */ class ShareControllerTest extends \Test\TestCase { - /** @var string */ private $user; /** @var string */ @@ -160,7 +158,6 @@ class ShareControllerTest extends \Test\TestCase { $this->config, $this->urlGenerator, $this->userManager, - $this->createMock(ILogger::class), $this->createMock(IManager::class), $this->shareManager, $this->session, diff --git a/apps/files_sharing/tests/MountProviderTest.php b/apps/files_sharing/tests/MountProviderTest.php index f7cf5156ec5..af552f48218 100644 --- a/apps/files_sharing/tests/MountProviderTest.php +++ b/apps/files_sharing/tests/MountProviderTest.php @@ -36,34 +36,33 @@ use OCP\Files\IRootFolder; use OCP\Files\Storage\IStorageFactory; use OCP\ICacheFactory; use OCP\IConfig; -use OCP\ILogger; use OCP\IUser; use OCP\IUserManager; use OCP\Share\IAttributes as IShareAttributes; use OCP\Share\IManager; use OCP\Share\IShare; +use Psr\Log\LoggerInterface; /** * @group DB */ class MountProviderTest extends \Test\TestCase { - /** @var MountProvider */ private $provider; - /** @var IConfig|\PHPUnit\Framework\MockObject\MockObject */ + /** @var IConfig|MockObject */ private $config; - /** @var IUser|\PHPUnit\Framework\MockObject\MockObject */ + /** @var IUser|MockObject */ private $user; - /** @var IStorageFactory|\PHPUnit\Framework\MockObject\MockObject */ + /** @var IStorageFactory|MockObject */ private $loader; - /** @var IManager|\PHPUnit\Framework\MockObject\MockObject */ + /** @var IManager|MockObject */ private $shareManager; - /** @var ILogger | \PHPUnit\Framework\MockObject\MockObject */ + /** @var LoggerInterface|MockObject */ private $logger; protected function setUp(): void { @@ -73,7 +72,7 @@ class MountProviderTest extends \Test\TestCase { $this->user = $this->getMockBuilder(IUser::class)->getMock(); $this->loader = $this->getMockBuilder('OCP\Files\Storage\IStorageFactory')->getMock(); $this->shareManager = $this->getMockBuilder(IManager::class)->getMock(); - $this->logger = $this->getMockBuilder(ILogger::class)->getMock(); + $this->logger = $this->getMockBuilder(LoggerInterface::class)->getMock(); $eventDispatcher = $this->createMock(IEventDispatcher::class); $cacheFactory = $this->createMock(ICacheFactory::class); $cacheFactory->method('createLocal')