Skip to content

Commit d4a78f3

Browse files
KocKoc
committed
Perf: Optimize shares loading by eliminating N+1 queries (code review fixes)
Signed-off-by: Kostiantyn Miakshyn <molodchick@gmail.com>
1 parent 706e0e1 commit d4a78f3

File tree

7 files changed

+80
-32
lines changed

7 files changed

+80
-32
lines changed

‎lib/AppInfo/Application.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
use OCP\Collaboration\Resources\LoadAdditionalScriptsEvent;
4242
use OCP\DB\Events\AddMissingIndicesEvent;
4343
use OCP\User\Events\BeforeUserDeletedEvent;
44+
use Psr\Container\ContainerInterface;
4445

4546
class Application extends App implements IBootstrap {
4647
public const APP_ID = 'tables';
@@ -73,7 +74,7 @@ public function register(IRegistrationContext $context): void {
7374
throw new Exception('Cannot include autoload. Did you run install dependencies using composer?');
7475
}
7576

76-
$context->registerService(AuditLogServiceInterface::class, fn (IAppContainer $c) => $c->query(DefaultAuditLogService::class));
77+
$context->registerService(AuditLogServiceInterface::class, fn (ContainerInterface $c) => $c->get(DefaultAuditLogService::class));
7778

7879
$context->registerEventListener(BeforeUserDeletedEvent::class, UserDeletedListener::class);
7980
$context->registerEventListener(DatasourceEvent::class, AnalyticsDatasourceListener::class);

‎lib/Db/BulkFetchTrait.php‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
<?php
2+
3+
/**
4+
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
5+
* SPDX-License-Identifier: AGPL-3.0-or-later
6+
*/
7+
8+
namespace OCA\Tables\Db;
9+
10+
use OCP\IDBConnection;
11+
12+
/**
13+
* Trait that helps mappers to avoid errors with too many parameters
14+
*/
15+
trait BulkFetchTrait {
16+
private function getMaxDbParameters(): int {
17+
return match($this->db->getDatabaseProvider()) {
18+
IDBConnection::PLATFORM_ORACLE => 1000,
19+
default => 65_535,
20+
};
21+
}
22+
}

‎lib/Db/ShareMapper.php‎

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818

1919
/** @template-extends QBMapper<Share> */
2020
class ShareMapper extends QBMapper {
21+
use BulkFetchTrait;
22+
2123
protected string $table = 'tables_shares';
2224
protected LoggerInterface $logger;
2325

@@ -100,14 +102,27 @@ public function findShareForNode(int $nodeId, string $nodeType, string $receiver
100102
* @throws Exception
101103
*/
102104
public function findAllSharesFor(string $nodeType, array $receivers, string $userId, ?string $receiverType = 'user'): array {
103-
$qb = $this->db->getQueryBuilder();
104-
$qb->select('*')
105-
->from($this->table)
106-
->where($qb->expr()->in('receiver', $qb->createNamedParameter($receivers, IQueryBuilder::PARAM_STR_ARRAY)))
107-
->andWhere($qb->expr()->neq('sender', $qb->createNamedParameter($userId, IQueryBuilder::PARAM_STR)))
108-
->andWhere($qb->expr()->eq('node_type', $qb->createNamedParameter($nodeType, IQueryBuilder::PARAM_STR)))
109-
->andWhere($qb->expr()->eq('receiver_type', $qb->createNamedParameter($receiverType, IQueryBuilder::PARAM_STR)));
110-
return $this->findEntities($qb);
105+
if (!$receivers) {
106+
return [];
107+
}
108+
109+
// deduct extra parameters (sender, node type, receiver type)
110+
$chunkSize = $this->getMaxDbParameters() - 3;
111+
$receiverChunks = array_chunk($receivers, $chunkSize);
112+
$chunks = [];
113+
foreach ($receiverChunks as $receiverChunk) {
114+
$qb = $this->db->getQueryBuilder();
115+
$qb->select('*')
116+
->from($this->table)
117+
->where($qb->expr()->in('receiver', $qb->createNamedParameter($receiverChunk, IQueryBuilder::PARAM_STR_ARRAY)))
118+
->andWhere($qb->expr()->neq('sender', $qb->createNamedParameter($userId)))
119+
->andWhere($qb->expr()->eq('node_type', $qb->createNamedParameter($nodeType)))
120+
->andWhere($qb->expr()->eq('receiver_type', $qb->createNamedParameter($receiverType)));
121+
122+
$chunks[] = $this->findEntities($qb);
123+
}
124+
125+
return array_merge(...$chunks);
111126
}
112127

113128
/**

‎lib/Db/TableMapper.php‎

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919

2020
/** @template-extends QBMapper<Table> */
2121
class TableMapper extends QBMapper {
22+
use BulkFetchTrait;
23+
2224
protected string $table = 'tables_tables';
2325
protected CappedMemoryCache $cache;
2426
public function __construct(
@@ -70,16 +72,20 @@ public function findMany(array $ids): array {
7072
return $result;
7173
}
7274

73-
$qb = $this->db->getQueryBuilder();
74-
$qb->select('*')
75-
->from($this->table)
76-
->where($qb->expr()->in('id', $qb->createNamedParameter(array_keys($missing), IQueryBuilder::PARAM_INT_ARRAY)));
75+
$missing = array_keys($missing);
76+
$chunkSize = $this->getMaxDbParameters();
77+
$missingChunks = array_chunk($missing, $chunkSize);
78+
foreach ($missingChunks as $missingChunk) {
79+
$qb = $this->db->getQueryBuilder();
80+
$qb->select('*')
81+
->from($this->table)
82+
->where($qb->expr()->in('id', $qb->createNamedParameter($missingChunk, IQueryBuilder::PARAM_INT_ARRAY)));
7783

78-
$entities = $this->findEntities($qb);
79-
foreach ($entities as $entity) {
80-
$id = $entity->getId();
81-
$this->cache[(string)$id] = $entity;
82-
$result[$id] = $entity;
84+
foreach ($this->findEntities($qb) as $entity) {
85+
$id = $entity->getId();
86+
$this->cache[(string)$id] = $entity;
87+
$result[$id] = $entity;
88+
}
8389
}
8490
return $result;
8591
}

‎lib/Db/ViewMapper.php‎

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919

2020
/** @template-extends QBMapper<View> */
2121
class ViewMapper extends QBMapper {
22+
use BulkFetchTrait;
23+
2224
protected string $table = 'tables_views';
2325

2426
protected CappedMemoryCache $cache;
@@ -70,17 +72,21 @@ public function findMany(array $ids): array {
7072
return $result;
7173
}
7274

73-
$qb = $this->db->getQueryBuilder();
74-
$qb->select('v.*', 't.ownership')
75-
->from($this->table, 'v')
76-
->innerJoin('v', 'tables_tables', 't', 't.id = v.table_id')
77-
->where($qb->expr()->in('v.id', $qb->createNamedParameter(array_keys($missing), IQueryBuilder::PARAM_INT_ARRAY)));
78-
79-
$entities = $this->findEntities($qb);
80-
foreach ($entities as $entity) {
81-
$id = $entity->getId();
82-
$this->cache[(string)$id] = $entity;
83-
$result[$id] = $entity;
75+
$missing = array_keys($missing);
76+
$chunkSize = $this->getMaxDbParameters();
77+
$missingChunks = array_chunk($missing, $chunkSize);
78+
foreach ($missingChunks as $missingChunk) {
79+
$qb = $this->db->getQueryBuilder();
80+
$qb->select('v.*', 't.ownership')
81+
->from($this->table, 'v')
82+
->innerJoin('v', 'tables_tables', 't', 't.id = v.table_id')
83+
->where($qb->expr()->in('v.id', $qb->createNamedParameter($missingChunk, IQueryBuilder::PARAM_INT_ARRAY)));
84+
85+
foreach ($this->findEntities($qb) as $entity) {
86+
$id = $entity->getId();
87+
$this->cache[(string)$id] = $entity;
88+
$result[$id] = $entity;
89+
}
8490
}
8591
return $result;
8692
}

‎lib/Helper/CircleHelper.php‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
use OCA\Circles\Model\Circle;
1212
use OCA\Circles\Model\Member;
1313
use OCA\Circles\Model\Probes\CircleProbe;
14-
use OCA\Tables\Errors\InternalError;
1514
use OCP\App\IAppManager;
1615
use OCP\IL10N;
1716
use OCP\Server;
@@ -77,7 +76,6 @@ public function getCircleDisplayName(string $circleId, string $userId): string {
7776
/**
7877
* @param string $userId
7978
* @return Circle[]
80-
* @throws InternalError
8179
*/
8280
public function getUserCircles(string $userId): array {
8381
if (!$this->circlesEnabled) {

‎lib/Service/ShareService.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -570,7 +570,6 @@ public function findSharedWithUserIds(int $elementId, string $elementType): arra
570570
}
571571
}
572572

573-
574573
/**
575574
* @param int $nodeId
576575
* @param array $share
@@ -624,4 +623,5 @@ public function importShare(int $nodeId, array $share, string $userId): void {
624623
$this->logger->error('Failed to import share: ' . $e->getMessage(), ['exception' => $e, 'share' => $share]);
625624
}
626625
}
626+
627627
}

0 commit comments

Comments
 (0)