diff --git a/src/commands.test.ts b/src/commands.test.ts index a5322fc2..3df51c52 100644 --- a/src/commands.test.ts +++ b/src/commands.test.ts @@ -219,7 +219,7 @@ async function loadPlugin(data: Record) { registerObsidianProtocolHandler: vi.fn(), registerEvent: vi.fn(), registerDomEvent: vi.fn(), - writeRefusal: null, + closing: null, writeInFlight: null, storeUnsubscribers: [], views: new Set(), diff --git a/src/main.activateView.test.ts b/src/main.activateView.test.ts index 1c2c050a..187820f3 100644 --- a/src/main.activateView.test.ts +++ b/src/main.activateView.test.ts @@ -322,7 +322,7 @@ describe("PodNotes onload wiring (#55)", () => { registerObsidianProtocolHandler: vi.fn(), registerEvent: vi.fn(), registerDomEvent: vi.fn(), - writeRefusal: null, + closing: null, writeInFlight: null, storeUnsubscribers: [], views: new Set(), @@ -487,7 +487,7 @@ describe("PodNotes onload wiring (#55)", () => { vi.mocked(plugin.app.vault.adapter.read).mockResolvedValue("{ cut off"); vi.spyOn(console, "warn").mockImplementation(() => undefined); await plugin.onExternalSettingsChange(); - await expect(plugin.saveSettingsStrict()).rejects.toThrow(/does not save over/); + await expect(plugin.saveSettingsStrict()).rejects.toThrow(/could not read/); const hide = vi.spyOn(Notice.prototype, "hide"); plugin.onunload(); diff --git a/src/main.fileRenames.test.ts b/src/main.fileRenames.test.ts index 5cd2ff27..6e22b894 100644 --- a/src/main.fileRenames.test.ts +++ b/src/main.fileRenames.test.ts @@ -70,7 +70,7 @@ async function loadPlugin(data: Record): Promise { registerObsidianProtocolHandler: vi.fn(), registerEvent: vi.fn(), registerDomEvent: vi.fn(), - writeRefusal: null, + closing: null, writeInFlight: null, storeUnsubscribers: [], views: new Set(), diff --git a/src/main.persistence.test.ts b/src/main.persistence.test.ts index f9eb4a9d..bff21997 100644 --- a/src/main.persistence.test.ts +++ b/src/main.persistence.test.ts @@ -85,6 +85,200 @@ afterEach(() => { vi.restoreAllMocks(); }); +describe("save refusal recovery", () => { + function recovering() { + vi.spyOn(console, "error").mockImplementation(() => undefined); + vi.spyOn(console, "warn").mockImplementation(() => undefined); + const saveData = vi.fn().mockResolvedValue(undefined); + const plugin = makePlugin(saveData, { dataFileExists: true }); + plugin.settings.savedFeeds = { + Recovery: { + title: "Recovery", + url: "https://example.com/recovery.xml", + artworkUrl: "", + }, + }; + const readable = encodePodNotesData(plugin.settings); + const state: { blocked: boolean; disk: unknown } = { blocked: false, disk: readable }; + const loadData = vi.fn(async () => (state.blocked ? null : structuredClone(state.disk))); + Object.assign(plugin, { + loadData, + saveData: async (data: unknown) => { + await saveData(data); + state.disk = structuredClone(data); + }, + }); + Object.assign(plugin.app.vault.adapter, { + exists: vi.fn(async () => state.disk !== null), + read: vi.fn(async () => { + if (state.blocked) throw new Error("read outage"); + return dataFileText(state.disk); + }), + }); + const clock = manualClock(); + attachDataFileWriter(plugin, clock); + const notices = vi.spyOn(obsidian, "Notice"); + return { plugin, state, readable, saveData, loadData, clock, notices }; + } + + it.each([1, 99])( + "#429 stops periodic reads for schema %s and reports the required action once", + async (schemaVersion) => { + const { plugin, state, readable, saveData, loadData, clock, notices } = recovering(); + state.disk = { ...readable, schemaVersion }; + plugin.settings.skipForwardLength = 25; + await saveInBackground(plugin); + const reads = loadData.mock.calls.length; + for (let i = 0; i < 6; i++) { + clock.advance(10_000); + clock.runAll(); + await settled(); + } + expect.soft(loadData).toHaveBeenCalledTimes(reads); + expect.soft(notices).toHaveBeenCalledTimes(1); + expect + .soft(notices.mock.calls[0]?.[0]) + .toMatch(schemaVersion === 99 ? /updat.*PodNotes/i : /reload|restart/i); + expect(saveData).not.toHaveBeenCalled(); + expect(state.disk).toEqual({ ...readable, schemaVersion }); + }, + ); + + it("#430 strict saves recover the read before rejecting an import or key selection", async () => { + const { plugin, state, saveData, notices } = recovering(); + state.blocked = true; + await plugin.onExternalSettingsChange(); + await plugin.onExternalSettingsChange(); + expect(notices).toHaveBeenCalledTimes(1); + expect(saveData).not.toHaveBeenCalled(); + state.blocked = false; + plugin.settings.openAISecretId = "podnotes-openai-api-key"; + await plugin.saveSettingsStrict(); + expect(saveData).toHaveBeenCalledTimes(1); + expect(saveData.mock.lastCall?.[0]).toMatchObject({ + openAISecretId: "podnotes-openai-api-key", + savedFeeds: { Recovery: { title: "Recovery" } }, + }); + }); + + it("#431 flushes a pending edit immediately once a runtime read recovers", async () => { + const { plugin, state, saveData } = recovering(); + state.blocked = true; + plugin.settings.skipForwardLength = 25; + await saveInBackground(plugin); + expect(saveData).not.toHaveBeenCalled(); + state.blocked = false; + await plugin.flushSaves(); + expect(saveData).toHaveBeenCalledTimes(1); + expect(saveData.mock.lastCall?.[0]).toMatchObject({ skipForwardLength: 25 }); + }); + + it("#431 restores the idle library once when the unreadable file is moved away", async () => { + const { plugin, state, saveData, clock } = recovering(); + state.blocked = true; + await plugin.onExternalSettingsChange(); + expect(saveData).not.toHaveBeenCalled(); + state.blocked = false; + state.disk = null; + await plugin.onExternalSettingsChange(); + clock.advance(10_000); + clock.runAll(); + await settled(); + expect(saveData).toHaveBeenCalledTimes(1); + expect(saveData.mock.lastCall?.[0]).toMatchObject({ + savedFeeds: { Recovery: { title: "Recovery" } }, + }); + clock.advance(10_000); + clock.runAll(); + await settled(); + expect(saveData).toHaveBeenCalledTimes(1); + }); + + it("#431 recovers an idle read outage without a file event or an edit", async () => { + const { plugin, state, saveData, clock, notices } = recovering(); + state.blocked = true; + await plugin.onExternalSettingsChange(); + clock.advance(10_000); + clock.runAll(); + await settled(); + expect(notices).toHaveBeenCalledTimes(1); + expect(saveData).not.toHaveBeenCalled(); + state.blocked = false; + clock.advance(10_000); + clock.runAll(); + await settled(); + expect(saveData).toHaveBeenCalledTimes(1); + expect(saveData.mock.lastCall?.[0]).toMatchObject({ + savedFeeds: { Recovery: { title: "Recovery" } }, + }); + clock.advance(10_000); + clock.runAll(); + await settled(); + expect(saveData).toHaveBeenCalledTimes(1); + }); + + it("#432 asks for readable startup data and a reload without writing defaults", async () => { + const { plugin, state, saveData, loadData, notices, clock } = recovering(); + state.blocked = true; + await plugin.loadSettings(); + attachDataFileWriter(plugin, clock); + expect + .soft(notices.mock.calls[0]?.[0]) + .toMatch( + /(?:make|restore)[^.]+(?:read|access)[^.]+(?:reload|restart)|(?:readable|read access)[^.]+(?:reload|restart)/i, + ); + state.blocked = false; + loadData.mockClear(); + plugin.settings.skipForwardLength = 25; + await saveInBackground(plugin); + await plugin.flushSaves(); + await expect(plugin.saveSettingsStrict()).rejects.toThrow(); + expect(loadData).not.toHaveBeenCalled(); + expect(saveData).not.toHaveBeenCalled(); + expect(clock.pending()).toBe(0); + }); + + it("retries a file with an invalid schema version until it reads, then writes the library once", async () => { + const { plugin, state, readable, saveData, clock, notices } = recovering(); + state.disk = { ...readable, schemaVersion: "two" }; + await plugin.onExternalSettingsChange(); + expect(notices.mock.calls[0]?.[0]).toMatch(/could not read/); + + state.disk = readable; + clock.advance(10_000); + clock.runAll(); + await settled(); + expect(saveData).toHaveBeenCalledTimes(1); + expect(saveData.mock.lastCall?.[0]).toMatchObject({ + savedFeeds: { Recovery: { title: "Recovery" } }, + }); + }); + + it("starts no recovery write when the window unloads during its read", async () => { + const { plugin, state, saveData, loadData, clock, notices } = recovering(); + state.blocked = true; + await plugin.onExternalSettingsChange(); + state.blocked = false; + let release = () => {}; + loadData.mockImplementationOnce(async () => { + await new Promise((resolve) => (release = resolve)); + return structuredClone(state.disk); + }); + + const flushing = plugin.flushSaves(); + await vi.waitFor(() => expect(loadData).toHaveBeenCalledTimes(2)); + Object.assign(plugin, { closing: "window-unloading" }); + release(); + await flushing; + + expect(saveData).not.toHaveBeenCalled(); + expect(notices.mock.calls.map(([message]) => message)).toEqual([ + "PodNotes could not read .obsidian/plugins/podnotes/data.json, so it leaves the file unchanged. It tries again automatically and saves your changes once the file can be read.", + ]); + expect(clock.pending()).toBe(0); + }); +}); + describe("PodNotes persistence integration", () => { it("loads legacy JSON dates as Date instances", async () => { const plugin = makePlugin(); @@ -735,7 +929,7 @@ describe("PodNotes persistence integration", () => { await vi.waitFor(() => expect(saveData).toHaveBeenCalledTimes(1)); plugin.settings.defaultVolume = 0.25; const queued = plugin.saveSettingsStrict(); - Object.assign(plugin, { writeRefusal: "window-unloading" }); + Object.assign(plugin, { closing: "window-unloading" }); const late = plugin.saveSettingsStrict(); resolvers[0](); @@ -755,7 +949,7 @@ describe("PodNotes persistence integration", () => { vi.spyOn(console, "error").mockImplementation(() => undefined); plugin.settings.defaultVolume = 0.5; - Object.assign(plugin, { writeRefusal: "obsidian-closing" }); + Object.assign(plugin, { closing: "obsidian-closing" }); await (plugin as unknown as { writer: { unload(): Promise } }).writer.unload(); expect(plugin.loadData).toHaveBeenCalledTimes(1); @@ -1173,7 +1367,7 @@ describe("PodNotes persistence integration", () => { const plugin = makePlugin(saveData); Object.assign(plugin, { loadData: vi.fn().mockResolvedValue({ defaultVolume: 0.4 }), - writeRefusal: "window-unloading", + closing: "window-unloading", }); vi.spyOn(console, "error").mockImplementation(() => undefined); @@ -1260,7 +1454,7 @@ describe("PodNotes persistence integration", () => { describe("while data.json cannot be read for a while", () => { const refusal = - "PodNotes could not read .obsidian/plugins/podnotes/data.json, so it leaves the file unchanged and saves nothing until the file can be read again."; + "PodNotes could not read .obsidian/plugins/podnotes/data.json, so it leaves the file unchanged. It tries again automatically and saves your changes once the file can be read."; function setup() { const file = { readable: true, writable: true }; @@ -1314,7 +1508,7 @@ describe("PodNotes persistence integration", () => { expect(clock.delays).toEqual([10_000]); }); - it("saves an edit made once it reads again, after the edits it could not save were undone", async () => { + it("keeps retrying after the edits it could not save were undone, and saves an edit made once it reads again", async () => { const { plugin, file, clock, notice, retry } = setup(); const hide = vi.spyOn(Notice.prototype, "hide"); file.readable = false; @@ -1323,21 +1517,20 @@ describe("PodNotes persistence integration", () => { plugin.settings.skipForwardLength = 15; await saveInBackground(plugin); await retry(); - expect(clock.pending()).toBe(0); + expect(clock.pending()).toBe(1); file.readable = true; plugin.settings.skipBackwardLength = 33; await saveInBackground(plugin); await saveInBackground(plugin); const armed = { pending: clock.pending(), delays: [...clock.delays] }; - clock.runAll(); - await settled(); + await retry(); expect(await plugin.loadData()).toMatchObject({ skipForwardLength: 15, skipBackwardLength: 33, }); - expect(armed).toEqual({ pending: 1, delays: [10_000, 0] }); + expect(armed).toEqual({ pending: 1, delays: [10_000, 10_000] }); expect(notice.mock.calls).toEqual([[refusal, 0]]); expect(hide).toHaveBeenCalledTimes(1); expect(clock.pending()).toBe(0); @@ -1461,7 +1654,7 @@ describe("PodNotes persistence integration", () => { disk.full = false; disk.text = '{"schemaVersion": 2, "savedFeeds": {"Other device'; await plugin.onExternalSettingsChange(); - await expect(plugin.saveSettingsStrict()).rejects.toThrow(/does not save over/); + await expect(plugin.saveSettingsStrict()).rejects.toThrow(/could not read/); disk.text = ""; await plugin.onExternalSettingsChange(); @@ -1478,7 +1671,7 @@ describe("PodNotes persistence integration", () => { const before = disk.text; await plugin.onExternalSettingsChange(); plugin.settings.defaultVolume = 0.25; - await expect(plugin.saveSettingsStrict()).rejects.toThrow(/does not save over/); + await expect(plugin.saveSettingsStrict()).rejects.toThrow(/could not read/); expect(disk.text).toBe(before); disk.denied = false; @@ -1506,8 +1699,7 @@ describe("PodNotes persistence integration", () => { disk.text = '{"schemaVersion": 2, "savedFeeds": {"Other device'; plugin.settings.defaultVolume = 0.25; await expect(plugin.saveSettingsStrict()).rejects.toThrow(/could not read/); - // Refused now, with the Notice up: no save even tries to read the file. - await expect(plugin.saveSettingsStrict()).rejects.toThrow(/does not save over/); + await expect(plugin.saveSettingsStrict()).rejects.toThrow(/could not read/); const hideNotice = vi.spyOn(Notice.prototype, "hide"); disk.text = readable; @@ -1753,7 +1945,7 @@ describe("a device that started before sync delivered its data.json", () => { expect(newDevice.plugin.settings).toEqual(established.plugin.settings); }); - it("keeps the library when a device that could not read its file at start writes a new one", async () => { + it("keeps the library when a device that could not read its file at start finds it emptied, then synced", async () => { const established = await startDevice(library); const device = makePlugin(); let text = '{"schemaVersion": 2, "savedFeeds": {"Library Fe'; @@ -1774,6 +1966,12 @@ describe("a device that started before sync delivered its data.json", () => { text = ""; await device.onExternalSettingsChange(); device.settings.defaultVolume = 0.5; + await expect(device.saveSettingsStrict()).rejects.toThrow(/could not read/); + expect(text).toBe(""); + + text = dataFileText(established.file()); + await device.onExternalSettingsChange(); + device.settings.defaultVolume = 0.5; await device.saveSettingsStrict(); await established.receive(JSON.parse(text)); diff --git a/src/main.ts b/src/main.ts index abe17448..d58d4ed8 100644 --- a/src/main.ts +++ b/src/main.ts @@ -33,6 +33,7 @@ import { PlayerController } from "./player/PlayerController"; import { bindMediaSession } from "./player/mediaSession"; import { decodePodNotesData, + type DecodedPodNotesData, encodePodNotesData, PODNOTES_DATA_SCHEMA_VERSION, newLibraryId, @@ -45,6 +46,8 @@ import { type Clock, type DataFileAccess, DataFileWriter, + type ReadOutcome, + type Refusal, WriteRefusedError, } from "./persistence/writer"; import { CredentialRepository } from "./services/CredentialRepository"; @@ -55,15 +58,9 @@ import { FeedRepository, loadFeed } from "./services/FeedRepository"; import { fetchChapters } from "./utility/fetchChapters"; import { onlineStatus } from "./services/online"; -type WriteRefusal = - | "unreadable-data-file" - | "older-data-file" - | "window-unloading" - | "obsidian-closing"; +type Closing = "window-unloading" | "obsidian-closing"; -const WRITE_REFUSALS: Record = { - "unreadable-data-file": "PodNotes does not save over a data.json it could not read.", - "older-data-file": "PodNotes does not save over a data.json from an older version.", +const CLOSING_REFUSALS: Record = { "window-unloading": "PodNotes does not start a save while the window unloads.", "obsidian-closing": "PodNotes does not start a save while Obsidian closes.", }; @@ -118,12 +115,10 @@ export default class PodNotes extends Plugin implements IPodNotes { private freshInstall = false; private podcastViewMountEnabled = true; private isReady = false; - // Why data.json must not be written, or null while writes are allowed. Set when the - // file exists but could not be read (a sync client's half-written copy, or one a - // reload cut off), when an older version's copy replaced it, and when the window - // starts unloading or Obsidian quits: a write started then is cut off after it - // truncates the file, which leaves data.json empty. - private writeRefusal: WriteRefusal | null = null; + // Set once the window starts unloading or Obsidian quits: a write started then is + // cut off after it truncates the file, which leaves data.json empty. + private closing: Closing | null = null; + private startupRefusal: Refusal | null = null; // The data.json write in progress, settled either way, or null. Writes never overlap. private writeInFlight: Promise | null = null; // Every save after load, and every change synced in from elsewhere, goes through it. @@ -135,14 +130,14 @@ export default class PodNotes extends Plugin implements IPodNotes { this.isUnloaded = false; this.podcastViewMountEnabled = !this.isMobileRuntime(); this.registerDomEvent(window, "pagehide", () => { - this.writeRefusal = "window-unloading"; + this.closing = "window-unloading"; }); // Obsidian closes the window once its quit tasks settle, so start no write after // quit and let a write in flight finish first. A reload fires no quit; pagehide // covers it. A rejected quit task would stall the close until Obsidian forces it. this.registerEvent( this.app.workspace.on("quit", (tasks) => { - this.writeRefusal = "obsidian-closing"; + this.closing = "obsidian-closing"; if (this.writeInFlight) tasks.addPromise(this.writeInFlight); }), ); @@ -373,10 +368,11 @@ export default class PodNotes extends Plugin implements IPodNotes { if (data == null && !(await this.dataFileHoldsNothing())) { this.settings = decodePodNotesData(undefined).settings; this.persistenceUnknownFields = { libraryId: newLibraryId() }; - this.refuseSaves( - "unreadable-data-file", - `PodNotes could not read ${this.dataFilePath()}, so it left the file unchanged and started with default settings. Nothing is saved until the file can be read again, or until you delete it and restart Obsidian.`, - ); + this.startupRefusal = { + recovery: "retry", + message: `PodNotes could not read ${this.dataFilePath()}, so it left the file unchanged and started with default settings. Nothing is saved until you restore read access to the file and restart Obsidian.`, + }; + this.showRefusal(this.startupRefusal); console.error( `PodNotes could not read ${this.dataFilePath()} and will not save over it`, ); @@ -459,16 +455,14 @@ export default class PodNotes extends Plugin implements IPodNotes { } saveSettings(): Promise { - if (!this.isReady) return Promise.resolve(); - if (!this.writeRefusal) this.writer?.request(); - else if (this.writeRefusal === "unreadable-data-file") this.writer?.retryPending(); + if (this.isReady && !this.closing) this.writer?.request(); return Promise.resolve(); } /** Awaitable save for migrations/imports that must not report success on failure. */ saveSettingsStrict(): Promise { if (!this.isReady || !this.writer) return Promise.resolve(); - if (this.writeRefusal) return Promise.reject(new Error(WRITE_REFUSALS[this.writeRefusal])); + if (this.closing) return Promise.reject(new Error(CLOSING_REFUSALS[this.closing])); return this.writer.save(); } @@ -480,8 +474,7 @@ export default class PodNotes extends Plugin implements IPodNotes { /** Write what is not saved yet now. Never rejects. */ flushSaves(): Promise { - // A refused write would only fail again. - if (this.writeRefusal) return Promise.resolve(); + if (this.closing) return Promise.resolve(); return this.writer?.flush() ?? Promise.resolve(); } @@ -490,39 +483,23 @@ export default class PodNotes extends Plugin implements IPodNotes { await this.writer.onExternalChange(); } - /** The file can be taken as it is again: saves resume. */ - private liftUnreadableRefusal(): void { - if (this.writeRefusal !== "unreadable-data-file") return; - this.writeRefusal = null; + private showRefusal(refusal: Refusal | null): void { this.refusalNotice?.hide(); - this.refusalNotice = null; - } - - /** No save until `refusal` lifts, if it does, with a Notice saying why. */ - private refuseSaves( - refusal: "unreadable-data-file" | "older-data-file", - message: string, - ): void { - // Closing refusals never lift; this one would. - if ( - this.isUnloaded || - this.writeRefusal === "window-unloading" || - this.writeRefusal === "obsidian-closing" - ) { - return; - } - if (this.writeRefusal === refusal) return; - this.writeRefusal = refusal; - this.refusalNotice?.hide(); - this.refusalNotice = new Notice(message, 0); + this.refusalNotice = + refusal && !this.isUnloaded && !this.closing ? new Notice(refusal.message, 0) : null; } private createWriter(clock: Clock): DataFileWriter { - return new DataFileWriter( + return new DataFileWriter( this.dataFileAccess(), clock, - this.writeRefusal ? null : this.localDocument(), - () => new Notice("PodNotes could not save your changes."), + this.startupRefusal + ? { base: null, refusal: this.startupRefusal } + : { base: this.localDocument() }, + { + onRefusal: (refusal) => this.showRefusal(refusal), + onWriteFailed: () => new Notice("PodNotes could not save your changes."), + }, ); } @@ -536,35 +513,46 @@ export default class PodNotes extends Plugin implements IPodNotes { private dataFileAccess(): DataFileAccess { return { - read: async (base) => { + read: async (base): Promise> => { const data: unknown = await this.loadData(); const path = this.dataFilePath(); + const unreadable: ReadOutcome = { + kind: "refused", + refusal: { + recovery: "retry", + message: `PodNotes could not read ${path}, so it leaves the file unchanged. It tries again automatically and saves your changes once the file can be read.`, + }, + }; if (data == null) { - if (await this.dataFileHoldsNothing()) { - this.liftUnreadableRefusal(); - return null; - } - this.refuseSaves( - "unreadable-data-file", - `PodNotes could not read ${path}, so it leaves the file unchanged and saves nothing until the file can be read again.`, - ); - throw new Error(`PodNotes could not read ${path}`); + return (await this.dataFileHoldsNothing()) ? { kind: "missing" } : unreadable; + } + let decoded: DecodedPodNotesData; + try { + decoded = decodePodNotesData(data); + } catch (error) { + if (!(error instanceof PodNotesDataError)) throw error; + if (error.code === "invalid-data") return unreadable; + return { + kind: "refused", + refusal: { + recovery: "stop", + message: `${path} was written by a newer version of PodNotes, likely synced from another device. PodNotes saves nothing until you update PodNotes.`, + }, + }; } - const decoded = decodePodNotesData(data); // Only startup moves an older file's API keys into SecretStorage; a save now // would drop them. if (decoded.sourceVersion !== PODNOTES_DATA_SCHEMA_VERSION) { - this.refuseSaves( - "older-data-file", - `${path} was replaced by a copy from an older version of PodNotes, likely synced from another device. PodNotes saves nothing until you restart Obsidian, which updates the file and keeps its API keys.`, - ); - throw new Error(WRITE_REFUSALS["older-data-file"]); + return { + kind: "refused", + refusal: { + recovery: "stop", + message: `${path} was replaced by a copy from an older version of PodNotes, likely synced from another device. PodNotes saves nothing until you restart Obsidian, which updates the file and keeps its API keys.`, + }, + }; } // With nothing loaded, an entry the codec drops has nothing to be kept from. - if (!base && decoded.changed) { - throw new Error(`PodNotes could not read ${path} without repairing it`); - } - this.liftUnreadableRefusal(); + if (!base && decoded.changed) return unreadable; // A private feed synced here raw is protected before it can be merged and written. const protection = protectPrivateFeeds(decoded.settings, this.feedUrls); const addresses = new Map([ @@ -584,7 +572,10 @@ export default class PodNotes extends Plugin implements IPodNotes { doc[key] = restoreDropped(value, doc[key], raw[key], [key]); } } - return redactPrivateFeedAddresses(doc, addresses) as PersistedPodNotesDataV2; + return { + kind: "file", + doc: redactPrivateFeedAddresses(doc, addresses) as PersistedPodNotesDataV2, + }; }, write: (doc) => this.writeProtectedDataFile( @@ -622,7 +613,6 @@ export default class PodNotes extends Plugin implements IPodNotes { this.player?.restore(this.settings.currentEpisode); } - // Every data.json write goes through here, so no path can write while writes are refused. /** * No data.json, or an empty one, as a write cut off on a full disk leaves: nothing * to lose. False when the file cannot be checked. @@ -641,7 +631,7 @@ export default class PodNotes extends Plugin implements IPodNotes { data: unknown, privateFeedAddresses: ReadonlyMap, ): Promise { - if (this.writeRefusal) throw new WriteRefusedError(WRITE_REFUSALS[this.writeRefusal]); + if (this.closing) throw new WriteRefusedError(CLOSING_REFUSALS[this.closing]); if (redactPrivateFeedAddresses(data, privateFeedAddresses) !== data) { throw new Error("PodNotes does not write a private feed URL to data.json."); } diff --git a/src/main.undo.test.ts b/src/main.undo.test.ts index 27a0269e..9e276e27 100644 --- a/src/main.undo.test.ts +++ b/src/main.undo.test.ts @@ -49,7 +49,7 @@ async function loadPlugin(secrets: obsidian.SecretStorage): Promise { registerObsidianProtocolHandler: vi.fn(), registerEvent: vi.fn(), registerDomEvent: vi.fn(), - writeRefusal: null, + closing: null, writeInFlight: null, storeUnsubscribers: [], views: new Set(), diff --git a/src/persistence/writer.test.ts b/src/persistence/writer.test.ts index 8f7b0a75..8eb4b2d5 100644 --- a/src/persistence/writer.test.ts +++ b/src/persistence/writer.test.ts @@ -1,6 +1,12 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { manualClock } from "../../tests/mocks/dataFileWriter"; -import { type DataFileAccess, DataFileWriter } from "./writer"; +import { + type DataFileAccess, + DataFileWriter, + type ReadOutcome, + type Refusal, + type WriterStart, +} from "./writer"; type Doc = { defaultVolume: number; @@ -16,6 +22,20 @@ function doc(): Doc { }; } +const STARTUP: Refusal = { recovery: "retry", message: "could not read at startup" }; +const UNREADABLE: Refusal = { recovery: "retry", message: "could not read" }; +const NEWER: Refusal = { recovery: "stop", message: "update PodNotes" }; + +function found(disk: Doc | null): ReadOutcome { + return disk ? { kind: "file", doc: structuredClone(disk) } : { kind: "missing" }; +} + +function refused(refusal: Refusal): ReadOutcome { + return { kind: "refused", refusal }; +} + +const settled = () => new Promise((resolve) => setTimeout(resolve, 0)); + /** data.json and the in-memory state as two plain documents. */ function memoryFile(initial: Doc = doc()) { const state = { @@ -26,7 +46,7 @@ function memoryFile(initial: Doc = doc()) { onPublish: () => {}, }; const access = { - read: vi.fn(async (): Promise => structuredClone(state.disk)), + read: vi.fn(async (): Promise> => found(state.disk)), write: vi.fn(async (next: Doc) => { state.writes.push(structuredClone(next)); state.disk = structuredClone(next); @@ -41,11 +61,12 @@ function memoryFile(initial: Doc = doc()) { return { state, access }; } -function setup(initial: Doc = doc()) { +function setup(initial: Doc = doc(), start: WriterStart = { base: structuredClone(initial) }) { const file = memoryFile(initial); const clock = manualClock(); - const writer = new DataFileWriter(file.access, clock, structuredClone(initial)); - return { ...file, clock, writer }; + const events = { onRefusal: vi.fn(), onWriteFailed: vi.fn() }; + const writer = new DataFileWriter(file.access, clock, start, events); + return { ...file, clock, events, writer }; } afterEach(() => { @@ -182,10 +203,7 @@ describe("DataFileWriter saves", () => { await writer.save(); let releaseRead = () => {}; access.read.mockImplementationOnce( - () => - new Promise( - (resolve) => (releaseRead = () => resolve(structuredClone(state.disk))), - ), + () => new Promise((resolve) => (releaseRead = () => resolve(found(state.disk)))), ); const external = writer.onExternalChange(); await vi.waitFor(() => expect(access.read).toHaveBeenCalledTimes(2)); @@ -358,45 +376,15 @@ describe("DataFileWriter saves", () => { }); it("writes nothing over a file it cannot read, even after a failed write", async () => { - const { access, state, writer } = setup(); + const { access, clock, state, writer } = setup(); access.write.mockRejectedValueOnce(new Error("disk full")); state.memory.defaultVolume = 0.5; await expect(writer.save()).rejects.toThrow("disk full"); - access.read.mockRejectedValue(new Error("newer schema")); - await expect(writer.save()).rejects.toThrow("newer schema"); + access.read.mockResolvedValue(refused(NEWER)); + await expect(writer.save()).rejects.toThrow("update PodNotes"); expect(access.write).toHaveBeenCalledTimes(1); - }); - - it("schedules no retry of a failed read before anything was loaded", async () => { - const file = memoryFile(); - const clock = manualClock(); - const writer = new DataFileWriter(file.access, clock, null); - file.access.read.mockRejectedValue(new Error("busy")); - - await expect(writer.save()).rejects.toThrow("busy"); - - expect(clock.pending()).toBe(0); - }); - - it("retries pending work once, 10 s after the last save, and nothing when nothing is pending", async () => { - const { access, clock, state, writer } = setup(); - writer.retryPending(); - expect(clock.pending()).toBe(0); - - state.memory.defaultVolume = 0.5; - await writer.save(); - clock.advance(4_000); - state.memory.defaultVolume = 0.25; - writer.retryPending(); - writer.retryPending(); - expect(clock.delays).toEqual([6_000]); - - clock.advance(6_000); - clock.runAll(); - await vi.waitFor(() => expect(state.disk?.defaultVolume).toBe(0.25)); - expect(access.write).toHaveBeenCalledTimes(2); expect(clock.pending()).toBe(0); }); @@ -492,29 +480,199 @@ describe("DataFileWriter changes made elsewhere", () => { }); it("leaves memory alone when the file is missing or cannot be read", async () => { - const { access, state, writer } = setup(); - const consoleWarn = vi.spyOn(console, "warn").mockImplementation(() => undefined); + const { access, events, state, writer } = setup(); state.disk = null; await writer.onExternalChange(); - access.read.mockRejectedValueOnce(new SyntaxError("Unexpected end of JSON input")); + access.read.mockResolvedValueOnce(refused(UNREADABLE)); await writer.onExternalChange(); expect(state.published).toHaveLength(0); - expect(consoleWarn).toHaveBeenCalledTimes(1); + expect(state.writes).toHaveLength(0); + expect(events.onRefusal.mock.calls).toEqual([[UNREADABLE]]); }); it("adopts the file when it had nothing to merge against", async () => { - const file = memoryFile(); - const writer = new DataFileWriter(file.access, manualClock(), null); - file.state.memory.defaultVolume = 0.3; - file.state.disk!.savedFeeds.Synced = { title: "Synced" }; + const { access, events, state, writer } = setup(doc(), { base: null, refusal: STARTUP }); + state.memory.defaultVolume = 0.3; + state.disk!.savedFeeds.Synced = { title: "Synced" }; + + await writer.onExternalChange(); + + expect(state.memory).toEqual(state.disk); + expect(events.onRefusal.mock.calls).toEqual([[null]]); + await writer.save(); + expect(access.write).not.toHaveBeenCalled(); + }); +}); + +describe("DataFileWriter refusals", () => { + it("reads and writes nothing before a file was loaded, even when flushed or unloaded", async () => { + const { access, clock, state, writer } = setup(doc(), { base: null, refusal: STARTUP }); + state.memory.defaultVolume = 0.5; + + writer.request(); + writer.checkpoint(() => writer.request()); + await writer.flush(); + expect(clock.pending()).toBe(0); + await expect(writer.save()).rejects.toThrow("could not read at startup"); + await writer.unload(); + + expect(access.read).not.toHaveBeenCalled(); + expect(state.writes).toHaveLength(0); + }); + + it("keeps refusing before a file was loaded when the file is missing, empty or unreadable", async () => { + const { access, events, state, writer } = setup(doc(), { base: null, refusal: STARTUP }); + state.memory.defaultVolume = 0.5; + state.disk = null; + await writer.onExternalChange(); + access.read.mockResolvedValueOnce(refused(UNREADABLE)); + await writer.onExternalChange(); + + await expect(writer.save()).rejects.toThrow("could not read at startup"); + await writer.flush(); + expect(access.read).toHaveBeenCalledTimes(2); + expect(state.writes).toHaveLength(0); + expect(events.onRefusal).not.toHaveBeenCalled(); + }); + + it("replaces startup protection with a schema stop and reads no further", async () => { + const { access, clock, events, state, writer } = setup(doc(), { + base: null, + refusal: STARTUP, + }); + state.memory.defaultVolume = 0.5; + access.read.mockResolvedValue(refused(NEWER)); + await writer.onExternalChange(); + await writer.onExternalChange(); + writer.request(); + await writer.flush(); + + await expect(writer.save()).rejects.toThrow("update PodNotes"); + expect(access.read).toHaveBeenCalledTimes(1); + expect(state.writes).toHaveLength(0); + expect(clock.pending()).toBe(0); + expect(events.onRefusal.mock.calls).toEqual([[NEWER]]); + }); + + it("retries a refused read 10 s after the last attempt, also with edits, and writes once it reads", async () => { + const { access, clock, events, state, writer } = setup(); + vi.spyOn(console, "error").mockImplementation(() => undefined); + access.read.mockResolvedValue(refused(UNREADABLE)); + clock.advance(60_000); + await writer.onExternalChange(); + expect(clock.delays).toEqual([10_000]); + + clock.advance(5_000); + state.memory.defaultVolume = 0.5; + writer.request(); + await settled(); + expect(access.read).toHaveBeenCalledTimes(1); + + clock.advance(5_000); + clock.runAll(); + await settled(); + expect(access.read).toHaveBeenCalledTimes(2); + expect(clock.delays).toEqual([10_000, 10_000]); + + access.read.mockImplementation(async () => found(state.disk)); + clock.advance(10_000); + clock.runAll(); + await settled(); + expect(state.writes).toEqual([{ ...doc(), defaultVolume: 0.5 }]); + expect(events.onRefusal.mock.calls).toEqual([[UNREADABLE], [null]]); + expect(clock.pending()).toBe(0); + }); + + it("writes the library once when the file reads as missing again, even without an edit", async () => { + const { access, clock, state, writer } = setup(); + access.read.mockResolvedValueOnce(refused(UNREADABLE)); + await writer.onExternalChange(); + state.disk = null; + + await writer.onExternalChange(); + expect(state.writes).toEqual([doc()]); + + clock.advance(10_000); + clock.runAll(); + await writer.flush(); + await writer.onExternalChange(); + expect(access.write).toHaveBeenCalledTimes(1); + }); + + it("writes the library again when its recovery write fails", async () => { + const { access, clock, events, state, writer } = setup(); + vi.spyOn(console, "error").mockImplementation(() => undefined); + access.read.mockResolvedValueOnce(refused(UNREADABLE)); + await writer.onExternalChange(); + state.disk = null; + access.write.mockRejectedValueOnce(new Error("disk full")); + + clock.advance(10_000); + clock.runAll(); + await settled(); + expect(state.disk).toBeNull(); + expect(events.onWriteFailed).toHaveBeenCalledTimes(1); + + clock.advance(10_000); + clock.runAll(); + await settled(); + expect(state.disk).toEqual(doc()); + expect(access.write).toHaveBeenCalledTimes(2); + expect(clock.pending()).toBe(0); + }); + it("reports a refusal once per streak, and again after a recovery", async () => { + const { access, events, state, writer } = setup(); + access.read.mockResolvedValue(refused(UNREADABLE)); + await writer.onExternalChange(); await writer.onExternalChange(); + await expect(writer.save()).rejects.toThrow("could not read"); - expect(file.state.memory).toEqual(file.state.disk); + access.read.mockImplementation(async () => found(state.disk)); await writer.save(); - expect(file.access.write).not.toHaveBeenCalled(); + access.read.mockResolvedValue(refused(UNREADABLE)); + await writer.onExternalChange(); + + expect(events.onRefusal.mock.calls).toEqual([[UNREADABLE], [null], [UNREADABLE]]); + }); + + it("stops for a schema it must not write: no checkpoint, read or write until the next instance", async () => { + const { access, clock, events, state, writer } = setup(); + vi.spyOn(console, "error").mockImplementation(() => undefined); + access.read.mockResolvedValueOnce(refused(UNREADABLE)); + await writer.onExternalChange(); + let release = () => {}; + access.read.mockImplementationOnce( + () => new Promise((resolve) => (release = () => resolve(refused(NEWER)))), + ); + const external = writer.onExternalChange(); + const queuedSave = writer.save(); + const queuedExternal = writer.onExternalChange(); + await vi.waitFor(() => expect(access.read).toHaveBeenCalledTimes(2)); + state.memory.playedEpisodes["Pod::Ep"] = { time: 20 }; + writer.checkpoint(() => writer.request()); + expect(clock.pending()).toBe(1); + release(); + await external; + await expect(queuedSave).rejects.toThrow("update PodNotes"); + await queuedExternal; + expect(clock.pending()).toBe(0); + + state.memory.defaultVolume = 0.5; + writer.request(); + writer.checkpoint(() => writer.request()); + await writer.flush(); + await writer.onExternalChange(); + expect(clock.pending()).toBe(0); + await expect(writer.save()).rejects.toThrow("update PodNotes"); + await writer.unload(); + + expect(access.read).toHaveBeenCalledTimes(2); + expect(state.writes).toHaveLength(0); + expect(clock.pending()).toBe(0); + expect(events.onRefusal.mock.calls).toEqual([[UNREADABLE], [NEWER]]); }); }); @@ -525,7 +683,7 @@ describe("DataFileWriter lifecycle", () => { const held = new Promise((resolve) => (release = resolve)); access.read.mockImplementationOnce(async () => { await held; - return structuredClone(state.disk); + return found(state.disk); }); state.disk!.savedFeeds.Synced = { title: "Synced" }; const external = writer.onExternalChange(); @@ -543,7 +701,7 @@ describe("DataFileWriter lifecycle", () => { const held = new Promise((resolve) => (release = resolve)); access.read.mockImplementationOnce(async () => { await held; - return structuredClone(state.disk); + return found(state.disk); }); state.memory.defaultVolume = 0.5; @@ -563,7 +721,7 @@ describe("DataFileWriter lifecycle", () => { const held = new Promise((resolve) => (release = resolve)); access.read.mockImplementationOnce(async () => { await held; - return structuredClone(state.disk); + return found(state.disk); }); state.memory.defaultVolume = 0.5; writer.request(); @@ -611,7 +769,7 @@ describe("DataFileWriter lifecycle", () => { it("writes an edit whose save could not read the file when flushed", async () => { const { access, state, writer } = setup(); const consoleError = vi.spyOn(console, "error").mockImplementation(() => undefined); - access.read.mockRejectedValueOnce(new Error("busy")); + access.read.mockResolvedValueOnce(refused(UNREADABLE)); state.memory.defaultVolume = 0.5; writer.request(); await vi.waitFor(() => expect(consoleError).toHaveBeenCalled()); diff --git a/src/persistence/writer.ts b/src/persistence/writer.ts index c863fadb..b125906b 100644 --- a/src/persistence/writer.ts +++ b/src/persistence/writer.ts @@ -15,13 +15,27 @@ export class WriteRefusedError extends Error { } } +export interface Refusal { + /** + * retry: a later read may succeed, as once a sync client finishes its write. + * stop: the file has a schema this version must not write; only the next instance reads it. + */ + readonly recovery: "retry" | "stop"; + readonly message: string; +} + +export type ReadOutcome = + | { readonly kind: "file"; readonly doc: Doc } + | { readonly kind: "missing" } + | { readonly kind: "refused"; readonly refusal: Refusal }; + /** How the writer reaches data.json and the in-memory state, as plain JSON documents. */ export interface DataFileAccess { /** - * The file now, or null when it does not exist. Rejects when it cannot be read. - * Entries that had to be dropped to read it are reported as `base` has them. + * The file now. Entries that had to be dropped to read it are reported as `base` + * has them. Rejects only on a defect, which is not retried. */ - read(base: Doc | null): Promise; + read(base: Doc | null): Promise>; write(doc: Doc): Promise; /** The in-memory state as the document it would write. */ local(): Doc; @@ -29,6 +43,17 @@ export interface DataFileAccess { publish(doc: Doc): void; } +export type WriterStart = + | { readonly base: Doc } + | { readonly base: null; readonly refusal: Refusal }; + +export interface WriterEvents { + /** A refusal began or changed its reason, or ended (null). */ + onRefusal(refusal: Refusal | null): void; + /** A save's write failed; once per failure streak. */ + onWriteFailed(): void; +} + // 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; @@ -42,6 +67,10 @@ const CHECKPOINT_INTERVAL_MS = 10_000; * Obsidian's loadData and saveData offer no compare-and-swap, so a change that * lands between a save's read and its write is overwritten. The read happens * right before the write to keep that window short. + * + * A read that cannot be used refuses writes. Obsidian reports no rename, delete or + * permission change of the file, so a loaded library retries on the checkpoint + * interval and is written once a read succeeds, edited or not. */ export class DataFileWriter { private chain: Promise | null = null; @@ -57,23 +86,28 @@ export class DataFileWriter { private lastSaveAt = Number.NEGATIVE_INFINITY; /** A write that failed, and the memory it was made from. It may have reached disk anyway. */ private failedWrite: { local: Doc; written: Doc } | null = null; + /** What PodNotes last loaded or wrote; null until a readable file was loaded. */ + private base: Doc | null; + private refusal: Refusal | null; constructor( private readonly file: DataFileAccess, private readonly clock: Clock, - /** What PodNotes last loaded or wrote; null until a readable file was loaded. */ - private base: Doc | null, - private readonly onWriteFailed: () => void = () => {}, - ) {} + start: WriterStart, + private readonly events: WriterEvents, + ) { + this.base = start.base; + this.refusal = "refusal" in start ? start.refusal : null; + } /** * Save after any save already running; changes made until it starts share it. - * Inside `checkpoint`, wait for the checkpoint interval instead. + * Inside `checkpoint`, or while a read is refused, wait for the checkpoint interval instead. */ request(): void { // Publishing sets stores, whose bindings ask for a save of what is already on disk. if (this.publishing) return; - if (this.checkpointing) { + if (this.checkpointing || this.refusal) { this.scheduleCheckpoint(); return; } @@ -93,21 +127,14 @@ export class DataFileWriter { /** Write what is not saved yet now. Resolves once queued work is done; never rejects. */ flush(): Promise { - if (this.timer !== null || this.failedWrite || this.hasUnsavedEdits()) { + if (!this.blocked() && (this.timer !== null || this.pending())) { this.cancelTimer(); if (!this.saveQueued) void this.saveLogged(); } return (this.chain ?? Promise.resolve()).catch(() => undefined); } - /** Retry pending work at the checkpoint interval; nothing before a file was loaded. */ - retryPending(): void { - if (this.base !== null && (this.failedWrite || this.hasUnsavedEdits())) { - this.scheduleCheckpoint(); - } - } - - /** Save now. Rejects when the save fails; a failed write is the caller's to report. */ + /** Save now. Rejects when the save fails or is refused; a failed write is the caller's to report. */ save(): Promise { this.cancelTimer(); return this.enqueue(() => this.persist(false)).catch((error: unknown) => { @@ -152,7 +179,7 @@ export class DataFileWriter { } private scheduleCheckpoint(): void { - if (this.unloaded) return; + if (this.unloaded || this.blocked()) return; this.timer ??= this.clock.setTimeout( () => { this.timer = null; @@ -162,6 +189,32 @@ export class DataFileWriter { ); } + private blocked(): Refusal | null { + const refusal = this.refusal; + return refusal && (refusal.recovery === "stop" || this.base === null) ? refusal : null; + } + + private pending(): boolean { + return this.refusal !== null || this.failedWrite !== null || this.hasUnsavedEdits(); + } + + private refuse(refusal: Refusal): void { + this.lastSaveAt = this.clock.now(); + const replaces = this.base !== null || refusal.recovery === "stop"; + if (replaces && this.refusal?.message !== refusal.message) { + this.refusal = refusal; + this.events.onRefusal(refusal); + } + if (this.blocked()) this.cancelTimer(); + else this.scheduleCheckpoint(); + } + + private lift(): void { + if (!this.refusal) return; + this.refusal = null; + this.events.onRefusal(null); + } + private local(): Doc { return this.memoryAtUnload ?? this.file.local(); } @@ -197,20 +250,23 @@ export class DataFileWriter { } private async persist(warnOnFailure: boolean): Promise { - // After a failed write the file is unknown, so memory equal to base proves nothing. - if (this.base && !this.failedWrite && !this.hasUnsavedEdits()) return; - // Before the read settles a failed write the file holds, which does not end its streak. - const reportFailure = warnOnFailure && !this.failedWrite; + const blocked = this.blocked(); + if (blocked) throw new WriteRefusedError(blocked.message); + if (!this.pending()) return; this.lastSaveAt = this.clock.now(); - let disk: Doc | null; - try { - disk = await this.file.read(this.base); - } catch (error) { - // A failed read can refuse later saves until a read succeeds, so retry what is pending. - if (this.failedWrite || this.hasUnsavedEdits()) this.scheduleCheckpoint(); - throw error; + const read = await this.file.read(this.base); + if (read.kind === "refused") { + this.refuse(read.refusal); + throw new WriteRefusedError(read.refusal.message); } + await this.writeMerged(read.kind === "file" ? read.doc : null, warnOnFailure); + } + + private async writeMerged(disk: Doc | null, warnOnFailure: boolean): Promise { + this.lift(); + // Before the read settles a failed write the file holds, which does not end its streak. + const reportFailure = warnOnFailure && !this.failedWrite; this.settleFailedWrite(disk); const base = this.base; // Captured after the read, so edits made while it was in flight are written too. @@ -228,7 +284,7 @@ export class DataFileWriter { await this.file.write(toWrite); } catch (error) { if (error instanceof WriteRefusedError) throw error; - if (reportFailure) this.onWriteFailed(); + if (reportFailure) this.events.onWriteFailed(); this.failedWrite = { local, written: toWrite }; this.scheduleCheckpoint(); throw error; @@ -260,16 +316,20 @@ export class DataFileWriter { } private async applyExternalChange(): Promise { - let disk: Doc | null; - try { - disk = await this.file.read(this.base); - } catch (error) { - // A sync client can leave a half-written file; its complete write calls again. - console.warn("PodNotes could not read its changed data.json", error); + if (this.refusal?.recovery === "stop") return; + const read = await this.file.read(this.base); + if (this.unloaded) return; + if (read.kind === "refused") { + this.refuse(read.refusal); + return; + } + const disk = read.kind === "file" ? read.doc : null; + if (this.refusal && this.base !== null) { + this.lastSaveAt = this.clock.now(); + await this.writeMerged(disk, true); return; } - // A missing file is not a request to reset everything. - if (!disk || this.unloaded) return; + if (!disk) return; this.settleFailedWrite(disk); const base = this.base; @@ -278,6 +338,7 @@ export class DataFileWriter { const local = this.local(); const merged = base ? mergeDataFile(base, local, disk) : structuredClone(disk); this.base = disk; + this.lift(); if (!jsonValuesEqual(merged, local)) this.publish(merged); if (jsonValuesEqual(merged, disk)) this.cancelTimer(); else this.request(); diff --git a/src/ui/settings/PodNotesSettingsTab.import.test.ts b/src/ui/settings/PodNotesSettingsTab.import.test.ts index bed0ca3b..75327e29 100644 --- a/src/ui/settings/PodNotesSettingsTab.import.test.ts +++ b/src/ui/settings/PodNotesSettingsTab.import.test.ts @@ -776,11 +776,12 @@ describe("PodNotesSettingsTab settings import", () => { clock.advance(10_000); clock.runAll(); await settled(); - expect(clock.pending()).toBe(0); + expect(clock.pending()).toBe(1); readable = true; savedFeeds.update((feeds) => ({ ...feeds, Later: feed("Later") })); const armed = clock.pending(); + clock.advance(10_000); clock.runAll(); await settled(); @@ -793,7 +794,7 @@ describe("PodNotesSettingsTab settings import", () => { expect(clock.pending()).toBe(0); expect(notice.mock.calls).toEqual([ [ - "PodNotes could not read .obsidian/plugins/podnotes/data.json, so it leaves the file unchanged and saves nothing until the file can be read again.", + "PodNotes could not read .obsidian/plugins/podnotes/data.json, so it leaves the file unchanged. It tries again automatically and saves your changes once the file can be read.", 0, ], [ diff --git a/tests/e2e/save-refusal-recovery.test.ts b/tests/e2e/save-refusal-recovery.test.ts new file mode 100644 index 00000000..6ca0dd66 --- /dev/null +++ b/tests/e2e/save-refusal-recovery.test.ts @@ -0,0 +1,317 @@ +import { createHash } from "node:crypto"; +import { mkdir, readFile, rename, rm, writeFile } from "node:fs/promises"; +import { resolve } from "node:path"; +import type { ObsidianClient } from "obsidian-e2e"; +import { describe, expect, onTestFinished, test } from "vitest"; +import { createPodNotesE2EHarness, type PodNotesData } from "./harness"; +import { closeSettings, openPodNotesSettings } from "./journey/actions"; +import { chooseFile, click, typeInto } from "./journey/input"; +import { ui } from "./journey/targets"; +import { jsLiteral, RELOAD_OPTIONS, waitForPodNotesReady } from "./runtime"; + +const getContext = createPodNotesE2EHarness("save-refusal-recovery"); +const POLL = { timeout: 15_000, interval: 200 }; +const progress = { + title: "Saved episode", + podcastName: "Recovery", + time: 42, + duration: 90, + finished: false, +}; + +async function seed() { + const context = getContext(); + await context.plugin.updateDataAndReload((data) => { + data.currentEpisode = undefined; + data.skipForwardLength = 15; + data.savedFeeds = { + Recovery: { + title: "Recovery", + url: "https://example.com/recovery.xml", + artworkUrl: "", + }, + }; + data.playedEpisodes = { "Recovery::Saved episode": progress }; + }, RELOAD_OPTIONS); + await waitForPodNotesReady(context.obsidian); + await context.obsidian.dev.evalJsonAsync(`(async () => { + const p = app.plugins.plugins.podnotes; + p.api.volume = 0.8; + await p.flushSaves(); + return true; + })()`); + return context; +} + +async function observe(obsidian: ObsidianClient) { + await obsidian.dev.evalJson(`(() => { + const adapter = app.vault.adapter; + const path = app.plugins.plugins.podnotes.manifest.dir + "/data.json"; + const read = adapter.read; + const write = adapter.write; + const probe = { blocked: false, reads: 0, writes: 0, notices: [] }; + const seen = new WeakSet(); + const observer = new MutationObserver(() => { + for (const el of document.querySelectorAll(".notice")) { + if (!seen.has(el)) { seen.add(el); probe.notices.push(el.textContent); } + } + }); + for (const el of document.querySelectorAll(".notice")) seen.add(el); + observer.observe(document.body, { childList: true, subtree: true }); + adapter.read = function(file, ...args) { + if (file === path) { + probe.reads++; + if (probe.blocked) return Promise.reject(new Error("PERS4 read outage")); + } + return read.call(this, file, ...args); + }; + adapter.write = function(file, ...args) { + if (file === path) probe.writes++; + return write.call(this, file, ...args); + }; + probe.restore = () => { adapter.read = read; adapter.write = write; observer.disconnect(); }; + window.__pers4 = probe; + return true; + })()`); + onTestFinished(async () => { + await obsidian.dev.evalJson("window.__pers4?.restore(), delete window.__pers4, true"); + }); +} + +async function sample(obsidian: ObsidianClient, label: string, path: string) { + const state = await obsidian.dev.evalJson<{ reads: number; writes: number; notices: string[] }>( + `({ reads: window.__pers4.reads, writes: window.__pers4.writes, notices: window.__pers4.notices })`, + ); + const bytes = await readFile(path).catch((error: NodeJS.ErrnoException) => { + if (error.code === "ENOENT") return null; + throw error; + }); + console.info( + JSON.stringify({ + label, + ...state, + sha256: bytes && createHash("sha256").update(bytes).digest("hex"), + }), + ); + await capture(obsidian, label); + return state; +} + +async function capture(obsidian: ObsidianClient, label: string) { + const directory = resolve(".obsidian-e2e-artifacts/save-refusal-recovery"); + await mkdir(directory, { recursive: true }); + await obsidian.dev.screenshot(resolve(directory, `${label}.png`)); +} + +async function externalChange(obsidian: ObsidianClient) { + await obsidian.dev.evalJsonAsync( + "app.plugins.plugins.podnotes.onExternalSettingsChange().then(() => true)", + ); +} + +async function outage(obsidian: ObsidianClient) { + await obsidian.dev.evalJson("window.__pers4.blocked = true"); + await externalChange(obsidian); + await expect + .poll( + async () => + (await obsidian.dev.notices()).some((n) => n.message.includes("could not read")), + POLL, + ) + .toBe(true); +} + +describe("save refusal recovery", () => { + test("#431 restores the full idle library after an unreadable file is moved away without an edit", async () => { + const { obsidian, plugin } = await seed(); + const path = await plugin.dataPath(); + const original = await readFile(path, "utf8"); + await observe(obsidian); + onTestFinished(async () => { + await rm(path + ".unreadable", { force: true }); + }); + await writeFile(path, "{"); + await expect + .poll( + async () => + (await obsidian.dev.notices()).some((n) => + n.message.includes("could not read"), + ), + POLL, + ) + .toBe(true); + await sample(obsidian, "idle-refused", path); + expect(await readFile(path, "utf8")).toBe("{"); + await rename(path, path + ".unreadable"); + await obsidian.sleep(12_000); + await sample(obsidian, "idle-recovered", path); + const restored = await readFile(path, "utf8").catch((error: NodeJS.ErrnoException) => { + if (error.code === "ENOENT") return "missing"; + throw error; + }); + expect(restored).not.toBe("missing"); + expect(JSON.parse(restored)).toEqual(JSON.parse(original)); + expect((await sample(obsidian, "idle-once", path)).writes).toBe(1); + }); + + test.each([99, 1])( + "#429 schema %s stops retries and shows one actionable Notice without writing", + async (version) => { + const { obsidian, plugin } = await seed(); + await openPodNotesSettings(obsidian); + onTestFinished(() => closeSettings(obsidian)); + await obsidian.dev.evalJsonAsync(`(async () => { + const p = app.plugins.plugins.podnotes; + p.api.volume = 0.95; + await p.flushSaves(); + p.checkpoint(() => { p.api.volume = 0.9; }); + return true; + })()`); + await observe(obsidian); + const data = await plugin.data().read(); + await plugin.data().write({ ...data, schemaVersion: version }); + const path = await plugin.dataPath(); + const protectedText = await readFile(path, "utf8"); + await externalChange(obsidian); + await typeInto(obsidian, ui.settingControl("Skip forward (s)", "input"), "25", { + replace: true, + }); + await obsidian.sleep(500); + const before = await sample(obsidian, `schema-${version}-refused`, path); + await obsidian.sleep(21_000); + const after = await sample(obsidian, `schema-${version}-waiting`, path); + expect.soft(after.notices.filter((text) => /PodNotes/.test(text))).toHaveLength(1); + expect + .soft(after.notices.join(" ")) + .toMatch(version === 99 ? /updat.*PodNotes/i : /restart|reload/i); + expect.soft(after.reads).toBe(before.reads); + expect(after.writes).toBe(0); + expect(await readFile(path, "utf8")).toBe(protectedText); + }, + ); + + test("#430 imports through the file picker immediately after a read outage clears", async () => { + const { obsidian, plugin, sandbox } = await seed(); + await openPodNotesSettings(obsidian); + onTestFinished(() => closeSettings(obsidian)); + const importPath = `${await obsidian.vaultPath()}/${sandbox.path("recovery.json")}`; + await writeFile( + importPath, + JSON.stringify({ + type: "podnotes-settings", + version: 2, + settings: { skipForwardLength: 37 }, + }), + ); + await observe(obsidian); + await outage(obsidian); + await obsidian.dev.evalJson("window.__pers4.blocked = false"); + await chooseFile(obsidian, ui.settingControl("Import settings", "button"), importPath); + await click(obsidian, { + name: "Import confirmation", + elements: + '[...document.querySelectorAll(".modal button.mod-destructive")].filter(el => el.textContent.trim() === "Import")', + }); + await obsidian.sleep(1000); + await sample(obsidian, "import-recovered", await plugin.dataPath()); + expect((await plugin.data().read()).skipForwardLength).toBe(37); + expect( + (await plugin.data().read()).playedEpisodes?.["Recovery::Saved episode"], + ).toEqual(progress); + }); + + test("#430 links an API key immediately after a read outage clears", async () => { + const { obsidian, plugin } = await seed(); + const initialIds = await obsidian.dev.evalJson("app.secretStorage.listSecrets()"); + await obsidian.dev.evalJson( + 'app.secretStorage.setSecret("podnotes-pers4-recovery", "sk-podnotes-e2e-recovery"), true', + ); + try { + await openPodNotesSettings(obsidian); + await observe(obsidian); + await outage(obsidian); + await obsidian.dev.evalJson("window.__pers4.blocked = false"); + await click(obsidian, ui.settingControl("OpenAI API key", "button")); + await click(obsidian, { + name: "Recovery secret", + elements: + '[...document.querySelectorAll(".modal .suggestion-item")].filter(el => el.textContent.includes("podnotes-pers4-recovery"))', + }); + await click(obsidian, { + name: "Save secret selection", + elements: + '[...document.querySelectorAll(".modal button")].filter(el => el.textContent.trim() === "Save")', + }); + await obsidian.sleep(500); + await sample(obsidian, "key-recovered", await plugin.dataPath()); + const savedId = (await plugin.data().read()).openAISecretId; + expect(savedId).toMatch(/^podnotes-openai-api-key(?:-\d+)?$/); + expect( + await obsidian.dev.evalJson( + `app.secretStorage.getSecret(${jsLiteral(savedId)}) === "sk-podnotes-e2e-recovery"`, + ), + ).toBe(true); + } finally { + await closeSettings(obsidian); + await obsidian.dev.evalJson(`(() => { + const initial = ${jsLiteral(initialIds)}; + for (const id of app.secretStorage.listSecrets()) { + if (!initial.includes(id)) app.secretStorage.setSecret(id, null); + } + return true; + })()`); + } + }); + + test("#431 visibility flush reads through a recovered outage before the retry interval", async () => { + const { obsidian, plugin } = await seed(); + await observe(obsidian); + await outage(obsidian); + await obsidian.dev.evalJson(`(() => { + const podnotes = app.plugins.plugins.podnotes; + podnotes.api.volume = 0.42; + window.__pers4.blocked = false; + Object.defineProperty(document, "visibilityState", { configurable: true, value: "hidden" }); + document.dispatchEvent(new Event("visibilitychange")); + delete document.visibilityState; + return true; + })()`); + await obsidian.sleep(300); + await sample(obsidian, "visibility-recovered", await plugin.dataPath()); + expect((await plugin.data().read()).defaultVolume).toBe(0.42); + }); + + test("#432 an unread startup file says to restore access and reload, and stays protected", async () => { + const { obsidian, plugin } = await seed(); + await plugin.disable(); + const path = await plugin.dataPath(); + await writeFile(path, "{"); + await plugin.enable(); + await waitForPodNotesReady(obsidian); + const notices = await obsidian.dev.notices(); + await capture(obsidian, "startup-refused"); + console.info( + JSON.stringify({ + label: "startup-refused", + notices, + sha256: createHash("sha256") + .update(await readFile(path)) + .digest("hex"), + }), + ); + expect + .soft(notices.map((notice) => notice.message).join(" ")) + .toMatch( + /(?:make|restore)[^.]+(?:read|access)[^.]+(?:reload|restart)|(?:readable|read access)[^.]+(?:reload|restart)/i, + ); + await obsidian.dev + .evalJsonAsync(`(async () => { + const p = app.plugins.plugins.podnotes; + p.api.volume = 0.42; + await p.flushSaves(); + return p.saveSettingsStrict().then(() => "saved", () => "refused"); + })()`) + .then((result) => expect(result).toBe("refused")); + expect(await readFile(path, "utf8")).toBe("{"); + }); +});