Merge pull request #7665 from nextcloud/fix/delete-old-sessions

Fix: Delete old sessions in cleanup cron

Author: mejo-

lib/Cron/Cleanup.php

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

lib/Db/SessionMapper.php

@@ -142,6 +142,42 @@ public function deleteInactiveWithoutSteps(?int $documentId = null): int {
 		return $deletedCount;
 	}
 
+	public function deleteOldSessions(int $ageInSeconds): int {
+		$startTime = microtime(true);
+		$maxExecutionSeconds = 30;
+		$batchSize = 1000;
+		$deletedCount = 0;
+		$ageThreshold = time() - $ageInSeconds;
+
+		do {
+			$oldSessionsQb = $this->db->getQueryBuilder();
+			$result = $oldSessionsQb->select('id')
+				->from('text_sessions')
+				->where($oldSessionsQb->expr()->lt('last_contact', $oldSessionsQb->createNamedParameter($ageThreshold)))
+				->setMaxResults($batchSize)
+				->executeQuery();
+
+			$sessionIds = array_map(function ($row) {
+				return (int)$row['id'];
+			}, $result->fetchAll());
+			$result->closeCursor();
+
+			if (empty($sessionIds)) {
+				break;
+			}
+
+			$deleteSessionsQb = $this->db->getQueryBuilder();
+			$batchDeleted = $deleteSessionsQb->delete('text_sessions')
+				->where($deleteSessionsQb->expr()->in('id', $deleteSessionsQb->createParameter('ids'), IQueryBuilder::PARAM_INT_ARRAY))
+				->setParameter('ids', $sessionIds, 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

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

tests/unit/Db/SessionMapperTest.php

@@ -98,4 +98,41 @@ public function testDeleteInactiveWithoutStepsMultiple() {
 
 		self::assertCount(0, $this->sessionMapper->findAll(1));
 	}
+
+	public function testDeleteOldSessions() {
+		$this->stepMapper->deleteAll(1);
+		$this->sessionMapper->deleteByDocumentId(1);
+
+		$fourMonthsAgo = time() - (120 * 24 * 60 * 60);
+		$oneWeekAgo = time() - (7 * 24 * 60 * 60);
+
+		// Create old and recent session
+		$oldSession = $this->sessionMapper->insert(Session::fromParams([
+			'userId' => 'admin',
+			'documentId' => 1,
+			'lastContact' => $fourMonthsAgo,
+			'token' => uniqid(),
+			'color' => '00ff00',
+		]));
+		$recentSession = $this->sessionMapper->insert(Session::fromParams([
+			'userId' => 'admin',
+			'documentId' => 1,
+			'lastContact' => $oneWeekAgo,
+			'token' => uniqid(),
+			'color' => 'ff0000',
+		]));
+
+		// Verify 2 sessions
+		self::assertCount(2, $this->sessionMapper->findAll(1));
+
+		// Delete sessions older than 90 days
+		$threeMonths = 90 * 24 * 60 * 60;
+		$deletedCount = $this->sessionMapper->deleteOldSessions($threeMonths);
+		self::assertEquals(1, $deletedCount);
+
+		// Should have 1 recent session remaining
+		$remainingSessions = $this->sessionMapper->findAll(1);
+		self::assertCount(1, $remainingSessions);
+		self::assertEquals($recentSession->getId(), $remainingSessions[0]->getId());
+	}
 }