diff --git a/docs/docs/local_files.md b/docs/docs/local_files.md index ecf4cf79..9c6cc0c1 100644 --- a/docs/docs/local_files.md +++ b/docs/docs/local_files.md @@ -2,6 +2,8 @@ PodNotes supports local media files. You can right-click any audio or video file In the podcast grid, you will see a playlist with a folder icon. This is a playlist of every episode you have available offline: both local media files you have played with `Play with PodNotes` and episodes you have downloaded. Removing a downloaded file also removes it from this playlist. +`Remove from Local Files` removes a local media file from the playlist and keeps the file in your vault. `Remove file` moves a downloaded episode's file to the trash. If PodNotes cannot confirm that it downloaded the file itself, which is the case for downloads made with an older PodNotes version, it asks first. `Keep file` removes the episode from the playlist and leaves the file where it is. + PodNotes will attempt to treat all local files as podcast episodes. This means all standard features are available, such as timestamps, note creation, and persisting playback time. Any exceptions will be added here. Local files and downloaded episodes are played through Obsidian's native media resource path (the same mechanism Obsidian uses for embedded media), so they work on mobile (iOS and Android) as well as desktop. diff --git a/src/downloadEpisode.test.ts b/src/downloadEpisode.test.ts index 1b1adfa8..fec75c20 100644 --- a/src/downloadEpisode.test.ts +++ b/src/downloadEpisode.test.ts @@ -1,5 +1,6 @@ // @vitest-environment jsdom import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { fireEvent, screen } from "@testing-library/svelte"; import { get } from "svelte/store"; import { Notice, requestUrl, TFile } from "obsidian"; import downloadEpisodeWithNotice, { @@ -10,6 +11,7 @@ import downloadEpisodeWithNotice, { } from "./downloadEpisode"; import { downloadedEpisodes, plugin } from "./store"; import type PodNotes from "./main"; +import type DownloadedEpisode from "./types/DownloadedEpisode"; import type { Episode } from "./types/Episode"; import type { LocalEpisode } from "./types/LocalEpisode"; @@ -645,21 +647,269 @@ describe("safeDownloadBasename (#183)", () => { }); describe("removeDownloadedEpisode", () => { - it("trashes the indexed download through FileManager", async () => { + const promptFor = (filePath: string) => + `PodNotes cannot confirm that it downloaded "${filePath}".`; + const keepFile = () => fireEvent.click(screen.getByRole("button", { name: "Keep file" })); + const deleteFile = () => fireEvent.click(screen.getByRole("button", { name: "Delete file" })); + const dismissPrompt = () => { + const closeButton = document.querySelector(".modal-header-button"); + if (!closeButton) throw new Error("The prompt has no close button."); + fireEvent.click(closeButton); + }; + + function putOnDisk(vault: ReturnType, filePath: string): void { + vault.present.add(filePath); + vault.disk.add(filePath); + } + + function expectKeptOnDisk(vault: ReturnType, filePath: string): void { + expect(vault.trashFile).not.toHaveBeenCalled(); + expect(vault.removeFile).not.toHaveBeenCalled(); + expect(vault.present.has(filePath)).toBe(true); + expect(vault.disk.has(filePath)).toBe(true); + } + + function mockAudioResponse(): void { + requestUrlMock.mockResolvedValue({ + status: 200, + headers: { "content-type": "audio/mpeg" }, + arrayBuffer: bytes(0x49, 0x44, 0x33, 0x04), + } as unknown as Awaited>); + } + + async function removeConfirmingAnyPrompt(episode: Episode): Promise<{ asked: boolean }> { + let settled = false; + const removal = removeDownloadedEpisode(episode).finally(() => { + settled = true; + }); + let asked = false; + await vi.waitFor(() => { + if (settled) return; + deleteFile(); + asked = true; + }); + await removal; + return { asked }; + } + + afterEach(() => { + document.body.replaceChildren(); + }); + + it("trashes a stored entry marked as PodNotes' own without asking", async () => { const vault = setupVault(); const episode = makeEpisode(); const filePath = "Podcasts/My Title.mp3"; - vault.present.add(filePath); - vault.disk.add(filePath); - downloadedEpisodes.addEpisode(episode, filePath, 4); + putOnDisk(vault, filePath); + const owned: DownloadedEpisode = { + ...episode, + filePath, + size: 4, + downloadedByPodNotes: true, + }; + downloadedEpisodes.set({ Pod: [owned] }); await removeDownloadedEpisode(episode); + expect(screen.queryByText(promptFor(filePath))).toBeNull(); expect(vault.trashFile).toHaveBeenCalledOnce(); expect(vault.trashFile.mock.calls[0][0].path).toBe(filePath); expect(vault.removeFile).not.toHaveBeenCalled(); - expect(get(downloadedEpisodes)[episode.podcastName]).toEqual([]); + expect(get(downloadedEpisodes).Pod).toEqual([]); + }); + + it.each([ + ["Keep file", keepFile, true], + ["Delete file", deleteFile, false], + ["a dismissal", dismissPrompt, true], + ])( + "asks before trashing a feed file PodNotes cannot confirm it downloaded, answered with %s", + async (_answer, answer, keptOnDisk) => { + const vault = setupVault(); + const episode = makeEpisode({ + title: "Fixture Episode 2", + streamUrl: "https://example.com/episode-2.wav", + }); + const filePath = "Recordings/Fixture Episode 2.wav"; + putOnDisk(vault, filePath); + downloadedEpisodes.set({ Pod: [{ ...episode, filePath, size: 4 }] }); + + const removal = removeDownloadedEpisode(episode); + await vi.waitFor(() => + expect({ + asking: screen.queryByText(promptFor(filePath)) !== null, + onDisk: vault.present.has(filePath), + }).toEqual({ asking: true, onDisk: true }), + ); + expect(screen.getByRole("button", { name: "Keep file" })).toHaveFocus(); + answer(); + await removal; + + expect(screen.queryByText(promptFor(filePath))).toBeNull(); + expect(get(downloadedEpisodes).Pod).toEqual([]); + expect(vault.present.has(filePath)).toBe(keptOnDisk); + expect(vault.trashFile).toHaveBeenCalledTimes(keptOnDisk ? 0 : 1); + expect(vault.removeFile).not.toHaveBeenCalled(); + }, + ); + + it("drops an unconfirmed entry whose file the vault has not indexed and leaves the file", async () => { + const vault = setupVault(); + const episode = makeEpisode({ title: "Fixture Episode 2" }); + const filePath = ".recordings/Fixture Episode 2.wav"; + vault.disk.add(filePath); + downloadedEpisodes.set({ Pod: [{ ...episode, filePath, size: 4 }] }); + + const { asked } = await removeConfirmingAnyPrompt(episode); + + expect({ + asked, + trashed: vault.trashFile.mock.calls.length, + unlinked: vault.removeFile.mock.calls.length, + onDisk: vault.disk.has(filePath), + }).toEqual({ asked: false, trashed: 0, unlinked: 0, onDisk: true }); + expect(get(downloadedEpisodes).Pod).toEqual([]); + }); + + it.each([ + ["whose file is gone", "Recordings/Gone.wav"], + ["without a file path", ""], + ])("drops an unconfirmed entry %s without asking", async (_case, filePath) => { + const vault = setupVault(); + const episode = makeEpisode(); + downloadedEpisodes.set({ Pod: [{ ...episode, filePath, size: 4 }] }); + + const { asked } = await removeConfirmingAnyPrompt(episode); + + expect(asked).toBe(false); + expect(get(downloadedEpisodes).Pod).toEqual([]); + expect(vault.trashFile).not.toHaveBeenCalled(); + expect(vault.removeFile).not.toHaveBeenCalled(); }); + + it("trashes the file it asked about even if the vault index loses its path meanwhile", async () => { + const vault = setupVault(); + const episode = makeEpisode({ title: "Fixture Episode 2" }); + const filePath = "Recordings/Fixture Episode 2.wav"; + putOnDisk(vault, filePath); + downloadedEpisodes.set({ Pod: [{ ...episode, filePath, size: 4 }] }); + + const removal = removeDownloadedEpisode(episode); + await vi.waitFor(() => expect(screen.getByText(promptFor(filePath))).toBeTruthy()); + vault.present.delete(filePath); + deleteFile(); + await removal; + + expect({ + trashed: vault.trashFile.mock.calls.map(([file]) => file.path), + unlinked: vault.removeFile.mock.calls.length, + }).toEqual({ trashed: [filePath], unlinked: 0 }); + }); + + const interviewPath = "Recordings/Interview.wav"; + const interview: LocalEpisode = { + title: "Interview", + description: "", + content: "", + podcastName: "local file", + url: `[[${interviewPath}]]`, + streamUrl: `app://local/${interviewPath}`, + }; + + it.each([ + ["without a media type", {}], + ["as audio", { mediaType: "audio" }], + ["as video", { mediaType: "video" }], + ] as const)("drops a local file stored %s but leaves it on disk", async (_case, media) => { + const vault = setupVault(); + putOnDisk(vault, interviewPath); + const stored = { ...interview, ...media, filePath: interviewPath, size: 4 }; + downloadedEpisodes.set({ "local file": [stored] }); + + await removeDownloadedEpisode(stored); + + expect(screen.queryByText(promptFor(interviewPath))).toBeNull(); + expect(get(downloadedEpisodes)["local file"]).toEqual([]); + expectKeptOnDisk(vault, interviewPath); + }); + + it("drops a local file removed through an older snapshot without its path", async () => { + const vault = setupVault(); + putOnDisk(vault, interviewPath); + downloadedEpisodes.set({ + "local file": [{ ...interview, filePath: interviewPath, size: 4 }], + }); + + await removeDownloadedEpisode(interview); + + expect(screen.queryByText(promptFor(interviewPath))).toBeNull(); + expect(get(downloadedEpisodes)["local file"]).toEqual([]); + expectKeptOnDisk(vault, interviewPath); + }); + + it("trashes a file the download flow wrote without asking", async () => { + const vault = setupVault(); + mockAudioResponse(); + const episode = makeEpisode(); + const filePath = "Podcasts/My Title.mp3"; + + await downloadEpisodeWithNotice(episode, "Podcasts/{{title}}"); + expect(get(downloadedEpisodes).Pod).toEqual([ + expect.objectContaining({ filePath, downloadedByPodNotes: true }), + ]); + await removeDownloadedEpisode(episode); + + expect(screen.queryByText(promptFor(filePath))).toBeNull(); + expect(vault.trashFile).toHaveBeenCalledOnce(); + expect(vault.present.has(filePath)).toBe(false); + expect(get(downloadedEpisodes).Pod).toEqual([]); + }); + + it.each([ + ["its URL extension", "https://example.com/ep.mp3"], + ["its sniffed destination", "https://example.com/stream?id=42"], + ])("keeps an earlier registration it reuses at %s unconfirmed", async (_case, streamUrl) => { + const vault = setupVault(); + mockAudioResponse(); + const episode = makeEpisode({ streamUrl }); + const filePath = "Podcasts/My Title.mp3"; + putOnDisk(vault, filePath); + const registered = { ...episode, filePath, size: 4 }; + downloadedEpisodes.set({ Pod: [registered] }); + + await downloadEpisodeWithNotice(episode, "Podcasts/{{title}}"); + + expect(vault.writeBinary).not.toHaveBeenCalled(); + expect(get(downloadedEpisodes).Pod).toEqual([registered]); + }); + + it.each([ + ["its URL extension", "https://example.com/ep.mp3", 1], + ["its sniffed destination", "https://example.com/stream?id=42", 2], + ])( + "keeps its own download confirmed when it reuses it at %s", + async (_case, streamUrl, requests) => { + const vault = setupVault(); + mockAudioResponse(); + const episode = makeEpisode({ streamUrl }); + const filePath = "Podcasts/My Title.mp3"; + + await downloadEpisodeWithNotice(episode, "Podcasts/{{title}}"); + await downloadEpisodeWithNotice(episode, "Podcasts/{{title}}"); + + expect(vault.writeBinary).toHaveBeenCalledOnce(); + expect(requestUrlMock).toHaveBeenCalledTimes(requests); + expect(get(downloadedEpisodes).Pod).toEqual([ + expect.objectContaining({ filePath, downloadedByPodNotes: true }), + ]); + + const { asked } = await removeConfirmingAnyPrompt(episode); + + expect(asked).toBe(false); + expect(vault.trashFile).toHaveBeenCalledOnce(); + expect(vault.present.has(filePath)).toBe(false); + }, + ); }); describe("safeDownloadFilePath (#22)", () => { @@ -1384,7 +1634,7 @@ describe("downloadEpisodeWithNotice (streaming range path)", () => { await downloadEpisodeWithNotice(episodeA, template); await expect(downloadEpisodeWithNotice(episodeB, template)).rejects.toThrow( - "A different episode already occupies the selected destination.", + 'A file already exists at "Podcasts/shared.mp3".', ); // Episode B pays one Range probe before the collision surfaces: the diff --git a/src/downloadEpisode.ts b/src/downloadEpisode.ts index 6fea28fe..00752341 100644 --- a/src/downloadEpisode.ts +++ b/src/downloadEpisode.ts @@ -1,6 +1,7 @@ import { normalizePath, Notice, TFile } from "obsidian"; import { get } from "svelte/store"; import { downloadedEpisodes, plugin } from "./store"; +import type DownloadedEpisode from "./types/DownloadedEpisode"; import { DownloadPathTemplateEngine, replaceIllegalFileNameCharactersInString, @@ -31,6 +32,7 @@ import { MAX_DOWNLOAD_SIZE, } from "./download/streaming"; import { detectAudioFileExtension } from "./download/mediaSignatures"; +import { confirmTrashUnownedFile } from "./ui/ConfirmTrashUnownedFileModal"; import { NetworkError, requestWithTimeout } from "./utility/networkRequest"; const WHOLE_FILE_DOWNLOAD_TIMEOUT_MS = 15 * 60_000; @@ -95,8 +97,8 @@ function downloadAlreadyInProgressError(): Error { return new Error("A download is already in progress for the selected destination."); } -function downloadDestinationCollisionError(): Error { - return new Error("A different episode already occupies the selected destination."); +function downloadDestinationCollisionError(filePath: string): Error { + return new Error(`A file already exists at "${filePath}".`); } // Single source of truth for "what extension and on-disk path does this download @@ -119,10 +121,10 @@ function resolveDownloadTarget( }; } -function registeredDownloadOwnsPath(episode: Episode, filePath: string): boolean { +function ownRegistrationAt(episode: Episode, filePath: string): DownloadedEpisode | undefined { const normalizedPath = normalizePath(filePath); const registered = downloadedEpisodes.getEpisode(episode); - if (!registered || normalizePath(registered.filePath) !== normalizedPath) return false; + if (!registered || normalizePath(registered.filePath) !== normalizedPath) return undefined; const anotherOwner = Object.values(get(downloadedEpisodes)) .flat() @@ -130,33 +132,23 @@ function registeredDownloadOwnsPath(episode: Episode, filePath: string): boolean (candidate) => candidate !== registered && normalizePath(candidate.filePath) === normalizedPath, ); - if (anotherOwner) return false; + if (anotherOwner) return undefined; - if (isLocalFile(episode)) { - return Boolean(episode.filePath && normalizePath(episode.filePath) === normalizedPath); - } - return isSameMediaSource(registered.streamUrl, episode.streamUrl); -} - -// Fast-path variant: the path here is only the URL-extension guess, so a foreign -// occupant is not a collision yet - the sniffed final path may differ. Returns a -// size only when this episode already owns the file. -function ownedExistingDownloadSize(episode: Episode, filePath: string): number | undefined { - const existing = get(plugin).app.vault.getAbstractFileByPath(filePath); - if (!(existing instanceof TFile)) return undefined; - if (!registeredDownloadOwnsPath(episode, filePath)) return undefined; - return downloadedEpisodes.getEpisode(episode)?.size; + const owns = isLocalFile(episode) + ? Boolean(episode.filePath && normalizePath(episode.filePath) === normalizedPath) + : isSameMediaSource(registered.streamUrl, episode.streamUrl); + return owns ? registered : undefined; } -// Resolved-destination variant: the final path is known, so a foreign occupant -// is a hard collision. -function reusableExistingDownloadSize(episode: Episode, filePath: string): number | undefined { +function reuseOwnRegistration(episode: Episode, filePath: string): "reused" | "free" | "foreign" { const existing = get(plugin).app.vault.getAbstractFileByPath(filePath); - if (!(existing instanceof TFile)) return undefined; - if (!registeredDownloadOwnsPath(episode, filePath)) { - throw downloadDestinationCollisionError(); - } - return downloadedEpisodes.getEpisode(episode)?.size; + if (!(existing instanceof TFile)) return "free"; + const own = ownRegistrationAt(episode, filePath); + if (!own) return "foreign"; + downloadedEpisodes.addEpisode(episode, filePath, own.size, { + downloadedByPodNotes: own.downloadedByPodNotes, + }); + return "reused"; } // Download an episode to a vault file with bounded memory using the binary append @@ -173,11 +165,8 @@ async function startDownloadEpisodeToDisk( const urlExtension = getUrlExtension(episode.streamUrl); if (urlExtension) { const provisionalPath = safeDownloadFilePath(downloadPathTemplate, episode, urlExtension); - const cachedSize = ownedExistingDownloadSize(episode, provisionalPath); - if (cachedSize !== undefined) { - downloadedEpisodes.addEpisode(episode, provisionalPath, cachedSize); - return provisionalPath; - } + // A foreign occupant is not a collision yet - the sniffed final path may differ. + if (reuseOwnRegistration(episode, provisionalPath) === "reused") return provisionalPath; } let probe; @@ -192,11 +181,9 @@ async function startDownloadEpisodeToDisk( probe.firstChunk, probe.contentType, ); - const existingSize = reusableExistingDownloadSize(episode, filePath); - if (existingSize !== undefined) { - downloadedEpisodes.addEpisode(episode, filePath, existingSize); - return filePath; - } + const reuse = reuseOwnRegistration(episode, filePath); + if (reuse === "reused") return filePath; + if (reuse === "foreign") throw downloadDestinationCollisionError(filePath); await ensureFolderExists(parentFolderPath(filePath)); @@ -218,10 +205,10 @@ async function startDownloadEpisodeToDisk( await moveIntoPlace(tmpPath, filePath); moved = true; - downloadedEpisodes.addEpisode(episode, filePath, total); + downloadedEpisodes.addEpisode(episode, filePath, total, { downloadedByPodNotes: true }); return filePath; } finally { - if (!moved) await deleteEpisodeFile(tmpPath); + if (!moved) await deleteFileWrittenByPodNotes(tmpPath); } } @@ -351,22 +338,27 @@ export function safeDownloadFilePath( return enforceMaxPathLength(`${basename}.${extension}`, `.${extension}`); } -/** - * Remove a downloaded episode: drop it from the offline set and delete its - * backing vault file. This composes the pure store removal with the file I/O so - * callers can't do one without the other (and leak files); the download store - * stays free of vault side effects. - */ export async function removeDownloadedEpisode(episode: Episode): Promise { - const removedFilePath = downloadedEpisodes.removeEpisode(episode); - if (removedFilePath) { - await deleteEpisodeFile(removedFilePath); + const removed = downloadedEpisodes.removeEpisode(episode); + if (!removed || isLocalFile(removed)) return; + if (removed.downloadedByPodNotes) { + await deleteFileWrittenByPodNotes(removed.filePath); + return; + } + + const { app } = get(plugin); + const file = app.vault.getAbstractFileByPath(removed.filePath); + if (!(file instanceof TFile) || !(await confirmTrashUnownedFile(app, file.path))) return; + try { + await app.fileManager.trashFile(file); + } catch (error) { + console.error(`Failed to trash "${file.path}":`, error); } } // Best-effort: a missing/already-removed file is a no-op, and failures are logged // rather than thrown so removing a stale entry never breaks the calling UI flow. -async function deleteEpisodeFile(filePath: string): Promise { +async function deleteFileWrittenByPodNotes(filePath: string): Promise { if (!filePath) return; const { app } = get(plugin); diff --git a/src/persistence/collectionCodecs.ts b/src/persistence/collectionCodecs.ts index e2e47f2d..22f35386 100644 --- a/src/persistence/collectionCodecs.ts +++ b/src/persistence/collectionCodecs.ts @@ -179,7 +179,13 @@ export function decodeDownloadedEpisodes( ? entry.size : 0; if (size !== entry.size) warn(warnings, `${path}.size`, "value was normalized"); - return [{ ...decoded, filePath: entry.filePath, size }]; + + const download: DownloadedEpisode = { ...decoded, filePath: entry.filePath, size }; + if ("downloadedByPodNotes" in entry && entry.downloadedByPodNotes !== true) { + delete download.downloadedByPodNotes; + warn(warnings, `${path}.downloadedByPodNotes`, "value was normalized"); + } + return [download]; }); } return downloads; diff --git a/src/persistence/podNotesData.test.ts b/src/persistence/podNotesData.test.ts index f97b8cc1..52b811ed 100644 --- a/src/persistence/podNotesData.test.ts +++ b/src/persistence/podNotesData.test.ts @@ -266,6 +266,42 @@ describe("PodNotes data schema", () => { expect(decoded.settings.downloadedEpisodes["Old Podcast"][0].size).toBe(0); }); + const storedDownload = (downloadedByPodNotes: unknown) => ({ + ...episode("Downloaded"), + episodeDate: episodeDate.toISOString(), + filePath: "Podcasts/downloaded.mp3", + size: 42, + downloadedByPodNotes, + }); + + it("keeps a download PodNotes recorded as its own through load and save", () => { + const decoded = decodePodNotesData({ + downloadedEpisodes: { "Test Podcast": [storedDownload(true)] }, + }); + const persisted = encodePodNotesData(decoded.settings, decoded.unknownFields); + + expect(decoded.warnings).toEqual([]); + expect(decoded.settings.downloadedEpisodes["Test Podcast"][0]).toMatchObject({ + downloadedByPodNotes: true, + }); + expect(persisted.downloadedEpisodes["Test Podcast"][0]).toMatchObject({ + downloadedByPodNotes: true, + }); + }); + + it.each([["true"], [1], [false]])("does not trust download ownership stored as %o", (value) => { + const decoded = decodePodNotesData({ + downloadedEpisodes: { "Test Podcast": [storedDownload(value)] }, + }); + + expect(decoded.settings.downloadedEpisodes["Test Podcast"][0]).not.toHaveProperty( + "downloadedByPodNotes", + ); + expect(decoded.warnings).toEqual([ + "downloadedEpisodes.Test Podcast[0].downloadedByPodNotes: value was normalized", + ]); + }); + it("preserves safe unknown root and nested fields across a schema upgrade", () => { const decoded = decodePodNotesData({ schemaVersion: 1, diff --git a/src/store/downloads.ts b/src/store/downloads.ts index 3d4626b6..0fd93a83 100644 --- a/src/store/downloads.ts +++ b/src/store/downloads.ts @@ -26,7 +26,12 @@ export const downloadedEpisodes = (() => { * collision), or `undefined` otherwise. The caller surfaces a Notice — this * store stays pure (no UI/side effects), per the PR #211/#212 layering (#LF-06). */ - addEpisode: (episode: Episode, filePath: string, size: number): string | undefined => { + addEpisode: ( + episode: Episode, + filePath: string, + size: number, + { downloadedByPodNotes }: { downloadedByPodNotes?: true } = {}, + ): string | undefined => { let replacedFilePath: string | undefined; update((downloadedEpisodes: { [podcastName: string]: DownloadedEpisode[] }) => { @@ -44,13 +49,9 @@ export const downloadedEpisodes = (() => { if (existingFilePath && existingFilePath !== filePath) { replacedFilePath = existingFilePath; } - podcastEpisodes[idx] = { ...episode, filePath, size }; + podcastEpisodes[idx] = { ...episode, filePath, size, downloadedByPodNotes }; } else { - podcastEpisodes.push({ - ...episode, - filePath, - size, - }); + podcastEpisodes.push({ ...episode, filePath, size, downloadedByPodNotes }); } downloadedEpisodes[episode.podcastName] = podcastEpisodes; @@ -59,13 +60,8 @@ export const downloadedEpisodes = (() => { return replacedFilePath; }, - /** - * Drops an episode from the offline set and returns the vault path of the - * file it backed (or `undefined` if it wasn't tracked). Pure state only: the - * caller deletes the returned file (see `removeDownloadedEpisode`). - */ - removeEpisode: (episode: Episode): string | undefined => { - let removedFilePath: string | undefined; + removeEpisode: (episode: Episode): DownloadedEpisode | undefined => { + let removed: DownloadedEpisode | undefined; update((downloadedEpisodes) => { const podcastEpisodes = downloadedEpisodes[episode.podcastName] || []; @@ -76,14 +72,13 @@ export const downloadedEpisodes = (() => { return downloadedEpisodes; } - removedFilePath = podcastEpisodes[index].filePath; - podcastEpisodes.splice(index, 1); + [removed] = podcastEpisodes.splice(index, 1); downloadedEpisodes[episode.podcastName] = podcastEpisodes; return downloadedEpisodes; }); - return removedFilePath; + return removed; }, getEpisode: (episode: Episode) => { return get(store)[episode.podcastName]?.find((e) => e.title === episode.title); diff --git a/src/store/index.test.ts b/src/store/index.test.ts index 205d925d..ec677f44 100644 --- a/src/store/index.test.ts +++ b/src/store/index.test.ts @@ -321,15 +321,17 @@ describe("downloadedEpisodes store — removeEpisode is pure (no vault I/O)", () downloadedEpisodes.set({}); }); - test("removes the entry and returns its backing file path", () => { + test("removes the entry and returns the stored download", () => { const downloaded = downloadedEpisode("Design Podcast", "Episode 1", { filePath: "podcasts/design/ep1.mp3", }); - downloadedEpisodes.addEpisode(downloaded, downloaded.filePath, 2048); + downloadedEpisodes.addEpisode(downloaded, downloaded.filePath, 2048, { + downloadedByPodNotes: true, + }); - const removedFilePath = downloadedEpisodes.removeEpisode(downloaded); + const removed = downloadedEpisodes.removeEpisode(ep("Episode 1", "Design Podcast")); - expect(removedFilePath).toBe("podcasts/design/ep1.mp3"); + expect(removed).toEqual({ ...downloaded, size: 2048, downloadedByPodNotes: true }); expect(downloadedEpisodes.isEpisodeDownloaded(downloaded)).toBe(false); expect(get(downloadedEpisodes)["Design Podcast"]).toEqual([]); }); @@ -339,9 +341,9 @@ describe("downloadedEpisodes store — removeEpisode is pure (no vault I/O)", () downloadedEpisodes.addEpisode(present, present.filePath, 1024); const missing = downloadedEpisode("Design Podcast", "Never downloaded"); - const removedFilePath = downloadedEpisodes.removeEpisode(missing); + const removed = downloadedEpisodes.removeEpisode(missing); - expect(removedFilePath).toBeUndefined(); + expect(removed).toBeUndefined(); expect(downloadedEpisodes.isEpisodeDownloaded(present)).toBe(true); }); }); diff --git a/src/types/DownloadedEpisode.ts b/src/types/DownloadedEpisode.ts index a10a9759..f189043d 100644 --- a/src/types/DownloadedEpisode.ts +++ b/src/types/DownloadedEpisode.ts @@ -3,4 +3,9 @@ import type { Episode } from "./Episode"; export default interface DownloadedEpisode extends Episode { filePath: string; size: number; + /** + * Older releases registered any file already at a download path, so an entry + * without it may point at the user's own file. + */ + downloadedByPodNotes?: true; } diff --git a/src/ui/ConfirmTrashUnownedFileModal.ts b/src/ui/ConfirmTrashUnownedFileModal.ts new file mode 100644 index 00000000..4bd81079 --- /dev/null +++ b/src/ui/ConfirmTrashUnownedFileModal.ts @@ -0,0 +1,44 @@ +import { type App, ButtonComponent, Modal } from "obsidian"; + +class ConfirmTrashUnownedFileModal extends Modal { + private filePath: string; + private onAnswer: (moveToTrash: boolean) => void; + private moveToTrash = false; + + constructor(app: App, filePath: string, onAnswer: (moveToTrash: boolean) => void) { + super(app); + this.filePath = filePath; + this.onAnswer = onAnswer; + } + + override onOpen(): void { + this.contentEl.createEl("p", { + text: `PodNotes cannot confirm that it downloaded "${this.filePath}".`, + }); + + const buttons = this.contentEl.createDiv({ cls: "modal-button-container" }); + const keepFile = new ButtonComponent(buttons) + .setButtonText("Keep file") + .setCta() + .onClick(() => this.close()); + new ButtonComponent(buttons) + .setButtonText("Delete file") + .setDestructive() + .onClick(() => { + this.moveToTrash = true; + this.close(); + }); + keepFile.buttonEl.focus(); + } + + override onClose(): void { + this.contentEl.empty(); + this.onAnswer(this.moveToTrash); + } +} + +export function confirmTrashUnownedFile(app: App, filePath: string): Promise { + return new Promise((resolve) => + new ConfirmTrashUnownedFileModal(app, filePath, resolve).open(), + ); +} diff --git a/src/ui/PodcastView/spawnEpisodeContextMenu.test.ts b/src/ui/PodcastView/spawnEpisodeContextMenu.test.ts index 5f6671f1..d4e762c1 100644 --- a/src/ui/PodcastView/spawnEpisodeContextMenu.test.ts +++ b/src/ui/PodcastView/spawnEpisodeContextMenu.test.ts @@ -1,9 +1,9 @@ // @vitest-environment jsdom -import { Menu } from "obsidian"; +import { Menu, MenuItem } from "obsidian"; import { beforeEach, describe, expect, test, vi } from "vitest"; import spawnEpisodeContextMenu from "./spawnEpisodeContextMenu"; -import { playedEpisodes, playlists, queue } from "src/store"; +import { downloadedEpisodes, playedEpisodes, playlists, queue } from "src/store"; import type { Episode } from "src/types/Episode"; const episode: Episode = { @@ -37,6 +37,7 @@ beforeEach(() => { }); playlists.set({}); playedEpisodes.set({}); + downloadedEpisodes.set({}); }); describe("spawnEpisodeContextMenu anchoring (CM-general)", () => { @@ -66,3 +67,30 @@ describe("spawnEpisodeContextMenu anchoring (CM-general)", () => { atPos.mockRestore(); }); }); + +describe("spawnEpisodeContextMenu removal item", () => { + function downloadItemTitles(target: Episode) { + const setTitle = vi.spyOn(MenuItem.prototype, "setTitle"); + spawnEpisodeContextMenu(target, { x: 0, y: 0 }, { ...allDisabled, download: false }); + const titles = setTitle.mock.calls.map(([title]) => title); + setTitle.mockRestore(); + return titles; + } + + test("offers to remove a tracked local file from Local Files", () => { + const localFile = { ...episode, title: "Interview", podcastName: "local file" }; + downloadedEpisodes.set({ + "local file": [{ ...localFile, filePath: "Recordings/Interview.wav", size: 4 }], + }); + + expect(downloadItemTitles(localFile)).toEqual(["Remove from Local Files"]); + }); + + test("offers to remove the file of a downloaded feed episode", () => { + downloadedEpisodes.set({ + Podcast: [{ ...episode, filePath: "Podcasts/Episode.mp3", size: 4 }], + }); + + expect(downloadItemTitles(episode)).toEqual(["Remove file"]); + }); +}); diff --git a/src/ui/PodcastView/spawnEpisodeContextMenu.ts b/src/ui/PodcastView/spawnEpisodeContextMenu.ts index fcce7192..2a779898 100644 --- a/src/ui/PodcastView/spawnEpisodeContextMenu.ts +++ b/src/ui/PodcastView/spawnEpisodeContextMenu.ts @@ -19,6 +19,7 @@ import { ViewState } from "src/types/ViewState"; import { get } from "svelte/store"; import { isEpisodeFinished } from "src/utility/episodeStatus"; import { isSameStoredEpisode } from "src/utility/episodeKey"; +import { isLocalFile } from "src/utility/isLocalFile"; import { buildQueueReorderMenuItems } from "./queueReorderMenu"; interface DisabledMenuItems { @@ -86,11 +87,12 @@ export default function spawnEpisodeContextMenu( if (!disabledMenuItems?.download) { const isDownloaded = downloadedEpisodes.isEpisodeDownloaded(episode); + const removeTitle = isLocalFile(episode) ? "Remove from Local Files" : "Remove file"; menu.addItem((item) => item .setIcon(isDownloaded ? "cross" : "download") - .setTitle(isDownloaded ? "Remove file" : "Download") + .setTitle(isDownloaded ? removeTitle : "Download") .onClick(() => { if (isDownloaded) { void removeDownloadedEpisode(episode); diff --git a/tests/e2e/harness.ts b/tests/e2e/harness.ts index 80bdcaff..2ac3dc30 100644 --- a/tests/e2e/harness.ts +++ b/tests/e2e/harness.ts @@ -187,6 +187,50 @@ export function evalJsonAsync(obsidian: ObsidianClient, code: string): Promis return obsidian.dev.evalJsonAsync(code); } +export async function writeSilentWav( + obsidian: ObsidianClient, + audioPath: string, + seconds: number, +): Promise { + await evalJsonAsync( + obsidian, + `(async () => { + const sampleRate = 8_000; + const sampleCount = sampleRate * ${seconds}; + const bytes = new Uint8Array(44 + sampleCount); + const view = new DataView(bytes.buffer); + const ascii = (offset, value) => { + for (let index = 0; index < value.length; index++) { + bytes[offset + index] = value.charCodeAt(index); + } + }; + + ascii(0, "RIFF"); + view.setUint32(4, 36 + sampleCount, true); + ascii(8, "WAVE"); + ascii(12, "fmt "); + view.setUint32(16, 16, true); + view.setUint16(20, 1, true); + view.setUint16(22, 1, true); + view.setUint32(24, sampleRate, true); + view.setUint32(28, sampleRate, true); + view.setUint16(32, 1, true); + view.setUint16(34, 8, true); + ascii(36, "data"); + view.setUint32(40, sampleCount, true); + bytes.fill(128, 44); + + const existing = app.vault.getAbstractFileByPath(${JSON.stringify(audioPath)}); + if (existing) { + await app.vault.modifyBinary(existing, bytes.buffer); + } else { + await app.vault.createBinary(${JSON.stringify(audioPath)}, bytes.buffer); + } + return true; + })()`, + ); +} + export async function waitForPodNotesReady(obsidian: ObsidianClient): Promise { await obsidian.waitFor(() => podNotesReady(obsidian), { ...WAIT_OPTS, diff --git a/tests/e2e/local-files-runtime.test.ts b/tests/e2e/local-files-runtime.test.ts new file mode 100644 index 00000000..ce12b327 --- /dev/null +++ b/tests/e2e/local-files-runtime.test.ts @@ -0,0 +1,388 @@ +import type { ObsidianClient } from "obsidian-e2e"; +import { describe, expect, test } from "vitest"; +import type { IPodNotesSettings } from "../../src/types/IPodNotesSettings"; +import { + createPodNotesE2EHarness, + evalJsonAsync, + openPodNotesView, + RELOAD_OPTIONS, + VIEW_TYPE, + WAIT_OPTS, + waitForPodNotesReady, + writeSilentWav, +} from "./harness"; + +type PodNotesData = Partial; + +const SILENT_WAV_SECOND_BYTES = 44 + 8_000; + +const getContext = createPodNotesE2EHarness("local-files-runtime"); + +describe("local files runtime", () => { + test("removing a local file from Local Files keeps the user's file in the vault", async () => { + const { obsidian, plugin, sandbox } = getContext(); + const title = `E2E Own Recording ${Date.now()}`; + const fileName = `${title}.wav`; + const audioPath = sandbox.path(fileName); + const inDownloads = (data: PodNotesData) => + data.downloadedEpisodes?.["local file"]?.some( + (entry) => entry.filePath === audioPath, + ) ?? false; + const inLocalFiles = (data: PodNotesData) => + data.localFiles?.episodes.some((episode) => episode.title === title) ?? false; + const trashedFixture = async () => + (await listTrash(obsidian)).filter((path) => path.includes(title)); + + await plugin.updateDataAndReload((data) => { + data.downloadedEpisodes = {}; + data.playedEpisodes = {}; + }, RELOAD_OPTIONS); + await waitForPodNotesReady(obsidian); + await writeSilentWav(obsidian, audioPath, 60); + await sandbox.waitForExists(fileName, WAIT_OPTS); + const fileExplorerWasEnabled = await setFileExplorerEnabled(obsidian, true); + + try { + await openFileMenu(obsidian, audioPath); + await clickMenuItem(obsidian, "Play with PodNotes"); + await plugin.waitForData( + (data) => inDownloads(data) && inLocalFiles(data), + WAIT_OPTS, + ); + // Until #355 keeps the player's