Migrate files sharing to PSR LoggerInterface

Signed-off-by: Benjamin Gaussorgues <benjamin.gaussorgues@nextcloud.com>

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;
 	}
 
 	/**

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, '/');
 		}

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 {

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

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) {

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,

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')