From a909ba5b4e5d62b85053c93c62ee91c77f45e527 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Mon, 26 Oct 2026 09:46:11 +0100 Subject: [PATCH] fix(context_chat): Reschedule indexing upon triggerInitialImport call triggerInitialImport() was a no-op, so wiping the context chat vector store left all previously indexed mails out of the index for good. ScheduleJob also only reset mailboxes that still had a pending task, which skipped exactly the fully imported ones. triggerInitialImport() now re-queues ScheduleJob for every account. ScheduleJob backfills each synchronized mailbox from the start and then removes itself; it waits for the initial sync because that sync does not emit NewMessagesSynchronized. The FixBackgroundJobs repair step re-adds ScheduleJob on every app upgrade, causing one re-scan per upgrade. Context chat skips re-embedding unchanged items, so this was accepted over adding a "done" column to mail_cc_tasks. Assisted-by: ClaudeCode:claude-opus-5.5 Signed-off-by: Marcel Klehr --- lib/BackgroundJob/ContextChat/ScheduleJob.php | 17 +- lib/ContextChat/ContextChatProvider.php | 4 + .../ContextChat/ScheduleJobTest.php | 158 ++++++++++++++++++ .../ContextChat/ContextChatProviderTest.php | 27 +++ 4 files changed, 202 insertions(+), 4 deletions(-) create mode 100644 tests/Unit/BackgroundJob/ContextChat/ScheduleJobTest.php diff --git a/lib/BackgroundJob/ContextChat/ScheduleJob.php b/lib/BackgroundJob/ContextChat/ScheduleJob.php index 8648b89b14..b10b7a94db 100644 --- a/lib/BackgroundJob/ContextChat/ScheduleJob.php +++ b/lib/BackgroundJob/ContextChat/ScheduleJob.php @@ -9,6 +9,7 @@ namespace OCA\Mail\BackgroundJob\ContextChat; +use OCA\Mail\Db\Mailbox; use OCA\Mail\Exception\ServiceException; use OCA\Mail\Service\AccountService; use OCA\Mail\Service\ContextChat\ContextChatSettingsService; @@ -69,13 +70,21 @@ protected function run($argument): void { return; } - foreach ($mailboxes as $mailbox) { + // Messages fetched by the initial sync don't emit NewMessagesSynchronized, so wait until it ran + $cachedMailboxes = array_filter($mailboxes, static fn (Mailbox $mailbox): bool => $mailbox->isCached()); + if ($cachedMailboxes === []) { + $this->logger->debug("No synchronized mailboxes for account <{$accountId}> yet, postponing context chat import"); + return; + } + + foreach ($cachedMailboxes as $mailbox) { try { - $this->taskService->findByMailboxId($mailbox->getId()); $this->taskService->updateOrCreate($mailbox->getId(), 0); - } catch (DoesNotExistException|MultipleObjectsReturnedException|Exception $e) { - $this->logger->warning("Could not schedule context chat indexing tasks for mailbox <{$mailbox->getId()}>"); + } catch (MultipleObjectsReturnedException|Exception $e) { + $this->logger->warning("Could not schedule context chat indexing task for mailbox <{$mailbox->getId()}>", ['exception' => $e]); } } + + $this->jobList->remove(self::class, $argument); } } diff --git a/lib/ContextChat/ContextChatProvider.php b/lib/ContextChat/ContextChatProvider.php index 3b5196c096..3a9bdd57f9 100644 --- a/lib/ContextChat/ContextChatProvider.php +++ b/lib/ContextChat/ContextChatProvider.php @@ -10,6 +10,7 @@ namespace OCA\Mail\ContextChat; use OCA\Mail\AppInfo\Application; +use OCA\Mail\BackgroundJob\ContextChat\ScheduleJob; use OCA\Mail\Db\Message; use OCA\Mail\Db\MessageMapper; use OCA\Mail\Events\MessageDeletedEvent; @@ -124,5 +125,8 @@ public function getItemUrl(string $id): string { */ #[\Override] public function triggerInitialImport(): void { + foreach ($this->accountService->getAllAcounts() as $account) { + $this->jobList->add(ScheduleJob::class, ['accountId' => $account->getId()]); + } } } diff --git a/tests/Unit/BackgroundJob/ContextChat/ScheduleJobTest.php b/tests/Unit/BackgroundJob/ContextChat/ScheduleJobTest.php new file mode 100644 index 0000000000..fda6f3c59d --- /dev/null +++ b/tests/Unit/BackgroundJob/ContextChat/ScheduleJobTest.php @@ -0,0 +1,158 @@ + 42]; + + private ITimeFactory&MockObject $time; + private TaskService&MockObject $taskService; + private AccountService&MockObject $accountService; + private MailManager&MockObject $mailManager; + private LoggerInterface&MockObject $logger; + private IJobList&MockObject $jobList; + private ContextChatSettingsService&MockObject $settingsService; + private IContentManager&MockObject $contentManager; + private ScheduleJob $job; + private Account $account; + + protected function setUp(): void { + parent::setUp(); + + if (!interface_exists(IContentManager::class)) { + $this->markTestSkipped(); + } + + $this->time = $this->createMock(ITimeFactory::class); + $this->taskService = $this->createMock(TaskService::class); + $this->accountService = $this->createMock(AccountService::class); + $this->mailManager = $this->createMock(MailManager::class); + $this->logger = $this->createMock(LoggerInterface::class); + $this->jobList = $this->createMock(IJobList::class); + $this->settingsService = $this->createMock(ContextChatSettingsService::class); + $this->contentManager = $this->createMock(IContentManager::class); + + $this->time->method('getTime')->willReturn(2 * 24 * 60 * 60); + $this->contentManager->method('isContextChatAvailable')->willReturn(true); + $mailAccount = new MailAccount(); + $mailAccount->setId(self::ARGUMENT['accountId']); + $mailAccount->setUserId('user'); + $this->account = new Account($mailAccount); + + $this->job = new ScheduleJob( + $this->time, + $this->taskService, + $this->accountService, + $this->mailManager, + $this->logger, + $this->jobList, + $this->settingsService, + $this->contentManager, + ); + $this->job->setArgument(self::ARGUMENT); + $this->job->setLastRun(0); + } + + private function createMailbox(int $id, bool $cached): Mailbox { + $mailbox = new Mailbox(); + $mailbox->setId($id); + if ($cached) { + $mailbox->setSyncNewToken('new'); + $mailbox->setSyncChangedToken('changed'); + $mailbox->setSyncVanishedToken('vanished'); + } + return $mailbox; + } + + public function testResetsCachedMailboxesAndRemovesItself(): void { + $this->accountService->method('findById')->willReturn($this->account); + $this->settingsService->method('isIndexingEnabled')->with('user')->willReturn(true); + $this->mailManager->method('getMailboxes')->willReturn([ + $this->createMailbox(1, true), + $this->createMailbox(2, false), + $this->createMailbox(3, true), + ]); + $scheduled = []; + $this->taskService->expects($this->exactly(2)) + ->method('updateOrCreate') + ->willReturnCallback(function (int $mailboxId, int $lastMessageId) use (&$scheduled) { + $scheduled[$mailboxId] = $lastMessageId; + return new Task(); + }); + $this->jobList->expects($this->once()) + ->method('remove') + ->with(ScheduleJob::class, self::ARGUMENT); + + $this->job->start($this->jobList); + + $this->assertSame([1 => 0, 3 => 0], $scheduled); + } + + public function testRemovesItselfWhenSchedulingATaskFails(): void { + $this->accountService->method('findById')->willReturn($this->account); + $this->settingsService->method('isIndexingEnabled')->willReturn(true); + $this->mailManager->method('getMailboxes')->willReturn([$this->createMailbox(1, true)]); + $this->taskService->method('updateOrCreate')->willThrowException(new Exception()); + $this->logger->expects($this->once())->method('warning'); + $this->jobList->expects($this->once()) + ->method('remove') + ->with(ScheduleJob::class, self::ARGUMENT); + + $this->job->start($this->jobList); + } + + public function testPostponesUntilInitialSync(): void { + $this->accountService->method('findById')->willReturn($this->account); + $this->settingsService->method('isIndexingEnabled')->willReturn(true); + $this->mailManager->method('getMailboxes')->willReturn([$this->createMailbox(1, false)]); + $this->taskService->expects($this->never())->method('updateOrCreate'); + $this->jobList->expects($this->never())->method('remove'); + + $this->job->start($this->jobList); + } + + public function testPostponesWhileIndexingIsDisabled(): void { + $this->accountService->method('findById')->willReturn($this->account); + $this->settingsService->method('isIndexingEnabled')->willReturn(false); + $this->mailManager->expects($this->never())->method('getMailboxes'); + $this->taskService->expects($this->never())->method('updateOrCreate'); + $this->jobList->expects($this->never())->method('remove'); + + $this->job->start($this->jobList); + } + + public function testRemovesItselfForDeletedAccount(): void { + $this->accountService->method('findById')->willThrowException(new DoesNotExistException('')); + $this->taskService->expects($this->never())->method('updateOrCreate'); + $this->jobList->expects($this->once()) + ->method('remove') + ->with(ScheduleJob::class, self::ARGUMENT); + + $this->job->start($this->jobList); + } +} diff --git a/tests/Unit/ContextChat/ContextChatProviderTest.php b/tests/Unit/ContextChat/ContextChatProviderTest.php index e9d9a449f3..6009649ba1 100644 --- a/tests/Unit/ContextChat/ContextChatProviderTest.php +++ b/tests/Unit/ContextChat/ContextChatProviderTest.php @@ -11,6 +11,7 @@ use ChristophWurst\Nextcloud\Testing\TestCase; use OCA\Mail\Account; +use OCA\Mail\BackgroundJob\ContextChat\ScheduleJob; use OCA\Mail\ContextChat\ContextChatProvider; use OCA\Mail\Db\MailAccount; use OCA\Mail\Db\Mailbox; @@ -148,6 +149,32 @@ public function testHandleWithContextChat($event) { $this->contextChatProvider->handle($event); } + public function testTriggerInitialImportSchedulesEveryAccount(): void { + $first = new MailAccount(); + $first->setId(1); + $second = new MailAccount(); + $second->setId(2); + $this->accountService->method('getAllAcounts')->willReturn([$first, $second]); + $scheduled = []; + $this->jobList->expects($this->exactly(2)) + ->method('add') + ->willReturnCallback(function (string $job, array $argument) use (&$scheduled): void { + $this->assertSame(ScheduleJob::class, $job); + $scheduled[] = $argument; + }); + + $this->contextChatProvider->triggerInitialImport(); + + $this->assertSame([['accountId' => 1], ['accountId' => 2]], $scheduled); + } + + public function testTriggerInitialImportWithoutAccounts(): void { + $this->accountService->method('getAllAcounts')->willReturn([]); + $this->jobList->expects($this->never())->method('add'); + + $this->contextChatProvider->triggerInitialImport(); + } + public function testGetId(): void { $this->assertEquals('mail', $this->contextChatProvider->getId()); }