From 21c7297ff3591e409af38dc6528f0f3fb3053b09 Mon Sep 17 00:00:00 2001 From: Finn Date: Thu, 2 Jul 2026 01:56:43 +0100 Subject: [PATCH] fix(xapian): invalidate getDocData cache after index reload getDocData uses a single-doc cache (currentXapianDocId / currentDocData) to avoid redundant lookups when the canvastable requests the same document multiple times per row. After a flag change (seen, flagged, answered), the worker adds or removes a term, commits, and fires indexUpdatedSubject. The main thread syncs from IndexedDB, reloads the Xapian database, and fires indexReloadedSubject. But the cache was never invalidated, so getDocData returned stale data for the last-accessed docid. This affected all flag-based display updates: bold/unread state, flag icons, answered indicators. The Angular 16 upgrade shifted rendering timing enough to expose the pre-existing cache bug. Fix: clear currentXapianDocId after reloadXapianDatabase() and before indexReloadedSubject.next(), so consumers always read fresh data from the reloaded database. Tests cover seen flag, flagged flag, multiple flags at once, and verify the cache still provides its performance benefit between reload cycles. --- src/app/xapian/searchservice.spec.ts | 112 ++++++++++++++++++++++++++- src/app/xapian/searchservice.ts | 3 + 2 files changed, 114 insertions(+), 1 deletion(-) diff --git a/src/app/xapian/searchservice.spec.ts b/src/app/xapian/searchservice.spec.ts index a6b7c9a83..7f86feacd 100644 --- a/src/app/xapian/searchservice.spec.ts +++ b/src/app/xapian/searchservice.spec.ts @@ -28,7 +28,7 @@ import { MatLegacySnackBarModule as MatSnackBarModule } from '@angular/material/ import { MessageListService } from '../rmmapi/messagelist.service'; import { XapianAPI } from '@runboxcom/runbox-searchindex/rmmxapianapi'; -import { firstValueFrom } from 'rxjs'; +import { AsyncSubject, BehaviorSubject, Subject, of, firstValueFrom } from 'rxjs'; import { xapianLoadedSubject } from './xapianwebloader'; import { PostMessageAction } from './messageactions'; @@ -328,3 +328,113 @@ describe('SearchService', () => { FS.unmount('/' + localdir); }); }); + +describe('SearchService.getDocData cache', () => { + + let searchService: SearchService; + let mockApi: any; + let mockModule: { documenttermlistresult: string[] }; + + beforeEach(() => { + mockModule = { documenttermlistresult: [] }; + (window as any).Module = mockModule; + (window as any).FS = { + syncfs: (_populate: boolean, cb: () => void) => cb(), + }; + + const RealWorker = (window as any).Worker; + (window as any).Worker = class { + postMessage() {} + onmessage: any = null; + onerror: any = null; + terminate() {} + addEventListener() {} + }; + + searchService = new SearchService( + { + me: new AsyncSubject(), + messageFlagChangeSubject: new Subject(), + getMessageContents: () => of({ status: 'success', text: { text: '' } }), + deleteCachedMessageContents: () => {}, + } as any, + {} as any, + {} as any, + {} as any, + { + folderListSubject: new BehaviorSubject([]), + refreshFolderList: () => Promise.resolve([]), + searchservice: new AsyncSubject(), + } as any, + ); + + (window as any).Worker = RealWorker; + + mockApi = { + getDocumentData: jasmine.createSpy().and.returnValue('Q123\tsender@test.com\tTest Subject'), + documentXTermList: jasmine.createSpy(), + reloadXapianDatabase: jasmine.createSpy(), + }; + searchService.api = mockApi; + searchService.messageTextCache = new Map([[123, 'preview text']]); + }); + + afterEach(() => { + delete (window as any).Module; + delete (window as any).FS; + }); + + it('returns fresh seen flag after index reload', () => { + mockModule.documenttermlistresult = []; + const before = searchService.getDocData(1); + expect(before.seen).toBeFalsy(); + + mockModule.documenttermlistresult = ['XFseen']; + searchService.indexUpdatedSubject.next(undefined); + + const after = searchService.getDocData(1); + expect(after.seen).toBe(true); + }); + + it('returns fresh flagged flag after index reload', () => { + mockModule.documenttermlistresult = []; + const before = searchService.getDocData(1); + expect(before.flagged).toBeFalsy(); + + mockModule.documenttermlistresult = ['XFflagged']; + searchService.indexUpdatedSubject.next(undefined); + + const after = searchService.getDocData(1); + expect(after.flagged).toBe(true); + }); + + it('reflects multiple flag changes in one reload', () => { + mockModule.documenttermlistresult = []; + searchService.getDocData(1); + + mockModule.documenttermlistresult = ['XFseen', 'XFflagged']; + searchService.indexUpdatedSubject.next(undefined); + + const after = searchService.getDocData(1); + expect(after.seen).toBe(true); + expect(after.flagged).toBe(true); + }); + + it('still caches between calls when no reload happened', () => { + mockModule.documenttermlistresult = []; + searchService.getDocData(1); + expect(mockApi.getDocumentData).toHaveBeenCalledTimes(1); + + searchService.getDocData(1); + expect(mockApi.getDocumentData).toHaveBeenCalledTimes(1); + }); + + it('does fresh lookup for a different docid without reload', () => { + mockModule.documenttermlistresult = []; + searchService.getDocData(1); + expect(mockApi.getDocumentData).toHaveBeenCalledTimes(1); + + searchService.getDocData(2); + expect(mockApi.getDocumentData).toHaveBeenCalledTimes(2); + }); +}); diff --git a/src/app/xapian/searchservice.ts b/src/app/xapian/searchservice.ts index 2bee05d38..eb5dad7e0 100644 --- a/src/app/xapian/searchservice.ts +++ b/src/app/xapian/searchservice.ts @@ -263,6 +263,9 @@ export class SearchService { // console.log(FS.stat(`${this.partitionsdir}/${f}`)); // }); this.api.reloadXapianDatabase(); + // Invalidate the single-doc cache so getDocData reads fresh data + // from the reloaded database instead of returning stale values + this.currentXapianDocId = null; this.indexReloadedSubject.next(undefined); }); });