Skip to content

Commit c57fae3

Browse files
committed
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 1c4f7ce commit c57fae3

2 files changed

Lines changed: 138 additions & 3 deletions

File tree

src/utility/searchEpisodes.test.ts

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
import { beforeEach, describe, expect, test, vi } from "vitest";
2+
import type { Episode } from "src/types/Episode";
3+
4+
// Wrap the real Fuse so we can count how many times an index is built. This lets
5+
// the cache tests assert that an unchanged list reuses its index (the #149
6+
// optimization) while a changed list rebuilds it - without altering Fuse's real
7+
// search behaviour.
8+
const { fuseConstructorSpy } = vi.hoisted(() => ({
9+
fuseConstructorSpy: vi.fn(),
10+
}));
11+
12+
vi.mock("fuse.js", async (importOriginal) => {
13+
const actual = await importOriginal<typeof import("fuse.js")>();
14+
const RealFuse = actual.default;
15+
16+
class CountingFuse extends RealFuse<Episode> {
17+
constructor(...args: ConstructorParameters<typeof RealFuse<Episode>>) {
18+
fuseConstructorSpy();
19+
super(...args);
20+
}
21+
}
22+
23+
return { ...actual, default: CountingFuse };
24+
});
25+
26+
import searchEpisodes from "./searchEpisodes";
27+
28+
function makeEpisode(
29+
title: string,
30+
streamUrl = `https://example.com/${title}.mp3`,
31+
): Episode {
32+
return {
33+
title,
34+
streamUrl,
35+
url: streamUrl,
36+
description: "",
37+
content: "",
38+
podcastName: "Test Podcast",
39+
};
40+
}
41+
42+
function titlesOf(episodes: Episode[]): string[] {
43+
return episodes.map((episode) => episode.title);
44+
}
45+
46+
describe("searchEpisodes", () => {
47+
beforeEach(() => {
48+
fuseConstructorSpy.mockClear();
49+
});
50+
51+
test("returns episodes whose title fuzzy-matches the query", () => {
52+
const episodes = [makeEpisode("Alpha"), makeEpisode("Beta")];
53+
54+
expect(titlesOf(searchEpisodes("Alpha", episodes))).toEqual(["Alpha"]);
55+
});
56+
57+
test("returns an empty array for an empty list without building an index", () => {
58+
expect(searchEpisodes("anything", [])).toEqual([]);
59+
expect(fuseConstructorSpy).not.toHaveBeenCalled();
60+
});
61+
62+
test("restores the full list for a whitespace-only query", () => {
63+
const episodes = [makeEpisode("Alpha"), makeEpisode("Beta")];
64+
65+
expect(searchEpisodes(" ", episodes)).toBe(episodes);
66+
// A whitespace query short-circuits, so no index is built.
67+
expect(fuseConstructorSpy).not.toHaveBeenCalled();
68+
});
69+
70+
test("reuses the cached index for an unchanged list across searches", () => {
71+
const episodes = [makeEpisode("Alpha"), makeEpisode("Beta")];
72+
73+
searchEpisodes("Alpha", episodes);
74+
searchEpisodes("Beta", episodes);
75+
76+
// The same array reference with unchanged content builds the index once.
77+
expect(fuseConstructorSpy).toHaveBeenCalledTimes(1);
78+
});
79+
80+
test("returns fresh results when the list is mutated in place at the same length", () => {
81+
const episodes = [makeEpisode("Alpha"), makeEpisode("Beta")];
82+
83+
// Populate the cache against the original contents.
84+
expect(titlesOf(searchEpisodes("Alpha", episodes))).toEqual(["Alpha"]);
85+
86+
// Replace both entries in place: same array reference, same length, new
87+
// content. A length-only cache check would return the stale index here.
88+
episodes[0] = makeEpisode("Gamma");
89+
episodes[1] = makeEpisode("Delta");
90+
91+
// The old title is gone...
92+
expect(searchEpisodes("Alpha", episodes)).toEqual([]);
93+
// ...and the new content is searchable.
94+
expect(titlesOf(searchEpisodes("Gamma", episodes))).toEqual(["Gamma"]);
95+
// The index was rebuilt because the content signature changed.
96+
expect(fuseConstructorSpy).toHaveBeenCalledTimes(2);
97+
});
98+
99+
test("rebuilds the index on a same-title swap to a different identity", () => {
100+
const episodes = [makeEpisode("Same", "https://example.com/first.mp3")];
101+
102+
searchEpisodes("Same", episodes);
103+
104+
// Same title, same length, but a different underlying episode (new
105+
// streamUrl). A length-only OR title-only signature would keep serving the
106+
// stale index here; folding streamUrl into the signature detects the swap.
107+
// Asserting the rebuild (not just the result) is what makes this test
108+
// meaningful: fuse.js reads the live array, so the streamUrl result is
109+
// correct either way - only the rebuild count distinguishes the fix.
110+
episodes[0] = makeEpisode("Same", "https://example.com/second.mp3");
111+
const results = searchEpisodes("Same", episodes);
112+
113+
expect(results[0].streamUrl).toBe("https://example.com/second.mp3");
114+
expect(fuseConstructorSpy).toHaveBeenCalledTimes(2);
115+
});
116+
});

src/utility/searchEpisodes.ts

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,17 +9,36 @@ const fuseOptions = {
99
keys: ["title"],
1010
};
1111

12-
const fuseCache = new WeakMap<Episode[], { fuse: Fuse<Episode>; size: number }>();
12+
const fuseCache = new WeakMap<
13+
Episode[],
14+
{ fuse: Fuse<Episode>; signature: string }
15+
>();
16+
17+
// Fingerprint the searchable content of the list. The Fuse index is keyed by the
18+
// array reference, but the same reference can be mutated in place (an entry
19+
// swapped or its title edited) while keeping the same length, so length alone is
20+
// too weak a validity check - it would hand back an index built from the old
21+
// contents. The signature captures each episode's title (the only indexed field)
22+
// and streamUrl (its stable identity, so a same-title swap is still detected) in
23+
// order, so any content or ordering change rebuilds the index while an unchanged
24+
// list keeps reusing it across keystrokes. JSON framing keeps it collision-free
25+
// regardless of what the titles and URLs contain.
26+
function contentSignature(episodes: Episode[]): string {
27+
return JSON.stringify(
28+
episodes.map((episode) => [episode.title, episode.streamUrl]),
29+
);
30+
}
1331

1432
function getFuse(episodes: Episode[]): Fuse<Episode> {
33+
const signature = contentSignature(episodes);
1534
const cached = fuseCache.get(episodes);
1635

17-
if (cached && cached.size === episodes.length) {
36+
if (cached && cached.signature === signature) {
1837
return cached.fuse;
1938
}
2039

2140
const newFuse = new Fuse(episodes, fuseOptions);
22-
fuseCache.set(episodes, { fuse: newFuse, size: episodes.length });
41+
fuseCache.set(episodes, { fuse: newFuse, signature });
2342

2443
return newFuse;
2544
}

0 commit comments

Comments
 (0)