fixup! fix(dav): move orphan cleaning logic to a chunked background jobfix/dav/orphan-cleanup-job

2 files changed, 36 insertions, 27 deletions

diff --git a/apps/dav/lib/BackgroundJob/CleanupOrphanedChildrenJob.php b/apps/dav/lib/BackgroundJob/CleanupOrphanedChildrenJob.php
index a51435ce580..8a5e34381a7 100644
--- a/apps/dav/lib/BackgroundJob/CleanupOrphanedChildrenJob.php
+++ b/apps/dav/lib/BackgroundJob/CleanupOrphanedChildrenJob.php
@@ -74,14 +74,15 @@ class CleanupOrphanedChildrenJob extends QueuedJob {
 		$result = $selectQb->executeQuery();
 		$rows = $result->fetchAll();
 		$result->closeCursor();
+		if (empty($rows)) {
+			return 0;
+		}
 
 		$orphanItems = array_map(static fn ($row) => $row['id'], $rows);
-		if (!empty($orphanItems)) {
-			$deleteQb = $this->connection->getQueryBuilder();
-			$deleteQb->delete($childTable)
-				->where($deleteQb->expr()->in('id', $deleteQb->createNamedParameter($orphanItems, IQueryBuilder::PARAM_INT_ARRAY)));
-			$deleteQb->executeStatement();
-		}
+		$deleteQb = $this->connection->getQueryBuilder();
+		$deleteQb->delete($childTable)
+			->where($deleteQb->expr()->in('id', $deleteQb->createNamedParameter($orphanItems, IQueryBuilder::PARAM_INT_ARRAY)));
+		$deleteQb->executeStatement();
 
 		return count($orphanItems);
 	}
diff --git a/apps/dav/tests/unit/BackgroundJob/CleanupOrphanedChildrenJobTest.php b/apps/dav/tests/unit/BackgroundJob/CleanupOrphanedChildrenJobTest.php
index 4e7d4a7d3b2..fe05616d609 100644
--- a/apps/dav/tests/unit/BackgroundJob/CleanupOrphanedChildrenJobTest.php
+++ b/apps/dav/tests/unit/BackgroundJob/CleanupOrphanedChildrenJobTest.php
@@ -77,13 +77,13 @@ class CleanupOrphanedChildrenJobTest extends TestCase {
 
 	public function testRunWithoutOrphans(): void {
 		$argument = $this->getArgument();
-		$qb = $this->getMockQueryBuilder();
+		$selectQb = $this->getMockQueryBuilder();
 		$result = $this->createMock(IResult::class);
 
 		$this->connection->expects(self::once())
 			->method('getQueryBuilder')
-			->willReturn($qb);
-		$qb->expects(self::once())
+			->willReturn($selectQb);
+		$selectQb->expects(self::once())
 			->method('executeQuery')
 			->willReturn($result);
 		$result->expects(self::once())
@@ -91,10 +91,6 @@ class CleanupOrphanedChildrenJobTest extends TestCase {
 			->willReturn([]);
 		$result->expects(self::once())
 			->method('closeCursor');
-		$qb->expects(self::never())
-			->method('delete');
-		$qb->expects(self::never())
-			->method('executeStatement');
 		$this->jobList->expects(self::never())
 			->method('add');
 
@@ -102,15 +98,21 @@ class CleanupOrphanedChildrenJobTest extends TestCase {
 	}
 
 	public function testRunWithPartialBatch(): void {
-		$batch = 0;
 		$argument = $this->getArgument();
-		$qb = $this->getMockQueryBuilder();
+		$selectQb = $this->getMockQueryBuilder();
+		$deleteQb = $this->getMockQueryBuilder();
 		$result = $this->createMock(IResult::class);
 
-		$this->connection->expects(self::once())
+		$qbInvocationCount = self::exactly(2);
+		$this->connection->expects($qbInvocationCount)
 			->method('getQueryBuilder')
-			->willReturn($qb);
-		$qb->expects(self::once())
+			->willReturnCallback(function () use ($qbInvocationCount, $selectQb, $deleteQb) {
+				return match ($qbInvocationCount->getInvocationCount()) {
+					1 => $selectQb,
+					2 => $deleteQb,
+				};
+			});
+		$selectQb->expects(self::once())
 			->method('executeQuery')
 			->willReturn($result);
 		$result->expects(self::once())
@@ -121,10 +123,10 @@ class CleanupOrphanedChildrenJobTest extends TestCase {
 			]);
 		$result->expects(self::once())
 			->method('closeCursor');
-		$qb->expects(self::once())
+		$deleteQb->expects(self::once())
 			->method('delete')
 			->willReturnSelf();
-		$qb->expects(self::once())
+		$deleteQb->expects(self::once())
 			->method('executeStatement');
 		$this->jobList->expects(self::never())
 			->method('add');
@@ -133,15 +135,21 @@ class CleanupOrphanedChildrenJobTest extends TestCase {
 	}
 
 	public function testRunWithFullBatch(): void {
-		$batch = 0;
 		$argument = $this->getArgument();
-		$qb = $this->getMockQueryBuilder();
+		$selectQb = $this->getMockQueryBuilder();
+		$deleteQb = $this->getMockQueryBuilder();
 		$result = $this->createMock(IResult::class);
 
-		$this->connection->expects(self::once())
+		$qbInvocationCount = self::exactly(2);
+		$this->connection->expects($qbInvocationCount)
 			->method('getQueryBuilder')
-			->willReturn($qb);
-		$qb->expects(self::once())
+			->willReturnCallback(function () use ($qbInvocationCount, $selectQb, $deleteQb) {
+				return match ($qbInvocationCount->getInvocationCount()) {
+					1 => $selectQb,
+					2 => $deleteQb,
+				};
+			});
+		$selectQb->expects(self::once())
 			->method('executeQuery')
 			->willReturn($result);
 		$result->expects(self::once())
@@ -149,10 +157,10 @@ class CleanupOrphanedChildrenJobTest extends TestCase {
 			->willReturn(array_map(static fn ($i) => ['id' => 42 + $i], range(0, 999)));
 		$result->expects(self::once())
 			->method('closeCursor');
-		$qb->expects(self::once())
+		$deleteQb->expects(self::once())
 			->method('delete')
 			->willReturnSelf();
-		$qb->expects(self::once())
+		$deleteQb->expects(self::once())
 			->method('executeStatement');
 		$this->jobList->expects(self::once())
 			->method('add')