Skip to content

Commit

Permalink
perf: improve performance of resolving group shares
Browse files Browse the repository at this point in the history
Signed-off-by: Robin Appelman <[email protected]>
  • Loading branch information
icewind1991 committed Mar 5, 2024
1 parent c18ffe0 commit d28c1f7
Showing 1 changed file with 30 additions and 49 deletions.
79 changes: 30 additions & 49 deletions lib/private/Share20/DefaultShareProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -822,7 +822,7 @@ public function getShareById($id, $recipientId = null) {

// If the recipient is set for a group share resolve to that user
if ($recipientId !== null && $share->getShareType() === IShare::TYPE_GROUP) {
$share = $this->resolveGroupShares([$share], $recipientId)[0];
$share = $this->resolveGroupShares([$share->getId() => $share], $recipientId)[0];

Check failure

Code scanning / Psalm

InvalidArgument Error

Argument 1 of OC\Share20\DefaultShareProvider::resolveGroupShares expects array<int, OC\Share20\Share>, but non-empty-array<string, OCP\Share\IShare> provided

Check failure on line 825 in lib/private/Share20/DefaultShareProvider.php

View workflow job for this annotation

GitHub Actions / static-code-analysis

InvalidArgument

lib/private/Share20/DefaultShareProvider.php:825:39: InvalidArgument: Argument 1 of OC\Share20\DefaultShareProvider::resolveGroupShares expects array<int, OC\Share20\Share>, but non-empty-array<string, OCP\Share\IShare> provided (see https://psalm.dev/004)
}

return $share;
Expand Down Expand Up @@ -999,7 +999,8 @@ public function getSharedWith($userId, $shareType, $node, $limit, $offset) {
}

if ($this->isAccessibleResult($data)) {
$shares2[] = $this->createShare($data);
$share = $this->createShare($data);
$shares2[$share->getId()] = $share;
}
}
$cursor->closeCursor();
Expand Down Expand Up @@ -1120,61 +1121,41 @@ private function createShare($data) {
}

/**
* @param Share[] $shares
* Update the data from group shares with any per-user modifications
*
* @param array<int, Share> $shareMap shares indexed by share id
* @param $userId
* @return Share[] The updates shares if no update is found for a share return the original
*/
private function resolveGroupShares($shares, $userId) {
$result = [];

$start = 0;
while (true) {
/** @var Share[] $shareSlice */
$shareSlice = array_slice($shares, $start, 100);
$start += 100;

if ($shareSlice === []) {
break;
}

/** @var int[] $ids */
$ids = [];
/** @var Share[] $shareMap */
$shareMap = [];

foreach ($shareSlice as $share) {
$ids[] = (int)$share->getId();
$shareMap[$share->getId()] = $share;
}

$qb = $this->dbConn->getQueryBuilder();

$query = $qb->select('*')
->from('share')
->where($qb->expr()->in('parent', $qb->createNamedParameter($ids, IQueryBuilder::PARAM_INT_ARRAY)))
->andWhere($qb->expr()->eq('share_with', $qb->createNamedParameter($userId)))
->andWhere($qb->expr()->orX(
$qb->expr()->eq('item_type', $qb->createNamedParameter('file')),
$qb->expr()->eq('item_type', $qb->createNamedParameter('folder'))
));

$stmt = $query->execute();
private function resolveGroupShares($shareMap, $userId) {
$qb = $this->dbConn->getQueryBuilder();
$query = $qb->select('*')
->from('share')
->where($qb->expr()->eq('share_with', $qb->createNamedParameter($userId)))
->andWhere($qb->expr()->eq('share_type', $qb->createNamedParameter(IShare::TYPE_USERGROUP)))
->andWhere($qb->expr()->orX(
$qb->expr()->eq('item_type', $qb->createNamedParameter('file')),
$qb->expr()->eq('item_type', $qb->createNamedParameter('folder'))
));

while ($data = $stmt->fetch()) {
$shareMap[$data['parent']]->setPermissions((int)$data['permissions']);
$shareMap[$data['parent']]->setStatus((int)$data['accepted']);
$shareMap[$data['parent']]->setTarget($data['file_target']);
$shareMap[$data['parent']]->setParent($data['parent']);
}
// this is called with either all group shares or one group share.
// for all shares it's easier to just only search by share_with,
// for a single share it's efficient to filter by parent
if (count($shareMap) === 1) {
$share = reset($shareMap);
$query->andWhere($qb->expr()->eq('parent', $qb->createNamedParameter($share->getId())));
}

$stmt->closeCursor();
$stmt = $query->execute();

foreach ($shareMap as $share) {
$result[] = $share;
}
while ($data = $stmt->fetch()) {
$shareMap[$data['parent']]->setPermissions((int)$data['permissions']);
$shareMap[$data['parent']]->setStatus((int)$data['accepted']);
$shareMap[$data['parent']]->setTarget($data['file_target']);
$shareMap[$data['parent']]->setParent($data['parent']);
}

return $result;
return array_values($shareMap);
}

/**
Expand Down

0 comments on commit d28c1f7

Please sign in to comment.