Skip to content

Commit 0202a72

Browse files
solracsfsolracsf
committed
perf: improve DB queries
Signed-off-by: Git'Fellow <12234510+solracsf@users.noreply.github.com>
1 parent 01fd051 commit 0202a72

15 files changed

+663
-72
lines changed

‎lib/Db/AttachmentMapper.php‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,34 @@ public function findByData(int $cardId, string $data): Attachment {
6464
return $this->findEntity($qb);
6565
}
6666

67+
/**
68+
* Returns a map of cardId => attachment count for the given card IDs in a single query.
69+
*
70+
* @param int[] $cardIds
71+
* @return array<int, int>
72+
* @throws \OCP\DB\Exception
73+
*/
74+
public function findCountByCardIds(array $cardIds): array {
75+
if (empty($cardIds)) {
76+
return [];
77+
}
78+
$qb = $this->db->getQueryBuilder();
79+
$qb->select('card_id')
80+
->selectAlias($qb->func()->count('id'), 'attachment_count')
81+
->from($this->getTableName())
82+
->where($qb->expr()->in('card_id', $qb->createNamedParameter($cardIds, IQueryBuilder::PARAM_INT_ARRAY)))
83+
->andWhere($qb->expr()->eq('deleted_at', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT)))
84+
->groupBy('card_id');
85+
86+
$counts = [];
87+
$cursor = $qb->executeQuery();
88+
while ($row = $cursor->fetch()) {
89+
$counts[(int)$row['card_id']] = (int)$row['attachment_count'];
90+
}
91+
$cursor->closeCursor();
92+
return $counts;
93+
}
94+
6795
/**
6896
* @return Entity[]
6997
* @throws \OCP\DB\Exception

‎lib/Db/BoardMapper.php‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -163,12 +163,14 @@ public function findAllForUser(string $userId, ?int $since = null, bool $include
163163
return $board->getId();
164164
}, $allBoards));
165165

166+
// Pre-group ACLs by board ID
167+
$aclsByBoard = [];
168+
foreach ($acls as $acl) {
169+
$aclsByBoard[$acl->getBoardId()][] = $acl;
170+
}
166171
/* @var Board $entry */
167172
foreach ($allBoards as $entry) {
168-
$boardAcls = array_values(array_filter($acls, function ($acl) use ($entry) {
169-
return $acl->getBoardId() === $entry->getId();
170-
}));
171-
$entry->setAcl($boardAcls);
173+
$entry->setAcl($aclsByBoard[$entry->getId()] ?? []);
172174
}
173175

174176
foreach ($allBoards as $board) {

‎lib/Db/CardMapper.php‎

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,7 @@ public function findCalendarEntries(int $boardId, ?int $limit = null, $offset =
237237
return $this->findEntities($qb);
238238
}
239239

240-
public function findAllArchived($stackId, $limit = null, $offset = null) {
240+
public function findAllArchived(int $stackId, ?int $limit = null, ?int $offset = null): array {
241241
$qb = $this->db->getQueryBuilder();
242242
$qb->select('*')
243243
->from('deck_cards')
@@ -250,7 +250,33 @@ public function findAllArchived($stackId, $limit = null, $offset = null) {
250250
return $this->findEntities($qb);
251251
}
252252

253-
public function findAllByStack($stackId, $limit = null, $offset = null) {
253+
/**
254+
* Batch-fetch all archived cards for multiple stacks in a single query.
255+
*
256+
* @param int[] $stackIds
257+
* @return array<int, Card[]> Map of stackId => Card[]
258+
* @throws \OCP\DB\Exception
259+
*/
260+
public function findAllArchivedForStacks(array $stackIds): array {
261+
if (empty($stackIds)) {
262+
return [];
263+
}
264+
$qb = $this->db->getQueryBuilder();
265+
$qb->select('*')
266+
->from('deck_cards')
267+
->where($qb->expr()->in('stack_id', $qb->createNamedParameter($stackIds, IQueryBuilder::PARAM_INT_ARRAY)))
268+
->andWhere($qb->expr()->eq('archived', $qb->createNamedParameter(true, IQueryBuilder::PARAM_BOOL)))
269+
->andWhere($qb->expr()->eq('deleted_at', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT)))
270+
->orderBy('last_modified');
271+
272+
$cards = array_fill_keys($stackIds, []);
273+
foreach ($this->findEntities($qb) as $card) {
274+
$cards[$card->getStackId()][] = $card;
275+
}
276+
return $cards;
277+
}
278+
279+
public function findAllByStack(int $stackId, ?int $limit = null, ?int $offset = null): array {
254280
$qb = $this->db->getQueryBuilder();
255281
$qb->select('*')
256282
->from('deck_cards')

‎lib/Db/LabelMapper.php‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,30 @@ public function findAll(int $boardId, ?int $limit = null, int $offset = 0): arra
3333
return $this->findEntities($qb);
3434
}
3535

36+
/**
37+
* Check if a label with the given title already exists on the board.
38+
* Pass $excludeId to skip a specific label (used during updates).
39+
*
40+
* @throws \OCP\DB\Exception
41+
*/
42+
public function existsByBoardIdAndTitle(int $boardId, string $title, ?int $excludeId = null): bool {
43+
$qb = $this->db->getQueryBuilder();
44+
$qb->select('id')
45+
->from($this->getTableName())
46+
->where($qb->expr()->eq('board_id', $qb->createNamedParameter($boardId, IQueryBuilder::PARAM_INT)))
47+
->andWhere($qb->expr()->eq('title', $qb->createNamedParameter($title, IQueryBuilder::PARAM_STR)))
48+
->setMaxResults(1);
49+
50+
if ($excludeId !== null) {
51+
$qb->andWhere($qb->expr()->neq('id', $qb->createNamedParameter($excludeId, IQueryBuilder::PARAM_INT)));
52+
}
53+
54+
$cursor = $qb->executeQuery();
55+
$exists = $cursor->fetch() !== false;
56+
$cursor->closeCursor();
57+
return $exists;
58+
}
59+
3660
public function delete(Entity $entity): Entity {
3761
// delete assigned labels
3862
$this->deleteLabelAssignments($entity->getId());

‎lib/Db/StackMapper.php‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,43 @@ public function find(int $id): Stack {
5353
return $this->stackCache[(string)$id];
5454
}
5555

56+
/**
57+
* Fetch multiple stacks by their IDs in a single query.
58+
*
59+
* @param int[] $ids
60+
* @return array<int, Stack> Map of stackId => Stack
61+
* @throws \OCP\DB\Exception
62+
*/
63+
public function findByIds(array $ids): array {
64+
if (empty($ids)) {
65+
return [];
66+
}
67+
68+
$stacks = [];
69+
$uncachedIds = [];
70+
foreach ($ids as $id) {
71+
if (isset($this->stackCache[(string)$id])) {
72+
$stacks[$id] = $this->stackCache[(string)$id];
73+
} else {
74+
$uncachedIds[] = $id;
75+
}
76+
}
77+
78+
if (!empty($uncachedIds)) {
79+
$qb = $this->db->getQueryBuilder();
80+
$qb->select('*')
81+
->from($this->getTableName())
82+
->where($qb->expr()->in('id', $qb->createNamedParameter($uncachedIds, IQueryBuilder::PARAM_INT_ARRAY)));
83+
84+
foreach ($this->findEntities($qb) as $stack) {
85+
$this->stackCache[(string)$stack->getId()] = $stack;
86+
$stacks[$stack->getId()] = $stack;
87+
}
88+
}
89+
90+
return $stacks;
91+
}
92+
5693
/**
5794
* @throws \OCP\DB\Exception
5895
*/

‎lib/Service/AttachmentService.php‎

Lines changed: 46 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -137,21 +137,57 @@ public function findAll(int $cardId, bool $withDeleted = false): array {
137137
* @throws \OCP\DB\Exception
138138
*/
139139
public function count(int $cardId): int {
140-
$count = $this->attachmentCacheHelper->getAttachmentCount($cardId);
141-
if ($count === null) {
142-
$count = count($this->attachmentMapper->findAll($cardId));
143-
144-
foreach (array_keys($this->services) as $attachmentType) {
145-
$service = $this->getService($attachmentType);
146-
if ($service instanceof ICustomAttachmentService) {
147-
$count += $service->getAttachmentCount($cardId);
140+
return ($this->countForCards([$cardId]))[$cardId] ?? 0;
141+
}
142+
143+
/**
144+
* Returns a map of cardId => attachment count for the given card IDs using batch queries.
145+
*
146+
* @param int[] $cardIds
147+
* @return array<int, int>
148+
* @throws \OCP\DB\Exception
149+
*/
150+
public function countForCards(array $cardIds): array {
151+
if (empty($cardIds)) {
152+
return [];
153+
}
154+
155+
$counts = [];
156+
$uncachedIds = [];
157+
foreach ($cardIds as $cardId) {
158+
$cached = $this->attachmentCacheHelper->getAttachmentCount($cardId);
159+
if ($cached !== null) {
160+
$counts[$cardId] = $cached;
161+
} else {
162+
$uncachedIds[] = $cardId;
163+
$counts[$cardId] = 0;
164+
}
165+
}
166+
167+
if (empty($uncachedIds)) {
168+
return $counts;
169+
}
170+
171+
// Batch query for deck_file attachments stored in the deck_attachment table
172+
foreach ($this->attachmentMapper->findCountByCardIds($uncachedIds) as $cardId => $count) {
173+
$counts[$cardId] = $count;
174+
}
175+
176+
// Add counts from custom attachment services (e.g. files shared via FilesAppService)
177+
foreach (array_keys($this->services) as $attachmentType) {
178+
$service = $this->getService($attachmentType);
179+
if ($service instanceof ICustomAttachmentService) {
180+
foreach ($service->getAttachmentCountForCards($uncachedIds) as $cardId => $count) {
181+
$counts[$cardId] = ($counts[$cardId] ?? 0) + $count;
148182
}
149183
}
184+
}
150185

151-
$this->attachmentCacheHelper->setAttachmentCount($cardId, $count);
186+
foreach ($uncachedIds as $cardId) {
187+
$this->attachmentCacheHelper->setAttachmentCount($cardId, $counts[$cardId]);
152188
}
153189

154-
return $count;
190+
return $counts;
155191
}
156192

157193
/**

‎lib/Service/BoardService.php‎

Lines changed: 31 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -721,13 +721,14 @@ private function cloneCards(Board $board, Board $newBoard, bool $withAssignments
721721
usort($stacks, $stackSorter);
722722
usort($newStacks, $stackSorter);
723723

724+
$stackIds = array_map(fn (Stack $stack) => $stack->getId(), $stacks);
725+
$activeCardsByStack = $this->cardMapper->findAllForStacks($stackIds);
726+
$archivedCardsByStack = $this->cardMapper->findAllArchivedForStacks($stackIds);
727+
724728
$i = 0;
725729
foreach ($stacks as $stack) {
726-
$cards = $this->cardMapper->findAll($stack->getId());
727-
$archivedCards = $this->cardMapper->findAllArchived($stack->getId());
728-
729730
/** @var Card[] $cards */
730-
$cards = array_merge($cards, $archivedCards);
731+
$cards = array_merge($activeCardsByStack[$stack->getId()] ?? [], $archivedCardsByStack[$stack->getId()] ?? []);
731732

732733
foreach ($cards as $card) {
733734
$targetStackId = $moveCardsToLeftStack ? $newStacks[0]->getId() : $newStacks[$i]->getId();
@@ -830,22 +831,38 @@ private function clearBoardFromCache(Board $board): void {
830831

831832
private function enrichWithCards(Board $board): void {
832833
$stacks = $this->stackMapper->findAll($board->getId());
834+
if (\count($stacks) === 0) {
835+
return;
836+
}
837+
838+
$stackIds = array_map(fn (Stack $stack) => $stack->getId(), $stacks);
839+
840+
// Fetch all active cards for all stacks in one query
841+
$cardsByStack = $this->cardMapper->findAllForStacks($stackIds);
842+
843+
$allCards = array_merge(...array_values(array_filter($cardsByStack)));
844+
$allCardIds = array_map(fn (Card $card) => $card->getId(), $allCards);
845+
846+
// Batch-fetch labels and assigned users for all cards
847+
$labelsByCard = [];
848+
foreach ($this->labelMapper->findAssignedLabelsForCards($allCardIds) as $label) {
849+
$labelsByCard[$label->getCardId()][] = $label;
850+
}
851+
$usersByCard = [];
852+
foreach ($this->assignedUsersMapper->findIn($allCardIds) as $assignment) {
853+
$usersByCard[$assignment->getCardId()][] = $assignment;
854+
}
855+
833856
foreach ($stacks as $stack) {
834-
$cards = $this->cardMapper->findAllByStack($stack->getId());
835857
$fullCards = [];
836-
foreach ($cards as $card) {
837-
$fullCard = $this->cardMapper->find($card->getId());
838-
$assignedUsers = $this->assignedUsersMapper->findAll($card->getId());
839-
$fullCard->setAssignedUsers($assignedUsers);
840-
array_push($fullCards, $fullCard);
858+
foreach ($cardsByStack[$stack->getId()] ?? [] as $card) {
859+
$card->setLabels($labelsByCard[$card->getId()] ?? []);
860+
$card->setAssignedUsers($usersByCard[$card->getId()] ?? []);
861+
$fullCards[] = $card;
841862
}
842863
$stack->setCards($fullCards);
843864
}
844865

845-
if (\count($stacks) === 0) {
846-
return;
847-
}
848-
849866
$board->setStacks($stacks);
850867
}
851868
}

‎lib/Service/CardService.php‎

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,11 @@
1111
use OCA\Deck\Activity\ChangeSet;
1212
use OCA\Deck\BadRequestException;
1313
use OCA\Deck\Db\Acl;
14-
use OCA\Deck\Db\Assignment;
1514
use OCA\Deck\Db\AssignmentMapper;
1615
use OCA\Deck\Db\BoardMapper;
1716
use OCA\Deck\Db\Card;
1817
use OCA\Deck\Db\CardMapper;
1918
use OCA\Deck\Db\ChangeHelper;
20-
use OCA\Deck\Db\Label;
2119
use OCA\Deck\Db\LabelMapper;
2220
use OCA\Deck\Db\StackMapper;
2321
use OCA\Deck\Event\CardCreatedEvent;
@@ -71,12 +69,19 @@ public function __construct(
7169
public function enrichCards(array $cards): array {
7270
$user = $this->userManager->get($this->userId);
7371

74-
$cardIds = array_map(function (Card $card) use ($user): int {
72+
$allCardIds = array_map(fn (Card $card) => $card->getId(), $cards);
73+
$attachmentCounts = $this->attachmentService->countForCards($allCardIds);
74+
75+
// Pre-fetch all stacks for this batch in one query and index by ID
76+
$stackIds = array_unique(array_map(fn (Card $card) => $card->getStackId(), $cards));
77+
$stacksById = $this->stackMapper->findByIds($stackIds);
78+
79+
$cardIds = array_map(function (Card $card) use ($user, $attachmentCounts, $stacksById): int {
7580
// Everything done in here might be heavy as it is executed for every card
7681
$cardId = $card->getId();
7782
$this->cardMapper->mapOwner($card);
7883

79-
$card->setAttachmentCount($this->attachmentService->count($cardId));
84+
$card->setAttachmentCount($attachmentCounts[$cardId] ?? 0);
8085

8186
// TODO We should find a better way just to get the comment count so we can save 1-3 queries per card here
8287
$countComments = $this->commentsManager->getNumberOfCommentsForObject('deckCard', (string)$card->getId());
@@ -85,7 +90,7 @@ public function enrichCards(array $cards): array {
8590
$card->setCommentsUnread($countUnreadComments);
8691
$card->setCommentsCount($countComments);
8792

88-
$stack = $this->stackMapper->find($card->getStackId());
93+
$stack = $stacksById[$card->getStackId()] ?? $this->stackMapper->find($card->getStackId());
8994
$board = $this->boardService->find($stack->getBoardId(), false);
9095
$card->setRelatedStack($stack);
9196
$card->setRelatedBoard($board);
@@ -96,15 +101,19 @@ public function enrichCards(array $cards): array {
96101
$assignedLabels = $this->labelMapper->findAssignedLabelsForCards($cardIds);
97102
$assignedUsers = $this->assignedUsersMapper->findIn($cardIds);
98103

104+
// Pre-group labels and users by card ID
105+
$labelsByCard = [];
106+
foreach ($assignedLabels as $label) {
107+
$labelsByCard[$label->getCardId()][] = $label;
108+
}
109+
$usersByCard = [];
110+
foreach ($assignedUsers as $assignment) {
111+
$usersByCard[$assignment->getCardId()][] = $assignment;
112+
}
113+
99114
foreach ($cards as $card) {
100-
$cardLabels = array_values(array_filter($assignedLabels, function (Label $label) use ($card) {
101-
return $label->getCardId() === $card->getId();
102-
}));
103-
$cardAssignedUsers = array_values(array_filter($assignedUsers, function (Assignment $assignment) use ($card) {
104-
return $assignment->getCardId() === $card->getId();
105-
}));
106-
$card->setLabels($cardLabels);
107-
$card->setAssignedUsers($cardAssignedUsers);
115+
$card->setLabels($labelsByCard[$card->getId()] ?? []);
116+
$card->setAssignedUsers($usersByCard[$card->getId()] ?? []);
108117
}
109118

110119
return array_map(

0 commit comments

Comments
 (0)