fix(xapian): invalidate getDocData cache after index reload#1990
Open
piratefinn wants to merge 1 commit into
Open
fix(xapian): invalidate getDocData cache after index reload#1990piratefinn wants to merge 1 commit into
piratefinn wants to merge 1 commit into
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Root cause
getDocDatainsearchservice.tsuses a single-doc cache (currentXapianDocId/currentDocData) to avoid redundant Xapian lookups when the display requests the same document multiple times per row render.After a flag change (seen, flagged, answered), the worker adds/removes the term, commits, and fires
indexUpdatedSubject. The main thread syncs from IndexedDB viaFS.syncfs, reloads the Xapian database, and firesindexReloadedSubject. But the cache was never invalidated, sogetDocDatareturned 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 (commit 50b8e5d) shifted rendering timing enough to expose the pre-existing cache bug.
Fix
One line: clear
currentXapianDocIdafterreloadXapianDatabase()and beforeindexReloadedSubject.next(), so consumers always read fresh data from the reloaded database.Tests
5 new tests in
searchservice.spec.ts, all running on master (no dependency on accessible-table or MessageDisplay code):Full suite: 262 passing, 0 failing.
Scope
Fix lives entirely in
searchservice.ts. Works on master and on the accessible-table branch. Tests are importable to either branch.