From 48551eb8645a971d469293dcfc51791f4e9a8d7e Mon Sep 17 00:00:00 2001 From: Edmond Date: Sun, 19 Jul 2026 14:01:59 -0400 Subject: [PATCH] fix(core): handle parameter limits in files metadata deletion Refactor dropMetadataForFiles and dropIndexForFiles to chunk by 500 and wrap in database transactions to avoid query parameter limit errors on Oracle and PostgreSQL. Assisted-by: Antigravity:Gemini-3.5-Flash Signed-off-by: Edmond --- .../Service/IndexRequestService.php | 27 +++++--- .../Service/MetadataRequestService.php | 21 ++++-- .../FilesMetadataManagerTest.php | 65 +++++++++++++++++++ 3 files changed, 96 insertions(+), 17 deletions(-) diff --git a/lib/private/FilesMetadata/Service/IndexRequestService.php b/lib/private/FilesMetadata/Service/IndexRequestService.php index bbf6185007175..9418832b35041 100644 --- a/lib/private/FilesMetadata/Service/IndexRequestService.php +++ b/lib/private/FilesMetadata/Service/IndexRequestService.php @@ -186,19 +186,26 @@ public function dropIndex(int $fileId, string $key = ''): void { * @throws DbException */ public function dropIndexForFiles(array $fileIds, string $key = ''): void { - $chunks = array_chunk($fileIds, IQueryBuilder::MAX_IN_PARAMETERS); + $chunks = array_chunk($fileIds, 500); - foreach ($chunks as $chunk) { - $qb = $this->dbConnection->getQueryBuilder(); - $expr = $qb->expr(); - $qb->delete(self::TABLE_METADATA_INDEX) - ->where($expr->in('file_id', $qb->createNamedParameter($fileIds, IQueryBuilder::PARAM_INT_ARRAY))); + $this->dbConnection->beginTransaction(); + try { + foreach ($chunks as $chunk) { + $qb = $this->dbConnection->getQueryBuilder(); + $expr = $qb->expr(); + $qb->delete(self::TABLE_METADATA_INDEX) + ->where($expr->in('file_id', $qb->createNamedParameter($chunk, IQueryBuilder::PARAM_INT_ARRAY))); - if ($key !== '') { - $qb->andWhere($expr->eq('meta_key', $qb->createNamedParameter($key))); - } + if ($key !== '') { + $qb->andWhere($expr->eq('meta_key', $qb->createNamedParameter($key))); + } - $qb->executeStatement(); + $qb->executeStatement(); + } + $this->dbConnection->commit(); + } catch (DbException $e) { + $this->dbConnection->rollBack(); + throw $e; } } } diff --git a/lib/private/FilesMetadata/Service/MetadataRequestService.php b/lib/private/FilesMetadata/Service/MetadataRequestService.php index 83d65a992df6b..3e0073373335b 100644 --- a/lib/private/FilesMetadata/Service/MetadataRequestService.php +++ b/lib/private/FilesMetadata/Service/MetadataRequestService.php @@ -150,14 +150,21 @@ public function dropMetadata(int $fileId): void { * @throws Exception */ public function dropMetadataForFiles(int $storage, array $fileIds): void { - $chunks = array_chunk($fileIds, IQueryBuilder::MAX_IN_PARAMETERS); + $chunks = array_chunk($fileIds, 500); - foreach ($chunks as $chunk) { - $qb = $this->dbConnection->getQueryBuilder(); - $qb->delete(self::TABLE_METADATA) - ->where($qb->expr()->in('file_id', $qb->createNamedParameter($fileIds, IQueryBuilder::PARAM_INT_ARRAY))) - ->hintShardKey('storage', $storage); - $qb->executeStatement(); + $this->dbConnection->beginTransaction(); + try { + foreach ($chunks as $chunk) { + $qb = $this->dbConnection->getQueryBuilder(); + $qb->delete(self::TABLE_METADATA) + ->where($qb->expr()->in('file_id', $qb->createNamedParameter($chunk, IQueryBuilder::PARAM_INT_ARRAY))) + ->hintShardKey('storage', $storage); + $qb->executeStatement(); + } + $this->dbConnection->commit(); + } catch (Exception $e) { + $this->dbConnection->rollBack(); + throw $e; } } diff --git a/tests/lib/FilesMetadata/FilesMetadataManagerTest.php b/tests/lib/FilesMetadata/FilesMetadataManagerTest.php index a349065f32f05..446e5fe13d201 100644 --- a/tests/lib/FilesMetadata/FilesMetadataManagerTest.php +++ b/tests/lib/FilesMetadata/FilesMetadataManagerTest.php @@ -94,4 +94,69 @@ public function testRefreshMetadata(): void { $this->assertEquals($file->getId(), $retrieved->getFileId()); $this->assertEquals('yes', $retrieved->getString('istest')); } + + public function testDropMetadataForFilesChunking(): void { + $connection = $this->createMock(IDBConnection::class); + $qb = $this->createMock(\OCP\DB\QueryBuilder\IQueryBuilder::class); + $expr = $this->createMock(\OCP\DB\QueryBuilder\IExpressionBuilder::class); + + $connection->expects($this->once())->method('beginTransaction'); + $connection->expects($this->once())->method('commit'); + $connection->expects($this->never())->method('rollBack'); + + $connection->method('getQueryBuilder')->willReturn($qb); + $qb->method('expr')->willReturn($expr); + $qb->method('delete')->willReturnSelf(); + $qb->method('where')->willReturnSelf(); + $qb->method('hintShardKey')->willReturnSelf(); + + // We chunk 2000 items into 500. So we expect 4 queries. + $fileIds = range(1, 2000); + + $qb->expects($this->exactly(4)) + ->method('createNamedParameter') + ->with($this->callback(function (array $chunk) { + return count($chunk) === 500; + }), \OCP\DB\QueryBuilder\IQueryBuilder::PARAM_INT_ARRAY) + ->willReturn(':param'); + + $qb->expects($this->exactly(4)) + ->method('executeStatement') + ->willReturn(1); + + $service = new MetadataRequestService($connection, $this->logger); + $service->dropMetadataForFiles(123, $fileIds); + } + + public function testDropIndexForFilesChunking(): void { + $connection = $this->createMock(IDBConnection::class); + $qb = $this->createMock(\OCP\DB\QueryBuilder\IQueryBuilder::class); + $expr = $this->createMock(\OCP\DB\QueryBuilder\IExpressionBuilder::class); + + $connection->expects($this->once())->method('beginTransaction'); + $connection->expects($this->once())->method('commit'); + $connection->expects($this->never())->method('rollBack'); + + $connection->method('getQueryBuilder')->willReturn($qb); + $qb->method('expr')->willReturn($expr); + $qb->method('delete')->willReturnSelf(); + $qb->method('where')->willReturnSelf(); + + $fileIds = range(1, 2000); + + $qb->expects($this->exactly(4)) + ->method('createNamedParameter') + ->with($this->callback(function (array $chunk) { + return count($chunk) === 500; + }), \OCP\DB\QueryBuilder\IQueryBuilder::PARAM_INT_ARRAY) + ->willReturn(':param'); + + $qb->expects($this->exactly(4)) + ->method('executeStatement') + ->willReturn(1); + + $service = new IndexRequestService($connection, $this->logger); + $service->dropIndexForFiles($fileIds); + } } +