Skip to content

Commit 071b333

Browse files
edmndedmnd
committed
fix(core): handle parameter limits in files metadata deletion
Chunk metadata and index deletions using IQueryBuilder::MAX_IN_PARAMETERS and bind each chunk instead of the full file ID list. Add coverage for large input sets and preserve atomic deletion across chunks. Resolves: #62325 Signed-off-by: Edmond <edmnd@users.noreply.github.com>
1 parent e68d903 commit 071b333

3 files changed

Lines changed: 94 additions & 16 deletions

File tree

‎lib/private/FilesMetadata/Service/IndexRequestService.php‎

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -188,17 +188,24 @@ public function dropIndex(int $fileId, string $key = ''): void {
188188
public function dropIndexForFiles(array $fileIds, string $key = ''): void {
189189
$chunks = array_chunk($fileIds, IQueryBuilder::MAX_IN_PARAMETERS);
190190

191-
foreach ($chunks as $chunk) {
192-
$qb = $this->dbConnection->getQueryBuilder();
193-
$expr = $qb->expr();
194-
$qb->delete(self::TABLE_METADATA_INDEX)
195-
->where($expr->in('file_id', $qb->createNamedParameter($chunk, IQueryBuilder::PARAM_INT_ARRAY)));
196-
197-
if ($key !== '') {
198-
$qb->andWhere($expr->eq('meta_key', $qb->createNamedParameter($key)));
199-
}
191+
$this->dbConnection->beginTransaction();
192+
try {
193+
foreach ($chunks as $chunk) {
194+
$qb = $this->dbConnection->getQueryBuilder();
195+
$expr = $qb->expr();
196+
$qb->delete(self::TABLE_METADATA_INDEX)
197+
->where($expr->in('file_id', $qb->createNamedParameter($chunk, IQueryBuilder::PARAM_INT_ARRAY)));
200198

201-
$qb->executeStatement();
199+
if ($key !== '') {
200+
$qb->andWhere($expr->eq('meta_key', $qb->createNamedParameter($key)));
201+
}
202+
203+
$qb->executeStatement();
204+
}
205+
$this->dbConnection->commit();
206+
} catch (DbException $e) {
207+
$this->dbConnection->rollBack();
208+
throw $e;
202209
}
203210
}
204211
}

‎lib/private/FilesMetadata/Service/MetadataRequestService.php‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -152,12 +152,19 @@ public function dropMetadata(int $fileId): void {
152152
public function dropMetadataForFiles(int $storage, array $fileIds): void {
153153
$chunks = array_chunk($fileIds, IQueryBuilder::MAX_IN_PARAMETERS);
154154

155-
foreach ($chunks as $chunk) {
156-
$qb = $this->dbConnection->getQueryBuilder();
157-
$qb->delete(self::TABLE_METADATA)
158-
->where($qb->expr()->in('file_id', $qb->createNamedParameter($fileIds, IQueryBuilder::PARAM_INT_ARRAY)))
159-
->hintShardKey('storage', $storage);
160-
$qb->executeStatement();
155+
$this->dbConnection->beginTransaction();
156+
try {
157+
foreach ($chunks as $chunk) {
158+
$qb = $this->dbConnection->getQueryBuilder();
159+
$qb->delete(self::TABLE_METADATA)
160+
->where($qb->expr()->in('file_id', $qb->createNamedParameter($chunk, IQueryBuilder::PARAM_INT_ARRAY)))
161+
->hintShardKey('storage', $storage);
162+
$qb->executeStatement();
163+
}
164+
$this->dbConnection->commit();
165+
} catch (Exception $e) {
166+
$this->dbConnection->rollBack();
167+
throw $e;
161168
}
162169
}
163170

‎tests/lib/FilesMetadata/FilesMetadataManagerTest.php‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,4 +94,68 @@ public function testRefreshMetadata(): void {
9494
$this->assertEquals($file->getId(), $retrieved->getFileId());
9595
$this->assertEquals('yes', $retrieved->getString('istest'));
9696
}
97+
98+
public function testDropMetadataForFilesChunking(): void {
99+
$connection = $this->createMock(IDBConnection::class);
100+
$qb = $this->createMock(\OCP\DB\QueryBuilder\IQueryBuilder::class);
101+
$expr = $this->createMock(\OCP\DB\QueryBuilder\IExpressionBuilder::class);
102+
103+
$connection->expects($this->once())->method('beginTransaction');
104+
$connection->expects($this->once())->method('commit');
105+
$connection->expects($this->never())->method('rollBack');
106+
107+
$connection->method('getQueryBuilder')->willReturn($qb);
108+
$qb->method('expr')->willReturn($expr);
109+
$qb->method('delete')->willReturnSelf();
110+
$qb->method('where')->willReturnSelf();
111+
$qb->method('hintShardKey')->willReturnSelf();
112+
113+
$fileIds = range(1, 3000);
114+
115+
$qb->expects($this->exactly(3))
116+
->method('createNamedParameter')
117+
->with($this->callback(function (array $chunk) {
118+
return count($chunk) <= \OCP\DB\QueryBuilder\IQueryBuilder::MAX_IN_PARAMETERS;
119+
}), \OCP\DB\QueryBuilder\IQueryBuilder::PARAM_INT_ARRAY)
120+
->willReturn(':param');
121+
122+
$qb->expects($this->exactly(3))
123+
->method('executeStatement')
124+
->willReturn(1);
125+
126+
$service = new MetadataRequestService($connection, $this->logger);
127+
$service->dropMetadataForFiles(123, $fileIds);
128+
}
129+
130+
public function testDropIndexForFilesChunking(): void {
131+
$connection = $this->createMock(IDBConnection::class);
132+
$qb = $this->createMock(\OCP\DB\QueryBuilder\IQueryBuilder::class);
133+
$expr = $this->createMock(\OCP\DB\QueryBuilder\IExpressionBuilder::class);
134+
135+
$connection->expects($this->once())->method('beginTransaction');
136+
$connection->expects($this->once())->method('commit');
137+
$connection->expects($this->never())->method('rollBack');
138+
139+
$connection->method('getQueryBuilder')->willReturn($qb);
140+
$qb->method('expr')->willReturn($expr);
141+
$qb->method('delete')->willReturnSelf();
142+
$qb->method('where')->willReturnSelf();
143+
144+
$fileIds = range(1, 3000);
145+
146+
$qb->expects($this->exactly(3))
147+
->method('createNamedParameter')
148+
->with($this->callback(function (array $chunk) {
149+
return count($chunk) <= \OCP\DB\QueryBuilder\IQueryBuilder::MAX_IN_PARAMETERS;
150+
}), \OCP\DB\QueryBuilder\IQueryBuilder::PARAM_INT_ARRAY)
151+
->willReturn(':param');
152+
153+
$qb->expects($this->exactly(3))
154+
->method('executeStatement')
155+
->willReturn(1);
156+
157+
$service = new IndexRequestService($connection, $this->logger);
158+
$service->dropIndexForFiles($fileIds);
159+
}
97160
}
161+

0 commit comments

Comments
 (0)