diff --git a/apps/dav/lib/Files/NodeAttributes/SharesProvider.php b/apps/dav/lib/Files/NodeAttributes/SharesProvider.php index 112dc8b19b31c..4ed7bc405f6f8 100644 --- a/apps/dav/lib/Files/NodeAttributes/SharesProvider.php +++ b/apps/dav/lib/Files/NodeAttributes/SharesProvider.php @@ -52,10 +52,9 @@ public function preload(NodeAttributeContext $context, array $nodes, array $attr return; } - foreach ($nodes as $node) { - if (!isset($this->shares[$userId][$node->getId()])) { - $this->shares[$userId][$node->getId()] = $this->getShares($userId, $node); - } + $nodes = array_values(array_filter($nodes, fn (Node $node): bool => !isset($this->shares[$userId][$node->getId()]))); + if ($nodes !== []) { + $this->shares[$userId] = $this->getShares($userId, $nodes) + ($this->shares[$userId] ?? []); } } @@ -104,20 +103,28 @@ public function getAttributes(NodeAttributeContext $context, Node $node, array $ } /** - * Shares the user created on the node, and the ones the user received for it + * Shares the user created on the nodes, and the ones the user received for them * - * @return list + * @param non-empty-list $nodes + * @return array> by file id */ - private function getShares(string $userId, Node $node): array { - $shares = []; + private function getShares(string $userId, array $nodes): array { + $shares = array_fill_keys(array_map(static fn (Node $node): int => $node->getId(), $nodes), []); foreach (self::SHARE_TYPES as $shareType) { - $shares[] = $this->shareManager->getSharesBy($userId, $shareType, $node, false, -1); + $typeShares = [$this->shareManager->getSharesByNodes($userId, $shareType, $nodes)]; try { - $shares[] = $this->shareManager->getSharedWith($userId, $shareType, $node, -1); + $typeShares[] = $this->shareManager->getSharedWithByNodes($userId, $shareType, $nodes); } catch (BackendError) { // a share provider that is not available has no shares to show } + foreach ($typeShares as $sharesByFileId) { + foreach ($sharesByFileId as $fileId => $fileShares) { + if (isset($shares[$fileId])) { + array_push($shares[$fileId], ...$fileShares); + } + } + } } - return array_values(array_merge(...$shares)); + return $shares; } } diff --git a/apps/dav/tests/unit/Files/NodeAttributes/SharesParityTest.php b/apps/dav/tests/unit/Files/NodeAttributes/SharesParityTest.php index 3bfc18ca46446..74cf7204e9244 100644 --- a/apps/dav/tests/unit/Files/NodeAttributes/SharesParityTest.php +++ b/apps/dav/tests/unit/Files/NodeAttributes/SharesParityTest.php @@ -91,8 +91,8 @@ private function shareManager(array $outgoing, array $incoming): IManager { }, array_filter($shares, static fn (array $data): bool => $data[0] === $type))); $shareManager = $this->createMock(IManager::class); - $shareManager->method('getSharesBy')->willReturnCallback(static fn (string $userId, int $type): array => $toShares($outgoing, $type)); - $shareManager->method('getSharedWith')->willReturnCallback(static fn (string $userId, int $type): array => $toShares($incoming, $type)); + $shareManager->method('getSharesByNodes')->willReturnCallback(static fn (string $userId, int $type): array => array_filter([42 => $toShares($outgoing, $type)])); + $shareManager->method('getSharedWithByNodes')->willReturnCallback(static fn (string $userId, int $type): array => array_filter([42 => $toShares($incoming, $type)])); return $shareManager; } diff --git a/apps/dav/tests/unit/Files/NodeAttributes/SharesProviderTest.php b/apps/dav/tests/unit/Files/NodeAttributes/SharesProviderTest.php index 4532da43904c0..063666431cff9 100644 --- a/apps/dav/tests/unit/Files/NodeAttributes/SharesProviderTest.php +++ b/apps/dav/tests/unit/Files/NodeAttributes/SharesProviderTest.php @@ -95,11 +95,11 @@ public static function dataShareTypes(): array { #[DataProvider('dataShareTypes')] public function testOutgoingSharesOfANode(array $shareTypes): void { $node = $this->node(File::class, 11, '/alice/files/report.md'); - $this->shareManager->method('getSharesBy') - ->willReturnCallback(fn (string $userId, int $type, Node $sharedNode, bool $reshares, int $limit): array => $userId === 'alice' && $sharedNode === $node && !$reshares && $limit === -1 && in_array($type, $shareTypes, true) - ? [$this->share($type)] + $this->shareManager->method('getSharesByNodes') + ->willReturnCallback(fn (string $userId, int $type, array $nodes): array => $userId === 'alice' && $nodes === [$node] && in_array($type, $shareTypes, true) + ? [11 => [$this->share($type)]] : []); - $this->shareManager->method('getSharedWith')->willReturn([]); + $this->shareManager->method('getSharedWithByNodes')->willReturn([]); [$types] = $this->getValues($node); self::assertSame($shareTypes, $types); @@ -107,11 +107,11 @@ public function testOutgoingSharesOfANode(array $shareTypes): void { public function testReceivedSharesAreIncludedForANode(): void { $node = $this->node(Folder::class, 12, '/alice/files/Shared'); - $this->shareManager->method('getSharesBy') - ->willReturnCallback(fn (string $userId, int $type): array => $type === IShare::TYPE_LINK ? [$this->share(IShare::TYPE_LINK)] : []); - $this->shareManager->method('getSharedWith') - ->willReturnCallback(fn (string $userId, int $type, ?Node $sharedNode, int $limit): array => $userId === 'alice' && $sharedNode === $node && $limit === -1 && $type === IShare::TYPE_GROUP - ? [$this->share(IShare::TYPE_GROUP, 'staff', 'Staff')] + $this->shareManager->method('getSharesByNodes') + ->willReturnCallback(fn (string $userId, int $type): array => $type === IShare::TYPE_LINK ? [12 => [$this->share(IShare::TYPE_LINK)]] : []); + $this->shareManager->method('getSharedWithByNodes') + ->willReturnCallback(fn (string $userId, int $type, array $nodes): array => $userId === 'alice' && $nodes === [$node] && $type === IShare::TYPE_GROUP + ? [12 => [$this->share(IShare::TYPE_GROUP, 'staff', 'Staff')]] : []); [$types, $sharees] = $this->getValues($node); @@ -124,9 +124,9 @@ public function testReceivedSharesAreIncludedForANode(): void { public function testUnavailableShareProviderIsSkipped(): void { $node = $this->node(File::class, 11, '/alice/files/report.md'); - $this->shareManager->method('getSharesBy') - ->willReturnCallback(fn (string $userId, int $type): array => $type === IShare::TYPE_USER ? [$this->share(IShare::TYPE_USER, 'bob', 'Bob')] : []); - $this->shareManager->method('getSharedWith') + $this->shareManager->method('getSharesByNodes') + ->willReturnCallback(fn (string $userId, int $type): array => $type === IShare::TYPE_USER ? [11 => [$this->share(IShare::TYPE_USER, 'bob', 'Bob')]] : []); + $this->shareManager->method('getSharedWithByNodes') ->willReturnCallback(static fn (string $userId, int $type): array => $type === IShare::TYPE_ROOM ? throw new BackendError() : []); [$types] = $this->getValues($node); @@ -135,17 +135,37 @@ public function testUnavailableShareProviderIsSkipped(): void { public function testShareTypesAreUnique(): void { $node = $this->node(File::class, 11, '/alice/files/report.md'); - $this->shareManager->method('getSharesBy') + $this->shareManager->method('getSharesByNodes') ->willReturnCallback(fn (string $userId, int $type): array => $type === IShare::TYPE_USER - ? [$this->share(IShare::TYPE_USER, 'bob', 'Bob'), $this->share(IShare::TYPE_USER, 'carol', 'Carol')] + ? [11 => [$this->share(IShare::TYPE_USER, 'bob', 'Bob'), $this->share(IShare::TYPE_USER, 'carol', 'Carol')]] : []); - $this->shareManager->method('getSharedWith')->willReturn([]); + $this->shareManager->method('getSharedWithByNodes')->willReturn([]); [$types, $sharees] = $this->getValues($node); self::assertSame([IShare::TYPE_USER], $types); self::assertSame(['bob', 'carol'], array_column($sharees, 'id')); } + public function testNodesAreLoadedTogether(): void { + $first = $this->node(File::class, 31, '/alice/files/a.md'); + $second = $this->node(File::class, 32, '/alice/files/Other/b.md'); + + // One call per share type and direction for all the nodes + $this->shareManager->expects(self::exactly(8)) + ->method('getSharesByNodes') + ->with('alice', self::anything(), [$first, $second]) + ->willReturnCallback(fn (string $userId, int $type): array => $type === IShare::TYPE_LINK ? [32 => [$this->share(IShare::TYPE_LINK)]] : []); + $this->shareManager->expects(self::exactly(8)) + ->method('getSharedWithByNodes') + ->with('alice', self::anything(), [$first, $second]) + ->willReturn([]); + + $this->provider->preload($this->context, [$first, $second], self::ATTRIBUTES); + + self::assertSame([], $this->getValues($first)[0]); + self::assertSame([IShare::TYPE_LINK], $this->getValues($second)[0]); + } + public function testFolderListingLoadsTheChildrenAtOnce(): void { $folder = $this->node(Folder::class, 20, '/alice/files/Projects'); $shared = $this->node(File::class, 21, '/alice/files/Projects/plan.md'); @@ -157,12 +177,12 @@ public function testFolderListingLoadsTheChildrenAtOnce(): void { ->willReturn([21 => [$this->share(IShare::TYPE_LINK)]]); // The folder itself is looked up on its own, its children are not $this->shareManager->expects(self::exactly(8)) - ->method('getSharesBy') - ->with('alice', self::anything(), $folder, false, -1) + ->method('getSharesByNodes') + ->with('alice', self::anything(), [$folder]) ->willReturn([]); $this->shareManager->expects(self::exactly(8)) - ->method('getSharedWith') - ->with('alice', self::anything(), $folder, -1) + ->method('getSharedWithByNodes') + ->with('alice', self::anything(), [$folder]) ->willReturn([]); $this->provider->preloadListing($this->context, $folder, [$shared, $notShared], self::ATTRIBUTES); @@ -178,10 +198,10 @@ public function testAFolderAndItsChildOutsideOfAListingGetTheirOwnShares(): void $received = $this->node(Folder::class, 41, '/alice/files/X/Y'); $this->shareManager->expects(self::never())->method('getSharesInFolder'); - $this->shareManager->method('getSharesBy')->willReturn([]); - $this->shareManager->method('getSharedWith') - ->willReturnCallback(fn (string $userId, int $type, Node $node): array => $node === $received && $type === IShare::TYPE_USER - ? [$this->share(IShare::TYPE_USER, 'alice', 'Alice')] + $this->shareManager->method('getSharesByNodes')->willReturn([]); + $this->shareManager->method('getSharedWithByNodes') + ->willReturnCallback(fn (string $userId, int $type, array $nodes): array => $nodes === [$folder, $received] && $type === IShare::TYPE_USER + ? [41 => [$this->share(IShare::TYPE_USER, 'alice', 'Alice')]] : []); $this->provider->preload($this->context, [$folder, $received], self::ATTRIBUTES); @@ -192,9 +212,9 @@ public function testAFolderAndItsChildOutsideOfAListingGetTheirOwnShares(): void public function testPreloadingTwiceOnlyLoadsNewNodes(): void { $this->shareManager->expects(self::exactly(16)) - ->method('getSharesBy') + ->method('getSharesByNodes') ->willReturn([]); - $this->shareManager->method('getSharedWith')->willReturn([]); + $this->shareManager->method('getSharedWithByNodes')->willReturn([]); $this->provider->preload($this->context, [$this->node(File::class, 1, '/alice/files/a')], self::ATTRIBUTES); $this->provider->preload($this->context, [$this->node(File::class, 1, '/alice/files/a')], self::ATTRIBUTES); @@ -204,7 +224,7 @@ public function testPreloadingTwiceOnlyLoadsNewNodes(): void { public function testNothingOnPublicShares(): void { $context = NodeAttributeContext::forShare($this->createMock(IShare::class)); $node = $this->node(File::class, 11, '/bob/files/Shared/report.md'); - $this->shareManager->expects(self::never())->method('getSharesBy'); + $this->shareManager->expects(self::never())->method('getSharesByNodes'); $this->provider->preload($context, [$node], self::ATTRIBUTES); self::assertSame([], $this->provider->getAttributes($context, $node, self::ATTRIBUTES)); diff --git a/apps/federatedfilesharing/lib/FederatedShareProvider.php b/apps/federatedfilesharing/lib/FederatedShareProvider.php index aedea7c7dd03f..b6c5ff68e6a70 100644 --- a/apps/federatedfilesharing/lib/FederatedShareProvider.php +++ b/apps/federatedfilesharing/lib/FederatedShareProvider.php @@ -28,6 +28,7 @@ use OCP\Share\IShare; use OCP\Share\IShareProvider; use OCP\Share\IShareProviderSupportsAllSharesInFolder; +use OCP\Share\IShareProviderSupportsFileIds; use Override; use Psr\Log\LoggerInterface; @@ -36,7 +37,7 @@ * * @package OCA\FederatedFileSharing */ -class FederatedShareProvider implements IShareProvider, IShareProviderSupportsAllSharesInFolder { +class FederatedShareProvider implements IShareProvider, IShareProviderSupportsAllSharesInFolder, IShareProviderSupportsFileIds { public const SHARE_TYPE_REMOTE = 6; /** @var string */ @@ -559,6 +560,19 @@ private function getSharesInFolderInternal(?string $userId, Folder $node, ?bool */ #[\Override] public function getSharesBy($userId, $shareType, $node, $reshares, $limit, $offset) { + return $this->getSharesByInternal($userId, $shareType, $node === null ? null : [$node->getId()], $reshares, $limit, $offset); + } + + #[\Override] + public function getSharesByFileIds(string $userId, int $shareType, array $fileIds): array { + return $this->getSharesByInternal($userId, $shareType, $fileIds, false, -1, 0); + } + + /** + * @param list|null $fileIds + * @return list + */ + private function getSharesByInternal($userId, $shareType, ?array $fileIds, $reshares, $limit, $offset): array { $qb = $this->dbConnection->getQueryBuilder(); $qb->select('*') ->from('share'); @@ -590,8 +604,8 @@ public function getSharesBy($userId, $shareType, $node, $reshares, $limit, $offs ); } - if ($node !== null) { - $qb->andWhere($qb->expr()->eq('file_source', $qb->createNamedParameter($node->getId()))); + if ($fileIds !== null) { + $qb->andWhere($qb->expr()->in('file_source', $qb->createNamedParameter($fileIds, IQueryBuilder::PARAM_INT_ARRAY))); } if ($limit !== -1) { @@ -673,6 +687,19 @@ public function getSharesByPath(Node $path) { */ #[\Override] public function getSharedWith($userId, $shareType, $node, $limit, $offset) { + return $this->getSharedWithInternal($userId, $shareType, $node === null ? null : [$node->getId()], $limit, $offset); + } + + #[\Override] + public function getSharedWithByFileIds(string $userId, int $shareType, array $fileIds): array { + return array_values($this->getSharedWithInternal($userId, $shareType, $fileIds, -1, 0)); + } + + /** + * @param list|null $fileIds + * @return IShare[] + */ + private function getSharedWithInternal($userId, $shareType, ?array $fileIds, $limit, $offset): array { /** @var IShare[] $shares */ $shares = []; @@ -695,8 +722,8 @@ public function getSharedWith($userId, $shareType, $node, $limit, $offset) { $qb->andWhere($qb->expr()->eq('share_with', $qb->createNamedParameter($userId))); // Filter by node if provided - if ($node !== null) { - $qb->andWhere($qb->expr()->eq('file_source', $qb->createNamedParameter($node->getId()))); + if ($fileIds !== null) { + $qb->andWhere($qb->expr()->in('file_source', $qb->createNamedParameter($fileIds, IQueryBuilder::PARAM_INT_ARRAY))); } $cursor = $qb->executeQuery(); diff --git a/apps/sharebymail/lib/ShareByMailProvider.php b/apps/sharebymail/lib/ShareByMailProvider.php index 71f984444b3ed..7bcff1323b463 100644 --- a/apps/sharebymail/lib/ShareByMailProvider.php +++ b/apps/sharebymail/lib/ShareByMailProvider.php @@ -839,6 +839,19 @@ public function restore(IShare $share, string $recipient): IShare { */ #[\Override] public function getSharesBy($userId, $shareType, $node, $reshares, $limit, $offset): array { + return $this->getSharesByInternal($userId, $shareType, $node === null ? null : [$node->getId()], $reshares, $limit, $offset); + } + + #[\Override] + public function getSharesByFileIds(string $userId, int $shareType, array $fileIds): array { + return $this->getSharesByInternal($userId, $shareType, $fileIds, false, -1, 0); + } + + /** + * @param list|null $fileIds + * @return list + */ + private function getSharesByInternal($userId, $shareType, ?array $fileIds, $reshares, $limit, $offset): array { $qb = $this->dbConnection->getQueryBuilder(); $qb->select('*') ->from('share'); @@ -861,7 +874,7 @@ public function getSharesBy($userId, $shareType, $node, $reshares, $limit, $offs $or1 ) ); - } elseif ($node === null) { + } elseif ($fileIds === null) { $qb->andWhere( $qb->expr()->orX( $qb->expr()->eq('uid_owner', $qb->createNamedParameter($userId)), @@ -870,8 +883,8 @@ public function getSharesBy($userId, $shareType, $node, $reshares, $limit, $offs ); } - if ($node !== null) { - $qb->andWhere($qb->expr()->eq('file_source', $qb->createNamedParameter($node->getId()))); + if ($fileIds !== null) { + $qb->andWhere($qb->expr()->in('file_source', $qb->createNamedParameter($fileIds, IQueryBuilder::PARAM_INT_ARRAY))); } if ($limit !== -1) { @@ -954,6 +967,19 @@ public function getSharesByPath(Node $path): array { */ #[\Override] public function getSharedWith($userId, $shareType, $node, $limit, $offset): array { + return $this->getSharedWithInternal($userId, $shareType, $node === null ? null : [$node->getId()], $limit, $offset); + } + + #[\Override] + public function getSharedWithByFileIds(string $userId, int $shareType, array $fileIds): array { + return array_values($this->getSharedWithInternal($userId, $shareType, $fileIds, -1, 0)); + } + + /** + * @param list|null $fileIds + * @return IShare[] + */ + private function getSharedWithInternal($userId, $shareType, ?array $fileIds, $limit, $offset): array { /** @var IShare[] $shares */ $shares = []; @@ -977,8 +1003,8 @@ public function getSharedWith($userId, $shareType, $node, $limit, $offset): arra $qb->andWhere($qb->expr()->eq('share_with', $qb->createNamedParameter($userId))); // Filter by node if provided - if ($node !== null) { - $qb->andWhere($qb->expr()->eq('file_source', $qb->createNamedParameter($node->getId()))); + if ($fileIds !== null) { + $qb->andWhere($qb->expr()->in('file_source', $qb->createNamedParameter($fileIds, IQueryBuilder::PARAM_INT_ARRAY))); } $cursor = $qb->executeQuery(); diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 941234b4a8b1b..f7dd900d247aa 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -971,6 +971,7 @@ 'OCP\\Share\\IShareProviderGetUsers' => $baseDir . '/lib/public/Share/IShareProviderGetUsers.php', 'OCP\\Share\\IShareProviderSupportsAccept' => $baseDir . '/lib/public/Share/IShareProviderSupportsAccept.php', 'OCP\\Share\\IShareProviderSupportsAllSharesInFolder' => $baseDir . '/lib/public/Share/IShareProviderSupportsAllSharesInFolder.php', + 'OCP\\Share\\IShareProviderSupportsFileIds' => $baseDir . '/lib/public/Share/IShareProviderSupportsFileIds.php', 'OCP\\Share\\IShareProviderWithNotification' => $baseDir . '/lib/public/Share/IShareProviderWithNotification.php', 'OCP\\Share\\ShareReview\\Events\\ShareReviewAccessCheckEvent' => $baseDir . '/lib/public/Share/ShareReview/Events/ShareReviewAccessCheckEvent.php', 'OCP\\Share\\ShareReview\\IShareReviewSource' => $baseDir . '/lib/public/Share/ShareReview/IShareReviewSource.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 65ee51a5eec52..c7330437544b5 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1017,6 +1017,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OCP\\Share\\IShareProviderGetUsers' => __DIR__ . '/../../..' . '/lib/public/Share/IShareProviderGetUsers.php', 'OCP\\Share\\IShareProviderSupportsAccept' => __DIR__ . '/../../..' . '/lib/public/Share/IShareProviderSupportsAccept.php', 'OCP\\Share\\IShareProviderSupportsAllSharesInFolder' => __DIR__ . '/../../..' . '/lib/public/Share/IShareProviderSupportsAllSharesInFolder.php', + 'OCP\\Share\\IShareProviderSupportsFileIds' => __DIR__ . '/../../..' . '/lib/public/Share/IShareProviderSupportsFileIds.php', 'OCP\\Share\\IShareProviderWithNotification' => __DIR__ . '/../../..' . '/lib/public/Share/IShareProviderWithNotification.php', 'OCP\\Share\\ShareReview\\Events\\ShareReviewAccessCheckEvent' => __DIR__ . '/../../..' . '/lib/public/Share/ShareReview/Events/ShareReviewAccessCheckEvent.php', 'OCP\\Share\\ShareReview\\IShareReviewSource' => __DIR__ . '/../../..' . '/lib/public/Share/ShareReview/IShareReviewSource.php', diff --git a/lib/private/Share20/DefaultShareProvider.php b/lib/private/Share20/DefaultShareProvider.php index 965f22fae4822..52e66eaa91127 100644 --- a/lib/private/Share20/DefaultShareProvider.php +++ b/lib/private/Share20/DefaultShareProvider.php @@ -44,6 +44,7 @@ use OCP\Share\IShareProviderGetUsers; use OCP\Share\IShareProviderSupportsAccept; use OCP\Share\IShareProviderSupportsAllSharesInFolder; +use OCP\Share\IShareProviderSupportsFileIds; use OCP\Share\IShareProviderWithNotification; use OCP\Util; use Psr\Log\LoggerInterface; @@ -57,6 +58,7 @@ * @package OC\Share20 */ class DefaultShareProvider implements + IShareProviderSupportsFileIds, IShareProviderWithNotification, IShareProviderSupportsAccept, IShareProviderSupportsAllSharesInFolder, @@ -726,6 +728,19 @@ private function getSharesInFolderInternal(?string $userId, Folder $node, ?bool */ #[\Override] public function getSharesBy($userId, $shareType, $node, $reshares, $limit, $offset) { + return $this->getSharesByInternal($userId, $shareType, $node === null ? null : [$node->getId()], $reshares, $limit, $offset); + } + + #[\Override] + public function getSharesByFileIds(string $userId, int $shareType, array $fileIds): array { + return $this->getSharesByInternal($userId, $shareType, $fileIds, false, -1, 0); + } + + /** + * @param list|null $fileIds + * @return list + */ + private function getSharesByInternal($userId, $shareType, ?array $fileIds, $reshares, $limit, $offset): array { $qb = $this->dbConn->getQueryBuilder(); $qb->select('*') ->from('share') @@ -739,7 +754,7 @@ public function getSharesBy($userId, $shareType, $node, $reshares, $limit, $offs if ($reshares === false) { $qb->andWhere($qb->expr()->eq('uid_initiator', $qb->createNamedParameter($userId))); } else { - if ($node === null) { + if ($fileIds === null) { $qb->andWhere( $qb->expr()->orX( $qb->expr()->eq('uid_owner', $qb->createNamedParameter($userId)), @@ -749,8 +764,8 @@ public function getSharesBy($userId, $shareType, $node, $reshares, $limit, $offs } } - if ($node !== null) { - $qb->andWhere($qb->expr()->eq('file_source', $qb->createNamedParameter($node->getId()))); + if ($fileIds !== null) { + $qb->andWhere($qb->expr()->in('file_source', $qb->createNamedParameter($fileIds, IQueryBuilder::PARAM_INT_ARRAY))); } if ($limit !== -1) { @@ -869,7 +884,12 @@ private function isAccessibleResult($data) { #[\Override] public function getSharedWith($userId, $shareType, $node, $limit, $offset) { - return $this->_getSharedWith($userId, $shareType, $limit, $offset, $node); + return $this->_getSharedWith($userId, $shareType, $limit, $offset, $node === null ? null : [$node->getId()]); + } + + #[\Override] + public function getSharedWithByFileIds(string $userId, int $shareType, array $fileIds): array { + return array_values($this->_getSharedWith($userId, $shareType, -1, 0, $fileIds)); } #[\Override] @@ -885,6 +905,7 @@ public function getSharedWithByPath( } /** + * @param list|null $fileIds only the shares of these files * @return IShare[] */ private function _getSharedWith( @@ -892,7 +913,7 @@ private function _getSharedWith( int $shareType, int $limit, int $offset, - ?Node $node = null, + ?array $fileIds = null, ?string $path = null, ?bool $forChildren = false, ): iterable { @@ -940,9 +961,9 @@ private function _getSharedWith( ->andWhere($qb->expr()->eq('share_with', $qb->createNamedParameter($userId))) ->andWhere($qb->expr()->in('item_type', $qb->createNamedParameter(['file', 'folder'], IQueryBuilder::PARAM_STR_ARRAY))); - // Filter by node if provided - if ($node !== null) { - $qb->andWhere($qb->expr()->eq('file_source', $qb->createNamedParameter($node->getId()))); + // Filter by files if provided + if ($fileIds !== null) { + $qb->andWhere($qb->expr()->in('file_source', $qb->createNamedParameter($fileIds, IQueryBuilder::PARAM_INT_ARRAY))); } if ($path !== null) { @@ -1007,9 +1028,9 @@ private function _getSharedWith( $qb->setMaxResults($limit - count($shares)); } - // Filter by node if provided - if ($node !== null) { - $qb->andWhere($qb->expr()->eq('file_source', $qb->createNamedParameter($node->getId()))); + // Filter by files if provided + if ($fileIds !== null) { + $qb->andWhere($qb->expr()->in('file_source', $qb->createNamedParameter($fileIds, IQueryBuilder::PARAM_INT_ARRAY))); } if ($path !== null) { diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index 2d2720dfb8f5b..2ced393890f4f 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -79,6 +79,7 @@ use OCP\Share\IShareProviderGetUsers; use OCP\Share\IShareProviderSupportsAccept; use OCP\Share\IShareProviderSupportsAllSharesInFolder; +use OCP\Share\IShareProviderSupportsFileIds; use OCP\Share\IShareProviderWithNotification; use Override; use Psr\Log\LoggerInterface; @@ -1234,6 +1235,81 @@ public function getSharesBy(string $userId, int $shareType, ?Node $path = null, return $shares; } + #[Override] + public function getSharesByNodes(string $userId, int $shareType, array $nodes): array { + try { + $provider = $this->factory->getProviderForType($shareType); + } catch (ProviderException) { + return []; + } + + if ($this->config->getAppValue('files_sharing', 'hide_disabled_user_shares', 'no') === 'yes' + && $this->userManager->get($userId)?->isEnabled() === false) { + return []; + } + + $shares = []; + $lookup = []; + foreach ($nodes as $node) { + if ($node->getMountPoint() instanceof IShareOwnerlessMount) { + // Every share of the path, not only the ones the user created + $shares[] = array_filter($provider->getSharesByPath($node), static fn (IShare $share): bool => $share->getShareType() === $shareType); + } elseif ($provider instanceof IShareProviderSupportsFileIds) { + $lookup[] = $node->getId(); + } else { + $shares[] = $provider->getSharesBy($userId, $shareType, $node, false, -1, 0); + } + } + foreach (array_chunk(array_values(array_unique($lookup)), 1000) as $fileIds) { + /** @var IShareProviderSupportsFileIds $provider */ + $shares[] = $provider->getSharesByFileIds($userId, $shareType, $fileIds); + } + + return $this->groupValidSharesByFileId(array_merge(...$shares), $userId); + } + + #[Override] + public function getSharedWithByNodes(string $userId, int $shareType, array $nodes): array { + try { + $provider = $this->factory->getProviderForType($shareType); + } catch (ProviderException) { + return []; + } + + $shares = []; + if ($provider instanceof IShareProviderSupportsFileIds) { + $fileIds = array_values(array_unique(array_map(static fn (Node $node): int => $node->getId(), $nodes))); + foreach (array_chunk($fileIds, 1000) as $chunk) { + $shares[] = $provider->getSharedWithByFileIds($userId, $shareType, $chunk); + } + } else { + foreach ($nodes as $node) { + $shares[] = $provider->getSharedWith($userId, $shareType, $node, -1, 0); + } + } + + return $this->groupValidSharesByFileId(array_merge(...$shares), $userId); + } + + /** + * Drop the expired and invalid shares, like `getSharesBy()` and `getSharedWith()` do + * + * @param list $shares + * @return array> + */ + private function groupValidSharesByFileId(array $shares, string $userId): array { + $result = []; + foreach ($shares as $share) { + try { + $this->checkShare($share, viewer: $userId); + } catch (ShareNotFound) { + continue; + } + $result[$share->getNodeId()][] = $share; + } + return $result; + } + #[Override] public function getSharedWith(string $userId, int $shareType, ?Node $node = null, int $limit = 50, int $offset = 0): array { try { diff --git a/lib/public/Share/IManager.php b/lib/public/Share/IManager.php index d6bc7725c60de..b2b1a0ccfea39 100644 --- a/lib/public/Share/IManager.php +++ b/lib/public/Share/IManager.php @@ -134,6 +134,28 @@ public function getSharesBy(string $userId, int $shareType, ?Node $path = null, */ public function getSharedWith(string $userId, int $shareType, ?Node $node = null, int $limit = 50, int $offset = 0): array; + /** + * Get the shares the user created on each of these nodes, + * the same as `getSharesBy($userId, $shareType, $node, false, -1)` for each node + * + * @param IShare::TYPE_* $shareType + * @param list $nodes + * @return array> shares by file id, nodes without shares are left out + * @since 36.0.0 + */ + public function getSharesByNodes(string $userId, int $shareType, array $nodes): array; + + /** + * Get the shares of each of these nodes the user received, + * the same as `getSharedWith($userId, $shareType, $node, -1)` for each node + * + * @param IShare::TYPE_* $shareType + * @param list $nodes + * @return array> shares by file id, nodes without shares are left out + * @since 36.0.0 + */ + public function getSharedWithByNodes(string $userId, int $shareType, array $nodes): array; + /** * Get shares shared with a $user filtering by $path. * diff --git a/lib/public/Share/IShareProviderSupportsFileIds.php b/lib/public/Share/IShareProviderSupportsFileIds.php new file mode 100644 index 0000000000000..788c78e65446a --- /dev/null +++ b/lib/public/Share/IShareProviderSupportsFileIds.php @@ -0,0 +1,37 @@ + $fileIds at most 1000 + * @return list + * @since 36.0.0 + */ + public function getSharesByFileIds(string $userId, int $shareType, array $fileIds): array; + + /** + * Get the shares of these files the user received, like `getSharedWith()` for each file + * + * @param IShare::TYPE_* $shareType + * @param non-empty-list $fileIds at most 1000 + * @return list + * @since 36.0.0 + */ + public function getSharedWithByFileIds(string $userId, int $shareType, array $fileIds): array; +} diff --git a/tests/lib/Share20/DefaultShareProviderTest.php b/tests/lib/Share20/DefaultShareProviderTest.php index 7b64acee33f29..ee82c2e799214 100644 --- a/tests/lib/Share20/DefaultShareProviderTest.php +++ b/tests/lib/Share20/DefaultShareProviderTest.php @@ -3272,4 +3272,64 @@ public function testSendNote(): void { self::invokePrivate($this->provider, 'sendNote', [[$recipient], $share]); } + + public function testLookupByFileIdsMatchesLookupByNode(): void { + $storageId = $this->createTestStorageEntry('home::shareOwner'); + $fileIds = [ + $this->createTestFileEntry('files/a.txt', $storageId), + $this->createTestFileEntry('files/b.txt', $storageId), + $this->createTestFileEntry('files/c.txt', $storageId), + ]; + [$a, $b, $c] = $fileIds; + + // Created by alice + $this->addShareToDB(IShare::TYPE_USER, 'bob', 'alice', 'shareOwner', 'file', $a, '/a.txt', 19, null, null); + $this->addShareToDB(IShare::TYPE_USER, 'carol', 'alice', 'shareOwner', 'file', $b, '/b.txt', 19, null, null); + $this->addShareToDB(IShare::TYPE_LINK, null, 'alice', 'shareOwner', 'file', $b, null, 1, 'token1', null); + // Created by someone else + $this->addShareToDB(IShare::TYPE_USER, 'dave', 'bob', 'shareOwner', 'file', $a, '/a.txt', 19, null, null); + // Received by alice, directly and through her groups + $this->addShareToDB(IShare::TYPE_USER, 'alice', 'bob', 'shareOwner', 'file', $c, '/c.txt', 19, null, null); + $this->addShareToDB(IShare::TYPE_GROUP, 'staff', 'bob', 'shareOwner', 'file', $a, '/a.txt', 19, null, null); + $left = $this->addShareToDB(IShare::TYPE_GROUP, 'staff', 'bob', 'shareOwner', 'file', $c, '/c.txt', 19, null, null); + // alice left that one + $this->addShareToDB(IShare::TYPE_USERGROUP, 'alice', 'bob', 'shareOwner', 'file', $c, '/c.txt', 0, null, null, $left); + $this->addShareToDB(IShare::TYPE_GROUP, 'other', 'bob', 'shareOwner', 'file', $b, '/b.txt', 19, null, null); + + $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn('alice'); + $this->userManager->method('get')->willReturn($user); + $this->groupManager->method('getUserGroupIds')->willReturn(['staff']); + $this->rootFolder->method('getUserFolder')->willReturnSelf(); + $this->rootFolder->method('getFirstNodeById')->willReturn($this->createMock(File::class)); + + $ids = static fn (array $shares): array => array_map(static fn (IShare $share): string => $share->getId(), $shares); + foreach ([IShare::TYPE_USER, IShare::TYPE_GROUP, IShare::TYPE_LINK] as $shareType) { + $byFileIds = $this->provider->getSharesByFileIds('alice', $shareType, $fileIds); + // Like getSharedWith(), the provider receives no link shares + $withByFileIds = $shareType === IShare::TYPE_LINK ? [] : $this->provider->getSharedWithByFileIds('alice', $shareType, $fileIds); + $byNode = []; + $withByNode = []; + foreach ($fileIds as $fileId) { + $node = $this->createMock(File::class); + $node->method('getId')->willReturn($fileId); + $byNode[] = $this->provider->getSharesBy('alice', $shareType, $node, false, -1, 0); + if ($shareType !== IShare::TYPE_LINK) { + $withByNode[] = $this->provider->getSharedWith('alice', $shareType, $node, -1, 0); + } + } + + $this->assertEqualsCanonicalizing($ids(array_merge(...$byNode)), $ids($byFileIds), 'shares by alice, type ' . $shareType); + if ($shareType !== IShare::TYPE_LINK) { + $this->assertEqualsCanonicalizing($ids(array_merge(...$withByNode)), $ids($withByFileIds), 'shares with alice, type ' . $shareType); + } + } + + // The expected shares are found at all + $this->assertCount(2, $this->provider->getSharesByFileIds('alice', IShare::TYPE_USER, $fileIds)); + $this->assertCount(1, $this->provider->getSharedWithByFileIds('alice', IShare::TYPE_USER, $fileIds)); + // The group share alice left comes back with no permissions, like with getSharedWith() + $groupShares = $this->provider->getSharedWithByFileIds('alice', IShare::TYPE_GROUP, $fileIds); + $this->assertEqualsCanonicalizing([19, 0], array_map(static fn (IShare $share): int => $share->getPermissions(), $groupShares)); + } } diff --git a/tests/lib/Share20/ManagerTest.php b/tests/lib/Share20/ManagerTest.php index 1d62c7301c519..e04de55b61365 100644 --- a/tests/lib/Share20/ManagerTest.php +++ b/tests/lib/Share20/ManagerTest.php @@ -5749,6 +5749,100 @@ public function testCurrentUserCanEnumerateTargetUser(bool $currentUserIsGuest, $this->assertSame($expected, $manager->currentUserCanEnumerateTargetUser($currentUser, $targetUser)); } + + private function nodeWithId(int $id, bool $ownerless = false): File&MockObject { + $node = $this->createMock(File::class); + $node->method('getId')->willReturn($id); + $node->method('getMountPoint')->willReturn($this->createMock($ownerless ? IShareOwnerlessMount::class : IMountPoint::class)); + return $node; + } + + private function shareOn(int $fileId, int $shareType = IShare::TYPE_USER): IShare { + $share = $this->manager->newShare(); + $share->setNodeId($fileId); + $share->setShareType($shareType); + return $share; + } + + public function testGetSharesByNodesInOneLookup(): void { + $nodes = [$this->nodeWithId(1), $this->nodeWithId(2), $this->nodeWithId(3)]; + $first = $this->shareOn(1); + $second = $this->shareOn(1); + $third = $this->shareOn(3); + + $this->defaultProvider->expects($this->once()) + ->method('getSharesByFileIds') + ->with('user', IShare::TYPE_USER, [1, 2, 3]) + ->willReturn([$first, $third, $second]); + $this->defaultProvider->expects($this->never())->method('getSharesBy'); + + $this->assertSame([1 => [$first, $second], 3 => [$third]], $this->manager->getSharesByNodes('user', IShare::TYPE_USER, $nodes)); + } + + public function testGetSharesByNodesOnOwnerlessMounts(): void { + $ownerless = $this->nodeWithId(1, true); + $regular = $this->nodeWithId(2); + $byOthers = $this->shareOn(1); + $otherType = $this->shareOn(1, IShare::TYPE_LINK); + $mine = $this->shareOn(2); + + // Every share of the path, of the requested type + $this->defaultProvider->expects($this->once()) + ->method('getSharesByPath') + ->with($ownerless) + ->willReturn([$byOthers, $otherType]); + $this->defaultProvider->expects($this->once()) + ->method('getSharesByFileIds') + ->with('user', IShare::TYPE_USER, [2]) + ->willReturn([$mine]); + + $this->assertSame([1 => [$byOthers], 2 => [$mine]], $this->manager->getSharesByNodes('user', IShare::TYPE_USER, [$ownerless, $regular])); + } + + public function testGetSharedWithByNodesInOneLookup(): void { + $nodes = [$this->nodeWithId(1), $this->nodeWithId(2)]; + $share = $this->shareOn(2, IShare::TYPE_GROUP); + + $this->defaultProvider->expects($this->once()) + ->method('getSharedWithByFileIds') + ->with('user', IShare::TYPE_GROUP, [1, 2]) + ->willReturn([$share]); + $this->defaultProvider->expects($this->never())->method('getSharedWith'); + + $this->assertSame([2 => [$share]], $this->manager->getSharedWithByNodes('user', IShare::TYPE_GROUP, $nodes)); + } + + public function testLookupByNodesFallsBackToEachNode(): void { + $provider = $this->createMock(IShareProvider::class); + $this->factory->setProvider($provider); + $nodes = [$this->nodeWithId(1), $this->nodeWithId(2)]; + $outgoing = $this->shareOn(2); + $incoming = $this->shareOn(1); + + $provider->expects($this->exactly(2)) + ->method('getSharesBy') + ->willReturnCallback(fn (string $userId, int $shareType, Node $node, bool $reshares, int $limit, int $offset): array => $node->getId() === 2 && !$reshares && $limit === -1 ? [$outgoing] : []); + $provider->expects($this->exactly(2)) + ->method('getSharedWith') + ->willReturnCallback(fn (string $userId, int $shareType, Node $node, int $limit, int $offset): array => $node->getId() === 1 && $limit === -1 ? [$incoming] : []); + + $this->assertSame([2 => [$outgoing]], $this->manager->getSharesByNodes('user', IShare::TYPE_USER, $nodes)); + $this->assertSame([1 => [$incoming]], $this->manager->getSharedWithByNodes('user', IShare::TYPE_USER, $nodes)); + } + + public function testLookupByNodesSkipsExpiredShares(): void { + $expired = $this->shareOn(1); + $expired->setExpirationDate(new \DateTime('-1 day')); + $valid = $this->shareOn(1); + + $manager = $this->createManagerMock() + ->onlyMethods(['deleteShare']) + ->getMock(); + $manager->expects($this->once())->method('deleteShare')->with($expired); + $this->defaultProvider->method('getSharesByFileIds')->willReturn([$expired, $valid]); + + $this->assertSame([1 => [$valid]], $manager->getSharesByNodes('user', IShare::TYPE_USER, [$this->nodeWithId(1)])); + } } class DummyFactory implements IProviderFactory {