diff --git a/apps/files_sharing/lib/Controller/RemoteController.php b/apps/files_sharing/lib/Controller/RemoteController.php index 47523e08639d4..7618834e0590f 100644 --- a/apps/files_sharing/lib/Controller/RemoteController.php +++ b/apps/files_sharing/lib/Controller/RemoteController.php @@ -28,17 +28,10 @@ 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 ae21259e21e7a..f61665fe067fa 100644 --- a/apps/files_sharing/lib/Controller/ShareController.php +++ b/apps/files_sharing/lib/Controller/ShareController.php @@ -49,14 +49,9 @@ 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\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 @@ * @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 @@ protected function showIdentificationResult(bool $success = false): TemplateResp * @return bool */ protected function validateIdentity(?string $identityToken = null): bool { - if ($this->share->getShareType() !== IShare::TYPE_EMAIL) { return false; } @@ -496,7 +458,6 @@ public function downloadShare($token, $files = null, $path = '', $downloadStartS 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 65ad555f5bd4c..9eb69de7e83f1 100644 --- a/apps/files_sharing/lib/Listener/ShareInteractionListener.php +++ b/apps/files_sharing/lib/Listener/ShareInteractionListener.php @@ -30,10 +30,10 @@ 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 2ad7ede8e40a6..d4cc13842a3fa 100644 --- a/apps/files_sharing/lib/MountProvider.php +++ b/apps/files_sharing/lib/MountProvider.php @@ -36,54 +36,26 @@ 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 @@ public function getMountsForUser(IUser $user, IStorageFactory $loader) { $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 @@ private function buildSuperShares(array $allShares, \OCP\IUser $user) { // 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 5243c8d1594ac..0b4940446b9f5 100644 --- a/apps/files_sharing/lib/SharedStorage.php +++ b/apps/files_sharing/lib/SharedStorage.php @@ -54,6 +54,7 @@ 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,11 +81,6 @@ class SharedStorage extends \OC\Files\Storage\Wrapper\Jail implements ISharedSto /** @var string */ private $user; - /** - * @var \OCP\ILogger - */ - private $logger; - /** @var IStorage */ private $nonMaskedStorage; @@ -98,9 +94,11 @@ class SharedStorage extends \OC\Files\Storage\Wrapper\Jail implements ISharedSto private string $sourcePath = ''; - public function __construct($arguments) { + public function __construct( + array $arguments, + private LoggerInterface $logger, + ) { $this->ownerView = $arguments['ownerView']; - $this->logger = \OC::$server->getLogger(); $this->superShare = $arguments['superShare']; $this->groupedShares = $arguments['groupedShares']; @@ -173,7 +171,7 @@ private function init() { $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 7445fa1d7ab37..f29271ac3538e 100644 --- a/apps/files_sharing/tests/Controller/ShareControllerTest.php +++ b/apps/files_sharing/tests/Controller/ShareControllerTest.php @@ -54,7 +54,6 @@ use OCP\Files\Storage; use OCP\IConfig; use OCP\IL10N; -use OCP\ILogger; use OCP\IPreview; use OCP\IRequest; use OCP\ISession; @@ -71,6 +70,7 @@ use OC\Share20\Manager; use OCA\Files_Sharing\DefaultPublicShareTemplateProvider; use OCP\Share\IPublicShareTemplateFactory; +use Psr\Log\LoggerInterface; /** * @group DB @@ -78,7 +78,6 @@ * @package OCA\Files_Sharing\Controllers */ class ShareControllerTest extends \Test\TestCase { - /** @var string */ private $user; /** @var string */ @@ -160,7 +159,7 @@ protected function setUp(): void { $this->config, $this->urlGenerator, $this->userManager, - $this->createMock(ILogger::class), + $this->createMock(LoggerInterface::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 f7cf5156ec53f..af552f482181c 100644 --- a/apps/files_sharing/tests/MountProviderTest.php +++ b/apps/files_sharing/tests/MountProviderTest.php @@ -36,34 +36,33 @@ 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 @@ protected function setUp(): void { $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')