Skip to content

Commit ae16a28

Browse files
provokateurinprovokateurin
committed
perf(Share20): Add interface to query all shares in a folder without filtering by user
Signed-off-by: provokateurin <kate@provokateurin.de>
1 parent 4783459 commit ae16a28

File tree

8 files changed

+121
-59
lines changed

8 files changed

+121
-59
lines changed

‎apps/federatedfilesharing/lib/FederatedShareProvider.php‎

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -26,14 +26,15 @@
2626
use OCP\Share\Exceptions\ShareNotFound;
2727
use OCP\Share\IShare;
2828
use OCP\Share\IShareProvider;
29+
use OCP\Share\IShareProviderSupportsAllSharesInFolder;
2930
use Psr\Log\LoggerInterface;
3031

3132
/**
3233
* Class FederatedShareProvider
3334
*
3435
* @package OCA\FederatedFileSharing
3536
*/
36-
class FederatedShareProvider implements IShareProvider {
37+
class FederatedShareProvider implements IShareProvider, IShareProviderSupportsAllSharesInFolder {
3738
public const SHARE_TYPE_REMOTE = 6;
3839

3940
/** @var string */
@@ -553,7 +554,17 @@ public function getSharesInFolder($userId, Folder $node, $reshares, $shallow = t
553554
if (!$shallow) {
554555
throw new \Exception('non-shallow getSharesInFolder is no longer supported');
555556
}
557+
return $this->getSharesInFolderInternal($userId, $node, $reshares);
558+
}
559+
560+
public function getAllSharesInFolder(Folder $node): array {
561+
return $this->getSharesInFolderInternal(null, $node, null);
562+
}
556563

564+
/**
565+
* @return array<int, list<IShare>>
566+
*/
567+
private function getSharesInFolderInternal(?string $userId, Folder $node, ?bool $reshares): array {
557568
$qb = $this->dbConnection->getQueryBuilder();
558569
$qb->select('*')
559570
->from('share', 's')
@@ -562,18 +573,20 @@ public function getSharesInFolder($userId, Folder $node, $reshares, $shallow = t
562573
$qb->expr()->eq('share_type', $qb->createNamedParameter(IShare::TYPE_REMOTE))
563574
);
564575

565-
/**
566-
* Reshares for this user are shares where they are the owner.
567-
*/
568-
if ($reshares === false) {
569-
$qb->andWhere($qb->expr()->eq('uid_initiator', $qb->createNamedParameter($userId)));
570-
} else {
571-
$qb->andWhere(
572-
$qb->expr()->orX(
573-
$qb->expr()->eq('uid_owner', $qb->createNamedParameter($userId)),
574-
$qb->expr()->eq('uid_initiator', $qb->createNamedParameter($userId))
575-
)
576-
);
576+
if ($userId !== null) {
577+
/**
578+
* Reshares for this user are shares where they are the owner.
579+
*/
580+
if ($reshares !== true) {
581+
$qb->andWhere($qb->expr()->eq('uid_initiator', $qb->createNamedParameter($userId)));
582+
} else {
583+
$qb->andWhere(
584+
$qb->expr()->orX(
585+
$qb->expr()->eq('uid_owner', $qb->createNamedParameter($userId)),
586+
$qb->expr()->eq('uid_initiator', $qb->createNamedParameter($userId))
587+
)
588+
);
589+
}
577590
}
578591

579592
$qb->innerJoin('s', 'filecache', 'f', $qb->expr()->eq('s.file_source', 'f.fileid'));

‎apps/sharebymail/lib/ShareByMailProvider.php‎

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1112,6 +1112,17 @@ protected function getRawShare(int $id): array {
11121112
}
11131113

11141114
public function getSharesInFolder($userId, Folder $node, $reshares, $shallow = true): array {
1115+
return $this->getSharesInFolderInternal($userId, $node, $reshares);
1116+
}
1117+
1118+
public function getAllSharesInFolder(Folder $node): array {
1119+
return $this->getSharesInFolderInternal(null, $node, null);
1120+
}
1121+
1122+
/**
1123+
* @return array<int, list<IShare>>
1124+
*/
1125+
private function getSharesInFolderInternal(?string $userId, Folder $node, ?bool $reshares): array {
11151126
$qb = $this->dbConnection->getQueryBuilder();
11161127
$qb->select('*')
11171128
->from('share', 's')
@@ -1120,18 +1131,20 @@ public function getSharesInFolder($userId, Folder $node, $reshares, $shallow = t
11201131
$qb->expr()->eq('share_type', $qb->createNamedParameter(IShare::TYPE_EMAIL))
11211132
);
11221133

1123-
/**
1124-
* Reshares for this user are shares where they are the owner.
1125-
*/
1126-
if ($reshares === false) {
1127-
$qb->andWhere($qb->expr()->eq('uid_initiator', $qb->createNamedParameter($userId)));
1128-
} else {
1129-
$qb->andWhere(
1130-
$qb->expr()->orX(
1131-
$qb->expr()->eq('uid_owner', $qb->createNamedParameter($userId)),
1132-
$qb->expr()->eq('uid_initiator', $qb->createNamedParameter($userId))
1133-
)
1134-
);
1134+
if ($userId !== null) {
1135+
/**
1136+
* Reshares for this user are shares where they are the owner.
1137+
*/
1138+
if ($reshares !== true) {
1139+
$qb->andWhere($qb->expr()->eq('uid_initiator', $qb->createNamedParameter($userId)));
1140+
} else {
1141+
$qb->andWhere(
1142+
$qb->expr()->orX(
1143+
$qb->expr()->eq('uid_owner', $qb->createNamedParameter($userId)),
1144+
$qb->expr()->eq('uid_initiator', $qb->createNamedParameter($userId))
1145+
)
1146+
);
1147+
}
11351148
}
11361149

11371150
$qb->innerJoin('s', 'filecache', 'f', $qb->expr()->eq('s.file_source', 'f.fileid'));

‎lib/composer/composer/autoload_classmap.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -778,6 +778,7 @@
778778
'OCP\\Share\\IShareHelper' => $baseDir . '/lib/public/Share/IShareHelper.php',
779779
'OCP\\Share\\IShareProvider' => $baseDir . '/lib/public/Share/IShareProvider.php',
780780
'OCP\\Share\\IShareProviderSupportsAccept' => $baseDir . '/lib/public/Share/IShareProviderSupportsAccept.php',
781+
'OCP\\Share\\IShareProviderSupportsAllSharesInFolder' => $baseDir . '/lib/public/Share/IShareProviderSupportsAllSharesInFolder.php',
781782
'OCP\\Share\\IShareProviderWithNotification' => $baseDir . '/lib/public/Share/IShareProviderWithNotification.php',
782783
'OCP\\Share_Backend' => $baseDir . '/lib/public/Share_Backend.php',
783784
'OCP\\Share_Backend_Collection' => $baseDir . '/lib/public/Share_Backend_Collection.php',

‎lib/composer/composer/autoload_static.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -819,6 +819,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
819819
'OCP\\Share\\IShareHelper' => __DIR__ . '/../../..' . '/lib/public/Share/IShareHelper.php',
820820
'OCP\\Share\\IShareProvider' => __DIR__ . '/../../..' . '/lib/public/Share/IShareProvider.php',
821821
'OCP\\Share\\IShareProviderSupportsAccept' => __DIR__ . '/../../..' . '/lib/public/Share/IShareProviderSupportsAccept.php',
822+
'OCP\\Share\\IShareProviderSupportsAllSharesInFolder' => __DIR__ . '/../../..' . '/lib/public/Share/IShareProviderSupportsAllSharesInFolder.php',
822823
'OCP\\Share\\IShareProviderWithNotification' => __DIR__ . '/../../..' . '/lib/public/Share/IShareProviderWithNotification.php',
823824
'OCP\\Share_Backend' => __DIR__ . '/../../..' . '/lib/public/Share_Backend.php',
824825
'OCP\\Share_Backend_Collection' => __DIR__ . '/../../..' . '/lib/public/Share_Backend_Collection.php',

‎lib/private/Share20/DefaultShareProvider.php‎

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
* SPDX-FileCopyrightText: 2016 ownCloud, Inc.
66
* SPDX-License-Identifier: AGPL-3.0-only
77
*/
8+
89
namespace OC\Share20;
910

1011
use OC\Files\Cache\Cache;
@@ -31,6 +32,7 @@
3132
use OCP\Share\IManager;
3233
use OCP\Share\IShare;
3334
use OCP\Share\IShareProviderSupportsAccept;
35+
use OCP\Share\IShareProviderSupportsAllSharesInFolder;
3436
use OCP\Share\IShareProviderWithNotification;
3537
use Psr\Log\LoggerInterface;
3638
use function str_starts_with;
@@ -40,7 +42,7 @@
4042
*
4143
* @package OC\Share20
4244
*/
43-
class DefaultShareProvider implements IShareProviderWithNotification, IShareProviderSupportsAccept {
45+
class DefaultShareProvider implements IShareProviderWithNotification, IShareProviderSupportsAccept, IShareProviderSupportsAllSharesInFolder {
4446
// Special share type for user modified group shares
4547
public const SHARE_TYPE_USERGROUP = 2;
4648

@@ -603,6 +605,17 @@ public function getSharesInFolder($userId, Folder $node, $reshares, $shallow = t
603605
throw new \Exception('non-shallow getSharesInFolder is no longer supported');
604606
}
605607

608+
return $this->getSharesInFolderInternal($userId, $node, $reshares);
609+
}
610+
611+
public function getAllSharesInFolder(Folder $node): array {
612+
return $this->getSharesInFolderInternal(null, $node, null);
613+
}
614+
615+
/**
616+
* @return array<int, list<IShare>>
617+
*/
618+
private function getSharesInFolderInternal(?string $userId, Folder $node, ?bool $reshares): array {
606619
$qb = $this->dbConn->getQueryBuilder();
607620
$qb->select('s.*',
608621
'f.fileid', 'f.path', 'f.permissions AS f_permissions', 'f.storage', 'f.path_hash',
@@ -613,18 +626,20 @@ public function getSharesInFolder($userId, Folder $node, $reshares, $shallow = t
613626

614627
$qb->andWhere($qb->expr()->in('share_type', $qb->createNamedParameter([IShare::TYPE_USER, IShare::TYPE_GROUP, IShare::TYPE_LINK], IQueryBuilder::PARAM_INT_ARRAY)));
615628

616-
/**
617-
* Reshares for this user are shares where they are the owner.
618-
*/
619-
if ($reshares === false) {
620-
$qb->andWhere($qb->expr()->eq('uid_initiator', $qb->createNamedParameter($userId)));
621-
} else {
622-
$qb->andWhere(
623-
$qb->expr()->orX(
624-
$qb->expr()->eq('uid_owner', $qb->createNamedParameter($userId)),
625-
$qb->expr()->eq('uid_initiator', $qb->createNamedParameter($userId))
626-
)
627-
);
629+
if ($userId !== null) {
630+
/**
631+
* Reshares for this user are shares where they are the owner.
632+
*/
633+
if ($reshares !== true) {
634+
$qb->andWhere($qb->expr()->eq('uid_initiator', $qb->createNamedParameter($userId)));
635+
} else {
636+
$qb->andWhere(
637+
$qb->expr()->orX(
638+
$qb->expr()->eq('uid_owner', $qb->createNamedParameter($userId)),
639+
$qb->expr()->eq('uid_initiator', $qb->createNamedParameter($userId))
640+
)
641+
);
642+
}
628643
}
629644

630645
// todo? maybe get these from the oc_mounts table
@@ -656,7 +671,6 @@ public function getSharesInFolder($userId, Folder $node, $reshares, $shallow = t
656671

657672
foreach ($chunks as $chunk) {
658673
$qb->setParameter('chunk', $chunk, IQueryBuilder::PARAM_INT_ARRAY);
659-
$a = $qb->getSQL();
660674
$cursor = $qb->executeQuery();
661675
while ($data = $cursor->fetch()) {
662676
$shares[$data['fileid']][] = $this->createShare($data);

‎lib/private/Share20/Manager.php‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@
5151
use OCP\Share\IShare;
5252
use OCP\Share\IShareProvider;
5353
use OCP\Share\IShareProviderSupportsAccept;
54+
use OCP\Share\IShareProviderSupportsAllSharesInFolder;
5455
use OCP\Share\IShareProviderWithNotification;
5556
use Psr\Log\LoggerInterface;
5657

@@ -1213,11 +1214,13 @@ public function getSharesInFolder($userId, Folder $node, $reshares = false, $sha
12131214
$shares = [];
12141215
foreach ($providers as $provider) {
12151216
if ($isOwnerless) {
1216-
foreach ($node->getDirectoryListing() as $childNode) {
1217-
$data = $provider->getSharesByPath($childNode);
1218-
$fid = $childNode->getId();
1219-
$shares[$fid] ??= [];
1220-
$shares[$fid] = array_merge($shares[$fid], $data);
1217+
// If the provider does not implement the additional interface,
1218+
// we lack a performant way of querying all shares and therefore ignore the provider.
1219+
if ($provider instanceof IShareProviderSupportsAllSharesInFolder) {
1220+
foreach ($provider->getAllSharesInFolder($node) as $fid => $data) {
1221+
$shares[$fid] ??= [];
1222+
$shares[$fid] = array_merge($shares[$fid], $data);
1223+
}
12211224
}
12221225
} else {
12231226
foreach ($provider->getSharesInFolder($userId, $node, $reshares) as $fid => $data) {

‎lib/public/Share/IShareProviderSupportsAllSharesInFolder.php‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
<?php
2+
3+
/**
4+
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
5+
* SPDX-License-Identifier: AGPL-3.0-or-later
6+
*/
7+
namespace OCP\Share;
8+
9+
use OCP\Files\Folder;
10+
11+
/**
12+
* Allows defining a IShareProvider with support for the getAllSharesInFolder method.
13+
*
14+
* @since 32.0.0
15+
*/
16+
interface IShareProviderSupportsAllSharesInFolder extends IShareProvider {
17+
/**
18+
* Get all shares in a folder.
19+
*
20+
* @return array<int, list<IShare>>
21+
* @since 32.0.0
22+
*/
23+
public function getAllSharesInFolder(Folder $node): array;
24+
}

‎tests/lib/Share20/ManagerTest.php‎

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@
5555
use OCP\Share\IProviderFactory;
5656
use OCP\Share\IShare;
5757
use OCP\Share\IShareProvider;
58+
use OCP\Share\IShareProviderSupportsAllSharesInFolder;
5859
use PHPUnit\Framework\MockObject\MockBuilder;
5960
use PHPUnit\Framework\MockObject\MockObject;
6061
use Psr\Log\LoggerInterface;
@@ -4551,36 +4552,28 @@ public function testGetSharesInFolderOwnerless(): void {
45514552
$manager = $this->createManager($factory);
45524553

45534554
$factory->setProvider($this->defaultProvider);
4554-
$extraProvider = $this->createMock(IShareProvider::class);
4555+
$extraProvider = $this->createMock(IShareProviderSupportsAllSharesInFolder::class);
45554556
$factory->setSecondProvider($extraProvider);
45564557

45574558
$share1 = $this->createMock(IShare::class);
45584559
$share2 = $this->createMock(IShare::class);
45594560

45604561
$mount = $this->createMock(IShareOwnerlessMount::class);
45614562

4562-
$file = $this->createMock(File::class);
4563-
$file
4564-
->method('getId')
4565-
->willReturn(1);
4566-
45674563
$folder = $this->createMock(Folder::class);
45684564
$folder
45694565
->method('getMountPoint')
45704566
->willReturn($mount);
4571-
$folder
4572-
->method('getDirectoryListing')
4573-
->willReturn([$file]);
45744567

45754568
$this->defaultProvider
4576-
->method('getSharesByPath')
4577-
->with($file)
4578-
->willReturn([$share1]);
4569+
->method('getAllSharesInFolder')
4570+
->with($folder)
4571+
->willReturn([1 => [$share1]]);
45794572

45804573
$extraProvider
4581-
->method('getSharesByPath')
4582-
->with($file)
4583-
->willReturn([$share2]);
4574+
->method('getAllSharesInFolder')
4575+
->with($folder)
4576+
->willReturn([1 => [$share2]]);
45844577

45854578
$this->assertSame([
45864579
1 => [$share1, $share2],

0 commit comments

Comments
 (0)