fix(feed): stop refreshes from re-sorting the updated list
Two fixes to refresh order stability (My Shows / Feed sort by lastUpdated): - A refresh that fetches identical episodes no longer bumps lastUpdated (id-set comparison via sameEpisodes), so unchanged feeds keep their position instead of reordering every cycle. - refreshAllFeeds now fetches in parallel and applies ONE atomic update instead of a per-feed setFeeds, which re-sorted the list once per completion and made order flap until the batch finished. Adds feed-refresh regression tests with mocked clock.
This commit is contained in:
@@ -43,6 +43,17 @@ function saveSources(sources: PodcastSource[]): void {
|
|||||||
saveSourcesToFile(sources);
|
saveSourcesToFile(sources);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** True when two episode lists hold the same episodes (id-set equality,
|
||||||
|
* order-insensitive). Refreshes compare fetched content against this so an
|
||||||
|
* unchanged feed keeps its `lastUpdated` — and therefore its place in the
|
||||||
|
* "updated" sort — instead of reordering the list on every background
|
||||||
|
* refresh. */
|
||||||
|
function sameEpisodes(a: Episode[], b: Episode[]): boolean {
|
||||||
|
if (a.length !== b.length) return false;
|
||||||
|
const ids = new Set(a.map((e) => e.id));
|
||||||
|
return b.every((e) => ids.has(e.id));
|
||||||
|
}
|
||||||
|
|
||||||
/** Create feed store */
|
/** Create feed store */
|
||||||
function createFeedStore() {
|
function createFeedStore() {
|
||||||
const [feeds, setFeeds] = createSignal<Feed[]>([]);
|
const [feeds, setFeeds] = createSignal<Feed[]>([]);
|
||||||
@@ -249,6 +260,26 @@ function createFeedStore() {
|
|||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
|
/** Apply a freshly fetched episode list to one feed, bumping `lastUpdated`
|
||||||
|
* only when the content actually changed (see sameEpisodes). Returns the
|
||||||
|
* ORIGINAL array reference when nothing changed so callers skip
|
||||||
|
* persistence entirely — a refresh that fetched identical episodes must
|
||||||
|
* not re-sort the "updated" view. */
|
||||||
|
const applyRefreshedEpisodes = (
|
||||||
|
prev: Feed[],
|
||||||
|
feedId: string,
|
||||||
|
episodes: Episode[],
|
||||||
|
): Feed[] => {
|
||||||
|
let changed = false;
|
||||||
|
const updated = prev.map((f) => {
|
||||||
|
if (f.id !== feedId) return f;
|
||||||
|
if (sameEpisodes(f.episodes, episodes)) return f;
|
||||||
|
changed = true;
|
||||||
|
return { ...f, episodes, lastUpdated: new Date() };
|
||||||
|
});
|
||||||
|
return changed ? updated : prev;
|
||||||
|
};
|
||||||
|
|
||||||
/** Refresh a single feed - re-fetch latest 50 episodes */
|
/** Refresh a single feed - re-fetch latest 50 episodes */
|
||||||
const refreshFeed = async (feedId: string) => {
|
const refreshFeed = async (feedId: string) => {
|
||||||
const feed = getFeed(feedId);
|
const feed = getFeed(feedId);
|
||||||
@@ -259,10 +290,8 @@ function createFeedStore() {
|
|||||||
feedId,
|
feedId,
|
||||||
);
|
);
|
||||||
setFeeds((prev) => {
|
setFeeds((prev) => {
|
||||||
const updated = prev.map((f) =>
|
const updated = applyRefreshedEpisodes(prev, feedId, episodes);
|
||||||
f.id === feedId ? { ...f, episodes, lastUpdated: new Date() } : f,
|
if (updated !== prev) saveFeeds(updated);
|
||||||
);
|
|
||||||
saveFeeds(updated);
|
|
||||||
return updated;
|
return updated;
|
||||||
});
|
});
|
||||||
|
|
||||||
@@ -271,14 +300,35 @@ function createFeedStore() {
|
|||||||
runAutoDownload();
|
runAutoDownload();
|
||||||
};
|
};
|
||||||
|
|
||||||
/** Refresh all feeds */
|
/** Refresh all feeds — fetch every feed in parallel, then apply ONE
|
||||||
|
* atomic update. Per-feed incremental setFeeds re-sorted the list once
|
||||||
|
* per completion (each refresh bumped lastUpdated and the "updated" sort
|
||||||
|
* re-ran), which showed up as the list order flapping until the batch
|
||||||
|
* finished. */
|
||||||
const refreshAllFeeds = async () => {
|
const refreshAllFeeds = async () => {
|
||||||
setIsLoadingFeeds(true);
|
setIsLoadingFeeds(true);
|
||||||
try {
|
try {
|
||||||
const currentFeeds = feeds();
|
const currentFeeds = feeds();
|
||||||
for (const feed of currentFeeds) {
|
const results = await Promise.all(
|
||||||
await refreshFeed(feed.id);
|
currentFeeds.map(async (feed) => [
|
||||||
}
|
feed.id,
|
||||||
|
await fetchEpisodes(
|
||||||
|
feed.podcast.feedUrl,
|
||||||
|
MAX_EPISODES_REFRESH,
|
||||||
|
feed.id,
|
||||||
|
),
|
||||||
|
] as const),
|
||||||
|
);
|
||||||
|
setFeeds((prev) => {
|
||||||
|
let updated = prev;
|
||||||
|
for (const [feedId, episodes] of results) {
|
||||||
|
updated = applyRefreshedEpisodes(updated, feedId, episodes);
|
||||||
|
}
|
||||||
|
if (updated !== prev) saveFeeds(updated);
|
||||||
|
return updated;
|
||||||
|
});
|
||||||
|
// Global auto-download: one idempotent pass after the batch.
|
||||||
|
runAutoDownload();
|
||||||
} finally {
|
} finally {
|
||||||
setIsLoadingFeeds(false);
|
setIsLoadingFeeds(false);
|
||||||
}
|
}
|
||||||
|
|||||||
155
tests/feed-refresh.test.ts
Normal file
155
tests/feed-refresh.test.ts
Normal file
@@ -0,0 +1,155 @@
|
|||||||
|
/**
|
||||||
|
* Feed refresh order-stability regression test.
|
||||||
|
*
|
||||||
|
* My Shows / Feed sort by `lastUpdated` ("updated") by default, and every
|
||||||
|
* refresh bumped it unconditionally — so a startup refresh-all re-sorted the
|
||||||
|
* list once per feed as each fetch landed (order flapping until the batch
|
||||||
|
* finished). These tests pin the contract:
|
||||||
|
*
|
||||||
|
* 1. A refresh that fetches identical episodes does NOT bump lastUpdated —
|
||||||
|
* the feed object is untouched, so the list cannot reorder.
|
||||||
|
* 2. A refresh that fetches genuinely new episodes DOES bump lastUpdated.
|
||||||
|
* 3. refreshAllFeeds applies one atomic update: unchanged feeds keep their
|
||||||
|
* order and timestamps after a full refresh.
|
||||||
|
*
|
||||||
|
* The clock is mocked (fake timers) so the "did lastUpdated advance?" checks
|
||||||
|
* are deterministic — no real sleeps that would race under load.
|
||||||
|
*/
|
||||||
|
|
||||||
|
import { test, expect, beforeAll, afterAll, beforeEach, vi } from "bun:test";
|
||||||
|
import { mkdtempSync, rmSync } from "fs";
|
||||||
|
import { tmpdir } from "os";
|
||||||
|
import { join } from "path";
|
||||||
|
|
||||||
|
// Point the config dir at a throwaway directory BEFORE importing the stores
|
||||||
|
// (their module-level init reads it).
|
||||||
|
const configHome = mkdtempSync(join(tmpdir(), "podtui-refresh-"));
|
||||||
|
process.env.XDG_CONFIG_HOME = configHome;
|
||||||
|
|
||||||
|
import { useFeedStore } from "../src/stores/feed";
|
||||||
|
import type { Podcast } from "../src/types/podcast";
|
||||||
|
|
||||||
|
interface ServedEpisode {
|
||||||
|
title: string;
|
||||||
|
date: string;
|
||||||
|
}
|
||||||
|
|
||||||
|
let server: ReturnType<typeof Bun.serve> | null = null;
|
||||||
|
let servedEpisodes: ServedEpisode[] = [];
|
||||||
|
let feedAId = "";
|
||||||
|
|
||||||
|
/** XML for the current served episode list (episode ids = feedUrl#index). */
|
||||||
|
function feedXml(episodes: ServedEpisode[], origin: string): string {
|
||||||
|
const items = episodes
|
||||||
|
.map(
|
||||||
|
(ep, i) => `<item>
|
||||||
|
<title>${ep.title}</title>
|
||||||
|
<pubDate>${ep.date}</pubDate>
|
||||||
|
<enclosure url="${origin}/audio-${i}.mp3" length="12345" type="audio/mpeg"/>
|
||||||
|
</item>`,
|
||||||
|
)
|
||||||
|
.join("\n");
|
||||||
|
return `<?xml version="1.0" encoding="UTF-8"?>
|
||||||
|
<rss version="2.0"><channel>
|
||||||
|
<title>Test Show</title>
|
||||||
|
<description>Regression test feed</description>
|
||||||
|
${items}
|
||||||
|
</channel></rss>`;
|
||||||
|
}
|
||||||
|
|
||||||
|
const makePodcast = (feedUrl: string): Podcast => ({
|
||||||
|
id: feedUrl,
|
||||||
|
title: "Test Show",
|
||||||
|
description: "Regression test feed",
|
||||||
|
author: "tester",
|
||||||
|
feedUrl,
|
||||||
|
lastUpdated: new Date(),
|
||||||
|
isSubscribed: true,
|
||||||
|
});
|
||||||
|
|
||||||
|
beforeAll(() => {
|
||||||
|
server = Bun.serve({
|
||||||
|
port: 0,
|
||||||
|
fetch(req) {
|
||||||
|
const url = new URL(req.url);
|
||||||
|
if (url.pathname.endsWith(".xml")) {
|
||||||
|
return new Response(feedXml(servedEpisodes, url.origin), {
|
||||||
|
headers: { "Content-Type": "application/rss+xml" },
|
||||||
|
});
|
||||||
|
}
|
||||||
|
return new Response("not found", { status: 404 });
|
||||||
|
},
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
vi.useFakeTimers();
|
||||||
|
});
|
||||||
|
|
||||||
|
afterAll(() => {
|
||||||
|
vi.useRealTimers();
|
||||||
|
server?.stop(true);
|
||||||
|
rmSync(configHome, { recursive: true, force: true });
|
||||||
|
});
|
||||||
|
|
||||||
|
test("refresh with identical episodes does not bump lastUpdated", async () => {
|
||||||
|
const store = useFeedStore();
|
||||||
|
servedEpisodes = [
|
||||||
|
{ title: "Ep 3", date: "2026-08-03T00:00:00Z" },
|
||||||
|
{ title: "Ep 2", date: "2026-08-02T00:00:00Z" },
|
||||||
|
{ title: "Ep 1", date: "2026-08-01T00:00:00Z" },
|
||||||
|
];
|
||||||
|
const feedUrl = `http://127.0.0.1:${server!.port}/show-a.xml`;
|
||||||
|
const feed = await store.addFeed(makePodcast(feedUrl), "test-source");
|
||||||
|
expect(feed).not.toBeNull();
|
||||||
|
feedAId = feed!.id;
|
||||||
|
|
||||||
|
const before = store.getFeed(feedAId)!;
|
||||||
|
const beforeUpdated = before.lastUpdated.getTime();
|
||||||
|
|
||||||
|
// Advance the (mocked) clock, then refresh with identical content.
|
||||||
|
vi.advanceTimersByTime(60_000);
|
||||||
|
await store.refreshFeed(feedAId);
|
||||||
|
|
||||||
|
const after = store.getFeed(feedAId)!;
|
||||||
|
expect(after).toBe(before); // same object: no update applied at all
|
||||||
|
expect(after.lastUpdated.getTime()).toBe(beforeUpdated);
|
||||||
|
expect(after.episodes.length).toBe(3);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("refresh with a genuinely new episode bumps lastUpdated", async () => {
|
||||||
|
const store = useFeedStore();
|
||||||
|
servedEpisodes.push({ title: "Ep 0 (new)", date: "2026-08-04T00:00:00Z" });
|
||||||
|
|
||||||
|
const before = store.getFeed(feedAId)!.lastUpdated.getTime();
|
||||||
|
vi.advanceTimersByTime(60_000);
|
||||||
|
await store.refreshFeed(feedAId);
|
||||||
|
|
||||||
|
const after = store.getFeed(feedAId)!;
|
||||||
|
expect(after.lastUpdated.getTime()).toBeGreaterThan(before);
|
||||||
|
expect(after.episodes.length).toBe(4);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("refreshAllFeeds keeps unchanged feeds' order and timestamps", async () => {
|
||||||
|
const store = useFeedStore();
|
||||||
|
// Feed B: distinct URL, identical served content, so refreshing it is a
|
||||||
|
// no-op too.
|
||||||
|
const feedBUrl = `http://127.0.0.1:${server!.port}/show-b.xml`;
|
||||||
|
const feedB = await store.addFeed(makePodcast(feedBUrl), "test-source");
|
||||||
|
expect(feedB).not.toBeNull();
|
||||||
|
const feedBId = feedB!.id;
|
||||||
|
|
||||||
|
const orderBefore = store.getFilteredFeeds().map((f) => f.id);
|
||||||
|
const tsBefore: Record<string, number> = {};
|
||||||
|
for (const id of [feedAId, feedBId]) {
|
||||||
|
tsBefore[id] = store.getFeed(id)!.lastUpdated.getTime();
|
||||||
|
}
|
||||||
|
|
||||||
|
vi.advanceTimersByTime(60_000);
|
||||||
|
await store.refreshAllFeeds();
|
||||||
|
|
||||||
|
expect(store.getFilteredFeeds().map((f) => f.id)).toEqual(orderBefore);
|
||||||
|
for (const id of [feedAId, feedBId]) {
|
||||||
|
expect(store.getFeed(id)!.lastUpdated.getTime()).toBe(tsBefore[id]);
|
||||||
|
}
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user