perf(cleanup): Optimize cleanup job

appinfo/info.xml

@@ -1,6 +1,6 @@
 <?xml version="1.0"?>
 <!--
-  - SPDX-FileCopyrightText: 2019-2024 Nextcloud GmbH and Nextcloud contributors
+  - SPDX-FileCopyrightText: 2019-2026 Nextcloud GmbH and Nextcloud contributors
   - SPDX-License-Identifier: AGPL-3.0-or-later
 -->
 <info xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
@@ -15,7 +15,7 @@
 - **💾 Open format:** Files are saved as [Markdown](https://en.wikipedia.org/wiki/Markdown), so you can edit them from any other text app too.
 - **✊ Strong foundation:** We use [🐈 tiptap](https://tiptap.scrumpy.io) which is based on [🦉 ProseMirror](https://prosemirror.net) – huge thanks to them!
 	]]></description>
-	<version>10.0.0-dev.1</version>
+	<version>10.0.0-dev.2</version>
 	<licence>agpl</licence>
 	<author mail="jus@bitgrid.net">Julius Härtl</author>
 	<namespace>Text</namespace>

composer/composer/autoload_classmap.php

@@ -83,6 +83,7 @@
     'OCA\\Text\\Migration\\Version090000Date20260817110024' => $baseDir . '/../lib/Migration/Version090000Date20260817110024.php',
     'OCA\\Text\\Migration\\Version090000Date20260819110024' => $baseDir . '/../lib/Migration/Version090000Date20260819110024.php',
     'OCA\\Text\\Migration\\Version090000Date20260820132113' => $baseDir . '/../lib/Migration/Version090000Date20260820132113.php',
+    'OCA\\Text\\Migration\\Version100000Date20261011000000' => $baseDir . '/../lib/Migration/Version100000Date20261011000000.php',
     'OCA\\Text\\Notification\\Notifier' => $baseDir . '/../lib/Notification/Notifier.php',
     'OCA\\Text\\Service\\AiTagService' => $baseDir . '/../lib/Service/AiTagService.php',
     'OCA\\Text\\Service\\ApiService' => $baseDir . '/../lib/Service/ApiService.php',

composer/composer/autoload_static.php

@@ -98,6 +98,7 @@ class ComposerStaticInitText
         'OCA\\Text\\Migration\\Version090000Date20260817110024' => __DIR__ . '/..' . '/../lib/Migration/Version090000Date20260817110024.php',
         'OCA\\Text\\Migration\\Version090000Date20260819110024' => __DIR__ . '/..' . '/../lib/Migration/Version090000Date20260819110024.php',
         'OCA\\Text\\Migration\\Version090000Date20260820132113' => __DIR__ . '/..' . '/../lib/Migration/Version090000Date20260820132113.php',
+        'OCA\\Text\\Migration\\Version100000Date20261011000000' => __DIR__ . '/..' . '/../lib/Migration/Version100000Date20261011000000.php',
         'OCA\\Text\\Notification\\Notifier' => __DIR__ . '/..' . '/../lib/Notification/Notifier.php',
         'OCA\\Text\\Service\\AiTagService' => __DIR__ . '/..' . '/../lib/Service/AiTagService.php',
         'OCA\\Text\\Service\\ApiService' => __DIR__ . '/..' . '/../lib/Service/ApiService.php',

lib/Cron/Cleanup.php

@@ -35,12 +35,14 @@ public function __construct(
 	 */
 	protected function run($argument): void {
 		$this->logger->debug('Run cleanup job for text documents');
-		foreach ($this->documentService->getAllWithNoActiveSession() as $document) {
-			if ($document->getContextType() !== 'file') {
+		foreach ($this->documentService->getAllPendingAttachmentCleanup() as $document) {
+			if ($this->documentService->hasUnsavedChanges($document)) {
+				$this->documentService->markAttachmentCleanup($document);
 				continue;
 			}
 			$this->setupManager->tearDown();
 			$this->attachmentService->cleanupAttachments($document->getContextId());
+			$this->documentService->markAttachmentCleanup($document);
 		}
 
 		$this->logger->debug('Run cleanup job for text sessions');

lib/Db/Document.php

@@ -42,6 +42,7 @@ class Document extends SnowflakeAwareEntity implements \JsonSerializable {
 	protected int $lastSavedVersion = 0;
 	protected int $initialVersion = 0;
 	protected int $lastSavedVersionTime = 0;
+	protected int $lastAttachmentCleanup = 0;
 	protected string $lastSavedVersionEtag = '';
 	protected string $baseVersionEtag = '';
 	protected ?string $checksum = null;
@@ -53,12 +54,21 @@ public function __construct() {
 		$this->addType('currentVersion', Types::INTEGER);
 		$this->addType('lastSavedVersion', Types::INTEGER);
 		$this->addType('lastSavedVersionTime', Types::INTEGER);
+		$this->addType('lastAttachmentCleanup', Types::BIGINT);
 		$this->addType('initialVersion', Types::INTEGER);
 		$this->addType('checksum', Types::STRING);
 		$this->addType('contextType', Types::STRING);
 		$this->addType('contextId', Types::INTEGER);
 	}
 
+	public function getLastAttachmentCleanup(): int {
+		return $this->lastAttachmentCleanup;
+	}
+
+	public function setLastAttachmentCleanup(int $time): void {
+		$this->setter('lastAttachmentCleanup', [$time]);
+	}
+
 	public function jsonSerialize(): array {
 		return [
 			'id' => $this->id,

lib/Db/DocumentMapper.php

@@ -72,12 +72,14 @@ public function findAll(): Generator {
 		}
 	}
 
-	public function findAllWithNoActiveSessions(): Generator {
+	public function findAllPendingAttachmentCleanup(): Generator {
 		$qb = $this->db->getQueryBuilder();
 		$result = $qb->select('d.*')
 			->from($this->getTableName(), 'd')
 			->leftJoin('d', 'text_sessions', 's', $qb->expr()->eq('s.document_id', 'd.id'))
 			->where($qb->expr()->isNull('s.id'))
+			->andWhere($qb->expr()->gt('d.last_saved_version_time', 'd.last_attachment_cleanup'))
+			->andWhere($qb->expr()->eq('d.context_type', $qb->createNamedParameter('file')))
 			->executeQuery();
 		try {
 			while ($row = $result->fetchAssociative()) {

lib/Migration/Version100000Date20261011000000.php

@@ -0,0 +1,36 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCA\Text\Migration;
+
+use Closure;
+use OCP\DB\ISchemaWrapper;
+use OCP\DB\Types;
+use OCP\Migration\Attributes\AddColumn;
+use OCP\Migration\Attributes\ColumnType;
+use OCP\Migration\IOutput;
+use OCP\Migration\SimpleMigrationStep;
+
+#[AddColumn(table: 'text_documents', name: 'last_attachment_cleanup', type: ColumnType::BIGINT, description: 'Last saved timestamp processed by attachment cleanup')]
+class Version100000Date20261011000000 extends SimpleMigrationStep {
+	public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
+		$schema = $schemaClosure();
+		$table = $schema->getTable('text_documents');
+		if (!$table->hasColumn('last_attachment_cleanup')) {
+			$table->addColumn('last_attachment_cleanup', Types::BIGINT, [
+				'notnull' => true,
+				'default' => 0,
+				'unsigned' => true,
+			]);
+			return $schema;
+		}
+
+		return null;
+	}
+}

lib/Service/DocumentService.php

@@ -462,9 +462,7 @@ public function resetDocument(string $contextType, int $contextId, bool $force =
 			return;
 		}
 
-		$stepsVersion = $this->stepMapper->getLatestVersion($document->id) ?: 0;
-		$docVersion = $document->getLastSavedVersion();
-		if (!$force && $stepsVersion !== $docVersion) {
+		if (!$force && $this->hasUnsavedChanges($document)) {
 			$this->logger->debug('Did not reset document with unsaved changes for ' . $contextString);
 			throw new DocumentHasUnsavedChangesException('Did not reset document, as it has unsaved changes');
 		}
@@ -486,8 +484,20 @@ public function getAll(): \Generator {
 		return $this->documentMapper->findAll();
 	}
 
-	public function getAllWithNoActiveSession(): \Generator {
-		return $this->documentMapper->findAllWithNoActiveSessions();
+	public function getAllPendingAttachmentCleanup(): \Generator {
+		return $this->documentMapper->findAllPendingAttachmentCleanup();
+	}
+
+	public function markAttachmentCleanup(Document $document): void {
+		$document->setLastAttachmentCleanup($document->getLastSavedVersionTime());
+		$this->documentMapper->update($document);
+	}
+
+	public function hasUnsavedChanges(Document $document): bool {
+		if ($document->id === null) {
+			throw new RuntimeException('Document needs an id to check unsaved changes');
+		}
+		return ($this->stepMapper->getLatestVersion($document->id) ?? 0) !== $document->getLastSavedVersion();
 	}
 
 	private function ensureDocumentsFolder(): bool {

tests/unit/CleanupTest.php

@@ -0,0 +1,118 @@
+<?php
+
+/**
+ * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCA\Text\Tests;
+
+use OCA\Text\Cron\Cleanup;
+use OCA\Text\Db\Document;
+use OCA\Text\Service\AttachmentService;
+use OCA\Text\Service\DocumentService;
+use OCA\Text\Service\FileService;
+use OCA\Text\Service\SessionService;
+use OCP\AppFramework\Utility\ITimeFactory;
+use OCP\Files\IFilenameValidator;
+use OCP\Files\IMimeTypeDetector;
+use OCP\Files\IRootFolder;
+use OCP\Files\ISetupManager;
+use OCP\Files\Node;
+use OCP\FilesMetadata\IFilesMetadataManager;
+use OCP\IPreview;
+use OCP\ISession;
+use OCP\IURLGenerator;
+use OCP\Share\IManager;
+use Psr\Log\LoggerInterface;
+
+class CleanupTest extends \PHPUnit\Framework\TestCase {
+	private Cleanup $cleanup;
+	private Document $document;
+	private DocumentService $documentService;
+	private AttachmentService $attachmentService;
+	private SessionService $sessionService;
+	private IRootFolder $rootFolder;
+	private ISetupManager $setupManager;
+
+	public function setUp(): void {
+		$this->document = Document::fromRow([
+			'id' => '123456789012345678',
+			'context_type' => 'file',
+			'context_id' => 123,
+			'last_saved_version_time' => 1000,
+		]);
+		$this->documentService = $this->createMock(DocumentService::class);
+		$this->rootFolder = $this->createMock(IRootFolder::class);
+		$this->attachmentService = new AttachmentService(
+			$this->rootFolder,
+			$this->createMock(IManager::class),
+			$this->createMock(IPreview::class),
+			$this->createMock(IMimeTypeDetector::class),
+			$this->createMock(IURLGenerator::class),
+			$this->createMock(IFilenameValidator::class),
+			$this->createMock(IFilesMetadataManager::class),
+			$this->createMock(ISession::class),
+			$this->createMock(FileService::class),
+		);
+		$this->sessionService = $this->createMock(SessionService::class);
+		$this->setupManager = $this->createMock(ISetupManager::class);
+		$this->documentService->expects(self::never())->method('resetDocument');
+		$this->documentService->expects(self::once())
+			->method('getAllPendingAttachmentCleanup')
+			->willReturn((function (): \Generator {
+				yield $this->document;
+			})());
+		$this->cleanup = new class($this->createMock(ITimeFactory::class), $this->sessionService, $this->documentService, $this->attachmentService, $this->createMock(LoggerInterface::class), $this->setupManager, ) extends Cleanup {
+			public function run($argument): void {
+				parent::run($argument);
+			}
+		};
+	}
+
+	public function testMarksCleanupOnlyAfterAttachmentsAreCleaned(): void {
+		$this->documentService->expects(self::once())
+			->method('hasUnsavedChanges')->with($this->document)->willReturn(false);
+		$this->setupManager->expects(self::once())->method('tearDown');
+		$cleaned = false;
+		$this->rootFolder->expects(self::once())
+			->method('getFirstNodeById')->with(123)
+			->willReturnCallback(function () use (&$cleaned): ?Node {
+				$cleaned = true;
+				return null;
+			});
+		$this->documentService->expects(self::once())
+			->method('markAttachmentCleanup')->with($this->document)
+			->willReturnCallback(function () use (&$cleaned): void {
+				self::assertTrue($cleaned);
+			});
+		$this->sessionService->expects(self::once())->method('removeInactiveSessionsWithoutSteps');
+		$this->sessionService->expects(self::once())->method('removeOldSessions');
+		$this->sessionService->expects(self::once())->method('removeOrphanedSteps');
+		$this->documentService->expects(self::once())->method('cleanupOldDocumentsFolders');
+
+		$this->cleanup->run([]);
+	}
+
+	public function testMarksUnsavedChangesWithoutCleanup(): void {
+		$this->documentService->expects(self::once())
+			->method('hasUnsavedChanges')->with($this->document)->willReturn(true);
+		$this->setupManager->expects(self::never())->method('tearDown');
+		$this->rootFolder->expects(self::never())->method('getFirstNodeById');
+		$this->documentService->expects(self::once())->method('markAttachmentCleanup')->with($this->document);
+
+		$this->cleanup->run([]);
+	}
+
+	public function testFailedCleanupRemainsPending(): void {
+		$this->documentService->method('hasUnsavedChanges')->willReturn(false);
+		$this->rootFolder->expects(self::once())
+			->method('getFirstNodeById')->with(123)
+			->willThrowException(new \RuntimeException('Cleanup failed'));
+		$this->documentService->expects(self::never())->method('markAttachmentCleanup');
+		$this->expectException(\RuntimeException::class);
+		$this->expectExceptionMessage('Cleanup failed');
+
+		$this->cleanup->run([]);
+	}
+}

tests/unit/Db/DocumentMapperTest.php

@@ -0,0 +1,79 @@
+<?php
+
+/**
+ * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCA\Text\Db;
+
+use OCP\Server;
+
+/**
+ * @group DB
+ */
+class DocumentMapperTest extends \Test\TestCase {
+	private DocumentMapper $documentMapper;
+	private SessionMapper $sessionMapper;
+	private Document $document;
+
+	public function setUp(): void {
+		parent::setUp();
+		$this->documentMapper = Server::get(DocumentMapper::class);
+		$this->sessionMapper = Server::get(SessionMapper::class);
+		$this->document = $this->documentMapper->insert(Document::fromParams([
+			'contextType' => 'file',
+			'contextId' => 900001,
+			'lastSavedVersionTime' => 1000,
+			'lastSavedVersionEtag' => 'saved-etag',
+			'baseVersionEtag' => 'base-etag',
+		]));
+	}
+
+	public function tearDown(): void {
+		$this->sessionMapper->deleteByDocumentId($this->document->getId());
+		$this->documentMapper->delete($this->document);
+		parent::tearDown();
+	}
+
+	private function pendingDocumentIds(): array {
+		return array_map(
+			static fn (Document $document): string => $document->getId(),
+			iterator_to_array($this->documentMapper->findAllPendingAttachmentCleanup()),
+		);
+	}
+
+	public function testUnprocessedDocumentsArePending(): void {
+		self::assertSame(0, $this->documentMapper->find($this->document->getId())->getLastAttachmentCleanup());
+		self::assertContains($this->document->getId(), $this->pendingDocumentIds());
+	}
+
+	public function testDocumentsWithSessionsAreExcluded(): void {
+		$session = $this->sessionMapper->insert(Session::fromParams([
+			'documentId' => $this->document->getId(),
+			'userId' => 'admin',
+			'token' => uniqid(),
+			'lastContact' => time(),
+		]));
+		self::assertNotContains($this->document->getId(), $this->pendingDocumentIds());
+		$this->sessionMapper->delete($session);
+		self::assertContains($this->document->getId(), $this->pendingDocumentIds());
+	}
+
+	public function testCleanedDocumentsAreExcludedUntilSavedAgain(): void {
+		$this->document->setLastAttachmentCleanup(1000);
+		$this->documentMapper->update($this->document);
+		self::assertNotContains($this->document->getId(), $this->pendingDocumentIds());
+
+		$this->document->setLastSavedVersionTime(1001);
+		$this->documentMapper->update($this->document);
+		self::assertContains($this->document->getId(), $this->pendingDocumentIds());
+	}
+
+	public function testOlderSavedTimestampsAreExcluded(): void {
+		$this->document->setLastAttachmentCleanup(1001);
+		$this->documentMapper->update($this->document);
+
+		self::assertNotContains($this->document->getId(), $this->pendingDocumentIds());
+	}
+}