Skip to content

Carl/refactor share #58365

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
CarlSchwan wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Carl/refactor share #58365

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a6b0c0a
perf(sharing): Avoid loading all shares from all users when unsharing
CarlSchwan Feb 4, 2026
a0a194a
perf(share): Refactor share APIs to be more efficient
CarlSchwan Feb 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 21 additions & 24 deletions apps/dav/lib/Connector/Sabre/SharesPlugin.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use OCP\Files\NotFoundException;
use OCP\Files\Storage\ISharedStorage;
use OCP\IUserSession;
use OCP\PaginationParameters;
use OCP\Share\IManager;
use OCP\Share\IShare;
use Sabre\DAV\Exception\NotFound;
Expand Down Expand Up @@ -79,10 +80,9 @@ public function initialize(Server $server) {
}

/**
* @param Node $node
* @return IShare[]
* @return list<IShare>
*/
private function getShare(Node $node): array {
private function getShare(Node $node, PaginationParameters $paginationParameters): array {
$result = [];
$requestedShareTypes = [
IShare::TYPE_USER,
Expand All @@ -95,26 +95,23 @@ private function getShare(Node $node): array {
IShare::TYPE_DECK,
];

foreach ($requestedShareTypes as $requestedShareType) {
$result[] = $this->shareManager->getSharesBy(
$result[] = $this->shareManager->getAllSharesBy(
$this->userId,
$node,
$paginationParameters,
false,
);

// Also check for shares where the user is the recipient
try {
$result[] = $this->shareManager->getAllSharedWith(
$this->userId,
$requestedShareType,
$requestedShareTypes,
$node,
false,
-1
$paginationParameters,
);

// Also check for shares where the user is the recipient
try {
$result[] = $this->shareManager->getSharedWith(
$this->userId,
$requestedShareType,
$node,
-1
);
} catch (BackendError $e) {
// ignore
}
} catch (BackendError $e) {
// ignore
}

return array_merge(...$result);
Expand Down Expand Up @@ -155,7 +152,7 @@ private function getShares(DavNode $sabreNode): array {
return [];
}

$shares = $this->getShare($node);
$shares = $this->getShare($node, new PaginationParameters(limit: null));
$this->cachedShares[$sabreNode->getId()] = $shares;
return $shares;
}
Expand Down Expand Up @@ -235,9 +232,9 @@ public function validateMoveOrCopy(string $source, string $target): bool {
return true;
}

$targetShares = $this->getShare($targetNode->getNode());
$targetShares = $this->getShare($targetNode->getNode(), new PaginationParameters(limit: null));
if (empty($targetShares)) {
// Target is not a share so no re-sharing inprogress
// Target is not a share so no re-sharing in-progress
return true;
}

Expand All @@ -253,7 +250,7 @@ public function validateMoveOrCopy(string $source, string $target): bool {
}
}

// if the share recipient is allow to delete from the share, they are allowed to move the file out of the share
// if the share recipient is allowed to delete from the share, they are allowed to move the file out of the share
// the user moving the file out of the share to their home storage would give them share permissions and allow moving into the share
//
// since the 2-step move is allowed, we also allow both steps at once
Expand Down
6 changes: 2 additions & 4 deletions apps/dav/tests/unit/Connector/Sabre/SharesPluginTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,12 +87,11 @@ public function testGetProperties(array $shareTypes): void {
});

$this->shareManager->expects($this->any())
->method('getSharedWith')
->method('getAllSharedWith')
->with(
$this->equalTo('user1'),
$this->anything(),
$this->equalTo($node),
$this->equalTo(-1)
)
->willReturn([]);

Expand Down Expand Up @@ -183,12 +182,11 @@ public function testPreloadThenGetProperties(array $shareTypes): void {
});

$this->shareManager->expects($this->any())
->method('getSharedWith')
->method('getAllSharedWith')
->with(
$this->equalTo('user1'),
$this->anything(),
$this->equalTo($node),
$this->equalTo(-1)
)
->willReturn([]);

Expand Down
Loading