Skip to content

Commit 609950c

Browse files
committed
fix(episodes): harden episodeListLimit per review (cache cap, sort order, import, load sanitize)
- Align MAX_EPISODE_LIST_LIMIT with the feed cache's per-feed retention (75) so a chosen limit is always serveable from a warm cache; selecting a podcast still shows its full archive. - Sort each feed by date before truncating so the per-feed limit keeps the NEWEST episodes even when the feed/cache is not newest-first. - Rehydrate the episodeListLimit store on settings import (was only persisted). - Sanitize episodeListLimit in loadSettings so a malformed persisted value is repaired in the settings object, not just clamped at runtime. - Settings input: don't clobber the saved limit on empty/mid-edit input and skip redundant saves. - Rebuild the latest list with a single sort+slice on limit change. Refs #114
1 parent 1550d54 commit 609950c

7 files changed

Lines changed: 87 additions & 24 deletions

File tree

‎docs/docs/podcasts.md‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,11 @@ podcast is selected, so episodes older than the limit will not appear in those
3838
search results.
3939

4040
You can change this with the **Latest episodes per podcast** setting in the
41-
PodNotes settings tab. It defaults to 10. Raise it to surface more of each
42-
feed's history in the Latest Episodes list and to search further back; lower it
43-
for a shorter list. Selecting an individual podcast still shows all of that
44-
podcast's episodes regardless of this setting.
41+
PodNotes settings tab. It defaults to 10 and can be raised up to 75. Raise it to
42+
surface more of each feed's history in the Latest Episodes list and to search
43+
further back; lower it for a shorter list. Selecting an individual podcast still
44+
shows all of that podcast's episodes regardless of this setting, so the full
45+
back catalogue of any one show always remains searchable from its own view.
4546

4647
## Context menu
4748
You can right-click (desktop) or long-press (mobile) on an episode in the episode list to open the context menu.

‎src/constants.ts‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,14 @@ export const VIEW_TYPE = "podcast_player_view";
1212
export const DEFAULT_EPISODE_LIST_LIMIT = 10;
1313

1414
/**
15-
* Upper bound for {@link DEFAULT_EPISODE_LIST_LIMIT}. Keeps an accidental huge
16-
* value (e.g. a fat-fingered settings entry) from materialising an unbounded
17-
* latest-episodes list.
15+
* Upper bound for {@link DEFAULT_EPISODE_LIST_LIMIT}. Kept in lockstep with the
16+
* feed cache's `MAX_EPISODES_PER_FEED` (see src/services/FeedCacheService.ts):
17+
* on a warm start the Latest Episodes list is rebuilt from the persisted cache,
18+
* which retains at most that many episodes per feed, so a limit larger than the
19+
* cap could never actually be served. Selecting an individual podcast still
20+
* shows that feed's full episode list, unbounded by this setting.
1821
*/
19-
export const MAX_EPISODE_LIST_LIMIT = 1000;
22+
export const MAX_EPISODE_LIST_LIMIT = 75;
2023

2124
type PlaylistSettings = Pick<
2225
Playlist,

‎src/main.ts‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -102,9 +102,9 @@ export default class PodNotes extends Plugin implements IPodNotes {
102102
currentEpisode.set(this.settings.currentEpisode);
103103
}
104104
hidePlayedEpisodes.set(this.settings.hidePlayedEpisodes);
105-
episodeListLimit.set(
106-
sanitizeEpisodeListLimit(this.settings.episodeListLimit),
107-
);
105+
// loadSettings() already sanitized this, so the store stays in sync with
106+
// the (repaired) persisted value.
107+
episodeListLimit.set(this.settings.episodeListLimit);
108108
volume.set(
109109
Math.min(1, Math.max(0, this.settings.defaultVolume ?? 1)),
110110
);
@@ -462,6 +462,12 @@ export default class PodNotes extends Plugin implements IPodNotes {
462462
this.settings.download.path = migrateDownloadPath(
463463
this.settings.download.path,
464464
);
465+
// Normalise the persisted limit so a malformed value (e.g. 0 from an older
466+
// data.json) is repaired in the settings object too, not just clamped for
467+
// runtime behaviour, and so a later saveSettings() can't re-persist it (#114).
468+
this.settings.episodeListLimit = sanitizeEpisodeListLimit(
469+
this.settings.episodeListLimit,
470+
);
465471
}
466472

467473
async saveSettings() {

‎src/services/FeedCacheService.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@ const STORAGE_KEY = "podnotes:feed-cache:v2";
2222
// orphan ~MBs of data (which could push v2 writes over the localStorage quota).
2323
const LEGACY_STORAGE_KEYS = ["podnotes:feed-cache:v1"];
2424
const DEFAULT_TTL_MS = 1000 * 60 * 60 * 6; // 6 hours.
25+
// Keep this >= MAX_EPISODE_LIST_LIMIT (src/constants.ts): the Latest Episodes
26+
// list is rebuilt from this persisted cache on a warm start, so a per-feed list
27+
// limit larger than what we retain here could never be served (issue #114).
2528
const MAX_EPISODES_PER_FEED = 75;
2629
const MAX_CACHE_SIZE_BYTES = 4 * 1024 * 1024; // 4MB to leave room for other localStorage usage
2730

‎src/store/index.test.ts‎

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -540,15 +540,27 @@ describe("sanitizeEpisodeListLimit (issue #114)", () => {
540540
});
541541

542542
test("clamps to the maximum", () => {
543+
expect(sanitizeEpisodeListLimit(MAX_EPISODE_LIST_LIMIT)).toBe(
544+
MAX_EPISODE_LIST_LIMIT,
545+
);
546+
expect(sanitizeEpisodeListLimit(MAX_EPISODE_LIST_LIMIT + 1)).toBe(
547+
MAX_EPISODE_LIST_LIMIT,
548+
);
543549
expect(sanitizeEpisodeListLimit(10 ** 9)).toBe(MAX_EPISODE_LIST_LIMIT);
544550
});
545551
});
546552

547553
describe("latestEpisodes respects episodeListLimit (issue #114)", () => {
548-
// Newest-first like a real RSS feed; the store re-sorts by date anyway.
549-
function feedEpisodes(podcastName: string, count: number): Episode[] {
550-
return Array.from({ length: count }, (_, i) => {
551-
const ordinal = count - i;
554+
// `ordinal` doubles as the day-of-month, so a higher ordinal is a newer
555+
// episode. `order: "newest-first"` mimics a typical RSS feed; "oldest-first"
556+
// stresses that the per-feed limit ranks by date before truncating.
557+
function feedEpisodes(
558+
podcastName: string,
559+
count: number,
560+
order: "newest-first" | "oldest-first" = "newest-first",
561+
): Episode[] {
562+
const episodes = Array.from({ length: count }, (_, i) => {
563+
const ordinal = i + 1;
552564
return {
553565
title: `${podcastName} #${ordinal}`,
554566
streamUrl: `https://example.com/${podcastName}/${ordinal}.mp3`,
@@ -559,6 +571,7 @@ describe("latestEpisodes respects episodeListLimit (issue #114)", () => {
559571
episodeDate: new Date(2020, 0, ordinal),
560572
} satisfies Episode;
561573
});
574+
return order === "newest-first" ? episodes.reverse() : episodes;
562575
}
563576

564577
function trackLatest(): { value: () => Episode[]; stop: () => void } {
@@ -636,6 +649,25 @@ describe("latestEpisodes respects episodeListLimit (issue #114)", () => {
636649
expect(tracker.value()).toHaveLength(10);
637650
tracker.stop();
638651
});
652+
653+
test("selects the newest episodes even when the feed is oldest-first", () => {
654+
const tracker = trackLatest();
655+
episodeListLimit.set(3);
656+
657+
// Episodes 1..25 in ascending (oldest-first) order. A naive slice-before-sort
658+
// would keep #1..#3 (the oldest); the limit must rank by date and keep the
659+
// newest three.
660+
episodeCache.set({
661+
"Show A": feedEpisodes("Show A", 25, "oldest-first"),
662+
});
663+
664+
expect(tracker.value().map((episode) => episode.title)).toEqual([
665+
"Show A #25",
666+
"Show A #24",
667+
"Show A #23",
668+
]);
669+
tracker.stop();
670+
});
639671
});
640672

641673
describe("currentEpisode.set finished guard (issue #94)", () => {

‎src/store/index.ts‎

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -223,9 +223,13 @@ function getLatestEpisodesForFeed(
223223
): Episode[] {
224224
if (!episodes?.length) return [];
225225

226-
return episodes
227-
.slice(0, perFeedLimit)
228-
.sort((a, b) => getEpisodeTimestamp(b) - getEpisodeTimestamp(a));
226+
// Sort by date first, THEN take the newest N. Slicing before sorting would
227+
// only keep the newest episodes when the feed is already newest-first; for a
228+
// feed (or cache) in any other order it would surface the wrong episodes, so
229+
// the per-feed limit must rank the whole feed before truncating (issue #114).
230+
return [...episodes]
231+
.sort((a, b) => getEpisodeTimestamp(b) - getEpisodeTimestamp(a))
232+
.slice(0, perFeedLimit);
229233
}
230234

231235
function shallowEqualEpisodes(a?: Episode[], b?: Episode[]): boolean {
@@ -376,7 +380,7 @@ export const latestEpisodes = readable<Episode[]>([], (set) => {
376380

377381
const nextSources: FeedEpisodeSources = new Map();
378382
const nextLatestByFeed: LatestEpisodesByFeed = new Map();
379-
let nextMerged: Episode[] = [];
383+
const collected: Episode[] = [];
380384

381385
for (const [feedTitle, episodes] of cacheEntries) {
382386
const nextLatestForFeed = getLatestEpisodesForFeed(
@@ -385,12 +389,16 @@ export const latestEpisodes = readable<Episode[]>([], (set) => {
385389
);
386390
nextSources.set(feedTitle, episodes);
387391
nextLatestByFeed.set(feedTitle, nextLatestForFeed);
388-
389-
for (const episode of nextLatestForFeed) {
390-
nextMerged = insertEpisodeSorted(nextMerged, episode, latestLimit);
391-
}
392+
collected.push(...nextLatestForFeed);
392393
}
393394

395+
// Merge once: sort the gathered per-feed slices by date and cap. A single
396+
// sort avoids the repeated full-array copies insertEpisodeSorted would do
397+
// per episode, keeping this user-triggered rebuild off the slow path.
398+
const nextMerged = collected
399+
.sort((a, b) => getEpisodeTimestamp(b) - getEpisodeTimestamp(a))
400+
.slice(0, latestLimit);
401+
394402
feedSources = nextSources;
395403
latestByFeed = nextLatestByFeed;
396404

‎src/ui/settings/PodNotesSettingsTab.ts‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,14 @@ export class PodNotesSettingsTab extends PluginSettingTab {
151151
)
152152
.setPlaceholder(`${DEFAULT_EPISODE_LIST_LIMIT}`)
153153
.onChange(async (value) => {
154-
const sanitized = sanitizeEpisodeListLimit(value);
154+
// Don't commit while the field is empty or mid-edit (e.g. cleared,
155+
// or a lone "-"): sanitizing "" would silently overwrite the saved
156+
// limit with the default. Wait for a parseable number, and skip
157+
// redundant saves so typing doesn't churn data.json each keystroke.
158+
const trimmed = value.trim();
159+
if (trimmed === "" || !Number.isFinite(Number(trimmed))) return;
160+
const sanitized = sanitizeEpisodeListLimit(trimmed);
161+
if (sanitized === this.plugin.settings.episodeListLimit) return;
155162
this.plugin.settings.episodeListLimit = sanitized;
156163
episodeListLimit.set(sanitized);
157164
await this.plugin.saveSettings();
@@ -770,6 +777,9 @@ export class PodNotesSettingsTab extends PluginSettingTab {
770777
queue.set(merged.queue);
771778
localFiles.set(merged.localFiles);
772779
hidePlayedEpisodes.set(merged.hidePlayedEpisodes);
780+
const sanitizedLimit = sanitizeEpisodeListLimit(merged.episodeListLimit);
781+
merged.episodeListLimit = sanitizedLimit;
782+
episodeListLimit.set(sanitizedLimit);
773783
const importedVolume = Number.isFinite(merged.defaultVolume)
774784
? merged.defaultVolume
775785
: 1;

0 commit comments

Comments
 (0)