Skip to content

Commit 8ad7aa5

Browse files
authored
fix(uri): preserve '+' in episode titles and paths for timestamp links (#181)
Timestamp links (obsidian://podnotes, generated by {{linktime}}) failed to switch to or resume episodes whose title — or local-file path — contained a literal '+' (e.g. "C++ Tips", "A+ Players"). encodePodnotesURI used URLSearchParams, which encodes spaces as '+'. Obsidian decodes protocol query values with decodeURIComponent only (no '+' -> space), so the handler compensated with a blanket .replace(/\+/g, ' ') that also collapsed real '+' characters into spaces. The corrupted title/path then failed to match, and FeedParser.findItemByTitle threw an unhandled rejection (a silent no-op for the user). This pre-dated the timestamp-switching fix and broke both switching-to and resume-on-click for any '+'-bearing episode. - encodePodnotesURI now percent-encodes spaces as %20, so links round-trip losslessly through Obsidian's decoder — literal '+' included. - URIHandler resolves the episode name/path via ordered, raw-first candidates with a '+'-as-space fallback, so legacy links keep resolving without corrupting current-format titles. Feed episodes are resolved via getEpisodes and a miss now shows an "Episode not found" Notice instead of rejecting. - Removed the now-unused FeedParser.findItemByTitle. - Tests: encodePodnotesURI round-trip, URIHandler '+'/legacy/local/not-found cases, and the e2e capture test now uses the production encoder. Refs #164
1 parent a27d23d commit 8ad7aa5

7 files changed

Lines changed: 393 additions & 226 deletions

File tree

src/URIHandler.test.ts

Lines changed: 259 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -13,19 +13,22 @@ import {
1313
currentEpisode,
1414
currentTime,
1515
isPaused,
16+
localFiles,
1617
playedEpisodes,
1718
requestedPlaybackTime,
1819
viewState,
1920
} from "./store";
2021
import type { Episode } from "./types/Episode";
22+
import type { LocalEpisode } from "./types/LocalEpisode";
23+
import type { Playlist } from "./types/Playlist";
2124
import { ViewState } from "./types/ViewState";
2225

23-
const mockFindItemByTitle = vi.fn();
26+
const mockGetEpisodes = vi.fn();
2427
const testFeedUrl = "https://pod.example.com/feed.xml";
2528

2629
vi.mock("./parser/feedParser", () => ({
2730
default: class {
28-
findItemByTitle = mockFindItemByTitle;
31+
getEpisodes = mockGetEpisodes;
2932
},
3033
}));
3134

@@ -39,13 +42,33 @@ const testEpisode: Episode = {
3942
feedUrl: testFeedUrl,
4043
};
4144

45+
// Title with a literal '+': current-format links carry it as a real '+' after Obsidian's decode.
46+
const plusEpisode: Episode = {
47+
title: "Episode 50: C++ Tips",
48+
streamUrl: "https://pod.example.com/cpp.mp3",
49+
url: "https://pod.example.com/cpp",
50+
description: "",
51+
content: "",
52+
podcastName: "Test Podcast",
53+
feedUrl: testFeedUrl,
54+
};
55+
56+
const emptyLocalFiles: Playlist = {
57+
icon: "folder",
58+
name: "Local Files",
59+
episodes: [],
60+
shouldEpisodeRemoveAfterPlay: false,
61+
shouldRepeat: false,
62+
};
63+
4264
function resetStores() {
4365
currentEpisode.update(() => undefined as unknown as Episode);
4466
currentTime.set(0);
4567
isPaused.set(true);
4668
playedEpisodes.set({});
4769
requestedPlaybackTime.set(null);
4870
viewState.set(ViewState.PodcastGrid);
71+
localFiles.set({ ...emptyLocalFiles, episodes: [] });
4972
}
5073

5174
const api = {
@@ -54,20 +77,21 @@ const api = {
5477
},
5578
};
5679

57-
beforeEach(() => {
58-
resetStores();
59-
mockFindItemByTitle.mockResolvedValue(testEpisode);
60-
80+
function setApp(getAbstractFileByPath: (path: string) => unknown) {
6181
(globalThis as { app?: unknown }).app = {
62-
vault: {
63-
getAbstractFileByPath: vi.fn(() => null),
64-
},
82+
vault: { getAbstractFileByPath: vi.fn(getAbstractFileByPath) },
6583
};
84+
}
85+
86+
beforeEach(() => {
87+
resetStores();
88+
mockGetEpisodes.mockResolvedValue([testEpisode]);
89+
setApp(() => null);
6690
});
6791

6892
afterEach(() => {
6993
resetStores();
70-
mockFindItemByTitle.mockReset();
94+
mockGetEpisodes.mockReset();
7195
delete (globalThis as { app?: unknown }).app;
7296
});
7397

@@ -92,7 +116,7 @@ describe("podNotesURIHandler", () => {
92116
expect(get(currentTime)).toBe(120);
93117
expect(get(isPaused)).toBe(false);
94118
expect(get(requestedPlaybackTime)).toBeNull();
95-
expect(mockFindItemByTitle).not.toHaveBeenCalled();
119+
expect(mockGetEpisodes).not.toHaveBeenCalled();
96120
});
97121

98122
test("keeps the requested time for the player to apply after loading metadata", async () => {
@@ -108,20 +132,235 @@ describe("podNotesURIHandler", () => {
108132
api as never,
109133
);
110134

111-
expect(mockFindItemByTitle).toHaveBeenCalledWith(
112-
testEpisode.title,
113-
testFeedUrl,
114-
);
115-
expect(get(currentEpisode)).toMatchObject({
116-
title: testEpisode.title,
117-
});
135+
expect(mockGetEpisodes).toHaveBeenCalledWith(testFeedUrl);
136+
expect(get(currentEpisode)).toMatchObject({ title: testEpisode.title });
118137
expect(get(viewState)).toBe(ViewState.Player);
119138
expect(get(requestedPlaybackTime)).toEqual({
120139
episodeKey: `${testEpisode.podcastName}::${testEpisode.title}`,
121140
time: 240,
122141
});
123-
expect(get(playedEpisodes)[`${testEpisode.podcastName}::${testEpisode.title}`]?.finished).toBe(
124-
true,
142+
expect(
143+
get(playedEpisodes)[`${testEpisode.podcastName}::${testEpisode.title}`]?.finished,
144+
).toBe(true);
145+
});
146+
147+
test("switches to a non-loaded episode whose title contains a literal '+'", async () => {
148+
mockGetEpisodes.mockResolvedValue([plusEpisode]);
149+
150+
await podNotesURIHandler(
151+
{
152+
action: "podnotes",
153+
url: testFeedUrl,
154+
// Current-format link: Obsidian decodes %2B back to a literal '+'.
155+
episodeName: "Episode 50: C++ Tips",
156+
time: "300",
157+
},
158+
api as never,
125159
);
160+
161+
expect(get(currentEpisode)).toMatchObject({ title: "Episode 50: C++ Tips" });
162+
expect(get(viewState)).toBe(ViewState.Player);
163+
expect(get(requestedPlaybackTime)).toEqual({
164+
episodeKey: `${plusEpisode.podcastName}::${plusEpisode.title}`,
165+
time: 300,
166+
});
167+
});
168+
169+
test("resumes a paused, already-loaded episode whose title contains a literal '+'", async () => {
170+
currentEpisode.set(plusEpisode);
171+
viewState.set(ViewState.Player);
172+
isPaused.set(true);
173+
174+
await podNotesURIHandler(
175+
{
176+
action: "podnotes",
177+
url: testFeedUrl,
178+
episodeName: "Episode 50: C++ Tips",
179+
time: "90",
180+
},
181+
api as never,
182+
);
183+
184+
expect(get(currentTime)).toBe(90);
185+
expect(get(isPaused)).toBe(false);
186+
expect(mockGetEpisodes).not.toHaveBeenCalled();
187+
});
188+
189+
test("resolves a legacy '+'-as-space link to a non-loaded episode", async () => {
190+
mockGetEpisodes.mockResolvedValue([testEpisode]);
191+
192+
await podNotesURIHandler(
193+
{
194+
action: "podnotes",
195+
url: testFeedUrl,
196+
// Legacy link: spaces were encoded as '+'.
197+
episodeName: "Finished+Episode",
198+
time: "60",
199+
},
200+
api as never,
201+
);
202+
203+
expect(get(currentEpisode)).toMatchObject({ title: "Finished Episode" });
204+
expect(get(requestedPlaybackTime)).toEqual({
205+
episodeKey: `${testEpisode.podcastName}::${testEpisode.title}`,
206+
time: 60,
207+
});
208+
});
209+
210+
test("resumes the loaded episode for a legacy '+'-as-space link without re-resolving", async () => {
211+
currentEpisode.set(testEpisode);
212+
viewState.set(ViewState.Player);
213+
isPaused.set(true);
214+
215+
await podNotesURIHandler(
216+
{
217+
action: "podnotes",
218+
url: testFeedUrl,
219+
episodeName: "Finished+Episode",
220+
time: "45",
221+
},
222+
api as never,
223+
);
224+
225+
expect(get(currentTime)).toBe(45);
226+
expect(get(isPaused)).toBe(false);
227+
expect(mockGetEpisodes).not.toHaveBeenCalled();
228+
});
229+
230+
test("resolves a local-file episode whose path and title both contain '+'", async () => {
231+
const localEpisode: LocalEpisode = {
232+
title: "C++ Tips",
233+
streamUrl: "Notes+/C++ Tips.mp3",
234+
url: "Notes+/C++ Tips.mp3",
235+
description: "",
236+
content: "",
237+
podcastName: "local file",
238+
filePath: "Notes+/C++ Tips.mp3",
239+
};
240+
localFiles.set({ ...emptyLocalFiles, episodes: [localEpisode] });
241+
// Only the raw (current-format) path resolves to a file.
242+
setApp((path) => (path === "Notes+/C++ Tips.mp3" ? {} : null));
243+
244+
await podNotesURIHandler(
245+
{
246+
action: "podnotes",
247+
url: "Notes+/C++ Tips.mp3",
248+
episodeName: "C++ Tips",
249+
time: "30",
250+
},
251+
api as never,
252+
);
253+
254+
expect(get(currentEpisode)).toMatchObject({ title: "C++ Tips" });
255+
expect(get(viewState)).toBe(ViewState.Player);
256+
expect(get(requestedPlaybackTime)).toEqual({
257+
episodeKey: "local file::C++ Tips",
258+
time: 30,
259+
});
260+
expect(mockGetEpisodes).not.toHaveBeenCalled();
261+
});
262+
263+
test("shows a notice and changes nothing when the episode cannot be found", async () => {
264+
mockGetEpisodes.mockResolvedValue([]);
265+
266+
await expect(
267+
podNotesURIHandler(
268+
{
269+
action: "podnotes",
270+
url: testFeedUrl,
271+
episodeName: "Nonexistent Episode",
272+
time: "10",
273+
},
274+
api as never,
275+
),
276+
).resolves.toBeUndefined();
277+
278+
expect(get(currentEpisode)).toBeUndefined();
279+
expect(get(viewState)).toBe(ViewState.PodcastGrid);
280+
});
281+
282+
test("does not reject when feed parsing throws (no silent unhandled rejection)", async () => {
283+
mockGetEpisodes.mockRejectedValue(new Error("network down"));
284+
285+
await expect(
286+
podNotesURIHandler(
287+
{
288+
action: "podnotes",
289+
url: testFeedUrl,
290+
episodeName: "Anything",
291+
time: "10",
292+
},
293+
api as never,
294+
),
295+
).resolves.toBeUndefined();
296+
297+
expect(get(currentEpisode)).toBeUndefined();
298+
expect(get(viewState)).toBe(ViewState.PodcastGrid);
299+
});
300+
301+
test("picks the '+'-variant feed episode (raw-first) even when its space-twin precedes it", async () => {
302+
const spaceVariant: Episode = {
303+
...testEpisode,
304+
title: "A B",
305+
url: "https://pod.example.com/space",
306+
streamUrl: "https://pod.example.com/space.mp3",
307+
};
308+
const plusVariant: Episode = {
309+
...testEpisode,
310+
title: "A+B",
311+
url: "https://pod.example.com/plus",
312+
streamUrl: "https://pod.example.com/plus.mp3",
313+
};
314+
// Space-twin first: an order-insensitive (membership) match would wrongly pick it.
315+
mockGetEpisodes.mockResolvedValue([spaceVariant, plusVariant]);
316+
317+
await podNotesURIHandler(
318+
{
319+
action: "podnotes",
320+
url: testFeedUrl,
321+
episodeName: "A+B",
322+
time: "75",
323+
},
324+
api as never,
325+
);
326+
327+
expect(get(currentEpisode)).toMatchObject({ title: "A+B" });
328+
expect(get(requestedPlaybackTime)).toEqual({
329+
episodeKey: `${plusVariant.podcastName}::A+B`,
330+
time: 75,
331+
});
332+
});
333+
334+
test("picks the '+'-variant local file (raw-first) even when its space-twin precedes it", async () => {
335+
const makeLocal = (title: string): LocalEpisode => ({
336+
title,
337+
streamUrl: `${title}.mp3`,
338+
url: `${title}.mp3`,
339+
description: "",
340+
content: "",
341+
podcastName: "local file",
342+
filePath: `${title}.mp3`,
343+
});
344+
localFiles.set({
345+
...emptyLocalFiles,
346+
episodes: [makeLocal("A B"), makeLocal("A+B")],
347+
});
348+
setApp((path) => (path === "A+B.mp3" ? {} : null));
349+
350+
await podNotesURIHandler(
351+
{
352+
action: "podnotes",
353+
url: "A+B.mp3",
354+
episodeName: "A+B",
355+
time: "20",
356+
},
357+
api as never,
358+
);
359+
360+
expect(get(currentEpisode)).toMatchObject({ title: "A+B" });
361+
expect(get(requestedPlaybackTime)).toEqual({
362+
episodeKey: "local file::A+B",
363+
time: 20,
364+
});
126365
});
127366
});

0 commit comments

Comments
 (0)