diff --git a/src/global.d.ts b/src/global.d.ts index 1fa29721..119ef79b 100644 --- a/src/global.d.ts +++ b/src/global.d.ts @@ -1,4 +1,4 @@ -import type { App } from "obsidian"; +import type { App, EventRef, Vault } from "obsidian"; interface Manifest { author: string; @@ -34,6 +34,10 @@ interface App2 extends App { open(): void; openTabById(id: string): void; }; + vault: Vault & { + /** Not in the public types: any change Obsidian sees to a path, config files included. */ + on(name: "raw", callback: (path: string) => unknown): EventRef; + }; } export default App2; diff --git a/src/main.activateView.test.ts b/src/main.activateView.test.ts index 187820f3..414451da 100644 --- a/src/main.activateView.test.ts +++ b/src/main.activateView.test.ts @@ -495,6 +495,44 @@ describe("PodNotes onload wiring (#55)", () => { expect(hide).toHaveBeenCalledTimes(1); }); + const syncedLibrary = () => + encodePodNotesData({ + ...structuredClone(DEFAULT_SETTINGS), + savedFeeds: { + Synced: { title: "Synced", url: "https://example.com/synced.xml", artworkUrl: "" }, + }, + }); + const pastReportSettle = () => new Promise((resolve) => window.setTimeout(resolve, 100)); + + it("takes a data.json change Obsidian reports only as a raw vault event", async () => { + const { plugin } = await loadPlugin({ schemaVersion: 2 }); + const listeners = vi.mocked(plugin.app.vault.on).mock.calls as unknown as Array< + [string, (path: string) => void] + >; + const report = (path: string) => { + for (const [name, listener] of listeners) if (name === "raw") listener(path); + }; + await pastReportSettle(); + const loadData = vi.mocked(plugin.loadData); + loadData.mockResolvedValue(syncedLibrary()); + const reads = loadData.mock.calls.length; + + report(".obsidian/plugins/other-plugin/data.json"); + await pastReportSettle(); + expect(loadData).toHaveBeenCalledTimes(reads); + + report(".obsidian/plugins/podnotes/data.json"); + await vi.waitFor(() => expect(Object.keys(plugin.settings.savedFeeds)).toEqual(["Synced"])); + }); + + it("takes a data.json change that landed while it loaded", async () => { + const { plugin } = await loadPlugin({ schemaVersion: 2 }); + + vi.mocked(plugin.loadData).mockResolvedValue(syncedLibrary()); + + await vi.waitFor(() => expect(Object.keys(plugin.settings.savedFeeds)).toEqual(["Synced"])); + }); + const savedEpisode: Episode = { title: "Saved Episode", streamUrl: "https://example.com/saved.mp3", diff --git a/src/main.ts b/src/main.ts index d58d4ed8..e6d37114 100644 --- a/src/main.ts +++ b/src/main.ts @@ -154,6 +154,18 @@ export default class PodNotes extends Plugin implements IPodNotes { await this.loadSettings(); this.writer = this.createWriter(MAIN_WINDOW_CLOCK); + // Obsidian calls onExternalSettingsChange only when data.json's modified time is later + // than PodNotes' last save. A sync client can keep the other device's earlier time, and + // a write right after a save can round to the same millisecond, so the writer checks + // every change Obsidian sees to the file, and once for a change made while it loaded. + const dataFile = this.dataFilePath(); + const writer = this.writer; + this.registerEvent( + this.app.vault.on("raw", (path) => { + if (path === dataFile) writer.report(); + }), + ); + writer.report(); this.setCollectionStores(); // The media lives in the main window, so playback outlasts the view and popouts. @@ -478,6 +490,7 @@ export default class PodNotes extends Plugin implements IPodNotes { return this.writer?.flush() ?? Promise.resolve(); } + // Obsidian refreshes an open settings tab once this resolves. override async onExternalSettingsChange(): Promise { if (!this.writer) return; await this.writer.onExternalChange(); @@ -515,6 +528,8 @@ export default class PodNotes extends Plugin implements IPodNotes { return { read: async (base): Promise> => { const data: unknown = await this.loadData(); + if (base && jsonValuesEqual(data, base)) + return { kind: "file", doc: structuredClone(base) }; const path = this.dataFilePath(); const unreadable: ReadOutcome = { kind: "refused", diff --git a/src/persistence/writer.test.ts b/src/persistence/writer.test.ts index 8eb4b2d5..16aa5b7a 100644 --- a/src/persistence/writer.test.ts +++ b/src/persistence/writer.test.ts @@ -492,6 +492,34 @@ describe("DataFileWriter changes made elsewhere", () => { expect(events.onRefusal.mock.calls).toEqual([[UNREADABLE]]); }); + it("reads the file once the changes reported to it settle", async () => { + const { access, clock, state, writer } = setup(); + state.disk!.savedFeeds.Synced = { title: "Synced" }; + + writer.report(); + writer.report(); + expect(access.read).not.toHaveBeenCalled(); + expect(clock.pending()).toBe(1); + clock.runAll(); + await writer.flush(); + + expect(access.read).toHaveBeenCalledTimes(1); + expect(state.memory.savedFeeds.Synced).toEqual({ title: "Synced" }); + }); + + it("reads nothing for a change reported before unload", async () => { + const { access, clock, state, writer } = setup(); + state.disk!.savedFeeds.Synced = { title: "Synced" }; + + writer.report(); + await writer.unload(); + writer.report(); + clock.runAll(); + await settled(); + + expect(access.read).not.toHaveBeenCalled(); + }); + it("adopts the file when it had nothing to merge against", async () => { const { access, events, state, writer } = setup(doc(), { base: null, refusal: STARTUP }); state.memory.defaultVolume = 0.3; diff --git a/src/persistence/writer.ts b/src/persistence/writer.ts index b125906b..c87a75b2 100644 --- a/src/persistence/writer.ts +++ b/src/persistence/writer.ts @@ -57,6 +57,9 @@ export interface WriterEvents { // Each save rewrites the whole file, and playback progress changes every few // seconds, so a progress checkpoint starts at least this long after the last save. const CHECKPOINT_INTERVAL_MS = 10_000; +// A program writing data.json in place empties the file first. Obsidian waits as +// long before its own check. +const REPORT_SETTLE_MS = 50; /** * Writes data.json without clobbering changes another device synced in. Every @@ -76,6 +79,8 @@ export class DataFileWriter { private chain: Promise | null = null; /** A checkpoint waiting for its interval. */ private timer: number | null = null; + /** A check of the file waiting for the changes reported to it to settle. */ + private reportTimer: number | null = null; /** A save is queued and has not started, so later changes are written by it. */ private saveQueued = false; private unloaded = false; @@ -153,10 +158,22 @@ export class DataFileWriter { }); } + /** data.json changed, maybe by PodNotes itself: check it once the changes stop. */ + report(): void { + if (this.unloaded) return; + if (this.reportTimer !== null) this.clock.clearTimeout(this.reportTimer); + this.reportTimer = this.clock.setTimeout(() => { + this.reportTimer = null; + void this.onExternalChange(); + }, REPORT_SETTLE_MS); + } + /** The next instance reads the file itself; the flushed save still merges with disk. */ unload(): Promise { this.memoryAtUnload = this.file.local(); this.unloaded = true; + if (this.reportTimer !== null) this.clock.clearTimeout(this.reportTimer); + this.reportTimer = null; return this.flush(); } diff --git a/tests/e2e/sync-persistence.test.ts b/tests/e2e/sync-persistence.test.ts index e4e9fc9b..cd5cd111 100644 --- a/tests/e2e/sync-persistence.test.ts +++ b/tests/e2e/sync-persistence.test.ts @@ -1,4 +1,4 @@ -import { readFile, rm, writeFile } from "node:fs/promises"; +import { readFile, rename, rm, utimes, writeFile } from "node:fs/promises"; import type { PluginHandle } from "obsidian-e2e"; import { describe, expect, onTestFinished, test } from "vitest"; import type { Episode } from "../../src/types/Episode"; @@ -19,7 +19,7 @@ import { click } from "./journey/input"; import { ui } from "./journey/targets"; import { isShown, listedEpisodes } from "./journey/wait"; import { type FixtureFeed, pubDate, startFixtureServer } from "./journey/fixtures"; -import { seedLibrary, waitForLibrary } from "./journey/library"; +import { recordWrites, seedLibrary, waitForLibrary } from "./journey/library"; import { jsLiteral, PLUGIN_ID, RELOAD_OPTIONS, WAIT_OPTS, waitForPodNotesReady } from "./runtime"; import { assertNoNewRuntimeErrors } from "./runtime-evidence"; @@ -212,6 +212,19 @@ describe("PodNotes data.json changed elsewhere", () => { test("does not treat its own saves as external changes", async () => { const { obsidian, plugin } = getContext(); + await plugin.updateDataAndReload((data) => { + addSyncedChanges(data); + data.queue = queueOf([ + { + title: "Queued Episode", + podcastName: "Synced Podcast", + streamUrl: "https://example.com/queued.mp3", + url: "https://example.com/queued", + description: "", + content: "", + }, + ]); + }, RELOAD_OPTIONS); // The harness rewrites data.json before each test, which is a change made // elsewhere. Let Obsidian report it before counting. await obsidian.sleep(2_500); @@ -225,6 +238,7 @@ describe("PodNotes data.json changed elsewhere", () => { }; return true; })()`); + const writes = await recordWrites(obsidian); await setVolume(obsidian, 0.55); await plugin.waitForData((data) => data.defaultVolume === 0.55, WAIT_OPTS); @@ -232,6 +246,12 @@ describe("PodNotes data.json changed elsewhere", () => { await obsidian.sleep(2_500); expect(await obsidian.dev.evalJson("window.__podnotesExternalChanges")).toBe(0); + expect(await writes.stop()).toMatchObject({ + attempted: 1, + completed: 1, + failed: 0, + intact: true, + }); }); test("saves an unsaved edit and keeps the change made elsewhere when disabled during their read", async () => { @@ -380,6 +400,32 @@ describe("PodNotes data.json changed elsewhere", () => { }); }); + test("shows a library synced in with the earlier modified time it had on the other device", async () => { + const { obsidian, plugin } = getContext(); + const dataPath = await plugin.dataPath(); + const synced = `${dataPath}.synced`; + onTestFinished(() => rm(synced, { force: true })); + const library = await plugin.data().read(); + addSyncedChanges(library); + await startAsNewDevice(plugin); + await setVolume(obsidian, 0.42); + await plugin.waitForData((data) => data.defaultVolume === 0.42, WAIT_OPTS); + + // A sync client that keeps modified times writes beside the file and moves it over. + const savedOnOtherDevice = new Date(Date.now() - 60_000); + await writeFile(synced, JSON.stringify(library)); + await utimes(synced, savedOnOtherDevice, savedOnOtherDevice); + await rename(synced, dataPath); + + const disk = await plugin.waitForData( + (data) => data.defaultVolume === 0.42 && Boolean(data.savedFeeds?.[SYNCED_FEED.title]), + WAIT_OPTS, + ); + expect(disk.playedEpisodes?.["Synced Podcast::Synced Episode"]).toEqual(SYNCED_PROGRESS); + await showLibrary(obsidian); + await expect.poll(() => isShown(obsidian, ui.podcastCard("Synced Feed")), POLL).toBe(true); + }); + test("shows both names when the library saved a podcast under an earlier name than a new device", async () => { const context = getContext(); const { obsidian, plugin } = context;