diff --git a/apps/readest-app/src/__tests__/components/ProofreadRules.test.tsx b/apps/readest-app/src/__tests__/components/ProofreadRules.test.tsx index 5a31d564..dce5d9a9 100644 --- a/apps/readest-app/src/__tests__/components/ProofreadRules.test.tsx +++ b/apps/readest-app/src/__tests__/components/ProofreadRules.test.tsx @@ -248,6 +248,63 @@ describe('ProofreadRulesManager', () => { expect(screen.getByText("'book-hit'")).toBeTruthy(); }); + it('hides tombstoned (deleted) book rules from the list', async () => { + (useSettingsStore.setState as unknown as (state: unknown) => void)({ + settings: { ...DEFAULT_SYSTEM_SETTINGS, globalViewSettings: { proofreadRules: [] } }, + }); + + const liveRule: ProofreadRule = { + id: 'live', + scope: 'book', + pattern: 'visible-pattern', + replacement: 'kept', + enabled: true, + isRegex: false, + caseSensitive: true, + order: 1, + wholeWord: true, + }; + const deletedRule: ProofreadRule = { + id: 'dead', + scope: 'book', + pattern: 'deleted-pattern', + replacement: 'gone', + enabled: true, + isRegex: false, + caseSensitive: true, + order: 2, + wholeWord: true, + updatedAt: 5, + deletedAt: 10, + }; + const rules = [liveRule, deletedRule]; + + (useReaderStore.setState as unknown as (state: unknown) => void)({ + viewStates: { book1: { viewSettings: { proofreadRules: rules } } }, + }); + (useBookDataStore.setState as unknown as (state: unknown) => void)({ + booksData: { + book1: { + id: 'book1', + book: null, + file: null, + config: { viewSettings: { proofreadRules: rules } }, + bookDoc: null, + isFixedLayout: false, + }, + }, + }); + useSidebarStore.setState({ sideBarBookKey: 'book1' }); + + renderWithProviders(); + await Promise.resolve(); + setProofreadRulesVisibility(true); + + await screen.findByRole('dialog'); + expect(screen.getByText('visible-pattern')).toBeTruthy(); + expect(screen.queryByText('deleted-pattern')).toBeNull(); + }); + it('renders a drag handle for each reorderable rule', async () => { const selectionRule: ProofreadRule = { id: 's1', diff --git a/apps/readest-app/src/__tests__/hooks/useProgressSync.test.tsx b/apps/readest-app/src/__tests__/hooks/useProgressSync.test.tsx index a72a4fbc..697cc102 100644 --- a/apps/readest-app/src/__tests__/hooks/useProgressSync.test.tsx +++ b/apps/readest-app/src/__tests__/hooks/useProgressSync.test.tsx @@ -37,11 +37,15 @@ const h = vi.hoisted(() => { user: { id: 'u1' }, syncConfigsMock: vi.fn(async () => {}), syncBooksMock: vi.fn(async () => {}), + saveConfigMock: vi.fn(async () => {}), + setViewSettingsMock: vi.fn(), + recreateViewerMock: vi.fn(), cfiCompareMock: vi.fn((_a: string, _b: string) => 0), view: { renderer: { getContents: () => [], primaryIndex: 0 }, goTo: vi.fn() }, state: { syncedConfigs: [] as unknown[] | null, progress: { location: 'cfi-loc' } as { location: string } | null, + viewSettings: { proofreadRules: [] } as { proofreadRules: unknown[] } | null, }, eventListeners: new Map void>>(), }; @@ -63,10 +67,15 @@ vi.mock('@/hooks/useTranslation', () => ({ useTranslation: () => (s: string) => s, })); +vi.mock('@/context/EnvContext', () => ({ + useEnv: () => ({ envConfig: {} }), +})); + vi.mock('@/store/bookDataStore', () => ({ useBookDataStore: h.makeStore({ getConfig: () => h.config, setConfig: vi.fn(), + saveConfig: h.saveConfigMock, getBookData: () => ({ book: h.book }), }), })); @@ -75,6 +84,9 @@ vi.mock('@/store/readerStore', () => ({ useReaderStore: h.makeStore({ getView: () => h.view, getProgress: () => h.state.progress, + getViewSettings: () => h.state.viewSettings, + setViewSettings: h.setViewSettingsMock, + recreateViewer: h.recreateViewerMock, setHoveredBookKey: vi.fn(), getViewState: () => ({ previewMode: false }), }), @@ -141,11 +153,15 @@ beforeEach(() => { vi.useFakeTimers(); h.syncConfigsMock.mockClear(); h.syncBooksMock.mockClear(); + h.saveConfigMock.mockClear(); + h.setViewSettingsMock.mockClear(); + h.recreateViewerMock.mockClear(); h.view.goTo.mockClear(); h.cfiCompareMock.mockReset(); h.cfiCompareMock.mockReturnValue(0); h.state.syncedConfigs = []; h.state.progress = { location: 'cfi-loc' }; + h.state.viewSettings = { proofreadRules: [] }; h.eventListeners.clear(); }); @@ -287,6 +303,55 @@ describe('useProgressSync', () => { expect(pullCallCount()).toBe(callsBeforeRefresh + 2); }); + test('merges synced book-scope proofread rules into local config by id', async () => { + // Local has a book rule; the remote config carries a different book rule + // plus a library-scope rule (which must be ignored — it syncs separately + // via the settings replica). After the pull both book rules should be + // merged into local viewSettings, persisted, and the live view refreshed. + h.cfiCompareMock.mockReturnValue(0); + h.state.viewSettings = { + proofreadRules: [{ id: 'local', scope: 'book', pattern: 'a', updatedAt: 100 }], + }; + h.state.syncedConfigs = [ + { + bookHash: 'h1', + metaHash: 'm1', + viewSettings: { + proofreadRules: [ + { id: 'remote', scope: 'book', pattern: 'b', updatedAt: 200 }, + { id: 'lib', scope: 'library', pattern: 'c', updatedAt: 200 }, + ], + }, + }, + ]; + renderHook(() => useProgressSync('h1-view1')); + await advance(0); + + expect(h.setViewSettingsMock).toHaveBeenCalledTimes(1); + const mergedRules = ( + h.setViewSettingsMock.mock.calls[0]![1] as { proofreadRules: { id: string }[] } + ).proofreadRules; + // Both book rules merged; the library-scope rule is excluded. + expect(mergedRules.map((r) => r.id).sort()).toEqual(['local', 'remote']); + expect(h.saveConfigMock).toHaveBeenCalledTimes(1); + expect(h.recreateViewerMock).toHaveBeenCalledTimes(1); + }); + + test('does not touch the view when synced proofread rules match local', async () => { + h.cfiCompareMock.mockReturnValue(0); + const rule = { id: 'same', scope: 'book', pattern: 'a', updatedAt: 100 }; + h.state.viewSettings = { proofreadRules: [rule] }; + h.state.syncedConfigs = [ + { bookHash: 'h1', metaHash: 'm1', viewSettings: { proofreadRules: [rule] } }, + ]; + renderHook(() => useProgressSync('h1-view1')); + await advance(0); + + expect(h.setViewSettingsMock).not.toHaveBeenCalled(); + expect(h.saveConfigMock).not.toHaveBeenCalled(); + expect(h.recreateViewerMock).not.toHaveBeenCalled(); + }); + test('sync-book-progress flushes the pending cloud push on book close', async () => { // Reproduces issue #4532: the reader is closed inside the 3s auto-sync // debounce window, so the pending Readest cloud push would otherwise be diff --git a/apps/readest-app/src/__tests__/store/proofread-store.test.ts b/apps/readest-app/src/__tests__/store/proofread-store.test.ts index 4e7fb8d8..a9aa2a2a 100644 --- a/apps/readest-app/src/__tests__/store/proofread-store.test.ts +++ b/apps/readest-app/src/__tests__/store/proofread-store.test.ts @@ -196,7 +196,10 @@ describe('proofreadStore', () => { replacement: 'the', }); - expect(rule.id).toBe('uid-1'); + // Book rules now carry a stable content-derived id (not a random uid) + // so the same rule made on two devices dedupes on sync. + expect(rule.id).toBeTruthy(); + expect(rule.id).not.toBe('uid-1'); expect(rule.scope).toBe('book'); expect(rule.pattern).toBe('teh'); expect(rule.replacement).toBe('the'); @@ -246,6 +249,25 @@ describe('proofreadStore', () => { expect(mockSaveConfig).toHaveBeenCalledTimes(1); }); + + test('derives a stable content id (same pattern/scope/isRegex on two devices)', async () => { + mockViewSettingsMap['bookA'] = emptyViewSettings(); + mockViewSettingsMap['bookB'] = emptyViewSettings(); + + const a = await useProofreadStore.getState().addRule(envConfig, 'bookA', { + scope: 'book', + pattern: 'teh', + replacement: 'the', + }); + // Different replacement, same pattern — identity ignores replacement. + const b = await useProofreadStore.getState().addRule(envConfig, 'bookB', { + scope: 'book', + pattern: 'teh', + replacement: 'THE', + }); + + expect(a.id).toBe(b.id); + }); }); // ----------------------------------------------------------------------- @@ -282,6 +304,20 @@ describe('proofreadStore', () => { const rules = mockViewSettingsMap['book1']!.proofreadRules!; expect(rules.length).toBe(2); }); + + test('uses a per-instance unique id (not a content hash)', async () => { + mockViewSettingsMap['book1'] = emptyViewSettings(); + + const rule = await useProofreadStore.getState().addRule(envConfig, 'book1', { + scope: 'selection', + pattern: 'dup', + replacement: 'new', + }); + + // Selection rules keep the random uniqueId so two selections of the same + // text are never collapsed. + expect(rule.id).toBe('uid-1'); + }); }); // ----------------------------------------------------------------------- @@ -331,17 +367,22 @@ describe('proofreadStore', () => { // removeRule – book scope // ----------------------------------------------------------------------- describe('removeRule (book scope)', () => { - test('removes rule by id', async () => { + test('tombstones the rule (sets deletedAt) instead of hard-removing it', async () => { const rule = makeRule({ id: 'r1' }); mockViewSettingsMap['book1'] = emptyViewSettings({ proofreadRules: [rule] }); await useProofreadStore.getState().removeRule(envConfig, 'book1', 'r1', 'book'); + // The row is retained with a deletedAt tombstone so the deletion can sync + // across devices (the per-id config merge would otherwise resurrect it). const rules = mockViewSettingsMap['book1']!.proofreadRules!; - expect(rules.length).toBe(0); + expect(rules.length).toBe(1); + expect(typeof rules[0]!.deletedAt).toBe('number'); + // Hidden from consumers. + expect(useProofreadStore.getState().getBookRules('book1')).toEqual([]); }); - test('does not remove unmatched rule', async () => { + test('does not tombstone an unmatched rule', async () => { const rule = makeRule({ id: 'r1' }); mockViewSettingsMap['book1'] = emptyViewSettings({ proofreadRules: [rule] }); @@ -349,6 +390,78 @@ describe('proofreadStore', () => { const rules = mockViewSettingsMap['book1']!.proofreadRules!; expect(rules.length).toBe(1); + expect(rules[0]!.deletedAt ?? null).toBeNull(); + }); + }); + + // ----------------------------------------------------------------------- + // CRDT sync fields (updatedAt / deletedAt) + // ----------------------------------------------------------------------- + describe('CRDT sync fields', () => { + test('addRule stamps updatedAt', async () => { + mockViewSettingsMap['book1'] = emptyViewSettings(); + const rule = await useProofreadStore.getState().addRule(envConfig, 'book1', { + scope: 'book', + pattern: 'teh', + replacement: 'the', + }); + expect(typeof rule.updatedAt).toBe('number'); + expect(rule.updatedAt!).toBeGreaterThan(0); + }); + + test('updateRule bumps updatedAt', async () => { + const rule = makeRule({ id: 'r1', updatedAt: 1 }); + mockViewSettingsMap['book1'] = emptyViewSettings({ proofreadRules: [rule] }); + + await useProofreadStore.getState().updateRule(envConfig, 'book1', 'r1', { + replacement: 'baz', + }); + + const updated = mockViewSettingsMap['book1']!.proofreadRules!.find((r) => r.id === 'r1'); + expect(updated!.updatedAt!).toBeGreaterThan(1); + }); + + test('getBookRules hides tombstoned rules', () => { + mockViewSettingsMap['book1'] = emptyViewSettings({ + proofreadRules: [makeRule({ id: 'a' }), makeRule({ id: 'b', deletedAt: 123 })], + }); + expect( + useProofreadStore + .getState() + .getBookRules('book1') + .map((r) => r.id), + ).toEqual(['a']); + }); + + test('getMergedRules hides tombstoned rules from both stores', () => { + mockGlobalViewSettingsHolder.current = emptyViewSettings({ + proofreadRules: [makeRule({ id: 'g', scope: 'library', deletedAt: 9 })], + }); + mockViewSettingsMap['book1'] = emptyViewSettings({ + proofreadRules: [makeRule({ id: 'b' })], + }); + expect( + useProofreadStore + .getState() + .getMergedRules('book1') + .map((r) => r.id), + ).toEqual(['b']); + }); + + test('re-adding a tombstoned book pattern creates a fresh live rule', async () => { + mockViewSettingsMap['book1'] = emptyViewSettings({ + proofreadRules: [makeRule({ id: 'old', pattern: 'teh', deletedAt: 999 })], + }); + + await useProofreadStore.getState().addRule(envConfig, 'book1', { + scope: 'book', + pattern: 'teh', + replacement: 'the', + }); + + const live = mockViewSettingsMap['book1']!.proofreadRules!.filter((r) => !r.deletedAt); + expect(live.length).toBe(1); + expect(live[0]!.pattern).toBe('teh'); }); }); diff --git a/apps/readest-app/src/__tests__/utils/proofread.test.ts b/apps/readest-app/src/__tests__/utils/proofread.test.ts new file mode 100644 index 00000000..4547cc6e --- /dev/null +++ b/apps/readest-app/src/__tests__/utils/proofread.test.ts @@ -0,0 +1,126 @@ +import { describe, test, expect } from 'vitest'; +import type { ProofreadRule } from '@/types/book'; +import { ensureRuleId, mergeProofreadRules } from '@/utils/proofread'; + +function rule(overrides: Partial = {}): ProofreadRule { + return { + id: 'r1', + scope: 'book', + pattern: 'foo', + replacement: 'bar', + isRegex: false, + enabled: true, + caseSensitive: true, + order: 1000, + wholeWord: true, + onlyForTTS: false, + ...overrides, + }; +} + +describe('mergeProofreadRules', () => { + test('unions rules with disjoint ids', () => { + const local = [rule({ id: 'a' })]; + const remote = [rule({ id: 'b' })]; + const merged = mergeProofreadRules(local, remote); + expect(merged.map((r) => r.id).sort()).toEqual(['a', 'b']); + }); + + test('same id: remote wins when its updatedAt is newer', () => { + const local = [rule({ id: 'a', replacement: 'local', updatedAt: 100 })]; + const remote = [rule({ id: 'a', replacement: 'remote', updatedAt: 200 })]; + const merged = mergeProofreadRules(local, remote); + expect(merged).toHaveLength(1); + expect(merged[0]!.replacement).toBe('remote'); + }); + + test('same id: local wins when its updatedAt is newer', () => { + const local = [rule({ id: 'a', replacement: 'local', updatedAt: 300 })]; + const remote = [rule({ id: 'a', replacement: 'remote', updatedAt: 200 })]; + const merged = mergeProofreadRules(local, remote); + expect(merged).toHaveLength(1); + expect(merged[0]!.replacement).toBe('local'); + }); + + test('remote deletion (tombstone) wins over an older local edit', () => { + const local = [rule({ id: 'a', updatedAt: 100, deletedAt: null })]; + const remote = [rule({ id: 'a', updatedAt: 100, deletedAt: 200 })]; + const merged = mergeProofreadRules(local, remote); + expect(merged).toHaveLength(1); + expect(merged[0]!.deletedAt).toBe(200); + }); + + test('a deleted rule is not resurrected by a stale peer copy', () => { + // Device A deleted the rule (tombstone). Device B still has the live copy. + const localDeleted = [rule({ id: 'a', updatedAt: 50, deletedAt: 300 })]; + const remoteLive = [rule({ id: 'a', updatedAt: 50, deletedAt: null })]; + const merged = mergeProofreadRules(localDeleted, remoteLive); + expect(merged).toHaveLength(1); + expect(merged[0]!.deletedAt).toBe(300); + }); + + test('treats missing updatedAt as 0 and keeps the local copy on a tie', () => { + const local = [rule({ id: 'a', replacement: 'local' })]; + const remote = [rule({ id: 'a', replacement: 'remote' })]; + const merged = mergeProofreadRules(local, remote); + expect(merged).toHaveLength(1); + expect(merged[0]!.replacement).toBe('local'); + }); + + test('handles empty inputs', () => { + expect(mergeProofreadRules([], [])).toEqual([]); + expect(mergeProofreadRules([rule({ id: 'a' })], [])).toHaveLength(1); + expect(mergeProofreadRules([], [rule({ id: 'b' })])).toHaveLength(1); + }); + + // Rules with no id (legacy / hand-edited / foreign sync peer). Without + // backfilling a content-based id they would all collide on the Map's + // `undefined` key — distinct rules would clobber each other (silent loss), + // not duplicate. + test('two id-less identical rules merge to one', () => { + const local = [rule({ id: '', scope: 'book', pattern: 'teh', replacement: 'the' })]; + const remote = [rule({ id: '', scope: 'book', pattern: 'teh', replacement: 'the' })]; + const merged = mergeProofreadRules(local, remote); + expect(merged).toHaveLength(1); + }); + + test('two id-less DIFFERENT rules across sides are both kept', () => { + const local = [rule({ id: '', scope: 'book', pattern: 'teh' })]; + const remote = [rule({ id: '', scope: 'book', pattern: 'recieve' })]; + const merged = mergeProofreadRules(local, remote); + expect(merged.map((r) => r.pattern).sort()).toEqual(['recieve', 'teh']); + }); + + test('two DIFFERENT id-less rules on the SAME side are both kept (no undefined-key collapse)', () => { + const local = [ + rule({ id: '', scope: 'book', pattern: 'teh' }), + rule({ id: '', scope: 'book', pattern: 'recieve' }), + ]; + const merged = mergeProofreadRules(local, []); + expect(merged).toHaveLength(2); + }); +}); + +describe('ensureRuleId', () => { + test('backfills a missing id deterministically from content', () => { + const a = ensureRuleId(rule({ id: '', scope: 'book', pattern: 'teh', replacement: 'the' })); + const b = ensureRuleId(rule({ id: '', scope: 'book', pattern: 'teh', replacement: 'THE' })); + expect(a.id).toBeTruthy(); + // Identity mirrors the in-store dedup (scope + pattern + isRegex); the + // replacement is NOT part of it, so the same pattern collapses to one id. + expect(b.id).toBe(a.id); + }); + + test('a different pattern / scope / isRegex yields a different id', () => { + const base = ensureRuleId(rule({ id: '', scope: 'book', pattern: 'teh' })).id; + expect(ensureRuleId(rule({ id: '', scope: 'book', pattern: 'other' })).id).not.toBe(base); + expect(ensureRuleId(rule({ id: '', scope: 'library', pattern: 'teh' })).id).not.toBe(base); + expect( + ensureRuleId(rule({ id: '', scope: 'book', pattern: 'teh', isRegex: true })).id, + ).not.toBe(base); + }); + + test('leaves an existing id untouched', () => { + expect(ensureRuleId(rule({ id: 'fixed' })).id).toBe('fixed'); + }); +}); diff --git a/apps/readest-app/src/app/reader/components/ProofreadRules.tsx b/apps/readest-app/src/app/reader/components/ProofreadRules.tsx index b312bef1..c03f38a3 100644 --- a/apps/readest-app/src/app/reader/components/ProofreadRules.tsx +++ b/apps/readest-app/src/app/reader/components/ProofreadRules.tsx @@ -246,14 +246,20 @@ const useReplacementRules = (bookKey: string | null) => { const persistedConfig = bookKey ? getConfig(bookKey) : null; const persistedBookRules = persistedConfig?.viewSettings?.proofreadRules || []; - // Prefer persisted rules; fall back to in-memory - const bookRuleSource = persistedBookRules.length ? persistedBookRules : inMemoryRules; + // Prefer persisted rules; fall back to in-memory. Drop tombstoned rules + // (deletedAt set) — deletion is a soft tombstone now so it can sync across + // devices (see store/proofreadStore.ts), but it must not show in the list. + const bookRuleSource = (persistedBookRules.length ? persistedBookRules : inMemoryRules).filter( + (r: ProofreadRule) => !r.deletedAt, + ); const singleRules = bookRuleSource .filter((r: ProofreadRule) => r.scope === 'selection') .sort(byOrder); const bookScopedRules = bookRuleSource.filter((r: ProofreadRule) => r.scope === 'book'); - const globalRules = settings?.globalViewSettings?.proofreadRules || []; + const globalRules = (settings?.globalViewSettings?.proofreadRules || []).filter( + (r: ProofreadRule) => !r.deletedAt, + ); // Merge book-scoped and global rules const globalRuleIds = new Set(globalRules.map((gr: ProofreadRule) => gr.id)); diff --git a/apps/readest-app/src/app/reader/hooks/useProgressSync.ts b/apps/readest-app/src/app/reader/hooks/useProgressSync.ts index 574411df..0d15a2ca 100644 --- a/apps/readest-app/src/app/reader/hooks/useProgressSync.ts +++ b/apps/readest-app/src/app/reader/hooks/useProgressSync.ts @@ -1,5 +1,6 @@ import { useCallback, useEffect, useRef } from 'react'; import { useAuth } from '@/context/AuthContext'; +import { useEnv } from '@/context/EnvContext'; import { useSync } from '@/hooks/useSync'; import { BookConfig, FIXED_LAYOUT_FORMATS } from '@/types/book'; import { useBookDataStore } from '@/store/bookDataStore'; @@ -7,6 +8,7 @@ import { useReaderStore } from '@/store/readerStore'; import { useBookProgress } from '@/store/readerProgressStore'; import { useSettingsStore } from '@/store/settingsStore'; import { useTranslation } from '@/hooks/useTranslation'; +import { mergeProofreadRules } from '@/utils/proofread'; import { serializeConfig } from '@/utils/serializer'; import { CFI } from '@/libs/document'; import { debounce } from '@/utils/debounce'; @@ -27,9 +29,14 @@ export const useProgressSync = (bookKey: string) => { // to the WHOLE bookDataStore — saveConfig writes booksData on every // throttled save and would otherwise re-render the entire reader subtree. const getConfig = useBookDataStore((s) => s.getConfig); + const saveConfig = useBookDataStore((s) => s.saveConfig); const getBookData = useBookDataStore((s) => s.getBookData); const getView = useReaderStore((s) => s.getView); + const getViewSettings = useReaderStore((s) => s.getViewSettings); + const setViewSettings = useReaderStore((s) => s.setViewSettings); + const recreateViewer = useReaderStore((s) => s.recreateViewer); const setHoveredBookKey = useReaderStore((s) => s.setHoveredBookKey); + const { envConfig } = useEnv(); const { settings } = useSettingsStore(); const { syncedConfigs, syncConfigs } = useSync(bookKey); const { user } = useAuth(); @@ -237,8 +244,9 @@ export const useProgressSync = (bookKey: string) => { remoteCFILocation = candidateCFI; } } - // Currently, only reading progress is synced. - // TODO: Configuration sync will be handled through a more robust profile-based solution in the future. + // Reading progress applies below. Proofread (find/replace) rules merge + // separately just after; other config fields remain device-local. + // TODO: general config sync via a more robust profile-based solution. if (remoteCFILocation && configCFI) { if (CFI.compare(configCFI, remoteCFILocation) < 0) { // While previewing a deep-link target, do NOT yank the view to the @@ -256,6 +264,38 @@ export const useProgressSync = (bookKey: string) => { } } } + // Merge book/selection-scope proofread rules from the remote config by id. + // Library-scope rules sync via the settings replica, so they're excluded. + // Item-level CRDT (see utils/proofread.ts) keeps a concurrent edit on + // another device from being lost to whole-config last-writer-wins, and + // tombstones stop a deleted rule from being resurrected by a stale peer. + const remoteRules = (syncedConfig.viewSettings?.proofreadRules ?? []).filter( + (r) => r.scope !== 'library', + ); + const localViewSettings = getViewSettings(bookKey); + const localRules = localViewSettings?.proofreadRules ?? []; + if (localViewSettings && (remoteRules.length || localRules.length)) { + const mergedRules = mergeProofreadRules(localRules, remoteRules); + if (JSON.stringify(mergedRules) !== JSON.stringify(localRules)) { + const updatedViewSettings = { ...localViewSettings, proofreadRules: mergedRules }; + setViewSettings(bookKey, updatedViewSettings); + if (config) { + await saveConfig( + envConfig, + bookKey, + { ...config, viewSettings: updatedViewSettings, updatedAt: Date.now() }, + settings, + ); + } + // Refresh a live view so merged rules take effect immediately; a + // not-yet-rendered view picks them up from viewSettings on first + // render. Skip while previewing a deep-link target. + const isPreview = useReaderStore.getState().getViewState(bookKey)?.previewMode; + if (getView(bookKey) && !isPreview) { + recreateViewer(envConfig, bookKey); + } + } + } } }; diff --git a/apps/readest-app/src/services/transformers/proofread.ts b/apps/readest-app/src/services/transformers/proofread.ts index db8655bc..40d20d39 100644 --- a/apps/readest-app/src/services/transformers/proofread.ts +++ b/apps/readest-app/src/services/transformers/proofread.ts @@ -219,7 +219,7 @@ export const proofreadTransformer: Transformer = { if (!merged.length) return ctx.content; const processed = merged - .filter((r) => r.enabled && r.pattern.trim()) + .filter((r) => r.enabled && !r.deletedAt && r.pattern.trim()) .filter((r) => (onlyForTTS ? r.onlyForTTS : !r.onlyForTTS)) .map((r) => ({ ...r, diff --git a/apps/readest-app/src/store/proofreadStore.ts b/apps/readest-app/src/store/proofreadStore.ts index f2a3770d..17388ec9 100644 --- a/apps/readest-app/src/store/proofreadStore.ts +++ b/apps/readest-app/src/store/proofreadStore.ts @@ -6,6 +6,7 @@ import { useReaderStore } from '@/store/readerStore'; import { useSettingsStore } from '@/store/settingsStore'; import { useBookDataStore } from '@/store/bookDataStore'; import { uniqueId } from '@/utils/misc'; +import { ensureRuleId } from '@/utils/proofread'; export interface CreateProofreadRuleOptions { scope: ProofreadScope; @@ -55,8 +56,11 @@ interface ProofreadStoreState { } function createProofreadRule(opts: CreateProofreadRuleOptions): ProofreadRule { - return { - id: uniqueId(), + const rule: ProofreadRule = { + // Selection rules are per-instance unique. Book/library rules get a stable + // content-derived id (filled by ensureRuleId below) so the same rule + // created on two devices dedupes on sync instead of duplicating. + id: opts.scope === 'selection' ? uniqueId() : '', scope: opts.scope, pattern: opts.pattern, replacement: opts.replacement, @@ -68,7 +72,9 @@ function createProofreadRule(opts: CreateProofreadRuleOptions): ProofreadRule { order: opts.order ?? 1000, wholeWord: opts.wholeWord ?? true, onlyForTTS: opts.onlyForTTS ?? false, + updatedAt: Date.now(), }; + return ensureRuleId(rule); } function mergeRules( @@ -80,19 +86,21 @@ function mergeRules( (globalRules ?? []).forEach((r) => map.set(r.id, r)); (bookRules ?? []).forEach((r) => map.set(r.id, r)); - return [...map.values()].sort((a, b) => (a.order ?? 0) - (b.order ?? 0)); + return [...map.values()] + .filter((r) => !r.deletedAt) + .sort((a, b) => (a.order ?? 0) - (b.order ?? 0)); } export const useProofreadStore = create(() => ({ getBookRules: (bookKey: string) => { const { getViewSettings } = useReaderStore.getState(); const viewSettings = getViewSettings(bookKey); - return viewSettings?.proofreadRules || []; + return (viewSettings?.proofreadRules || []).filter((r) => !r.deletedAt); }, getGlobalRules: () => { const { settings } = useSettingsStore.getState(); - return settings.globalViewSettings.proofreadRules || []; + return (settings.globalViewSettings.proofreadRules || []).filter((r) => !r.deletedAt); }, getMergedRules: (bookKey: string) => { @@ -155,7 +163,7 @@ export const useProofreadStore = create(() => ({ const order = orderById.get(r.id); if (order === undefined) return r; bookTouched = true; - return { ...r, order }; + return { ...r, order, updatedAt: Date.now() }; }); if (bookTouched) await updateBookViewSettings(envConfig, bookKey, updated); } @@ -168,7 +176,7 @@ export const useProofreadStore = create(() => ({ const order = orderById.get(r.id); if (order === undefined) return r; globalTouched = true; - return { ...r, order }; + return { ...r, order, updatedAt: Date.now() }; }); if (globalTouched) await updateGlobalSettings(envConfig, updated); } @@ -192,9 +200,15 @@ async function addBookRule( // Always add new single-instance rules (each has unique ID) existingRules.push(rule); } else { - // Check for duplicates in book scope + // Check for duplicates in book scope (ignore tombstoned rows so a deleted + // pattern can be re-added as a fresh live rule rather than reviving the + // tombstone in place). const existing = existingRules.find( - (r) => r.pattern === rule.pattern && r.isRegex === rule.isRegex && r.scope !== 'selection', + (r) => + !r.deletedAt && + r.pattern === rule.pattern && + r.isRegex === rule.isRegex && + r.scope !== 'selection', ); if (existing) { @@ -202,6 +216,7 @@ async function addBookRule( replacement: rule.replacement, enabled: rule.enabled, order: rule.order, + updatedAt: Date.now(), }); } else { existingRules.push(rule); @@ -225,7 +240,9 @@ async function updateBookRule( } const existingRules = viewSettings.proofreadRules || []; - const updatedRules = existingRules.map((r) => (r.id === ruleId ? { ...r, ...updates } : r)); + const updatedRules = existingRules.map((r) => + r.id === ruleId ? { ...r, ...updates, updatedAt: Date.now() } : r, + ); await updateBookViewSettings(envConfig, bookKey, updatedRules); } @@ -242,10 +259,16 @@ async function removeBookRule( throw new Error(`No viewSettings found for book: ${bookKey}`); } + // Tombstone instead of hard-removing: book/selection rules ride the per-id + // book-config sync (see utils/proofread.ts), so a removed row must survive the + // merge with a `deletedAt` marker or the peer's still-live copy resurrects it. const existingRules = viewSettings.proofreadRules || []; - const filteredRules = existingRules.filter((r) => r.id !== ruleId); + const now = Date.now(); + const tombstonedRules = existingRules.map((r) => + r.id === ruleId ? { ...r, deletedAt: now, updatedAt: now } : r, + ); - await updateBookViewSettings(envConfig, bookKey, filteredRules); + await updateBookViewSettings(envConfig, bookKey, tombstonedRules); } async function updateBookViewSettings( @@ -287,7 +310,7 @@ async function addGlobalRule(envConfig: EnvConfigType, rule: ProofreadRule): Pro const globalRules = settings.globalViewSettings.proofreadRules || []; const existing = globalRules.find( - (r) => r.pattern === rule.pattern && r.isRegex === rule.isRegex, + (r) => !r.deletedAt && r.pattern === rule.pattern && r.isRegex === rule.isRegex, ); if (existing) { @@ -295,6 +318,7 @@ async function addGlobalRule(envConfig: EnvConfigType, rule: ProofreadRule): Pro replacement: rule.replacement, enabled: rule.enabled, order: rule.order, + updatedAt: Date.now(), }); await updateGlobalSettings(envConfig, globalRules); } else { @@ -310,7 +334,9 @@ async function updateGlobalRule( const { settings } = useSettingsStore.getState(); const globalRules = settings.globalViewSettings.proofreadRules || []; - const updatedRules = globalRules.map((r) => (r.id === ruleId ? { ...r, ...updates } : r)); + const updatedRules = globalRules.map((r) => + r.id === ruleId ? { ...r, ...updates, updatedAt: Date.now() } : r, + ); await updateGlobalSettings(envConfig, updatedRules); } diff --git a/apps/readest-app/src/types/book.ts b/apps/readest-app/src/types/book.ts index c4fbd464..660c662a 100644 --- a/apps/readest-app/src/types/book.ts +++ b/apps/readest-app/src/types/book.ts @@ -380,6 +380,13 @@ export interface ProofreadRule { wholeWord?: boolean; // Match whole words only (uses \b word boundaries) caseSensitive?: boolean; // Case-sensitive matching (default true) onlyForTTS?: boolean; // Only replace text for TTS, not in the book display (only for book/library scope) + // CRDT sync fields (book/selection scope rides the book-config sync). `updatedAt` + // is the last-write-wins key for the per-id merge; `deletedAt` is a tombstone so a + // deletion survives the merge instead of being resurrected by the peer's copy. + // Library-scope rules sync via the settings replica (whole-field LWW) and don't + // need a tombstone, so these stay optional for back-compat with older configs. + updatedAt?: number; + deletedAt?: number | null; } export interface ProofreadRulesConfig { diff --git a/apps/readest-app/src/utils/proofread.ts b/apps/readest-app/src/utils/proofread.ts new file mode 100644 index 00000000..5be32b58 --- /dev/null +++ b/apps/readest-app/src/utils/proofread.ts @@ -0,0 +1,66 @@ +import { md5 } from 'js-md5'; +import { ProofreadRule } from '@/types/book'; + +// Deterministic identity key for a rule. Mirrors the in-store dedup +// (scope + pattern + isRegex for book/library) so identity is consistent +// however a rule is created; selection-scope rules are per-instance, so their +// location (sectionHref/cfi) is part of the key. The replacement and the +// case/whole-word flags are intentionally excluded: the in-store dedup keys on +// pattern + isRegex, so the same pattern resolves to one rule on merge too. +const ruleContentKey = (r: ProofreadRule): string => { + const flags = r.isRegex ? 'r' : ''; + if (r.scope === 'selection') { + return ['selection', r.sectionHref ?? '', r.cfi ?? '', flags, r.pattern].join(' '); + } + return [r.scope, flags, r.pattern].join(' '); +}; + +/** + * Backfill a stable, content-derived id when a rule has none (legacy data, + * hand-edited config, or a foreign sync peer). Without this, mergeProofreadRules + * keys every id-less rule on the Map's `undefined` slot, so distinct rules + * clobber each other (silent loss). A content-based id also makes the SAME rule + * authored independently on two devices collapse on merge instead of + * duplicating. A rule that already has an id is returned untouched: the id is + * assigned once and frozen, so later edits never re-key it. + */ +export const ensureRuleId = (rule: ProofreadRule): ProofreadRule => + rule.id ? rule : { ...rule, id: `ph-${md5(ruleContentKey(rule))}` }; + +/** + * Per-rule merge keyed by `id`, mirroring `mergeNotes` in WebDAVSync / + * `processNewNote` in useNotesSync. Book- and selection-scope proofread rules + * ride the book-config sync, so two devices can each edit the same array; a + * blind whole-array overwrite would lose one side's concurrent edit. Merging by + * id keeps both, and the `updatedAt`/`deletedAt` last-write-wins decides which + * copy of a shared id survives. A deletion (tombstone) wins over an older edit + * so a removed rule is never resurrected by the peer's stale live copy. + */ +export const mergeProofreadRules = ( + local: ProofreadRule[], + remote: ProofreadRule[], +): ProofreadRule[] => { + const byId = new Map(); + for (const raw of local) { + const r = ensureRuleId(raw); + byId.set(r.id, r); + } + for (const raw of remote) { + const r = ensureRuleId(raw); + const l = byId.get(r.id); + if (!l) { + byId.set(r.id, r); + continue; + } + const lUpdated = l.updatedAt ?? 0; + const rUpdated = r.updatedAt ?? 0; + const lDeleted = l.deletedAt ?? 0; + const rDeleted = r.deletedAt ?? 0; + if (rUpdated > lUpdated || rDeleted > lDeleted) { + byId.set(r.id, { ...l, ...r }); + } else { + byId.set(r.id, { ...r, ...l }); + } + } + return Array.from(byId.values()); +};