diff --git a/src/main.persistence.test.ts b/src/main.persistence.test.ts index 9639d145..2c276112 100644 --- a/src/main.persistence.test.ts +++ b/src/main.persistence.test.ts @@ -6,11 +6,14 @@ import * as obsidian from "obsidian"; import { DEFAULT_SETTINGS } from "./constants"; import PodNotes from "./main"; import * as privateFeedUrls from "./utility/privateFeedUrl"; +import { getProgressKey } from "./utility/episodeKey"; +import { getPlayedEpisode } from "./utility/episodeStatus"; +import type { Episode } from "./types/Episode"; import * as persistenceMerge from "./persistence/merge"; import { encodePodNotesData } from "./persistence/podNotesData"; import * as podNotesData from "./persistence/podNotesData"; import { PlayerController } from "./player/PlayerController"; -import { downloadedEpisodes, localFiles, plugin as pluginStore } from "./store"; +import { downloadedEpisodes, localFiles, playedEpisodes, plugin as pluginStore } from "./store"; import { bindStoresToSettings } from "./store/persistence"; import { CredentialRepository } from "./services/CredentialRepository"; import { @@ -340,6 +343,8 @@ describe("private addresses adopted from sync", () => { await settled(); expect(plugin.settings.playedEpisodes["PRIV2 Both::PRIV2 Episode"].time).toBe(30); + const fromFeed = { ...plugin.settings.currentEpisode!, guid: "priv2-episode" }; + expect(getPlayedEpisode(plugin.settings.playedEpisodes, fromFeed)?.time).toBe(30); expect(await plugin.loadData()).toEqual(released2211); expect(save).not.toHaveBeenCalled(); }); @@ -938,6 +943,66 @@ describe("PodNotes persistence integration", () => { ); }); + it("lends a 2.21.1 library's progress to its episode but not to two that share a title, and a write adds one key", async () => { + const fromFeed = (title: string, guid: string, sharedTitle?: true): Episode => ({ + title, + podcastName: "PRIV2 Both", + guid, + sharedTitle, + streamUrl: "", + url: "", + description: "", + content: "", + }); + const released = { + ...structuredClone(released2211), + playedEpisodes: { + ...released2211.playedEpisodes, + "PRIV2 Both::Rerun": { + title: "Rerun", + podcastName: "PRIV2 Both", + time: 180, + duration: 180, + finished: true, + }, + }, + }; + const saveData = vi.fn().mockResolvedValue(undefined); + const plugin = makePlugin(saveData, { dataFileExists: true }); + Object.assign(plugin, { loadData: vi.fn().mockResolvedValue(structuredClone(released)) }); + const episode = fromFeed("PRIV2 Episode", "priv2-episode"); + + await plugin.loadSettings(); + attachDataFileWriter(plugin); + (plugin as unknown as { setCollectionStores(): void }).setCollectionStores(); + onTestFinished(bindStoresToSettings(plugin)); + + expect(playedEpisodes.get(episode)).toEqual( + released.playedEpisodes["PRIV2 Both::PRIV2 Episode"], + ); + expect(playedEpisodes.get(fromFeed("Rerun", "rerun-1", true))).toBeUndefined(); + expect(playedEpisodes.get(fromFeed("Rerun", "rerun-2", true))).toBeUndefined(); + + playedEpisodes.setEpisodeTime(episode, 40, 180, false); + await plugin.flushSaves(); + + expect(saveData).toHaveBeenCalledTimes(1); + expect(saveData.mock.calls[0][0]).toEqual({ + ...savedByThisVersion(released), + libraryId: "", + playedEpisodes: { + ...released.playedEpisodes, + [getProgressKey(episode)]: { + title: "PRIV2 Episode", + podcastName: "PRIV2 Both", + time: 40, + duration: 180, + finished: false, + }, + }, + }); + }); + it("moves a 2.21.1 library's path-secret feed address into SecretStorage once, for each device to set", async () => { const rawUrl = "https://priv3.supercast.com/feeds/RELEASED_SENTINEL"; const released = JSON.parse( diff --git a/src/parser/feedDocumentParser.ts b/src/parser/feedDocumentParser.ts index 2ac603a0..64ebeb22 100644 --- a/src/parser/feedDocumentParser.ts +++ b/src/parser/feedDocumentParser.ts @@ -43,6 +43,8 @@ export interface ParsedEpisodeDocument { duration?: number; chaptersUrl?: string; guid?: string; + /** In the feed cache: another item of the full feed has this title, though the cut may have dropped it. */ + sharedTitle?: true; transcripts?: TranscriptLink[]; mediaType: EpisodeMediaType; } diff --git a/src/persistence/episodeCodec.ts b/src/persistence/episodeCodec.ts index 30156020..096ec7f6 100644 --- a/src/persistence/episodeCodec.ts +++ b/src/persistence/episodeCodec.ts @@ -48,6 +48,10 @@ export function decodeEpisode( setOptionalString(decoded, value, "feedUrl", warnings, path); setOptionalString(decoded, value, "guid", warnings, path); + if (value.sharedTitle !== undefined && value.sharedTitle !== true) { + delete decoded.sharedTitle; + warn(warnings, `${path}.sharedTitle`, "expected true"); + } setOptionalString(decoded, value, "artworkUrl", warnings, path); setOptionalString(decoded, value, "itunesTitle", warnings, path); setOptionalString(decoded, value, "chaptersUrl", warnings, path); diff --git a/src/persistence/merge.ts b/src/persistence/merge.ts index 1727712b..3e71a14d 100644 --- a/src/persistence/merge.ts +++ b/src/persistence/merge.ts @@ -51,7 +51,7 @@ function isEpisodeEntry(value: unknown): value is EpisodeEntry { ); } -/** The identity played state and downloads use, so same-titled episodes of two podcasts stay apart. */ +/** The identity playlists and downloads use, so same-titled episodes of two podcasts stay apart. */ function episodeEntryKey(entry: EpisodeEntry): string { return getEpisodeKey(entry as unknown as Episode); } diff --git a/src/persistence/podNotesData.test.ts b/src/persistence/podNotesData.test.ts index d76ccab9..e55aa36e 100644 --- a/src/persistence/podNotesData.test.ts +++ b/src/persistence/podNotesData.test.ts @@ -564,6 +564,17 @@ describe("PodNotes data schema", () => { ).toEqual(stored); }); + it("keeps an episode's shared title mark only when it is true", () => { + const decoded = decodePodNotesData({ + currentEpisode: { ...episode("Restored"), sharedTitle: "no" }, + queue: { episodes: [{ ...episode("Queued"), sharedTitle: true }] }, + }); + + expect(decoded.settings.currentEpisode).not.toHaveProperty("sharedTitle"); + expect(decoded.settings.queue.episodes[0].sharedTitle).toBe(true); + expect(decoded.warnings).toEqual(["currentEpisode.sharedTitle: expected true"]); + }); + it("drops a guid that is not a string", () => { const decoded = decodePodNotesData({ currentEpisode: { ...episode("Restored"), guid: 12345 }, diff --git a/src/player/PlayerController.test.ts b/src/player/PlayerController.test.ts index 81465ec8..7f545db7 100644 --- a/src/player/PlayerController.test.ts +++ b/src/player/PlayerController.test.ts @@ -795,6 +795,32 @@ describe("switching episodes (#353)", () => { expect(media().currentTime).toBe(1234); }); + test.each([ + ["another item of its feed", episode("Rerun", { guid: "rerun-1", sharedTitle: true })], + ["a copy saved before guids", episode("Rerun")], + ])( + "starts an episode at its own position after %s with its title paused (#448)", + (_, first) => { + const second = episode("Rerun", { + guid: "rerun-2", + sharedTitle: true, + streamUrl: "https://cdn.example.com/Rerun-2.mp3", + }); + playedEpisodes.setEpisodeTime(second, 12, 3600, false); + const { player, media } = setup(); + player.play(first); + metadata(media()); + reach(media(), 30); + player.pause(); + + player.play(second); + metadata(media()); + + expect(media().currentTime).toBe(12); + expect(playedEpisodes.get(first)?.time).toBe(30); + }, + ); + test("does not queue the episode switched away from when the queue is not automatic", () => { const { player } = setup({ autoQueue: false }); player.restore(A); diff --git a/src/player/PlayerController.ts b/src/player/PlayerController.ts index a78326b0..f6e97604 100644 --- a/src/player/PlayerController.ts +++ b/src/player/PlayerController.ts @@ -6,7 +6,7 @@ import type { Episode, EpisodeMediaType } from "src/types/Episode"; import type { IPodNotesSettings } from "src/types/IPodNotesSettings"; import type { PlayedEpisode } from "src/types/PlayedEpisode"; import { createMediaUrlObjectFromFilePath } from "src/utility/createMediaUrlObjectFromFilePath"; -import { getEpisodeKey, isSameEpisode, isSameStoredEpisode } from "src/utility/episodeKey"; +import { getEpisodeKey, isSameFeedItem, isSameStoredEpisode } from "src/utility/episodeKey"; import { getPlayedEpisode } from "src/utility/episodeStatus"; import { getEpisodeMediaType } from "src/utility/mediaType"; import { normalizePlaybackRate } from "src/utility/playbackRate"; @@ -150,7 +150,7 @@ export class PlayerController { return; } if (!episode || (status !== "empty" && status !== "restored")) return; - this.moved &&= !!shown && isSameEpisode(shown, episode); + this.moved &&= !!shown && isSameFeedItem(shown, episode); this.setState({ status: "restored", episode, @@ -197,7 +197,7 @@ export class PlayerController { const source = selectSource(episode, get(downloadedEpisodes), this.plugin.app.vault); this.noteStoppedMove(); const { status, episode: outgoing, mediaType } = this.snapshot; - const sameEpisode = !!outgoing && isSameEpisode(outgoing, episode); + const sameEpisode = !!outgoing && isSameFeedItem(outgoing, episode); this.select(episode); const session = this.session; @@ -370,7 +370,7 @@ export class PlayerController { const shown = this.snapshot.episode; // Reloading the shown episode, as on its first play after a restart, keeps the user's speed. const playbackRate = - shown && isSameEpisode(shown, episode) + shown && isSameFeedItem(shown, episode) ? this.snapshot.playbackRate : normalizePlaybackRate(this.plugin.settings.defaultPlaybackRate); if (!source.src) { diff --git a/src/services/FeedRepository.test.ts b/src/services/FeedRepository.test.ts index 5bc1acf8..8bea9508 100644 --- a/src/services/FeedRepository.test.ts +++ b/src/services/FeedRepository.test.ts @@ -4,6 +4,7 @@ import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; import type { ParsedEpisodeDocument, ParsedFeedEpisodes } from "src/parser/feedDocumentParser"; import { encodeEpisode } from "src/persistence/episodeCodec"; import type { PodcastFeed } from "src/types/PodcastFeed"; +import { getPlayedEpisode } from "src/utility/episodeStatus"; import { privateFeedPlaceholder } from "src/utility/privateFeedUrl"; import { deferred, MemoryStorage, stubLegacyStorage } from "../../tests/mocks/feeds"; import { FeedRepository } from "./FeedRepository"; @@ -562,6 +563,20 @@ describe("FeedRepository", () => { ]); }); + test("marks each item whose title another item of the feed has", async () => { + const t = setup(); + const items = [item(1, { title: "Rerun" }), item(2), item(3, { title: "Rerun" })]; + await loaded(t, { Alpha: alpha }, doc("Alpha", items)); + + expect( + t.feed("Alpha")!.episodes.map(({ title, sharedTitle }) => [title, sharedTitle]), + ).toEqual([ + ["Rerun", true], + ["Episode 2", undefined], + ["Rerun", true], + ]); + }); + test("a guid whose twin the cache cut identifies no episode after a restart", async () => { const first = setup(); const items = Array.from({ length: 76 }, (_, index) => @@ -582,6 +597,30 @@ describe("FeedRepository", () => { expect(guids.get("Episode 75")).toBe("own-75"); }); + test("a title whose twin the cache cut still lends neither its old record after a failed refresh (#448)", async () => { + const first = setup(); + const items = Array.from({ length: 76 }, (_, index) => item(76 - index)); + items[0] = item(76, { title: "Rerun", guid: "rerun" }); + items[75] = item(1, { title: "Rerun", guid: "original" }); + await loaded(first, { Alpha: alpha }, doc("Alpha", items)); + + const t = setup({ storage: first.storage }); + t.repo.setSubscriptions({ Alpha: alpha }); + const refresh = t.repo.load(["Alpha"], "network"); + t.requests[0].respond.reject(new Error("503")); + await refresh; + + const rerun = t.feed("Alpha")!.episodes.find(({ guid }) => guid === "rerun")!; + const saved = { + title: "Rerun", + podcastName: "Alpha", + time: 40, + duration: 40, + finished: true, + }; + expect(getPlayedEpisode({ "Alpha::Rerun": saved }, rerun)).toBeUndefined(); + }); + test("reads an episode's transcripts from its own subscription's feed, by guid first", async () => { const t = setup(); const transcripts = [{ url: "https://media.test/b.vtt", type: "text/vtt" }]; diff --git a/src/services/FeedRepository.ts b/src/services/FeedRepository.ts index 9af6dcd3..549149b0 100644 --- a/src/services/FeedRepository.ts +++ b/src/services/FeedRepository.ts @@ -348,7 +348,8 @@ function projectEpisodes( items: readonly ParsedEpisodeDocument[], feed: Pick, ): Episode[] { - const repeated = repeatedGuids(items); + const repeated = repeatedValues(items, "guid"); + const sharedTitles = repeatedValues(items, "title"); return items.map((item) => ({ title: item.title, streamUrl: item.streamUrl, @@ -361,6 +362,7 @@ function projectEpisodes( feedUrl: feed.url, // A guid that several items repeat identifies none of them, so notes and links never get it. guid: item.guid && !repeated.has(item.guid) ? item.guid : undefined, + sharedTitle: item.sharedTitle === true || sharedTitles.has(item.title) || undefined, itunesTitle: item.itunesTitle || "", episodeNumber: item.episodeNumber, duration: item.duration, @@ -393,21 +395,28 @@ const MAX_CACHE_BYTES = 4 * 1024 * 1024; function keepNewestItems(doc: FeedDocument): FeedDocument { if (doc.items.length <= MAX_ITEMS_PER_FEED) return doc; const kept = new Set([...doc.items].sort(newestFirst).slice(0, MAX_ITEMS_PER_FEED)); - // The cut can drop one of two items that share a guid, which would make the other look unique. - const repeated = repeatedGuids(doc.items); + // The cut can drop one of two items that share a guid or a title, which would make the other look unique. + const repeated = repeatedValues(doc.items, "guid"); + const sharedTitles = repeatedValues(doc.items, "title"); const items = doc.items .filter((item) => kept.has(item)) - .map((item) => - item.guid && repeated.has(item.guid) ? { ...item, guid: undefined } : item, - ); + .map((item) => ({ + ...item, + guid: item.guid && !repeated.has(item.guid) ? item.guid : undefined, + sharedTitle: sharedTitles.has(item.title) || undefined, + })); return { ...doc, items, complete: false }; } -/** Guids that more than one item carries. */ -function repeatedGuids(items: readonly ParsedEpisodeDocument[]): Set { +/** Values of `field` that more than one item carries. */ +function repeatedValues( + items: readonly ParsedEpisodeDocument[], + field: "guid" | "title", +): Set { const seen = new Set(); const repeated = new Set(); - for (const { guid } of items) if (guid) (seen.has(guid) ? repeated : seen).add(guid); + for (const { [field]: value } of items) + if (value) (seen.has(value) ? repeated : seen).add(value); return repeated; } diff --git a/src/store/index.test.ts b/src/store/index.test.ts index ecbfc7c5..1f482745 100644 --- a/src/store/index.test.ts +++ b/src/store/index.test.ts @@ -86,6 +86,81 @@ describe("playedEpisodes store", () => { expect(stored[episode.title]?.finished).toBe(false); expect(stored["Design Podcast::Shared title"]?.finished).toBe(false); }); + + test("keeps apart two podcasts' episodes that carry one guid", () => { + playedEpisodes.setEpisodeTime({ ...episode, guid: "1" }, 30, 60, false); + playedEpisodes.markAsPlayed({ ...episode, podcastName: "Other Podcast", guid: "1" }); + + expect(playedEpisodes.get({ ...episode, guid: "1" })).toMatchObject({ + time: 30, + finished: false, + }); + }); + + describe("for two episodes of one feed with one title", () => { + const twinA: Episode = { ...episode, guid: "guid-a", sharedTitle: true }; + const twinB: Episode = { ...episode, guid: "guid-b", sharedTitle: true }; + const unshared: Episode = { ...episode, guid: "guid-own" }; + const legacy: PlayedEpisode = { + title: episode.title, + podcastName: "Design Podcast", + time: 45, + duration: 120, + finished: false, + }; + + test("keeps each one's own progress and finished state", () => { + playedEpisodes.setEpisodeTime(twinA, 30, 60, false); + playedEpisodes.markAsPlayed(twinB); + + expect(Object.keys(get(playedEpisodes))).toHaveLength(2); + expect(playedEpisodes.get(twinA)).toEqual({ ...legacy, time: 30, duration: 60 }); + expect(playedEpisodes.get(twinB)).toEqual({ + ...legacy, + time: 0, + duration: 0, + finished: true, + }); + }); + + test.each([ + [ + "saving a position", + () => playedEpisodes.setEpisodeTime(unshared, 50, 120, false), + { ...legacy, time: 50 }, + ], + [ + "marking it played", + () => playedEpisodes.markAsPlayed(unshared), + { ...legacy, time: 120, finished: true }, + ], + ])( + "lends the record saved under the title to neither, and to an unshared episode until %s", + (_, write, written) => { + playedEpisodes.set({ "Design Podcast::Shared title": { ...legacy } }); + + expect(playedEpisodes.get(twinA)).toBeUndefined(); + expect(playedEpisodes.get(twinB)).toBeUndefined(); + expect(playedEpisodes.get(unshared)).toEqual(legacy); + + write(); + + expect(get(playedEpisodes)["Design Podcast::Shared title"]).toEqual(legacy); + expect(Object.keys(get(playedEpisodes))).toHaveLength(2); + expect(playedEpisodes.get(unshared)).toEqual(written); + }, + ); + + test("marking one unplayed leaves the other played", () => { + playedEpisodes.markAsPlayed(twinA); + playedEpisodes.markAsPlayed(twinB); + + playedEpisodes.markAsUnplayed(twinA); + + expect(playedEpisodes.get(twinA)?.finished).toBe(false); + expect(playedEpisodes.get(twinB)?.finished).toBe(true); + }); + }); }); function downloadedEpisode( diff --git a/src/store/index.ts b/src/store/index.ts index 79e4a5e9..531e98d6 100644 --- a/src/store/index.ts +++ b/src/store/index.ts @@ -6,7 +6,7 @@ import type { Playlist } from "src/types/Playlist"; import type DownloadedEpisode from "src/types/DownloadedEpisode"; import type { LocalEpisode } from "src/types/LocalEpisode"; import { FAVORITES_SETTINGS, LOCAL_FILES_SETTINGS, QUEUE_SETTINGS } from "src/constants"; -import { getEpisodeKey, isSameEpisode } from "src/utility/episodeKey"; +import { getEpisodeKey, getProgressKey, isSameEpisode } from "src/utility/episodeKey"; import { getPlayedEpisode, getPlayedEpisodeAliasKeys } from "src/utility/episodeStatus"; import { recordStore } from "./recordStore"; @@ -42,7 +42,7 @@ export const playedEpisodes = (() => { if (!episode) return; update((playedEpisodes) => { - const key = getEpisodeKey(episode); + const key = getProgressKey(episode); if (!key) return playedEpisodes; playedEpisodes[key] = { @@ -60,21 +60,17 @@ export const playedEpisodes = (() => { if (!episode) return; update((playedEpisodes) => { - const key = getEpisodeKey(episode); + const key = getProgressKey(episode); if (!key) return playedEpisodes; - const playedEpisode = getPlayedEpisode(playedEpisodes, episode) || { + const duration = getPlayedEpisode(playedEpisodes, episode)?.duration ?? 0; + playedEpisodes[key] = { title: episode.title, podcastName: episode.podcastName, - time: 0, - duration: 0, - finished: false, + time: duration, + duration, + finished: true, }; - - playedEpisode.time = playedEpisode.duration; - playedEpisode.finished = true; - - playedEpisodes[key] = playedEpisode; return playedEpisodes; }); }, @@ -82,7 +78,7 @@ export const playedEpisodes = (() => { if (!episode) return; update((playedEpisodes) => { - const key = getEpisodeKey(episode); + const key = getProgressKey(episode); if (!key) return playedEpisodes; markPlayedEpisodeAliasesAsUnplayed( diff --git a/src/types/Episode.ts b/src/types/Episode.ts index e6f8623d..bcc6a1f0 100644 --- a/src/types/Episode.ts +++ b/src/types/Episode.ts @@ -8,8 +8,10 @@ export interface Episode { content: string; podcastName: string; feedUrl?: string; - /** The feed item's ``. It finds the episode within its feed for links and notes, and is not a storage key. */ + /** The feed item's ``. It finds the episode within its feed for links and notes, and keys its progress. */ guid?: string; + /** Another item of its feed has its title, so a record saved under the title alone cannot be told apart for it. */ + sharedTitle?: true; artworkUrl?: string; episodeDate?: Date; itunesTitle?: string; diff --git a/src/ui/PodcastView/EpisodeList.svelte b/src/ui/PodcastView/EpisodeList.svelte index 2783a516..bddbb464 100644 --- a/src/ui/PodcastView/EpisodeList.svelte +++ b/src/ui/PodcastView/EpisodeList.svelte @@ -31,6 +31,7 @@ {cover} {date} resolved={isAvailable} + playedKey={entry.playedEpisodeKey} onplay={() => openEpisode(services, episode, isAvailable)} onmenu={(at) => showMenu( diff --git a/src/ui/PodcastView/EpisodeRow.svelte b/src/ui/PodcastView/EpisodeRow.svelte index 5e1011d4..73430ac9 100644 --- a/src/ui/PodcastView/EpisodeRow.svelte +++ b/src/ui/PodcastView/EpisodeRow.svelte @@ -13,6 +13,7 @@ cover = false, date = !cover, resolved = true, + playedKey, queue = false, dragging = false, onplay, @@ -26,6 +27,8 @@ date?: boolean; /** False for a Played record that matched no episode. */ resolved?: boolean; + /** A Played row's record, which its episode may not find on its own. */ + playedKey?: string; /** A queue row: centered, flush left, without a progress bar. */ queue?: boolean; /** Lifted while its grip drags it. */ @@ -48,6 +51,7 @@ const row = $derived( episodeRowModel(episode, { played: $playedEpisodes, + playedKey, downloads: $downloadedEpisodes, now, playing, diff --git a/src/ui/PodcastView/episodeRow.test.ts b/src/ui/PodcastView/episodeRow.test.ts index f172b294..648696a7 100644 --- a/src/ui/PodcastView/episodeRow.test.ts +++ b/src/ui/PodcastView/episodeRow.test.ts @@ -98,6 +98,18 @@ describe("an episode row", () => { ); }); + test("is not current for another item of its feed with its title and media", () => { + const twin = (guid: string): Episode => ({ ...episode, guid, sharedTitle: true }); + expect(row({ now: twin("original"), playing: true }, twin("rerun")).current).toBe(false); + }); + + test("shows a Played row's own record when its episode cannot find it", () => { + const played = { + "guid record": { ...record(60, 60, true)["Field Notes::#214 Reading slowly"] }, + }; + expect(row({ played, playedKey: "guid record" }).played).toBe(true); + }); + test("says it is downloaded and has a note", () => { const downloads = { "Field Notes": [{ ...episode, filePath: "a.mp3", size: 1 }] }; expect(row({ downloads, hasNote: true })).toMatchObject({ diff --git a/src/ui/PodcastView/episodeRow.ts b/src/ui/PodcastView/episodeRow.ts index 408a1067..dfa3921a 100644 --- a/src/ui/PodcastView/episodeRow.ts +++ b/src/ui/PodcastView/episodeRow.ts @@ -2,7 +2,7 @@ import { downloadedCopy, offlinePlayable } from "src/player/source"; import type DownloadedEpisode from "src/types/DownloadedEpisode"; import type { Episode } from "src/types/Episode"; import type { PlayedEpisode } from "src/types/PlayedEpisode"; -import { getEpisodeKey, isSameEpisode } from "src/utility/episodeKey"; +import { getEpisodeKey, isSameFeedItem } from "src/utility/episodeKey"; import { getPlayedEpisode, type PlayedEpisodeMap } from "src/utility/episodeStatus"; import { isLocalFile } from "src/utility/isLocalFile"; import { getEpisodeMediaType, isSameMediaSource } from "src/utility/mediaType"; @@ -28,6 +28,8 @@ export function availabilityOf( export interface RowState { readonly played: PlayedEpisodeMap; + /** A Played row's record, which its episode may not find on its own. */ + readonly playedKey?: string; readonly downloads: Downloads; readonly now: Episode | undefined; readonly playing: boolean; @@ -74,14 +76,11 @@ export function formatMinutes(seconds: number): string { return total >= 60 ? `${Math.floor(total / 60)} h ${total % 60} min` : `${total} min`; } -/** - * Whether `episode` is the player's. Same-titled episodes of one podcast share a key, - * so their media tells them apart, as it does for the player. - */ +/** Whether `episode` is the player's: one feed item, from the same media. */ export function isCurrentEpisode(now: Episode | undefined, episode: Episode): boolean { return ( now !== undefined && - isSameEpisode(now, episode) && + isSameFeedItem(now, episode) && isSameMediaSource(now.streamUrl, episode.streamUrl) ); } @@ -129,7 +128,9 @@ export function sourceOf(episode: Episode): string { } export function episodeRowModel(episode: Episode, state: RowState): EpisodeRowModel { - const record = getPlayedEpisode(state.played, episode); + const record = state.playedKey + ? state.played[state.playedKey] + : getPlayedEpisode(state.played, episode); const played = record?.finished ?? false; const left = leftOf(record); const duration = durationOf(episode, record); diff --git a/src/utility/episodeKey.ts b/src/utility/episodeKey.ts index 29ee33c4..f2a56faf 100644 --- a/src/utility/episodeKey.ts +++ b/src/utility/episodeKey.ts @@ -1,4 +1,5 @@ import type { Episode } from "src/types/Episode"; +import { isSameMediaSource } from "src/utility/mediaType"; const KEY_DELIMITER = "::"; @@ -125,3 +126,29 @@ export function isSameStoredEpisode( export function isSameEpisode(a: Episode, b: Episode): boolean { return getEpisodeKey(a) === getEpisodeKey(b); } + +/** + * Starts a progress key made from a podcast name and a guid. An escaped episode + * key has one NUL and then a non-NUL character, and feed text has none. + */ +const GUID_KEY_PREFIX = "\u0000\u0000"; + +/** The key of an episode's own played record: its guid within its podcast when it has one, else its episode key. */ +export function getProgressKey(episode: Episode): string { + if (!episode.guid) return getEpisodeKey(episode); + return `${GUID_KEY_PREFIX}${escapeKeyComponent(episode.podcastName)}:${escapeKeyComponent(episode.guid)}`; +} + +export function isGuidProgressKey(key: string): boolean { + return key.startsWith(GUID_KEY_PREFIX); +} + +/** + * One feed item: one episode key, and one guid when both carry one. A snapshot saved + * before guids still matches its item, and only its media when the title is shared. + */ +export function isSameFeedItem(a: Episode, b: Episode): boolean { + if (!isSameEpisode(a, b)) return false; + if (a.guid && b.guid) return a.guid === b.guid; + return !(a.sharedTitle || b.sharedTitle) || isSameMediaSource(a.streamUrl, b.streamUrl); +} diff --git a/src/utility/episodeListEntry.test.ts b/src/utility/episodeListEntry.test.ts index fffabbdb..8567a2c0 100644 --- a/src/utility/episodeListEntry.test.ts +++ b/src/utility/episodeListEntry.test.ts @@ -2,6 +2,7 @@ import { describe, expect, test } from "vitest"; import type { Episode } from "src/types/Episode"; import type { PlayedEpisode } from "src/types/PlayedEpisode"; +import { getProgressKey } from "./episodeKey"; import { buildPlayedEpisodeListEntries } from "./episodeListEntry"; function episode(title: string, podcastName: string, episodeDate: string): Episode { @@ -78,6 +79,36 @@ describe("episodeListEntry", () => { expect(entries.map((entry) => entry.episode.title)).toEqual(["Newer", "Older", "Missing"]); }); + + test("lists two played episodes of one feed with one title apart, without the title's record", () => { + const first = { + ...episode("Rerun", "Design Podcast", "2024-01-01T00:00:00.000Z"), + guid: "guid-a", + sharedTitle: true as const, + }; + const second = { + ...episode("Rerun", "Design Podcast", "2024-03-01T00:00:00.000Z"), + guid: "guid-b", + sharedTitle: true as const, + }; + + const entries = buildPlayedEpisodeListEntries( + { + "Design Podcast::Rerun": played("Rerun", "Design Podcast"), + [getProgressKey(first)]: played("Rerun", "Design Podcast"), + [getProgressKey(second)]: played("Rerun", "Design Podcast"), + }, + [[first, second]], + ); + + expect(entries.map(({ episode, playedEpisodeKey }) => [episode, playedEpisodeKey])).toEqual( + [ + [second, getProgressKey(second)], + [first, getProgressKey(first)], + ], + ); + }); + describe("a legacy record keyed by its title", () => { const showA = episode("Shared episode", "Show A", "2024-02-01T00:00:00.000Z"); const showB = episode("Shared episode", "Show B", "2024-02-01T00:00:00.000Z"); diff --git a/src/utility/episodeListEntry.ts b/src/utility/episodeListEntry.ts index 87893a03..35fbfbca 100644 --- a/src/utility/episodeListEntry.ts +++ b/src/utility/episodeListEntry.ts @@ -1,6 +1,6 @@ import type { Episode } from "src/types/Episode"; import type { PlayedEpisode } from "src/types/PlayedEpisode"; -import { getEpisodeKey } from "src/utility/episodeKey"; +import { getEpisodeKey, getProgressKey, isGuidProgressKey } from "src/utility/episodeKey"; import { getFinishedPlayedEpisodeRecords, getPlayedEpisodeRecordKey, @@ -60,7 +60,10 @@ function resolvePlayedEpisode( playedEpisode: PlayedEpisode, episodeLookup: Map, ): Episode | undefined { - const lookupKeys = [getPlayedEpisodeRecordKey(playedEpisode), key, playedEpisode.title]; + // A guid record's title can be its twin's. + const lookupKeys = isGuidProgressKey(key) + ? [key] + : [getPlayedEpisodeRecordKey(playedEpisode), key, playedEpisode.title]; for (const lookupKey of lookupKeys) { const episode = episodeLookup.get(lookupKey); @@ -77,6 +80,7 @@ function buildEpisodeLookup(episodes: Episode[]): Map { for (const episode of episodes) { const keys = [ + getProgressKey(episode), getEpisodeKey(episode), episode.podcastName ? `${episode.podcastName}::${episode.title}` : "", episode.title, diff --git a/src/utility/episodeStatus.ts b/src/utility/episodeStatus.ts index 19ece39b..df58b519 100644 --- a/src/utility/episodeStatus.ts +++ b/src/utility/episodeStatus.ts @@ -1,6 +1,6 @@ import type { Episode } from "src/types/Episode"; import type { PlayedEpisode } from "src/types/PlayedEpisode"; -import { getEpisodeKey } from "src/utility/episodeKey"; +import { getEpisodeKey, getProgressKey, isGuidProgressKey } from "src/utility/episodeKey"; export type PlayedEpisodeMap = Record; @@ -13,18 +13,34 @@ export function podcastNamesAgree(podcastName: string, otherPodcastName: string) return !podcastName || !otherPodcastName || podcastName === otherPodcastName; } +function own(playedEpisodes: PlayedEpisodeMap, key: string): PlayedEpisode | undefined { + return Object.prototype.hasOwnProperty.call(playedEpisodes, key) + ? playedEpisodes[key] + : undefined; +} + +/** A record saved under the title, as 2.21.1 saved all progress: the episode key, else the raw title when its podcast agrees. */ +function titleRecord( + playedEpisodes: PlayedEpisodeMap, + episode: Episode, +): PlayedEpisode | undefined { + const legacy = own(playedEpisodes, episode.title); + return ( + own(playedEpisodes, getEpisodeKey(episode)) ?? + (legacy && podcastNamesAgree(legacy.podcastName, episode.podcastName) ? legacy : undefined) + ); +} + export function getPlayedEpisode( playedEpisodes: PlayedEpisodeMap, episode: Episode | null | undefined, ): PlayedEpisode | undefined { if (!episode) return undefined; - const own = (key: string) => - Object.prototype.hasOwnProperty.call(playedEpisodes, key) ? playedEpisodes[key] : undefined; - const legacy = own(episode.title); + // A title record shared by two items of one feed cannot be told apart, so neither borrows it. return ( - own(getEpisodeKey(episode)) ?? - (legacy && podcastNamesAgree(legacy.podcastName, episode.podcastName) ? legacy : undefined) + own(playedEpisodes, getProgressKey(episode)) ?? + (episode.guid && episode.sharedTitle ? undefined : titleRecord(playedEpisodes, episode)) ); } @@ -54,7 +70,12 @@ export function getPlayedEpisodeAliasKeys( const targetKey = getPlayedEpisodeRecordKey(episode); for (const [key, playedEpisode] of Object.entries(playedEpisodes)) { - if (key === sourceKey || key === targetKey || isSamePlayedEpisode(playedEpisode, episode)) { + // A guid record may share its title with a twin, so only its own key names it. + if ( + key === sourceKey || + key === targetKey || + (!isGuidProgressKey(key) && isSamePlayedEpisode(playedEpisode, episode)) + ) { keys.add(key); } } @@ -66,11 +87,20 @@ export function getFinishedPlayedEpisodeRecords( playedEpisodes: PlayedEpisodeMap, ): PlayedEpisodeRecord[] { const recordsByEpisodeKey = new Map(); + const records = Object.entries(playedEpisodes); + // Reads prefer a guid record to the title record, and twins' title record belongs to neither. + const guidTitles = new Set( + records + .filter(([key]) => isGuidProgressKey(key)) + .map(([, episode]) => getPlayedEpisodeRecordKey(episode)), + ); - for (const [key, episode] of Object.entries(playedEpisodes)) { + for (const [key, episode] of records) { if (!episode.finished) continue; - const episodeKey = getPlayedEpisodeRecordKey(episode); + const byGuid = isGuidProgressKey(key); + const episodeKey = byGuid ? key : getPlayedEpisodeRecordKey(episode); + if (!byGuid && guidTitles.has(episodeKey)) continue; const existingRecord = recordsByEpisodeKey.get(episodeKey); if (!existingRecord || isCompositePlayedKey(key)) { recordsByEpisodeKey.set(episodeKey, { key, episode }); diff --git a/tests/e2e/journey-playback.test.ts b/tests/e2e/journey-playback.test.ts index e8e22872..6d66f815 100644 --- a/tests/e2e/journey-playback.test.ts +++ b/tests/e2e/journey-playback.test.ts @@ -5,6 +5,7 @@ import { closeSettings, openInReadingView, openPodNotesSettings, + reloadObsidian, restartObsidian, runCommand, showLibrary, @@ -12,7 +13,7 @@ import { } from "./journey/actions"; import { click } from "./journey/input"; import { ui } from "./journey/targets"; -import { waitUntilShown } from "./journey/wait"; +import { eventually, listedEpisodeLabels, waitUntilShown } from "./journey/wait"; import { type FixtureEpisode, type FixtureFeed, @@ -51,6 +52,24 @@ const OTHER_SHOW: FixtureFeed = { artwork: true, episodes: [{ title: LONG.title, seconds: 60, pubDate: pubDate(0) }], }; +const RERUN: FixtureEpisode = { + title: "Repeat Episode", + seconds: 40, + pubDate: pubDate(0), + guid: "repeat-rerun", +}; +const ORIGINAL: FixtureEpisode = { + title: "Repeat Episode", + seconds: 60, + pubDate: pubDate(30), + guid: "repeat-original", +}; +const REPEATS: FixtureFeed = { + title: "Journey Repeats", + slug: "repeats", + artwork: true, + episodes: [RERUN, ORIGINAL], +}; const isolatedInstance = Boolean(process.env.OBSIDIAN_E2E_OBSIDIAN_HOME); const getContext = createPodNotesE2EHarness("journey-playback"); @@ -94,7 +113,97 @@ async function playFromList(episode: FixtureEpisode): Promise { ); } +type RepeatProgress = Readonly<{ duration: number; seconds: number; finished: boolean }>; + +/** Saved progress for REPEATS' title, told apart by the length of the media that wrote it. */ +async function repeatProgress(): Promise { + const data = await getContext().plugin.data>().read(); + return Object.values>(data.playedEpisodes ?? {}) + .filter((record) => record.podcastName === REPEATS.title && record.title === RERUN.title) + .map((record) => ({ + duration: Math.round(record.duration), + seconds: Math.round(record.time), + finished: record.finished === true, + })) + .sort((a, b) => a.duration - b.duration); +} + +async function showRepeats(): Promise { + const { obsidian } = getContext(); + await showPodNotes(obsidian); + await showLibrary(obsidian); + await click(obsidian, ui.podcastCard(REPEATS.title)); + await waitUntilShown(obsidian, ui.episodeAt(RERUN.title, 1)); + return await listedEpisodeLabels(obsidian); +} + describe("playback journeys", () => { + test("#448 same-titled episodes of one podcast keep their own position and played state", async () => { + const context = getContext(); + const { obsidian } = context; + const server = await serve(REPEATS); + await seedLibrary(context, server, { subscriptions: [REPEATS] }); + + await showRepeats(); + await click(obsidian, ui.episodeAt(ORIGINAL.title, 1)); + await waitForPlayback( + obsidian, + (playback) => + playback.media?.src === server.mediaUrl(REPEATS, ORIGINAL) && + playback.playing && + playback.seconds > 0, + "the 60 s episode playing", + ); + await evalApi("api.currentTime = 30"); + await waitForPlayback(obsidian, (playback) => playback.seconds >= 30, "seeked to 30 s"); + await click(obsidian, ui.playbackToggle); + await waitForPlayback(obsidian, (playback) => !playback.playing, "paused"); + expect( + await eventually(obsidian, repeatProgress, [ + { duration: 60, seconds: 30, finished: false }, + ]), + ).toEqual([{ duration: 60, seconds: 30, finished: false }]); + + await showRepeats(); + await click(obsidian, ui.episodeAt(RERUN.title, 0)); + const started = await waitForPlayback( + obsidian, + (playback) => + playback.media?.src === server.mediaUrl(REPEATS, RERUN) && + playback.playing && + playback.seconds > 0, + "the 40 s episode playing", + ); + expect.soft(started.seconds, "the 40 s episode starts at its own position").toBeLessThan(5); + await evalApi("api.currentTime = 37"); + const finished = [ + { duration: 40, seconds: 40, finished: true }, + { duration: 60, seconds: 30, finished: false }, + ]; + expect.soft(await eventually(obsidian, repeatProgress, finished, 15_000)).toEqual(finished); + + const rows = ["Repeat Episode, played, paused", "Repeat Episode, 30 s left"]; + expect.soft(await showRepeats()).toEqual(rows); + + await reloadObsidian(obsidian); + expect(await repeatProgress()).toEqual(finished); + expect(await showRepeats()).toEqual(rows); + await click(obsidian, ui.episodeAt(ORIGINAL.title, 1)); + const resumed = await waitForPlayback( + obsidian, + (playback) => + playback.media?.src === server.mediaUrl(REPEATS, ORIGINAL) && + playback.playing && + playback.seconds > 0, + "the 60 s episode playing after the reload", + ); + expect( + resumed.seconds, + "the 60 s episode resumes at its own position", + ).toBeGreaterThanOrEqual(29.5); + expect(resumed.seconds).toBeLessThan(34); + }, 90_000); + test("J-LIST-08 clicking an episode in the list plays its real media", async () => { const context = getContext(); const server = await serve(); diff --git a/tests/e2e/journey/targets.ts b/tests/e2e/journey/targets.ts index 21f5c648..783d0486 100644 --- a/tests/e2e/journey/targets.ts +++ b/tests/e2e/journey/targets.ts @@ -78,6 +78,11 @@ export const ui = { name: `episode ${title}`, elements: `[...document.querySelectorAll(${jsLiteral(`${PLAYER_VIEW} .pn-ep-main`)})].filter((el) => !el.closest(".pn-player") && el.querySelector(".pn-ep-title")?.textContent.trim() === ${jsLiteral(title)})`, }), + /** The row at `index` among the shown episodes titled `title`, for a podcast that repeats a title. */ + episodeAt: (title: string, index: number): UiTarget => ({ + name: `episode ${title} #${index + 1}`, + elements: `[...document.querySelectorAll(${jsLiteral(`${PLAYER_VIEW} .pn-ep-main`)})].filter((el) => !el.closest(".pn-player") && el.getClientRects().length > 0 && el.querySelector(".pn-ep-title")?.textContent.trim() === ${jsLiteral(title)}).slice(${index}, ${index + 1})`, + }), episodeLoading: byCss( "episode list loading", `${PLAYER_VIEW} .pn-count[role="status"]`, diff --git a/tests/e2e/journey/wait.ts b/tests/e2e/journey/wait.ts index 561454a4..c118a796 100644 --- a/tests/e2e/journey/wait.ts +++ b/tests/e2e/journey/wait.ts @@ -44,6 +44,13 @@ export async function listedEpisodes(obsidian: ObsidianClient): Promise { + return await obsidian.dev.evalJson( + `[...document.querySelectorAll(${jsLiteral(`${PLAYER_VIEW} .pn-ep-main`)})].filter((el) => !el.closest(".pn-player") && el.getClientRects().length > 0).map((el) => el.getAttribute("aria-label") ?? "")`, + ); +} + export async function listedEpisodesWithHeadings(obsidian: ObsidianClient): Promise { return await obsidian.dev.evalJson( `[...document.querySelectorAll(${jsLiteral(`${PLAYER_VIEW} .pn-scroll :is(h2.pn-h2, .pn-ep-main .pn-ep-title)`)})].filter((el) => el.getClientRects().length > 0).map((el) => el.textContent.trim())`,