Fix view-only code after code review comments

Signed-off-by: Vincent Petry <[email protected]>

apps/dav/lib/Connector/Sabre/ServerFactory.php

@@ -161,7 +161,7 @@ public function createServer(string $baseUri,
 
 			// Allow view-only plugin for webdav requests
 			$server->addPlugin(new ViewOnlyPlugin(
-				\OC::$server->getLogger()
+				$this->logger
 			));
 
 			if ($this->userSession->isLoggedIn()) {

apps/dav/lib/DAV/ViewOnlyPlugin.php

@@ -26,7 +26,7 @@
 use OCA\DAV\Meta\MetaFile;
 use OCP\Files\FileInfo;
 use OCP\Files\NotFoundException;
-use OCP\ILogger;
+use Psr\Log\LoggerInterface;
 use Sabre\DAV\Server;
 use Sabre\DAV\ServerPlugin;
 use Sabre\HTTP\RequestInterface;
@@ -40,13 +40,13 @@ class ViewOnlyPlugin extends ServerPlugin {
 	/** @var Server $server */
 	private $server;
 
-	/** @var ILogger $logger */
+	/** @var LoggerInterface $logger */
 	private $logger;
 
 	/**
-	 * @param ILogger $logger
+	 * @param LoggerInterface $logger
 	 */
-	public function __construct(ILogger $logger) {
+	public function __construct(LoggerInterface $logger) {
 		$this->logger = $logger;
 		$this->server = null;
 	}

apps/dav/lib/Server.php

@@ -232,7 +232,7 @@ public function __construct(IRequest $request, string $baseUri) {
 
 		// Allow view-only plugin for webdav requests
 		$this->server->addPlugin(new ViewOnlyPlugin(
-			\OC::$server->getLogger()
+			$logger
 		));
 
 		if (BrowserErrorPagePlugin::isBrowserRequest($request)) {

apps/dav/tests/unit/DAV/ViewOnlyPluginTest.php

@@ -25,9 +25,9 @@
 use OCA\DAV\Connector\Sabre\File as DavFile;
 use OCP\Files\File;
 use OCP\Files\Storage\IStorage;
-use OCP\ILogger;
 use OCP\Share\IAttributes;
 use OCP\Share\IShare;
+use Psr\Log\LoggerInterface;
 use Sabre\DAV\Server;
 use Sabre\DAV\Tree;
 use Test\TestCase;
@@ -45,7 +45,7 @@ class ViewOnlyPluginTest extends TestCase {
 
 	public function setUp(): void {
 		$this->plugin = new ViewOnlyPlugin(
-			$this->createMock(ILogger::class)
+			$this->createMock(LoggerInterface::class)
 		);
 		$this->request = $this->createMock(RequestInterface::class);
 		$this->tree = $this->createMock(Tree::class);

apps/files_sharing/lib/AppInfo/Application.php

@@ -125,12 +125,12 @@ public function boot(IBootContext $context): void {
 	}
 
 
-	public function registerMountProviders(IMountProviderCollection $mountProviderCollection, MountProvider $mountProvider, ExternalMountProvider $externalMountProvider) {
+	public function registerMountProviders(IMountProviderCollection $mountProviderCollection, MountProvider $mountProvider, ExternalMountProvider $externalMountProvider): void {
 		$mountProviderCollection->registerProvider($mountProvider);
 		$mountProviderCollection->registerProvider($externalMountProvider);
 	}
 
-	public function registerEventsScripts(IEventDispatcher $dispatcher, EventDispatcherInterface $oldDispatcher) {
+	public function registerEventsScripts(IEventDispatcher $dispatcher, EventDispatcherInterface $oldDispatcher): void {
 		// sidebar and files scripts
 		$dispatcher->addServiceListener(LoadAdditionalScriptsEvent::class, LoadAdditionalListener::class);
 		$dispatcher->addServiceListener(BeforeTemplateRenderedEvent::class, LegacyBeforeTemplateRenderedListener::class);
@@ -159,12 +159,12 @@ public function registerDownloadEvents(
 		IEventDispatcher $dispatcher,
 		?IUserSession $userSession,
 		IRootFolder $rootFolder
-	) {
+	): void {
 
 		$dispatcher->addListener(
 			'file.beforeGetDirect',
 			function (GenericEvent $event) use ($userSession, $rootFolder) {
-				$pathsToCheck[] = $event->getArgument('path');
+				$pathsToCheck = [$event->getArgument('path')];
 
 				// Check only for user/group shares. Don't restrict e.g. share links
 				if ($userSession && $userSession->isLoggedIn()) {
@@ -213,7 +213,7 @@ function (GenericEvent $event) use ($userSession, $rootFolder) {
 		);
 	}
 
-	public function setupSharingMenus(IManager $shareManager, IFactory $l10nFactory, IUserSession $userSession) {
+	public function setupSharingMenus(IManager $shareManager, IFactory $l10nFactory, IUserSession $userSession): void {
 		if (!$shareManager->shareApiEnabled() || !class_exists('\OCA\Files\App')) {
 			return;
 		}

apps/files_sharing/lib/ViewOnly.php

@@ -42,7 +42,7 @@ public function __construct(Folder $userFolder) {
 	 * @param string[] $pathsToCheck
 	 * @return bool
 	 */
-	public function check($pathsToCheck) {
+	public function check(array $pathsToCheck): bool {
 		// If any of elements cannot be downloaded, prevent whole download
 		foreach ($pathsToCheck as $file) {
 			try {
@@ -70,7 +70,7 @@ public function check($pathsToCheck) {
 	 * @return bool
 	 * @throws NotFoundException
 	 */
-	private function dirRecursiveCheck(Folder $dirInfo) {
+	private function dirRecursiveCheck(Folder $dirInfo): bool {
 		if (!$this->checkFileInfo($dirInfo)) {
 			return false;
 		}
@@ -94,7 +94,7 @@ private function dirRecursiveCheck(Folder $dirInfo) {
 	 * @return bool
 	 * @throws NotFoundException
 	 */
-	private function checkFileInfo(Node $fileInfo) {
+	private function checkFileInfo(Node $fileInfo): bool {
 		// Restrict view-only to nodes which are shared
 		$storage = $fileInfo->getStorage();
 		if (!$storage->instanceOfStorage(SharedStorage::class)) {
@@ -105,9 +105,14 @@ private function checkFileInfo(Node $fileInfo) {
 		/** @var \OCA\Files_Sharing\SharedStorage $storage */
 		$share = $storage->getShare();
 
+		$canDownload = true;
+
 		// Check if read-only and on whether permission can download is both set and disabled.
+		$attributes = $share->getAttributes();
+		if ($attributes !== null) {
+			$canDownload = $attributes->getAttribute('permissions', 'download');
+		}
 
-		$canDownload = $share->getAttributes()->getAttribute('permissions', 'download');
 		if ($canDownload !== null && !$canDownload) {
 			return false;
 		}

lib/private/Share20/Share.php

@@ -338,7 +338,7 @@ public function getPermissions() {
 	/**
 	 * @inheritdoc
 	 */
-	public function newAttributes() {
+	public function newAttributes(): IAttributes {
 		return new ShareAttributes();
 	}
 

lib/public/Share/IShare.php

@@ -36,7 +36,9 @@
 use OCP\Share\Exceptions\IllegalIDChangeException;
 
 /**
- * Interface IShare
+ * This interface allows to represent a share object.
+ *
+ * This interface must not be implemented in your application.
  *
  * @since 9.0.0
  */
@@ -320,7 +322,7 @@ public function getPermissions();
 	 * @since 25.0.0
 	 * @return IAttributes
 	 */
-	public function newAttributes();
+	public function newAttributes(): IAttributes;
 
 	/**
 	 * Set share attributes