From 8a42ee05244659ee497639460e074c4d0ede97ef Mon Sep 17 00:00:00 2001 From: Maksim Sukharev Date: Fri, 7 Aug 2026 11:42:59 +0200 Subject: [PATCH 1/2] fix(shares): expose share of TYPE_CIRCLE to the FilesList in groupfolders - groupfolders PROPFIND collect data differently than normal folder, as it's missing an owner - different path only calls for providers that have an implementation - adding this to circles to allow server collect the data - align behvaiour with DefaultShareProvider Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Maksim Sukharev --- lib/Db/ShareWrapperRequest.php | 12 +- lib/Service/ShareWrapperService.php | 2 +- lib/ShareByCircleProvider.php | 53 ++++++- tests/unit/lib/ShareByCircleProviderTest.php | 145 +++++++++++++++++++ 4 files changed, 200 insertions(+), 12 deletions(-) create mode 100644 tests/unit/lib/ShareByCircleProviderTest.php diff --git a/lib/Db/ShareWrapperRequest.php b/lib/Db/ShareWrapperRequest.php index b08298976..961b5dcf4 100644 --- a/lib/Db/ShareWrapperRequest.php +++ b/lib/Db/ShareWrapperRequest.php @@ -434,7 +434,11 @@ public function getSharesBy( } /** - * @param FederatedUser $federatedUser + * A null $federatedUser returns the shares of every owner and initiator. This + * is needed for ownerless mounts (e.g. groupfolders), where no single user can + * be used to filter the shares. + * + * @param FederatedUser|null $federatedUser * @param Folder $node * @param bool $reshares * @param bool $shallow Whether the method should stop at the first level, or look into sub-folders. @@ -443,7 +447,7 @@ public function getSharesBy( * @throws RequestBuilderException */ public function getSharesInFolder( - FederatedUser $federatedUser, + ?FederatedUser $federatedUser, Folder $node, bool $reshares, bool $shallow = true, @@ -451,7 +455,9 @@ public function getSharesInFolder( $qb = $this->getShareSelectSql(); $qb->leftJoinCircle(CoreQueryBuilder::SHARE, null, 'share_with'); - $qb->limitToShareOwner(CoreQueryBuilder::SHARE, $federatedUser, $reshares); + if ($federatedUser !== null) { + $qb->limitToShareOwner(CoreQueryBuilder::SHARE, $federatedUser, $reshares); + } $qb->leftJoinFileCache(CoreQueryBuilder::SHARE); $aliasFileCache = $qb->generateAlias(CoreQueryBuilder::SHARE, CoreQueryBuilder::FILE_CACHE); diff --git a/lib/Service/ShareWrapperService.php b/lib/Service/ShareWrapperService.php index c9dff06f3..55cfc2bff 100644 --- a/lib/Service/ShareWrapperService.php +++ b/lib/Service/ShareWrapperService.php @@ -237,7 +237,7 @@ public function getSharesBy( * @throws RequestBuilderException */ public function getSharesInFolder( - FederatedUser $federatedUser, + ?FederatedUser $federatedUser, Folder $node, bool $reshares, bool $shallow = true, diff --git a/lib/ShareByCircleProvider.php b/lib/ShareByCircleProvider.php index f53bdfea3..236c96b4d 100644 --- a/lib/ShareByCircleProvider.php +++ b/lib/ShareByCircleProvider.php @@ -32,6 +32,7 @@ use OCA\Circles\FederatedItems\Files\FileShare; use OCA\Circles\FederatedItems\Files\FileUnshare; use OCA\Circles\Model\Federated\FederatedEvent; +use OCA\Circles\Model\FederatedUser; use OCA\Circles\Model\Member; use OCA\Circles\Model\Probes\CircleProbe; use OCA\Circles\Model\Probes\DataProbe; @@ -60,6 +61,7 @@ use OCP\Share\IShare; use OCP\Share\IShareProvider; use OCP\Share\IShareProviderGetUsers; +use OCP\Share\IShareProviderSupportsAllSharesInFolder; use Psr\Log\LoggerInterface; /** @@ -67,7 +69,7 @@ * * @package OCA\Circles */ -class ShareByCircleProvider implements IShareProvider, IPartialShareProvider, IShareProviderGetUsers { +class ShareByCircleProvider implements IShareProvider, IPartialShareProvider, IShareProviderGetUsers, IShareProviderSupportsAllSharesInFolder { use TArrayTools; use TStringTools; use TNCLogger; @@ -321,7 +323,7 @@ public function restore(IShare $share, string $recipient): IShare { * @param bool $reshares * @param bool $shallow Whether the method should stop at the first level, or look into sub-folders. * - * @return array + * @return array> * @throws ContactAddressBookNotFoundException * @throws ContactFormatException * @throws ContactNotFoundException @@ -336,12 +338,47 @@ public function restore(IShare $share, string $recipient): IShare { */ public function getSharesInFolder($userId, Folder $node, $reshares, $shallow = true): array { $federatedUser = $this->federatedUserService->getLocalFederatedUser($userId); - $wrappedShares = $this->shareWrapperService->getSharesInFolder( - $federatedUser, - $node, - $reshares, - $shallow - ); + + return $this->getSharesInFolderInternal($federatedUser, $node, $reshares, $shallow); + } + + /** + * Get all shares in a folder, regardless of the share owner or initiator. + * Used for ownerless mounts (e.g. groupfolders) where there is no single + * user to filter shares by. + * + * @return array> + * @throws ContactAddressBookNotFoundException + * @throws ContactFormatException + * @throws ContactNotFoundException + * @throws InvalidIdException + * @throws InvalidPathException + * @throws NotFoundException + * @throws RequestBuilderException + * @throws SingleCircleNotFoundException + */ + public function getAllSharesInFolder(Folder $node): array { + return $this->getSharesInFolderInternal(null, $node, false); + } + + /** + * @return array> + * @throws ContactAddressBookNotFoundException + * @throws ContactFormatException + * @throws ContactNotFoundException + * @throws InvalidIdException + * @throws InvalidPathException + * @throws NotFoundException + * @throws RequestBuilderException + * @throws SingleCircleNotFoundException + */ + private function getSharesInFolderInternal( + ?FederatedUser $federatedUser, + Folder $node, + bool $reshares, + bool $shallow = true, + ): array { + $wrappedShares = $this->shareWrapperService->getSharesInFolder($federatedUser, $node, $reshares, $shallow); $result = []; foreach ($wrappedShares as $wrappedShare) { diff --git a/tests/unit/lib/ShareByCircleProviderTest.php b/tests/unit/lib/ShareByCircleProviderTest.php new file mode 100644 index 000000000..3d7c653fc --- /dev/null +++ b/tests/unit/lib/ShareByCircleProviderTest.php @@ -0,0 +1,145 @@ +shareWrapperService = $this->createMock(ShareWrapperService::class); + $this->federatedUserService = $this->createMock(FederatedUserService::class); + + $this->provider = new ShareByCircleProvider( + $this->createMock(IUserManager::class), + $this->createMock(IRootFolder::class), + $this->createMock(IL10N::class), + $this->createMock(LoggerInterface::class), + $this->createMock(IURLGenerator::class), + $this->shareWrapperService, + $this->createMock(ShareTokenService::class), + $this->federatedUserService, + $this->createMock(FederatedEventService::class), + $this->createMock(CircleService::class), + $this->createMock(EventService::class), + ); + } + + public function testGetAllSharesInFolderQueriesWithoutFederatedUser(): void { + $node = $this->createMock(Folder::class); + + $this->federatedUserService->expects($this->never()) + ->method('getLocalFederatedUser'); + $this->shareWrapperService->expects($this->once()) + ->method('getSharesInFolder') + ->with(null, $node, false, true) + ->willReturn([]); + + $this->assertSame([], $this->provider->getAllSharesInFolder($node)); + } + + public function testGetAllSharesInFolderGroupsSharesByFileSource(): void { + $node = $this->createMock(Folder::class); + $firstShare = $this->createMock(IShare::class); + $secondShare = $this->createMock(IShare::class); + $thirdShare = $this->createMock(IShare::class); + + $this->shareWrapperService->method('getSharesInFolder') + ->willReturn([ + $this->createWrappedShare(42, $firstShare), + $this->createWrappedShare(42, $secondShare), + $this->createWrappedShare(1337, $thirdShare), + ]); + + $this->assertSame([ + 42 => [$firstShare, $secondShare], + 1337 => [$thirdShare], + ], $this->provider->getAllSharesInFolder($node)); + } + + public function testGetAllSharesInFolderSkipsInaccessibleShares(): void { + $node = $this->createMock(Folder::class); + + $this->shareWrapperService->method('getSharesInFolder') + ->willReturn([ + $this->createWrappedShare(42, $this->createMock(IShare::class), false), + ]); + + $this->assertSame([42 => []], $this->provider->getAllSharesInFolder($node)); + } + + public function testGetSharesInFolderQueriesWithFederatedUser(): void { + $node = $this->createMock(Folder::class); + $federatedUser = $this->createMock(FederatedUser::class); + + $this->federatedUserService->expects($this->once()) + ->method('getLocalFederatedUser') + ->with('test-user') + ->willReturn($federatedUser); + $this->shareWrapperService->expects($this->once()) + ->method('getSharesInFolder') + ->with($federatedUser, $node, true, true) + ->willReturn([]); + + $this->assertSame([], $this->provider->getSharesInFolder('test-user', $node, true)); + } + + public function testGetSharesInFolderForwardsNonShallow(): void { + $node = $this->createMock(Folder::class); + $federatedUser = $this->createMock(FederatedUser::class); + + $this->federatedUserService->method('getLocalFederatedUser') + ->willReturn($federatedUser); + $this->shareWrapperService->expects($this->once()) + ->method('getSharesInFolder') + ->with($federatedUser, $node, true, false) + ->willReturn([]); + + $this->assertSame([], $this->provider->getSharesInFolder('test-user', $node, true, false)); + } + + private function createWrappedShare( + int $fileSource, + IShare $share, + bool $accessible = true, + ): ShareWrapper&MockObject { + $fileCache = $this->createMock(FileCacheWrapper::class); + $fileCache->method('isAccessible')->willReturn($accessible); + $fileCache->method('getPath')->willReturn('__groupfolders/1/document.md'); + + $wrappedShare = $this->createMock(ShareWrapper::class); + $wrappedShare->method('getFileSource')->willReturn($fileSource); + $wrappedShare->method('getFileCache')->willReturn($fileCache); + $wrappedShare->method('getShare')->willReturn($share); + + return $wrappedShare; + } +} From ac21f252f14614b32ae43845d80d744e9a08a585 Mon Sep 17 00:00:00 2001 From: Maksim Sukharev Date: Fri, 7 Aug 2026 12:09:40 +0200 Subject: [PATCH 2/2] fixup! psalm Signed-off-by: Maksim Sukharev --- lib/ShareByCircleProvider.php | 6 ++++-- tests/psalm-baseline.xml | 1 - 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/ShareByCircleProvider.php b/lib/ShareByCircleProvider.php index 236c96b4d..462f983cd 100644 --- a/lib/ShareByCircleProvider.php +++ b/lib/ShareByCircleProvider.php @@ -386,8 +386,10 @@ private function getSharesInFolderInternal( $result[$wrappedShare->getFileSource()] = []; } if ($wrappedShare->getFileCache()->isAccessible()) { - $result[$wrappedShare->getFileSource()][] - = $wrappedShare->getShare($this->rootFolder, $this->userManager, $this->urlGenerator); + $share = $wrappedShare->getShare($this->rootFolder, $this->userManager, $this->urlGenerator); + if ($share !== null) { + $result[$wrappedShare->getFileSource()][] = $share; + } } else { $this->logger->debug('shared document is not available anymore', ['wrappedShare' => $wrappedShare]); if ($wrappedShare->getFileCache()->getPath() === '') { diff --git a/tests/psalm-baseline.xml b/tests/psalm-baseline.xml index eb9e79271..8cbb6e9a6 100644 --- a/tests/psalm-baseline.xml +++ b/tests/psalm-baseline.xml @@ -338,7 +338,6 @@ -