From c378e7401703146dcbfaa2be78b2d68675fd6742 Mon Sep 17 00:00:00 2001 From: provokateurin Date: Tue, 22 Oct 2024 13:19:18 +0200 Subject: [PATCH] feat(Share20\Manager): Return all shares on IShareOwnerlessMount Signed-off-by: provokateurin --- lib/private/Share20/Manager.php | 41 +++++++++++++----- tests/lib/Share20/ManagerTest.php | 69 +++++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+), 11 deletions(-) diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index 4856c051307..da90b2183ae 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -17,6 +17,7 @@ use OCP\Files\File; use OCP\Files\Folder; use OCP\Files\IRootFolder; use OCP\Files\Mount\IMountManager; +use OCP\Files\Mount\IShareOwnerlessMount; use OCP\Files\Node; use OCP\Files\NotFoundException; use OCP\HintException; @@ -1215,17 +1216,26 @@ class Manager implements IManager { throw new \Exception('non-shallow getSharesInFolder is no longer supported'); } - return array_reduce($providers, function ($shares, IShareProvider $provider) use ($userId, $node, $reshares) { - $newShares = $provider->getSharesInFolder($userId, $node, $reshares); - foreach ($newShares as $fid => $data) { - if (!isset($shares[$fid])) { - $shares[$fid] = []; - } + $isOwnerless = $node->getMountPoint() instanceof IShareOwnerlessMount; - $shares[$fid] = array_merge($shares[$fid], $data); + $shares = []; + foreach ($providers as $provider) { + if ($isOwnerless) { + foreach ($node->getDirectoryListing() as $childNode) { + $data = $provider->getSharesByPath($childNode); + $fid = $childNode->getId(); + $shares[$fid] ??= []; + $shares[$fid] = array_merge($shares[$fid], $data); + } + } else { + foreach ($provider->getSharesInFolder($userId, $node, $reshares) as $fid => $data) { + $shares[$fid] ??= []; + $shares[$fid] = array_merge($shares[$fid], $data); + } } - return $shares; - }, []); + } + + return $shares; } /** @@ -1244,7 +1254,11 @@ class Manager implements IManager { return []; } - $shares = $provider->getSharesBy($userId, $shareType, $path, $reshares, $limit, $offset); + if ($path?->getMountPoint() instanceof IShareOwnerlessMount) { + $shares = array_filter($provider->getSharesByPath($path), static fn (IShare $share) => $share->getShareType() === $shareType); + } else { + $shares = $provider->getSharesBy($userId, $shareType, $path, $reshares, $limit, $offset); + } /* * Work around so we don't return expired shares but still follow @@ -1288,7 +1302,12 @@ class Manager implements IManager { $offset += $added; // Fetch again $limit shares - $shares = $provider->getSharesBy($userId, $shareType, $path, $reshares, $limit, $offset); + if ($path?->getMountPoint() instanceof IShareOwnerlessMount) { + // We already fetched all shares, so end here + $shares = []; + } else { + $shares = $provider->getSharesBy($userId, $shareType, $path, $reshares, $limit, $offset); + } // No more shares means we are done if (empty($shares)) { diff --git a/tests/lib/Share20/ManagerTest.php b/tests/lib/Share20/ManagerTest.php index a4c1dd3803d..5a0ca230186 100644 --- a/tests/lib/Share20/ManagerTest.php +++ b/tests/lib/Share20/ManagerTest.php @@ -23,6 +23,7 @@ use OCP\Files\Folder; use OCP\Files\IRootFolder; use OCP\Files\Mount\IMountManager; use OCP\Files\Mount\IMountPoint; +use OCP\Files\Mount\IShareOwnerlessMount; use OCP\Files\Node; use OCP\Files\Storage\IStorage; use OCP\HintException; @@ -2895,6 +2896,31 @@ class ManagerTest extends \Test\TestCase { $this->assertSame($share, $shares[0]); } + public function testGetSharesByOwnerless(): void { + $mount = $this->createMock(IShareOwnerlessMount::class); + + $node = $this->createMock(Folder::class); + $node + ->expects($this->once()) + ->method('getMountPoint') + ->willReturn($mount); + + $share = $this->manager->newShare(); + $share->setNode($node); + $share->setShareType(IShare::TYPE_USER); + + $this->defaultProvider + ->expects($this->once()) + ->method('getSharesByPath') + ->with($this->equalTo($node)) + ->willReturn([$share]); + + $shares = $this->manager->getSharesBy('user', IShare::TYPE_USER, $node, true, 1, 1); + + $this->assertCount(1, $shares); + $this->assertSame($share, $shares[0]); + } + /** * Test to ensure we correctly remove expired link shares * @@ -4493,6 +4519,49 @@ class ManagerTest extends \Test\TestCase { $this->assertSame($expects, $result); } + public function testGetSharesInFolderOwnerless(): void { + $factory = new DummyFactory2($this->createMock(IServerContainer::class)); + + $manager = $this->createManager($factory); + + $factory->setProvider($this->defaultProvider); + $extraProvider = $this->createMock(IShareProvider::class); + $factory->setSecondProvider($extraProvider); + + $share1 = $this->createMock(IShare::class); + $share2 = $this->createMock(IShare::class); + + $mount = $this->createMock(IShareOwnerlessMount::class); + + $file = $this->createMock(File::class); + $file + ->method('getId') + ->willReturn(1); + + $folder = $this->createMock(Folder::class); + $folder + ->method('getMountPoint') + ->willReturn($mount); + $folder + ->method('getDirectoryListing') + ->willReturn([$file]); + + $this->defaultProvider + ->method('getSharesByPath') + ->with($file) + ->willReturn([$share1]); + + $extraProvider + ->method('getSharesByPath') + ->with($file) + ->willReturn([$share2]); + + $this->assertSame([ + 1 => [$share1, $share2], + ], $manager->getSharesInFolder('user', $folder)); + } + + public function testGetAccessList(): void { $factory = new DummyFactory2($this->createMock(IServerContainer::class));