Merge pull request #7165 from nextcloud/get-cards-at-once

perf(cards): fetch all cards at once
This commit is contained in:
Luka Trovic
2025-09-16 10:31:00 +02:00
committed by GitHub
5 changed files with 58 additions and 21 deletions

View File

@@ -131,7 +131,11 @@ class CardMapper extends QBMapper implements IPermissionMapper {
return $card;
}
public function findAll($stackId, $limit = null, $offset = null, $since = -1) {
/**
* @return Card[]
* @throws \OCP\DB\Exception
*/
public function findAll($stackId, ?int $limit = null, ?int $offset = null, int $since = -1) {
$qb = $this->db->getQueryBuilder();
$qb->select('*')
->from('deck_cards')
@@ -146,6 +150,32 @@ class CardMapper extends QBMapper implements IPermissionMapper {
return $this->findEntities($qb);
}
/**
* @param int[] $stackIds
* @return array<int, null|Card[]>
* @throws \OCP\DB\Exception
*/
public function findAllForStacks(array $stackIds, ?int $limit = null, ?int $offset = null, int $since = -1): array {
$qb = $this->db->getQueryBuilder();
$qb->select('*')
->from('deck_cards')
->where($qb->expr()->in('stack_id', $qb->createNamedParameter($stackIds, IQueryBuilder::PARAM_INT_ARRAY)))
->andWhere($qb->expr()->eq('archived', $qb->createNamedParameter(false, IQueryBuilder::PARAM_BOOL)))
->andWhere($qb->expr()->eq('deleted_at', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT)))
->andWhere($qb->expr()->gt('last_modified', $qb->createNamedParameter($since, IQueryBuilder::PARAM_INT)))
->setMaxResults($limit)
->setFirstResult($offset)
->orderBy('order')
->addOrderBy('id');
$rawCards = $this->findEntities($qb);
$cards = array_fill_keys($stackIds, null);
foreach ($rawCards as $card) {
$cards[$card->getStackId()][] = $card;
}
return $cards;
}
public function queryCardsByBoard(int $boardId): IQueryBuilder {
$qb = $this->db->getQueryBuilder();
$qb->select('c.*')

View File

@@ -15,6 +15,7 @@ use Sabre\VObject\Component\VCalendar;
* @method int getDeletedAt()
* @method int getLastModified()
* @method int getOrder()
* @method Card[] getCards()
*/
class Stack extends RelationalEntity {
protected $title;

View File

@@ -77,12 +77,10 @@ class StackMapper extends DeckMapper implements IPermissionMapper {
/**
* @param numeric $boardId
* @param int|null $limit
* @param int|null $offset
* @return Stack[]
* @throws \OCP\DB\Exception
*/
public function findAll($boardId, $limit = null, $offset = null): array {
public function findAll($boardId, ?int $limit = null, ?int $offset = null): array {
$qb = $this->db->getQueryBuilder();
$qb->select('*')
->from($this->getTableName())

View File

@@ -75,19 +75,22 @@ class StackService {
$this->stackServiceValidator = $stackServiceValidator;
}
private function enrichStackWithCards($stack, $since = -1) {
$cards = $this->cardMapper->findAll($stack->getId(), null, null, $since);
/** @param Stack[] $stacks */
private function enrichStacksWithCards(array $stacks, $since = -1): void {
$cardsByStackId = $this->cardMapper->findAllForStacks(array_map(fn (Stack $stack) => $stack->getId(), $stacks), null, null, $since);
if (\count($cards) === 0) {
return;
}
foreach ($cardsByStackId as $stackId => $cards) {
if (!$cards) {
continue;
}
$stack->setCards($this->cardService->enrichCards($cards));
}
private function enrichStacksWithCards($stacks, $since = -1) {
foreach ($stacks as $stack) {
$this->enrichStackWithCards($stack, $since);
foreach ($stacks as $stack) {
if ($stack->getId() === $stackId) {
$stack->setCards($this->cardService->enrichCards($cards));
break;
}
}
}
}
@@ -124,9 +127,9 @@ class StackService {
}
/**
* @param $boardId
* @param mixed $boardId
*
* @return array
* @return Stack[]
* @throws \OCA\Deck\NoPermissionException
* @throws BadRequestException
*/
@@ -247,7 +250,7 @@ class StackService {
);
$this->changeHelper->boardChanged($stack->getBoardId());
$this->eventDispatcher->dispatchTyped(new BoardUpdatedEvent($stack->getBoardId()));
$this->enrichStackWithCards($stack);
$this->enrichStacksWithCards([$stack]);
return $stack;
}

View File

@@ -129,12 +129,17 @@ class StackServiceTest extends TestCase {
}
)
);
$this->cardMapper->expects($this->any())->method('findAll')->willReturn($this->getCards(222));
$this->cardMapper->expects($this->any())->method('findAllForStacks')->willReturnCallback(function (array $stackIds) {
$r = [];
foreach ($stackIds as $stackId) {
$r[$stackId] = $this->getCards(222);
}
return $r;
});
$actual = $this->stackService->findAll(123);
for ($stackId = 0; $stackId < 3; $stackId++) {
for ($cardId = 0;$cardId < 10;$cardId++) {
for ($cardId = 0; $cardId < 10; $cardId++) {
$this->assertEquals($actual[0]->getCards()[$cardId]->getId(), $cardId);
$this->assertEquals($actual[0]->getCards()[$cardId]->getStackId(), 222);
$this->assertEquals($actual[0]->getCards()[$cardId]->getLabels(), $this->getLabels()[$cardId]);
@@ -211,7 +216,7 @@ class StackServiceTest extends TestCase {
$stackToBeDeleted->setBoardId(1);
$this->stackMapper->expects($this->once())->method('find')->willReturn($stackToBeDeleted);
$this->stackMapper->expects($this->once())->method('update')->willReturn($stackToBeDeleted);
$this->cardMapper->expects($this->once())->method('findAll')->willReturn([]);
$this->cardMapper->expects($this->once())->method('findAllForStacks')->willReturn([]);
$this->stackService->delete(123);
$this->assertTrue($stackToBeDeleted->getDeletedAt() <= time(), 'deletedAt is in the past');
$this->assertTrue($stackToBeDeleted->getDeletedAt() > 0, 'deletedAt is set');