Skip to content

Commit 053d51f

Browse files
authored
fix(feed/search): parse feed once + content-based search cache (#225)
* fix(feed): parse the feed once per getEpisodes call getEpisodes fetched and parsed the feed twice on a cold call: once via getFeed (to populate channel metadata) and again directly to read the items. This doubled the network round-trips and XML parses on every cold path, e.g. the URIHandler resume-link flow that constructs a FeedParser with no cached feed. Fetch and parse the document a single time and reuse it for both the channel metadata and the episode items. The metadata extraction moves to a private extractFeed(body, url) helper that getFeed and getEpisodes both call, so behavior (metadata population, the "Invalid RSS feed" guard, this.feed caching) is unchanged - only the redundant second fetch is removed. Resolves deepsec finding other-redundant-fetch. * fix(search): rebuild the Fuse cache on content change, not just length The Fuse search index was cached in a WeakMap keyed by the episodes array and reused whenever the cached size matched the array length. Length is a weak fingerprint: the same array reference mutated in place at the same length (an entry swapped or edited) would return the stale index built from the old contents. Validate the cache with a content signature (a JSON-framed list of each episode's title and streamUrl) instead of length, so any content or ordering change rebuilds the index while an unchanged list keeps reusing it across keystrokes (preserving the #149 optimization). Adds the first unit tests for searchEpisodes, including a Fuse-construction spy proving the index is reused when unchanged and rebuilt when the content changes. Resolves deepsec finding other-stale-cache.
1 parent edef281 commit 053d51f

4 files changed

Lines changed: 208 additions & 212 deletions

File tree

src/parser/feedParser.test.ts

Lines changed: 56 additions & 204 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,18 @@ import { requestWithTimeout } from "src/utility/networkRequest";
1010

1111
const mockRequestWithTimeout = vi.mocked(requestWithTimeout);
1212

13+
// Build the shape requestWithTimeout resolves to. Keeps the per-test mock setup
14+
// to a single line and makes the number of expected fetches obvious at a glance.
15+
function feedResponse(text: string) {
16+
return {
17+
text,
18+
status: 200,
19+
headers: {},
20+
arrayBuffer: new ArrayBuffer(0),
21+
json: {},
22+
};
23+
}
24+
1325
const sampleRssFeed = `<?xml version="1.0" encoding="UTF-8"?>
1426
<rss version="2.0" xmlns:itunes="http://www.itunes.com/dtds/podcast-1.0.dtd">
1527
<channel>
@@ -359,48 +371,27 @@ describe("FeedParser", () => {
359371
});
360372

361373
describe("getEpisodes", () => {
362-
test("parses all valid episodes from feed", async () => {
363-
// getEpisodes now calls getFeed first, then parseFeed again
364-
mockRequestWithTimeout
365-
.mockResolvedValueOnce({
366-
text: sampleRssFeed,
367-
status: 200,
368-
headers: {},
369-
arrayBuffer: new ArrayBuffer(0),
370-
json: {},
371-
})
372-
.mockResolvedValueOnce({
373-
text: sampleRssFeed,
374-
status: 200,
375-
headers: {},
376-
arrayBuffer: new ArrayBuffer(0),
377-
json: {},
378-
});
374+
test("parses all valid episodes from a single feed fetch", async () => {
375+
mockRequestWithTimeout.mockResolvedValueOnce(
376+
feedResponse(sampleRssFeed),
377+
);
379378

380379
const parser = new FeedParser();
381380
const episodes = await parser.getEpisodes("https://example.com/feed.xml");
382381

383382
expect(episodes).toHaveLength(2);
384383
expect(episodes[0].title).toBe("Episode 1");
385384
expect(episodes[1].title).toBe("Episode 2");
385+
// A cold getEpisodes call must fetch + parse the feed exactly ONCE. It
386+
// previously fetched twice (here, then again inside getFeed), doubling
387+
// the load on the feed host.
388+
expect(mockRequestWithTimeout).toHaveBeenCalledTimes(1);
386389
});
387390

388391
test("parses episode properties correctly and populates feed metadata", async () => {
389-
mockRequestWithTimeout
390-
.mockResolvedValueOnce({
391-
text: sampleRssFeed,
392-
status: 200,
393-
headers: {},
394-
arrayBuffer: new ArrayBuffer(0),
395-
json: {},
396-
})
397-
.mockResolvedValueOnce({
398-
text: sampleRssFeed,
399-
status: 200,
400-
headers: {},
401-
arrayBuffer: new ArrayBuffer(0),
402-
json: {},
403-
});
392+
mockRequestWithTimeout.mockResolvedValueOnce(
393+
feedResponse(sampleRssFeed),
394+
);
404395

405396
const parser = new FeedParser();
406397
const episodes = await parser.getEpisodes("https://example.com/feed.xml");
@@ -418,21 +409,9 @@ describe("FeedParser", () => {
418409
});
419410

420411
test("parses Podcasting 2.0 chapter URLs from episodes (#47)", async () => {
421-
mockRequestWithTimeout
422-
.mockResolvedValueOnce({
423-
text: rssFeedWithPodcastChapters,
424-
status: 200,
425-
headers: {},
426-
arrayBuffer: new ArrayBuffer(0),
427-
json: {},
428-
})
429-
.mockResolvedValueOnce({
430-
text: rssFeedWithPodcastChapters,
431-
status: 200,
432-
headers: {},
433-
arrayBuffer: new ArrayBuffer(0),
434-
json: {},
435-
});
412+
mockRequestWithTimeout.mockResolvedValueOnce(
413+
feedResponse(rssFeedWithPodcastChapters),
414+
);
436415

437416
const parser = new FeedParser();
438417
const episodes = await parser.getEpisodes("https://example.com/feed.xml");
@@ -456,21 +435,7 @@ describe("FeedParser", () => {
456435
</item>
457436
</channel>
458437
</rss>`;
459-
mockRequestWithTimeout
460-
.mockResolvedValueOnce({
461-
text: videoFeed,
462-
status: 200,
463-
headers: {},
464-
arrayBuffer: new ArrayBuffer(0),
465-
json: {},
466-
})
467-
.mockResolvedValueOnce({
468-
text: videoFeed,
469-
status: 200,
470-
headers: {},
471-
arrayBuffer: new ArrayBuffer(0),
472-
json: {},
473-
});
438+
mockRequestWithTimeout.mockResolvedValueOnce(feedResponse(videoFeed));
474439

475440
const parser = new FeedParser();
476441
const episodes = await parser.getEpisodes("https://example.com/feed.xml");
@@ -495,21 +460,9 @@ describe("FeedParser", () => {
495460
</item>
496461
</channel>
497462
</rss>`;
498-
mockRequestWithTimeout
499-
.mockResolvedValueOnce({
500-
text: audioMp4Feed,
501-
status: 200,
502-
headers: {},
503-
arrayBuffer: new ArrayBuffer(0),
504-
json: {},
505-
})
506-
.mockResolvedValueOnce({
507-
text: audioMp4Feed,
508-
status: 200,
509-
headers: {},
510-
arrayBuffer: new ArrayBuffer(0),
511-
json: {},
512-
});
463+
mockRequestWithTimeout.mockResolvedValueOnce(
464+
feedResponse(audioMp4Feed),
465+
);
513466

514467
const parser = new FeedParser();
515468
const episodes = await parser.getEpisodes("https://example.com/feed.xml");
@@ -544,21 +497,9 @@ describe("FeedParser", () => {
544497
</item>
545498
</channel>
546499
</rss>`;
547-
mockRequestWithTimeout
548-
.mockResolvedValueOnce({
549-
text: untypedAmbiguousFeed,
550-
status: 200,
551-
headers: {},
552-
arrayBuffer: new ArrayBuffer(0),
553-
json: {},
554-
})
555-
.mockResolvedValueOnce({
556-
text: untypedAmbiguousFeed,
557-
status: 200,
558-
headers: {},
559-
arrayBuffer: new ArrayBuffer(0),
560-
json: {},
561-
});
500+
mockRequestWithTimeout.mockResolvedValueOnce(
501+
feedResponse(untypedAmbiguousFeed),
502+
);
562503

563504
const parser = new FeedParser();
564505
const episodes = await parser.getEpisodes("https://example.com/feed.xml");
@@ -571,21 +512,9 @@ describe("FeedParser", () => {
571512
});
572513

573514
test("filters out invalid episodes missing required fields", async () => {
574-
mockRequestWithTimeout
575-
.mockResolvedValueOnce({
576-
text: rssFeedWithInvalidItem,
577-
status: 200,
578-
headers: {},
579-
arrayBuffer: new ArrayBuffer(0),
580-
json: {},
581-
})
582-
.mockResolvedValueOnce({
583-
text: rssFeedWithInvalidItem,
584-
status: 200,
585-
headers: {},
586-
arrayBuffer: new ArrayBuffer(0),
587-
json: {},
588-
});
515+
mockRequestWithTimeout.mockResolvedValueOnce(
516+
feedResponse(rssFeedWithInvalidItem),
517+
);
589518

590519
const parser = new FeedParser();
591520
const episodes = await parser.getEpisodes("https://example.com/feed.xml");
@@ -601,37 +530,23 @@ describe("FeedParser", () => {
601530
artworkUrl: "https://example.com/feed-artwork.jpg",
602531
};
603532

604-
mockRequestWithTimeout.mockResolvedValueOnce({
605-
text: sampleRssFeed,
606-
status: 200,
607-
headers: {},
608-
arrayBuffer: new ArrayBuffer(0),
609-
json: {},
610-
});
533+
mockRequestWithTimeout.mockResolvedValueOnce(
534+
feedResponse(sampleRssFeed),
535+
);
611536

612-
// When constructed with a feed, it skips calling getFeed
537+
// When constructed with a feed, it skips re-deriving feed metadata.
613538
const parser = new FeedParser(mockFeed);
614539
const episodes = await parser.getEpisodes("https://example.com/feed.xml");
615540

616541
expect(episodes[1].artworkUrl).toBe("https://example.com/feed-artwork.jpg");
542+
// A pre-populated, matching feed still fetches the items exactly once.
543+
expect(mockRequestWithTimeout).toHaveBeenCalledTimes(1);
617544
});
618545

619546
test("uses episode artwork from itunes:image when available", async () => {
620-
mockRequestWithTimeout
621-
.mockResolvedValueOnce({
622-
text: sampleRssFeedWithItunesImage,
623-
status: 200,
624-
headers: {},
625-
arrayBuffer: new ArrayBuffer(0),
626-
json: {},
627-
})
628-
.mockResolvedValueOnce({
629-
text: sampleRssFeedWithItunesImage,
630-
status: 200,
631-
headers: {},
632-
arrayBuffer: new ArrayBuffer(0),
633-
json: {},
634-
});
547+
mockRequestWithTimeout.mockResolvedValueOnce(
548+
feedResponse(sampleRssFeedWithItunesImage),
549+
);
635550

636551
const parser = new FeedParser();
637552
const episodes = await parser.getEpisodes("https://example.com/feed.xml");
@@ -642,21 +557,9 @@ describe("FeedParser", () => {
642557

643558
describe("episode number and duration (#34, #88)", () => {
644559
test("parses <itunes:episode>/<itunes:duration> with a title fallback", async () => {
645-
mockRequestWithTimeout
646-
.mockResolvedValueOnce({
647-
text: rssFeedWithEpisodeNumberAndDuration,
648-
status: 200,
649-
headers: {},
650-
arrayBuffer: new ArrayBuffer(0),
651-
json: {},
652-
})
653-
.mockResolvedValueOnce({
654-
text: rssFeedWithEpisodeNumberAndDuration,
655-
status: 200,
656-
headers: {},
657-
arrayBuffer: new ArrayBuffer(0),
658-
json: {},
659-
});
560+
mockRequestWithTimeout.mockResolvedValueOnce(
561+
feedResponse(rssFeedWithEpisodeNumberAndDuration),
562+
);
660563

661564
const parser = new FeedParser();
662565
const episodes = await parser.getEpisodes("https://example.com/feed.xml");
@@ -704,21 +607,7 @@ describe("FeedParser", () => {
704607
</channel>
705608
</rss>`;
706609

707-
mockRequestWithTimeout
708-
.mockResolvedValueOnce({
709-
text: emptyFeed,
710-
status: 200,
711-
headers: {},
712-
arrayBuffer: new ArrayBuffer(0),
713-
json: {},
714-
})
715-
.mockResolvedValueOnce({
716-
text: emptyFeed,
717-
status: 200,
718-
headers: {},
719-
arrayBuffer: new ArrayBuffer(0),
720-
json: {},
721-
});
610+
mockRequestWithTimeout.mockResolvedValueOnce(feedResponse(emptyFeed));
722611

723612
const parser = new FeedParser();
724613
const episodes = await parser.getEpisodes("https://example.com/feed.xml");
@@ -740,21 +629,7 @@ describe("FeedParser", () => {
740629
</channel>
741630
</rss>`;
742631

743-
mockRequestWithTimeout
744-
.mockResolvedValueOnce({
745-
text: minimalFeed,
746-
status: 200,
747-
headers: {},
748-
arrayBuffer: new ArrayBuffer(0),
749-
json: {},
750-
})
751-
.mockResolvedValueOnce({
752-
text: minimalFeed,
753-
status: 200,
754-
headers: {},
755-
arrayBuffer: new ArrayBuffer(0),
756-
json: {},
757-
});
632+
mockRequestWithTimeout.mockResolvedValueOnce(feedResponse(minimalFeed));
758633

759634
const parser = new FeedParser();
760635
const episodes = await parser.getEpisodes("https://example.com/feed.xml");
@@ -781,21 +656,7 @@ describe("FeedParser", () => {
781656
</channel>
782657
</rss>`;
783658

784-
mockRequestWithTimeout
785-
.mockResolvedValueOnce({
786-
text: cdataFeed,
787-
status: 200,
788-
headers: {},
789-
arrayBuffer: new ArrayBuffer(0),
790-
json: {},
791-
})
792-
.mockResolvedValueOnce({
793-
text: cdataFeed,
794-
status: 200,
795-
headers: {},
796-
arrayBuffer: new ArrayBuffer(0),
797-
json: {},
798-
});
659+
mockRequestWithTimeout.mockResolvedValueOnce(feedResponse(cdataFeed));
799660

800661
const parser = new FeedParser();
801662
const episodes = await parser.getEpisodes("https://example.com/feed.xml");
@@ -805,20 +666,8 @@ describe("FeedParser", () => {
805666

806667
test("getFeed sets internal feed state for subsequent calls", async () => {
807668
mockRequestWithTimeout
808-
.mockResolvedValueOnce({
809-
text: sampleRssFeed,
810-
status: 200,
811-
headers: {},
812-
arrayBuffer: new ArrayBuffer(0),
813-
json: {},
814-
})
815-
.mockResolvedValueOnce({
816-
text: sampleRssFeed,
817-
status: 200,
818-
headers: {},
819-
arrayBuffer: new ArrayBuffer(0),
820-
json: {},
821-
});
669+
.mockResolvedValueOnce(feedResponse(sampleRssFeed))
670+
.mockResolvedValueOnce(feedResponse(sampleRssFeed));
822671

823672
const parser = new FeedParser();
824673
await parser.getFeed("https://example.com/feed.xml");
@@ -827,6 +676,9 @@ describe("FeedParser", () => {
827676
// Episodes should have feed metadata populated
828677
expect(episodes[0].podcastName).toBe("Test Podcast");
829678
expect(episodes[0].feedUrl).toBe("https://example.com/feed.xml");
679+
// One fetch for getFeed, one for getEpisodes: getEpisodes reuses the
680+
// already-cached feed metadata and does NOT re-fetch it.
681+
expect(mockRequestWithTimeout).toHaveBeenCalledTimes(2);
830682
});
831683
});
832684
});

0 commit comments

Comments
 (0)