Skip to content

Commit ecef56b

Browse files
committed
perf: Enhance objects using constant sql queries count to get rid of N+1 queries (code review)
Signed-off-by: Kostiantyn Miakshyn <molodchick@gmail.com>
1 parent b53d629 commit ecef56b

3 files changed

Lines changed: 5 additions & 39 deletions

File tree

lib/Db/RowSleeveMapper.php

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -105,33 +105,6 @@ public function deleteAllForTable(int $tableId): int {
105105
return $qb->executeStatement();
106106
}
107107

108-
/**
109-
* @param int[] $tableIds
110-
* @return array<int, int>
111-
*/
112-
public function countRowsForTables(array $tableIds): array {
113-
if (empty($tableIds)) {
114-
return [];
115-
}
116-
117-
$counts = array_fill_keys($tableIds, 0);
118-
foreach (array_chunk($tableIds, 1000 - 1) as $tableIdsChunk) {
119-
$qb = $this->db->getQueryBuilder();
120-
$qb->select('table_id', $qb->func()->count('*', 'counter'))
121-
->from($this->table)
122-
->where($qb->expr()->in('table_id', $qb->createNamedParameter($tableIdsChunk, IQueryBuilder::PARAM_INT_ARRAY)))
123-
->groupBy('table_id');
124-
125-
$result = $qb->executeQuery();
126-
while ($row = $result->fetch()) {
127-
$counts[(int)$row['table_id']] = (int)$row['counter'];
128-
}
129-
$result->closeCursor();
130-
}
131-
132-
return $counts;
133-
}
134-
135108
/**
136109
* @param int[] $tableIds
137110
* @return array<int, int>

lib/Db/ShareMapper.php

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -119,22 +119,17 @@ public function findAllSharesFor(string $nodeType, array $receivers, string $use
119119
/**
120120
* @param string $nodeType
121121
* @param int $nodeId
122-
* @param string $sender
123122
* @param array<string> $excluded receiver types to exclude from results
124123
* @return Share[]
125124
* @throws Exception
126125
*/
127-
public function findAllSharesForNode(string $nodeType, int $nodeId, string $sender = '', array $excluded = []): array {
126+
public function findAllSharesForNode(string $nodeType, int $nodeId, array $excluded = []): array {
128127
$qb = $this->db->getQueryBuilder();
129128
$qb->select('*')
130129
->from($this->table)
131130
->andWhere($qb->expr()->eq('node_type', $qb->createNamedParameter($nodeType, IQueryBuilder::PARAM_STR)))
132131
->andWhere($qb->expr()->eq('node_id', $qb->createNamedParameter($nodeId, IQueryBuilder::PARAM_INT)));
133132

134-
if ($sender !== '') {
135-
$qb->andWhere($qb->expr()->eq('sender', $qb->createNamedParameter($sender, IQueryBuilder::PARAM_STR)));
136-
}
137-
138133
if (!empty($excluded)) {
139134
$qb->andWhere($qb->expr()->notIn('receiver_type', $qb->createNamedParameter($excluded, IQueryBuilder::PARAM_STR_ARRAY)));
140135
}

lib/Service/ShareService.php

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -79,12 +79,10 @@ public function __construct(
7979
* @throws InternalError
8080
* @return Share[]
8181
*/
82-
public function findAll(string $nodeType, int $nodeId, ?string $userId = null, bool $enhanceShares = true): array {
83-
$userId = $this->permissionsService->preCheckUserId($userId);
84-
82+
public function findAll(string $nodeType, int $nodeId, bool $enhanceShares = true): array {
8583
try {
8684
$excluded = !$this->circleHelper->isCirclesEnabled() ? [ShareReceiverType::CIRCLE] : [];
87-
$shares = $this->mapper->findAllSharesForNode($nodeType, $nodeId, $userId, $excluded);
85+
$shares = $this->mapper->findAllSharesForNode($nodeType, $nodeId, $excluded);
8886

8987
return $enhanceShares ? $this->addReceiverDisplayNames($shares) : $shares;
9088
} catch (Exception $e) {
@@ -920,7 +918,7 @@ public function deleteAllForContext(Context $context): void {
920918
* @return Share[]
921919
*/
922920
public function changeSenderForNode(string $nodeType, int $nodeId, string $newOwnerUserId, ?string $userId = null): array {
923-
$sharesForTable = $this->findAll($nodeType, $nodeId, $userId, false);
921+
$sharesForTable = $this->findAll($nodeType, $nodeId, false);
924922
$newShares = [];
925923

926924
foreach ($sharesForTable as $share) {
@@ -982,7 +980,7 @@ public function transferSharesForContext(int $contextId, string $newOwnerId, str
982980
*/
983981
public function findSharedWithUserIds(int $elementId, string $elementType): array {
984982
try {
985-
$shares = $this->mapper->findAllSharesForNode($elementType, $elementId, '');
983+
$shares = $this->mapper->findAllSharesForNode($elementType, $elementId);
986984
$sharedWithUserIds = [];
987985

988986
foreach ($shares as $share) {

0 commit comments

Comments
 (0)