Skip to content

Commit 44e315b

Browse files
committed
perf: Call countSharesForViews for a multiple views at once to get rid of N+1 queries
Signed-off-by: Kostiantyn Miakshyn <molodchick@gmail.com>
1 parent 5910950 commit 44e315b

2 files changed

Lines changed: 60 additions & 8 deletions

File tree

lib/Service/ShareService.php

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,36 @@ public function countSharesForTables(array $tableIds, ?string $userId = null): a
278278
}
279279
}
280280

281+
/**
282+
* @param int[] $viewIds
283+
* @param string|null $userId
284+
* @return array<int, int>
285+
* @throws InternalError
286+
*/
287+
public function countSharesForViews(array $viewIds, ?string $userId = null): array {
288+
$userId = $this->permissionsService->preCheckUserId($userId);
289+
290+
try {
291+
$excluded = !$this->circleHelper->isCirclesEnabled() ? [ShareReceiverType::CIRCLE] : [];
292+
$shares = $this->mapper->findAllSharesForNodes('view', $viewIds, $userId, $excluded);
293+
294+
$counts = [];
295+
foreach ($shares as $share) {
296+
$nodeId = $share->getNodeId();
297+
$counts[$nodeId] = ($counts[$nodeId] ?? 0) + 1;
298+
}
299+
300+
foreach ($viewIds as $viewId) {
301+
$counts[$viewId] ??= 0;
302+
}
303+
304+
return $counts;
305+
} catch (Exception $e) {
306+
$this->logger->error($e->getMessage());
307+
throw new InternalError($e->getMessage());
308+
}
309+
}
310+
281311
/**
282312
* @param int[] $tableIds
283313
* @return int[]

lib/Service/ViewService.php

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ public function findAll(Table $table, ?string $userId = null, ?int $tableRowsCou
111111
/**
112112
* @param Table[] $tables
113113
* @param string|null $userId
114-
* @param array<int, int> $tableRowsCounts
114+
* @param array<int, int|null> $tableRowsCounts
115115
* @return array<int, View[]>
116116
* @throws InternalError
117117
*/
@@ -459,7 +459,7 @@ private function enhanceView(View $view, string $userId, ?int $tableRowsCount =
459459
* $userId can be set or ''
460460
*
461461
* @param View[] $views
462-
* @param array<int, int> $tableRowsCounts
462+
* @param array<int, int|null> $tableRowsCounts
463463
*/
464464
private function enhanceViews(array $views, string $userId, array $tableRowsCounts = []): void {
465465
if (empty($views)) {
@@ -478,8 +478,26 @@ private function enhanceViews(array $views, string $userId, array $tableRowsCoun
478478

479479
$rowsCountCache = [];
480480

481+
$sharesCounts = [];
482+
if ($userId !== '') {
483+
$ownedViewIds = [];
484+
foreach ($views as $view) {
485+
if ($userId === $view->getOwnership()) {
486+
$ownedViewIds[] = $view->getId();
487+
}
488+
}
489+
490+
if (!empty($ownedViewIds)) {
491+
try {
492+
$sharesCounts = $this->shareService->countSharesForViews($ownedViewIds, $userId);
493+
} catch (InternalError $e) {
494+
$this->logger->error($e->getMessage(), ['exception' => $e]);
495+
}
496+
}
497+
}
498+
481499
foreach ($views as $view) {
482-
$this->setIsSharedState($view, $userId);
500+
$this->setIsSharedState($view, $userId, $sharesCounts[$view->getId()] ?? null);
483501

484502
if (!$this->permissionsService->canReadRowsByElement($view, 'view', $userId)) {
485503
continue;
@@ -535,7 +553,7 @@ static function (array $sortRule) use ($view): array {
535553
}
536554
}
537555

538-
private function setIsSharedState(View $view, string $userId): void {
556+
private function setIsSharedState(View $view, string $userId, ?int $sharesCount = null): void {
539557
// set if this is a shared table with you (somebody else shared it with you)
540558
// (senseless if we have no user in context)
541559
if ($userId !== '') {
@@ -575,10 +593,14 @@ private function setIsSharedState(View $view, string $userId): void {
575593
} else {
576594
// set hasShares if this table is shared by you (you share it with somebody else)
577595
// (senseless if we have no user in context)
578-
try {
579-
$allShares = $this->shareService->findAll('view', $view->getId());
580-
$view->setHasShares(count($allShares) !== 0);
581-
} catch (InternalError $e) {
596+
if ($sharesCount !== null) {
597+
$view->setHasShares($sharesCount > 0);
598+
} else {
599+
try {
600+
$allShares = $this->shareService->findAll('view', $view->getId(), $userId);
601+
$view->setHasShares(count($allShares) !== 0);
602+
} catch (InternalError $e) {
603+
}
582604
}
583605
}
584606
} else {

0 commit comments

Comments
 (0)