Merge pull request #7653 from nextcloud/fix/steps-cleanup-orphaned

Fix: Add cleanup for orphaned text steps

Author: mejo-

lib/Cron/Cleanup.php

@@ -58,6 +58,10 @@ protected function run($argument): void {
 		$removedOldSessions = $this->sessionService->removeOldSessions();
 		$this->logger->debug('Removed ' . $removedOldSessions . ' old sessions');
 
+		$this->logger->debug('Run cleanup job for orphaned steps');
+		$removedSteps = $this->sessionService->removeOrphanedSteps();
+		$this->logger->debug('Removed ' . $removedSteps . ' orphaned steps');
+
 		$this->logger->debug('Run cleanup job for obsolete documents folders');
 		$this->documentService->cleanupOldDocumentsFolders();
 	}

lib/Db/SessionMapper.php

@@ -178,6 +178,51 @@ public function deleteOldSessions(int $ageInSeconds): int {
 		return $deletedCount;
 	}
 
+	public function deleteOrphanedSteps(int $ageInSeconds): int {
+		$startTime = microtime(true);
+		$maxExecutionSeconds = 30;
+		$batchSize = 1000;
+		$deletedCount = 0;
+		$ageThreshold = time() - $ageInSeconds;
+
+		do {
+			$orphanedStepsQb = $this->db->getQueryBuilder();
+			$orphanedStepsQb->select('st.id')
+				->from('text_steps', 'st')
+				->leftJoin('st', 'text_sessions', 's', $orphanedStepsQb->expr()->eq('st.document_id', 's.document_id'))
+				->leftJoin('st', 'text_documents', 'd', $orphanedStepsQb->expr()->eq('st.document_id', 'd.id'))
+				->where($orphanedStepsQb->expr()->isNull('s.id'))
+				->andWhere($orphanedStepsQb->expr()->lt('st.timestamp', $orphanedStepsQb->createNamedParameter($ageThreshold)))
+				->andWhere(
+					$orphanedStepsQb->expr()->orX(
+						$orphanedStepsQb->expr()->isNull('d.id'),
+						$orphanedStepsQb->expr()->lt('st.id', 'd.last_saved_version')
+					)
+				)
+				->setMaxResults($batchSize);
+
+			$result = $orphanedStepsQb->executeQuery();
+			$stepIds = array_map(function ($row) {
+				return (int)$row['id'];
+			}, $result->fetchAll());
+			$result->closeCursor();
+
+			if (empty($stepIds)) {
+				break;
+			}
+
+			$deleteQb = $this->db->getQueryBuilder();
+			$batchDeleted = $deleteQb->delete('text_steps')
+				->where($deleteQb->expr()->in('id', $deleteQb->createParameter('ids'), IQueryBuilder::PARAM_INT_ARRAY))
+				->setParameter('ids', $stepIds, IQueryBuilder::PARAM_INT_ARRAY)
+				->executeStatement();
+
+			$deletedCount += $batchDeleted;
+		} while ((microtime(true) - $startTime) < $maxExecutionSeconds);
+
+		return $deletedCount;
+	}
+
 	public function deleteByDocumentId(int $documentId): int {
 		$qb = $this->db->getQueryBuilder();
 		$qb->delete($this->getTableName())

lib/Service/SessionService.php

@@ -142,6 +142,10 @@ public function removeOldSessions(int $ageInSeconds = 7776000): int {
 		return $this->sessionMapper->deleteOldSessions($ageInSeconds);
 	}
 
+	public function removeOrphanedSteps(int $ageInSeconds = 604800): int {
+		return $this->sessionMapper->deleteOrphanedSteps($ageInSeconds);
+	}
+
 	public function getSession(int $documentId, int $sessionId, string $token): ?Session {
 		if ($this->session !== null) {
 			return $this->session;

tests/unit/Db/SessionMapperTest.php

@@ -9,12 +9,13 @@ class SessionMapperTest extends \Test\TestCase {
 
 	private SessionMapper $sessionMapper;
 	private StepMapper $stepMapper;
+	private DocumentMapper $documentMapper;
 
 	public function setUp(): void {
 		parent::setUp();
 		$this->sessionMapper = \OCP\Server::get(SessionMapper::class);
 		$this->stepMapper = \OCP\Server::get(StepMapper::class);
-
+		$this->documentMapper = \OCP\Server::get(DocumentMapper::class);
 	}
 
 	public function testDeleteInactiveWithoutSteps() {
@@ -135,4 +136,66 @@ public function testDeleteOldSessions() {
 		self::assertCount(1, $remainingSessions);
 		self::assertEquals($recentSession->getId(), $remainingSessions[0]->getId());
 	}
+
+	public function testDeleteOrphanedSteps() {
+		$this->documentMapper->clearAll();
+		$this->sessionMapper->clearAll();
+		$this->stepMapper->clearAll();
+
+		$eightDaysAgo = time() - (8 * 24 * 60 * 60);
+
+		// Create document
+		$document = $this->documentMapper->insert(Document::fromParams([
+			'id' => 1,
+			'currentVersion' => 0,
+			'lastSavedVersion' => 100,
+			'lastSavedVersionTime' => time()
+		]));
+
+		// Create Orphaned step without document (delete)
+		$this->stepMapper->insert(Step::fromParams([
+			'sessionId' => 99999,
+			'documentId' => 99999,
+			'data' => 'ORPHANED_NO_DOC',
+			'version' => 1
+		]));
+
+		// Orphaned "old" step with document and old version (delete)
+		$this->stepMapper->insert(Step::fromParams([
+			'id' => 1,
+			'sessionId' => 99999,
+			'documentId' => $document->getId(),
+			'data' => 'ORPHANED_OLD_VERSION',
+			'timestamp' => $eightDaysAgo,
+			'version' => 1
+		]));
+
+		// Orphaned "new" step with document and current version (keep)
+		$this->stepMapper->insert(Step::fromParams([
+			'id' => 100,
+			'sessionId' => 99999,
+			'documentId' => $document->getId(),
+			'data' => 'ORPHANED_CURRENT_VERSION',
+			'version' => 2
+		]));
+
+		// Orphaned step with document and new version (keep)
+		$this->stepMapper->insert(Step::fromParams([
+			'id' => 101,
+			'sessionId' => 99999,
+			'documentId' => $document->getId(),
+			'data' => 'ORPHANED_NEW_VERSION',
+			'timestamp' => $eightDaysAgo,
+			'version' => 3
+		]));
+
+		// Verify steps for document 1 and 99999
+		self::assertCount(3, $this->stepMapper->find(1, 0));
+		self::assertCount(1, $this->stepMapper->find(99999, 0));
+
+		// Delete orphaned steps older than 7 days
+		$sevenDays = 7 * 24 * 60 * 60;
+		$deletedCount = $this->sessionMapper->deleteOrphanedSteps($sevenDays);
+		self::assertEquals(2, $deletedCount);
+	}
 }