From c04b1ed857221ea68cd4da3ec0597cc531ffd2cd Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Sat, 3 Jun 2023 01:19:15 +0200 Subject: [PATCH 1/6] fix: expect interface, not a specific implementation - fixes a regression when deleting folders while music app was enabled, for a LazyRoot was passed to this method. Signed-off-by: Arthur Schiwon --- lib/private/Files/Cache/QuerySearchHelper.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/private/Files/Cache/QuerySearchHelper.php b/lib/private/Files/Cache/QuerySearchHelper.php index 30b3c7225ac00..52663213b162e 100644 --- a/lib/private/Files/Cache/QuerySearchHelper.php +++ b/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; @@ -198,7 +197,7 @@ public function searchInCaches(ISearchQuery $searchQuery, array $caches): array /** * @return array{array, array} */ - 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(); From faea2440ed8b7bf9614556b88fc0c8357ac9cf84 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Sat, 3 Jun 2023 01:34:06 +0200 Subject: [PATCH 2/6] fix: add typehine for IRootFolder Signed-off-by: Arthur Schiwon --- lib/private/Files/Node/Node.php | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/lib/private/Files/Node/Node.php b/lib/private/Files/Node/Node.php index 0eef27161410a..4804f7046aa4e 100644 --- a/lib/private/Files/Node/Node.php +++ b/lib/private/Files/Node/Node.php @@ -34,6 +34,7 @@ use OC\Files\Utils\PathHelper; use OCP\Files\FileInfo; use OCP\Files\InvalidPathException; +use OCP\Files\IRootFolder; use OCP\Files\NotFoundException; use OCP\Files\NotPermittedException; use OCP\Lock\LockedException; @@ -47,10 +48,7 @@ class Node implements \OCP\Files\Node { */ 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 +70,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'); } @@ -402,7 +400,7 @@ public function unlock($type) { /** * @param string $targetPath - * @return \OC\Files\Node\Node + * @return \OCP\Files\Node * @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 \OCP\Files\Node * @throws InvalidPathException * @throws NotFoundException * @throws NotPermittedException if move not allowed or failed From f724760f67bbd8c177dd716fa3b95e990a787bda Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Thu, 8 Jun 2023 23:32:16 +0200 Subject: [PATCH 3/6] chore: ugly type juggling Signed-off-by: Arthur Schiwon --- lib/private/Files/Cache/QuerySearchHelper.php | 18 ++++++++++++++---- lib/private/Files/FileInfo.php | 3 --- lib/private/Files/Node/Folder.php | 11 ++++++++--- lib/private/Files/Node/Node.php | 16 ++++++++-------- lib/private/Files/Node/Root.php | 4 ++-- tests/lib/Files/Node/FolderTest.php | 8 +++++--- 6 files changed, 37 insertions(+), 23 deletions(-) diff --git a/lib/private/Files/Cache/QuerySearchHelper.php b/lib/private/Files/Cache/QuerySearchHelper.php index 52663213b162e..b4376eb2d985c 100644 --- a/lib/private/Files/Cache/QuerySearchHelper.php +++ b/lib/private/Files/Cache/QuerySearchHelper.php @@ -195,23 +195,33 @@ public function searchInCaches(ISearchQuery $searchQuery, array $caches): array } /** - * @return array{array, array} + * @return list{0?: array, 1?: array} */ public function getCachesAndMountPointsForSearch(IRootFolder $root, string $path, bool $limitToHome = false): array { $rootLength = strlen($path); - $mount = $root->getMount($path); - $storage = $mount->getStorage(); + $storage = null; + if (method_exists($root, 'getMount')) { + /** @var IMountPoint $mount */ + $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) { + if (!$limitToHome && method_exists($root, 'getMountsIn')) { /** @var IMountPoint[] $mounts */ $mounts = $root->getMountsIn($path); foreach ($mounts as $mount) { diff --git a/lib/private/Files/FileInfo.php b/lib/private/Files/FileInfo.php index d9b773cc2a660..2b6b83a25462c 100644 --- a/lib/private/Files/FileInfo.php +++ b/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; } diff --git a/lib/private/Files/Node/Folder.php b/lib/private/Files/Node/Folder.php index 1d6d88bafe6b3..cf58a0b1ef1e4 100644 --- a/lib/private/Files/Node/Folder.php +++ b/lib/private/Files/Node/Folder.php @@ -330,8 +330,13 @@ protected function getAppDataDirectoryName(): string { * @return array */ protected function getByIdInRootMount(int $id): array { - $mount = $this->root->getMount(''); - $cacheEntry = $mount->getStorage()->getCache($this->path)->get($id); + $storage = null; + if (\method_exists($this->root, 'getMount')) { + /** @var IMountPoint $mount */ + $mount = $this->root->getMount(''); + $storage = $mount->getStorage(); + } + $cacheEntry = $storage?->getCache($this->path)->get($id); if (!$cacheEntry) { return []; } @@ -346,7 +351,7 @@ protected function getByIdInRootMount(int $id): array { return [$this->root->createNode( $absolutePath, new \OC\Files\FileInfo( $absolutePath, - $mount->getStorage(), + $storage, $cacheEntry->getPath(), $cacheEntry, $mount diff --git a/lib/private/Files/Node/Node.php b/lib/private/Files/Node/Node.php index 4804f7046aa4e..4ba2c472c674a 100644 --- a/lib/private/Files/Node/Node.php +++ b/lib/private/Files/Node/Node.php @@ -35,6 +35,7 @@ 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; @@ -42,7 +43,7 @@ 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 */ @@ -128,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)); } } @@ -288,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 === '/') { @@ -400,7 +400,7 @@ public function unlock($type) { /** * @param string $targetPath - * @return \OCP\Files\Node + * @return INode * @throws InvalidPathException * @throws NotFoundException * @throws NotPermittedException if copy not allowed or failed @@ -426,7 +426,7 @@ public function copy($targetPath) { /** * @param string $targetPath - * @return \OCP\Files\Node + * @return INode * @throws InvalidPathException * @throws NotFoundException * @throws NotPermittedException if move not allowed or failed diff --git a/lib/private/Files/Node/Root.php b/lib/private/Files/Node/Root.php index e9fb14e5364b9..0ed1bcab2aedf 100644 --- a/lib/private/Files/Node/Root.php +++ b/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; @@ -339,10 +340,9 @@ public function isShareable() { } /** - * @return Node * @throws \OCP\Files\NotFoundException */ - public function getParent() { + public function getParent(): INode|IRootFolder { throw new NotFoundException(); } diff --git a/tests/lib/Files/Node/FolderTest.php b/tests/lib/Files/Node/FolderTest.php index d745a05ba17f4..0bcf69c5c1303 100644 --- a/tests/lib/Files/Node/FolderTest.php +++ b/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)); } From 09c4ba257c41c68fe58c3a5d298e82a32034b9db Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Thu, 15 Jun 2023 23:21:56 +0200 Subject: [PATCH 4/6] refactor: declare getMount() and getMountsIn() at IRootFolder Signed-off-by: Arthur Schiwon --- lib/private/Files/Cache/QuerySearchHelper.php | 11 +++-------- lib/private/Files/Node/Folder.php | 8 ++------ lib/private/Files/Node/LazyFolder.php | 7 ++++--- lib/private/Files/Node/Root.php | 8 ++------ lib/public/Files/IRootFolder.php | 13 +++++++++++++ 5 files changed, 24 insertions(+), 23 deletions(-) diff --git a/lib/private/Files/Cache/QuerySearchHelper.php b/lib/private/Files/Cache/QuerySearchHelper.php index b4376eb2d985c..75e203d5fd959 100644 --- a/lib/private/Files/Cache/QuerySearchHelper.php +++ b/lib/private/Files/Cache/QuerySearchHelper.php @@ -199,12 +199,8 @@ public function searchInCaches(ISearchQuery $searchQuery, array $caches): array */ public function getCachesAndMountPointsForSearch(IRootFolder $root, string $path, bool $limitToHome = false): array { $rootLength = strlen($path); - $storage = null; - if (method_exists($root, 'getMount')) { - /** @var IMountPoint $mount */ - $mount = $root->getMount($path); - $storage = $mount->getStorage(); - } + $mount = $root->getMount($path); + $storage = $mount->getStorage(); if ($storage === null) { return []; } @@ -221,8 +217,7 @@ public function getCachesAndMountPointsForSearch(IRootFolder $root, string $path /** @var IMountPoint[] $mountByMountPoint */ $mountByMountPoint = ['' => $mount]; - if (!$limitToHome && method_exists($root, 'getMountsIn')) { - /** @var IMountPoint[] $mounts */ + if (!$limitToHome) { $mounts = $root->getMountsIn($path); foreach ($mounts as $mount) { $storage = $mount->getStorage(); diff --git a/lib/private/Files/Node/Folder.php b/lib/private/Files/Node/Folder.php index cf58a0b1ef1e4..7e41c8bc111ed 100644 --- a/lib/private/Files/Node/Folder.php +++ b/lib/private/Files/Node/Folder.php @@ -330,12 +330,8 @@ protected function getAppDataDirectoryName(): string { * @return array */ protected function getByIdInRootMount(int $id): array { - $storage = null; - if (\method_exists($this->root, 'getMount')) { - /** @var IMountPoint $mount */ - $mount = $this->root->getMount(''); - $storage = $mount->getStorage(); - } + $mount = $this->root->getMount(''); + $storage = $mount->getStorage(); $cacheEntry = $storage?->getCache($this->path)->get($id); if (!$cacheEntry) { return []; diff --git a/lib/private/Files/Node/LazyFolder.php b/lib/private/Files/Node/LazyFolder.php index c843baabade00..9f9528f69bd54 100644 --- a/lib/private/Files/Node/LazyFolder.php +++ b/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()); } diff --git a/lib/private/Files/Node/Root.php b/lib/private/Files/Node/Root.php index 0ed1bcab2aedf..7bd88981ff2f3 100644 --- a/lib/private/Files/Node/Root.php +++ b/lib/private/Files/Node/Root.php @@ -154,11 +154,7 @@ 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); } @@ -166,7 +162,7 @@ public function getMount($mountPoint) { * @param string $mountPoint * @return \OC\Files\Mount\MountPoint[] */ - public function getMountsIn($mountPoint) { + public function getMountsIn(string $mountPoint): array { return $this->mountManager->findIn($mountPoint); } diff --git a/lib/public/Files/IRootFolder.php b/lib/public/Files/IRootFolder.php index 452c0fd31573b..1fee0b3595e33 100644 --- a/lib/public/Files/IRootFolder.php +++ b/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; } From 29e11a4273547a910b55f54d82c14ec12ca17ded Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Sat, 17 Jun 2023 00:15:49 +0200 Subject: [PATCH 5/6] ci: pro forma check of existence of internal method createNode() is protected and used by Folder, but being an internal-only method it shall not be exposed in the Folder or IRootFolder interface. Signed-off-by: Arthur Schiwon --- lib/private/Files/Node/Folder.php | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/lib/private/Files/Node/Folder.php b/lib/private/Files/Node/Folder.php index 7e41c8bc111ed..38a7645631c59 100644 --- a/lib/private/Files/Node/Folder.php +++ b/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,6 +326,12 @@ 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(''); $storage = $mount->getStorage(); $cacheEntry = $storage?->getCache($this->path)->get($id); @@ -385,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( From 5ba5c2d88e51f94138e27e3eb2c4a4b83d62bb1f Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Mon, 26 Jun 2023 14:44:17 +0200 Subject: [PATCH 6/6] docs: adjust @since version to backport Signed-off-by: Arthur Schiwon --- lib/public/Files/IRootFolder.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/public/Files/IRootFolder.php b/lib/public/Files/IRootFolder.php index 1fee0b3595e33..49d41ab998cc5 100644 --- a/lib/public/Files/IRootFolder.php +++ b/lib/public/Files/IRootFolder.php @@ -60,12 +60,12 @@ public function getByIdInPath(int $id, string $path); /** * @return IMountPoint[] * - * @since 28.0.0 + * @since 27.0.1 */ public function getMountsIn(string $mountPoint): array; /** - * @since 28.0.0 + * @since 27.0.1 */ public function getMount(string $mountPoint): IMountPoint; }