Merge pull request #38625 from nextcloud/fix/noid/querysearchehelper-…

…narrow-type

fix: expect interface, not a specific implementation

lib/private/Files/Cache/QuerySearchHelper.php

@@ -26,15 +26,14 @@
 namespace OC\Files\Cache;
 
 use OC\Files\Cache\Wrapper\CacheJail;
-use OC\Files\Node\Root;
 use OC\Files\Search\QueryOptimizer\QueryOptimizer;
 use OC\Files\Search\SearchBinaryOperator;
 use OC\SystemConfig;
 use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCP\Files\Cache\ICache;
 use OCP\Files\Cache\ICacheEntry;
-use OCP\Files\Folder;
 use OCP\Files\IMimeTypeLoader;
+use OCP\Files\IRootFolder;
 use OCP\Files\Mount\IMountPoint;
 use OCP\Files\Search\ISearchBinaryOperator;
 use OCP\Files\Search\ISearchQuery;
@@ -196,24 +195,29 @@ public function searchInCaches(ISearchQuery $searchQuery, array $caches): array
 	}
 
 	/**
-	 * @return array{array<string, ICache>, array<string, IMountPoint>}
+	 * @return list{0?: array<array-key, ICache>, 1?: array<array-key, IMountPoint>}
 	 */
-	public function getCachesAndMountPointsForSearch(Root $root, string $path, bool $limitToHome = false): array {
+	public function getCachesAndMountPointsForSearch(IRootFolder $root, string $path, bool $limitToHome = false): array {
 		$rootLength = strlen($path);
 		$mount = $root->getMount($path);
 		$storage = $mount->getStorage();
+		if ($storage === null) {
+			return [];
+		}
 		$internalPath = $mount->getInternalPath($path);
 
 		if ($internalPath !== '') {
 			// a temporary CacheJail is used to handle filtering down the results to within this folder
+			/** @var ICache[] $caches */
 			$caches = ['' => new CacheJail($storage->getCache(''), $internalPath)];
 		} else {
+			/** @var ICache[] $caches */
 			$caches = ['' => $storage->getCache('')];
 		}
+		/** @var IMountPoint[] $mountByMountPoint */
 		$mountByMountPoint = ['' => $mount];
 
 		if (!$limitToHome) {
-			/** @var IMountPoint[] $mounts */
 			$mounts = $root->getMountsIn($path);
 			foreach ($mounts as $mount) {
 				$storage = $mount->getStorage();

lib/private/Files/FileInfo.php

@@ -143,9 +143,6 @@ public function getPath() {
 		return $this->path;
 	}
 
-	/**
-	 * @return \OCP\Files\Storage
-	 */
 	public function getStorage() {
 		return $this->storage;
 	}

lib/private/Files/Node/Folder.php

@@ -40,6 +40,7 @@
 use OCP\Files\Cache\ICacheEntry;
 use OCP\Files\FileInfo;
 use OCP\Files\Mount\IMountPoint;
+use OCP\Files\Node as INode;
 use OCP\Files\NotFoundException;
 use OCP\Files\NotPermittedException;
 use OCP\Files\Search\ISearchBinaryOperator;
@@ -109,12 +110,7 @@ public function getDirectoryListing() {
 		}, $folderContent);
 	}
 
-	/**
-	 * @param string $path
-	 * @param FileInfo $info
-	 * @return File|Folder
-	 */
-	protected function createNode($path, FileInfo $info = null, bool $infoHasSubMountsIncluded = true) {
+	protected function createNode(string $path, ?FileInfo $info = null, bool $infoHasSubMountsIncluded = true): INode {
 		if (is_null($info)) {
 			$isDir = $this->view->is_dir($path);
 		} else {
@@ -330,8 +326,15 @@ protected function getAppDataDirectoryName(): string {
 	 * @return array
 	 */
 	protected function getByIdInRootMount(int $id): array {
+		if (!method_exists($this->root, 'createNode')) {
+			// Always expected to be false. Being a method of Folder, this is
+			// always implemented. For it is an internal method and should not
+			// be exposed and made public, it is not part of an interface.
+			return [];
+		}
 		$mount = $this->root->getMount('');
-		$cacheEntry = $mount->getStorage()->getCache($this->path)->get($id);
+		$storage = $mount->getStorage();
+		$cacheEntry = $storage?->getCache($this->path)->get($id);
 		if (!$cacheEntry) {
 			return [];
 		}
@@ -346,7 +349,7 @@ protected function getByIdInRootMount(int $id): array {
 		return [$this->root->createNode(
 			$absolutePath, new \OC\Files\FileInfo(
 				$absolutePath,
-				$mount->getStorage(),
+				$storage,
 				$cacheEntry->getPath(),
 				$cacheEntry,
 				$mount
@@ -384,7 +387,7 @@ public function getNonExistingName($name) {
 	/**
 	 * @param int $limit
 	 * @param int $offset
-	 * @return \OCP\Files\Node[]
+	 * @return INode[]
 	 */
 	public function getRecent($limit, $offset = 0) {
 		$filterOutNonEmptyFolder = new SearchBinaryOperator(

lib/private/Files/Node/LazyFolder.php

@@ -29,6 +29,7 @@
 use OC\Files\Utils\PathHelper;
 use OCP\Files\Folder;
 use OCP\Constants;
+use OCP\Files\Mount\IMountPoint;
 
 /**
  * Class LazyFolder
@@ -111,14 +112,14 @@ public function mount($storage, $mountPoint, $arguments = []) {
 	/**
 	 * @inheritDoc
 	 */
-	public function getMount($mountPoint) {
+	public function getMount(string $mountPoint): IMountPoint {
 		return $this->__call(__FUNCTION__, func_get_args());
 	}
 
 	/**
-	 * @inheritDoc
+	 * @return IMountPoint[]
 	 */
-	public function getMountsIn($mountPoint) {
+	public function getMountsIn(string $mountPoint): array {
 		return $this->__call(__FUNCTION__, func_get_args());
 	}
 

lib/private/Files/Node/Node.php

@@ -34,23 +34,22 @@
 use OC\Files\Utils\PathHelper;
 use OCP\Files\FileInfo;
 use OCP\Files\InvalidPathException;
+use OCP\Files\IRootFolder;
+use OCP\Files\Node as INode;
 use OCP\Files\NotFoundException;
 use OCP\Files\NotPermittedException;
 use OCP\Lock\LockedException;
 use OCP\PreConditionNotMetException;
 use Symfony\Component\EventDispatcher\GenericEvent;
 
 // FIXME: this class really should be abstract
-class Node implements \OCP\Files\Node {
+class Node implements INode {
 	/**
 	 * @var \OC\Files\View $view
 	 */
 	protected $view;
 
-	/**
-	 * @var \OC\Files\Node\Root $root
-	 */
-	protected $root;
+	protected IRootFolder $root;
 
 	/**
 	 * @var string $path Absolute path to the node (e.g. /admin/files/folder/file)
@@ -72,7 +71,7 @@ class Node implements \OCP\Files\Node {
 	 * @param string $path
 	 * @param FileInfo $fileInfo
 	 */
-	public function __construct($root, $view, $path, $fileInfo = null, ?Node $parent = null, bool $infoHasSubMountsIncluded = true) {
+	public function __construct(IRootFolder $root, $view, $path, $fileInfo = null, ?Node $parent = null, bool $infoHasSubMountsIncluded = true) {
 		if (Filesystem::normalizePath($view->getRoot()) !== '/') {
 			throw new PreConditionNotMetException('The view passed to the node should not have any fake root set');
 		}
@@ -130,7 +129,9 @@ protected function sendHooks($hooks, array $args = null) {
 		$args = !empty($args) ? $args : [$this];
 		$dispatcher = \OC::$server->getEventDispatcher();
 		foreach ($hooks as $hook) {
-			$this->root->emit('\OC\Files', $hook, $args);
+			if (method_exists($this->root, 'emit')) {
+				$this->root->emit('\OC\Files', $hook, $args);
+			}
 			$dispatcher->dispatch('\OCP\Files::' . $hook, new GenericEvent($args));
 		}
 	}
@@ -290,10 +291,7 @@ public function isCreatable() {
 		return $this->getFileInfo(false)->isCreatable();
 	}
 
-	/**
-	 * @return Node
-	 */
-	public function getParent() {
+	public function getParent(): INode|IRootFolder {
 		if ($this->parent === null) {
 			$newPath = dirname($this->path);
 			if ($newPath === '' || $newPath === '.' || $newPath === '/') {
@@ -402,7 +400,7 @@ public function unlock($type) {
 
 	/**
 	 * @param string $targetPath
-	 * @return \OC\Files\Node\Node
+	 * @return INode
 	 * @throws InvalidPathException
 	 * @throws NotFoundException
 	 * @throws NotPermittedException if copy not allowed or failed
@@ -428,7 +426,7 @@ public function copy($targetPath) {
 
 	/**
 	 * @param string $targetPath
-	 * @return \OC\Files\Node\Node
+	 * @return INode
 	 * @throws InvalidPathException
 	 * @throws NotFoundException
 	 * @throws NotPermittedException if move not allowed or failed

lib/private/Files/Node/Root.php

@@ -45,6 +45,7 @@
 use OCP\Files\Events\Node\FilesystemTornDownEvent;
 use OCP\Files\IRootFolder;
 use OCP\Files\Mount\IMountPoint;
+use OCP\Files\Node as INode;
 use OCP\Files\NotFoundException;
 use OCP\Files\NotPermittedException;
 use OCP\IUser;
@@ -153,19 +154,15 @@ public function mount($storage, $mountPoint, $arguments = []) {
 		$this->mountManager->addMount($mount);
 	}
 
-	/**
-	 * @param string $mountPoint
-	 * @return \OC\Files\Mount\MountPoint
-	 */
-	public function getMount($mountPoint) {
+	public function getMount(string $mountPoint): IMountPoint {
 		return $this->mountManager->find($mountPoint);
 	}
 
 	/**
 	 * @param string $mountPoint
 	 * @return \OC\Files\Mount\MountPoint[]
 	 */
-	public function getMountsIn($mountPoint) {
+	public function getMountsIn(string $mountPoint): array {
 		return $this->mountManager->findIn($mountPoint);
 	}
 
@@ -339,10 +336,9 @@ public function isShareable() {
 	}
 
 	/**
-	 * @return Node
 	 * @throws \OCP\Files\NotFoundException
 	 */
-	public function getParent() {
+	public function getParent(): INode|IRootFolder {
 		throw new NotFoundException();
 	}
 

lib/public/Files/IRootFolder.php

@@ -26,6 +26,7 @@
 
 use OC\Hooks\Emitter;
 use OC\User\NoUserException;
+use OCP\Files\Mount\IMountPoint;
 
 /**
  * Interface IRootFolder
@@ -55,4 +56,16 @@ public function getUserFolder($userId);
 	 * @since 24.0.0
 	 */
 	public function getByIdInPath(int $id, string $path);
+
+	/**
+	 * @return IMountPoint[]
+	 *
+	 * @since 28.0.0
+	 */
+	public function getMountsIn(string $mountPoint): array;
+
+	/**
+	 * @since 28.0.0
+	 */
+	public function getMount(string $mountPoint): IMountPoint;
 }

tests/lib/Files/Node/FolderTest.php

@@ -24,6 +24,7 @@
 use OC\Files\Storage\Temporary;
 use OC\Files\Storage\Wrapper\Jail;
 use OCP\Files\Cache\ICacheEntry;
+use OCP\Files\IRootFolder;
 use OCP\Files\Mount\IMountPoint;
 use OCP\Files\NotFoundException;
 use OCP\Files\Search\ISearchComparison;
@@ -462,12 +463,13 @@ public function testSearchSubStorages() {
 	}
 
 	public function testIsSubNode() {
-		$file = new Node(null, $this->view, '/foo/bar');
-		$folder = new Folder(null, $this->view, '/foo');
+		$rootFolderMock = $this->createMock(IRootFolder::class);
+		$file = new Node($rootFolderMock, $this->view, '/foo/bar');
+		$folder = new Folder($rootFolderMock, $this->view, '/foo');
 		$this->assertTrue($folder->isSubNode($file));
 		$this->assertFalse($folder->isSubNode($folder));
 
-		$file = new Node(null, $this->view, '/foobar');
+		$file = new Node($rootFolderMock, $this->view, '/foobar');
 		$this->assertFalse($folder->isSubNode($file));
 	}