diff --git a/README.md b/README.md index b514b7f..d950361 100644 --- a/README.md +++ b/README.md @@ -54,6 +54,8 @@ Every deploy waits for approval in its GitHub environment. 1. **Stage:** merge to `main`. The push deploys the main app and every feed app with a stage id to stage. 2. **Production:** after checking stage, tag that commit on `main` with the next CalVer version, `vYY.M.PATCH` (for example `v26.11.0`, then `v26.11.1`; check `git tag --sort=-v:refname | head -1` for the latest), and push the tag. This deploys the main app and every feed app with a production id to production. Tags not on `main` are refused, and tags that aren't `v` followed by a digit (such as `vrc1`) are ignored. + > **Any** tag that starts with `v` and a digit deploys to production, including pre-release tags such as `v26.11.0-rc.1`. Don't push one to test; use stage. (This comes from the shared edge-apps-actions tag filter, so it applies to every Edge App.) + ```bash git checkout main && git pull git tag vYY.M.PATCH @@ -98,21 +100,22 @@ The initialize summary also says to set a repo variable. Ignore that for feed ap - **Its name, description, icon, header title or feed URL:** edit its entry in `deployed-apps.yml` and release as usual. Merging deploys the change to stage, and the next tag deploys it to production. (Changing `app_name` after the app exists doesn't rename it in Screenly; it is only used when the app is created.) - **Code or other settings:** change the code or `screenly.yml`. Every feed app picks it up on the next release, with no change to `deployed-apps.yml`. -- **A one-off redeploy or test:** run **Actions → Update RSS Reader Apps**, from `main` for stage or from a release tag for production. Leave the app key empty for every app, or enter one such as `bbc-news`. The optional overrides (description, header title, icon URL, feed URL) apply to that run only; to keep a change, put it in `deployed-apps.yml`. +- **A one-off redeploy or test:** run **Actions → Update RSS Reader Apps**, from `main` for stage or from a release tag for production. Leave the app key empty for every app, or enter one such as `bbc-news`. The optional overrides (description, header title, icon URL, feed URL) apply to that run only, so set an app key when you use one; with the key empty, an override changes every feed app. To keep a change, put it in `deployed-apps.yml`. +- **Redeploying the main RSS Reader app:** a manual run of **Update Edge App** from `main` redeploys stage. For production, re-run the workflow run of the latest release tag; a manual run from a tag does nothing for the main app. - **Removing an app from deploys:** clear its id for that environment in `deployed-apps.yml`. This stops deploying to it but doesn't delete the app in Screenly. ## Configuration -| Setting | Description | Type | Default | -| ------------------- | ------------------------------------------------------------------------------------------------------------ | -------- | -------------------------------------- | -| `bypass_cors` | Route the RSS feed URL through the CORS proxy | optional | `true` | -| `cache_interval` | How often (in seconds) to refresh the feed | required | `1800` | -| `display_errors` | Show errors on screen for debugging purposes | optional | `false` | -| `mode` | Layout: `headlines` (lead story with image, plus a list) or `list` (headlines only, for maximum readability) | optional | `headlines` | -| `override_locale` | Override the locale used for date formatting (e.g. `en`, `fr`, `de`) | optional | `en` | -| `override_timezone` | Override the timezone for date display (e.g. `Europe/London`) | optional | system timezone | -| `rss_title` | Source name shown in the header | required | `BBC News` | -| `rss_url` | URL of the RSS feed to display | required | `http://feeds.bbci.co.uk/news/rss.xml` | +| Setting | Description | Type | Default | +| ------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------- | -------------------------------------- | +| `bypass_cors` | Route the RSS feed URL through the CORS proxy | optional | `true` | +| `cache_interval` | How often (in seconds) to refresh the feed, from 60 to 86400; values outside that range are clamped. If a refresh fails or the feed is empty, the last stories from this feed stay up for up to 24 hours | required | `1800` | +| `display_errors` | Show errors on screen for debugging purposes | optional | `false` | +| `mode` | Layout: `headlines` (lead story with image, plus a list) or `list` (headlines only, for maximum readability) | optional | `headlines` | +| `override_locale` | Override the locale used for date formatting (e.g. `en`, `fr`, `de`) | optional | `en` | +| `override_timezone` | Override the timezone for date display (e.g. `Europe/London`) | optional | system timezone | +| `rss_title` | Source name shown in the header | required | `BBC News` | +| `rss_url` | URL of the RSS feed to display | required | `http://feeds.bbci.co.uk/news/rss.xml` | ## Example Feeds diff --git a/screenly.yml b/screenly.yml index ca60a25..89d6bbe 100644 --- a/screenly.yml +++ b/screenly.yml @@ -21,7 +21,9 @@ settings: title: Cache Interval optional: false help_text: | - Specify the interval (in seconds) used to periodically cache feeds. + How often, in seconds, to refresh the feed (60 to 86400; values + outside that range are clamped). If a refresh fails, the last stories + from this feed stay on screen for up to 24 hours. display_errors: type: string default_value: 'false' diff --git a/src/main.ts b/src/main.ts index 8e54f05..60e4767 100644 --- a/src/main.ts +++ b/src/main.ts @@ -18,6 +18,7 @@ import { import { getNewsMode, type NewsMode } from './mode' import { getTitleSizeClass, + getRefreshIntervalMs, loadCache, resizeImageUrl, saveCache, @@ -196,23 +197,26 @@ async function loadAndRender( ) { try { const entries = await fetchFeed(config) - if (entries.length === 0) { - showError(els) + if (entries.length > 0) { + saveCache(config.rssUrl, entries) + render(els, mode, entries) + showBody(els) return } - saveCache(entries) - render(els, mode, entries) - showBody(els) + // An empty feed is usually a hiccup at the source; keep showing the last + // stories rather than blanking the screen + console.warn('RSS feed returned no entries') } catch (err) { console.error('RSS fetch failed:', err) - const cached = loadCache() - if (cached.length === 0) { - showError(els) - return - } - render(els, mode, cached) - showBody(els) } + + const cached = loadCache(config.rssUrl) + if (cached.length === 0) { + showError(els) + return + } + render(els, mode, cached) + showBody(els) } function updateDate(els: NewsElements, config: FeedConfig) { @@ -257,8 +261,9 @@ document.addEventListener('DOMContentLoaded', async () => { locale: await getLocale(), timezone: await getTimeZone(), } - const cacheInterval = - getSettingWithDefault('cache_interval', 1800) * 1000 + const cacheInterval = getRefreshIntervalMs( + getSettingWithDefault('cache_interval', 1800), + ) updateDate(els, config) setInterval(() => updateDate(els, config), DATE_UPDATE_INTERVAL_MS) diff --git a/src/utils.test.ts b/src/utils.test.ts index ad9f8ba..99e46a5 100644 --- a/src/utils.test.ts +++ b/src/utils.test.ts @@ -1,5 +1,7 @@ import { describe, test, expect, beforeEach } from 'bun:test' import { + CACHE_MAX_AGE_MS, + getRefreshIntervalMs, getTitleSizeClass, loadCache, resizeImageUrl, @@ -10,14 +12,30 @@ import type { RssEntry } from './utils' const localStorageMock = (() => { let store: Record = {} + let quota = Infinity return { getItem: (key: string) => store[key] ?? null, setItem: (key: string, value: string) => { + if (!(key in store) && Object.keys(store).length >= quota) { + throw new DOMException('quota exceeded', 'QuotaExceededError') + } store[key] = value }, + setQuota: (keys: number) => { + quota = keys + }, + removeItem: (key: string) => { + delete store[key] + }, + key: (index: number) => Object.keys(store)[index] ?? null, + get length() { + return Object.keys(store).length + }, clear: () => { store = {} + quota = Infinity }, + keys: () => Object.keys(store), } })() @@ -58,29 +76,142 @@ describe('stripHtml', () => { }) }) +const BBC = 'http://feeds.bbci.co.uk/news/rss.xml' +const NPR = 'https://feeds.npr.org/1004/rss.xml' +const NOW = 1_800_000_000_000 + describe('loadCache / saveCache', () => { beforeEach(() => { localStorageMock.clear() }) test('returns empty array when cache is empty', () => { - expect(loadCache()).toEqual([]) + expect(loadCache(BBC, NOW)).toEqual([]) + }) + + test('returns saved entries for the same feed', () => { + saveCache(BBC, ENTRIES, NOW) + expect(loadCache(BBC, NOW)).toEqual(ENTRIES) + }) + + test('overwrites previous cache for the same feed', () => { + saveCache(BBC, ENTRIES, NOW) + saveCache(BBC, [ENTRIES[0]], NOW) + expect(loadCache(BBC, NOW)).toEqual([ENTRIES[0]]) + }) + + test("never returns another feed's stories", () => { + // e.g. rss_url changed and the first fetch of the new feed failed + saveCache(BBC, ENTRIES, NOW) + expect(loadCache(NPR, NOW)).toEqual([]) + }) + + test('keeps separate caches per feed', () => { + saveCache(BBC, ENTRIES, NOW) + saveCache(NPR, [ENTRIES[1]], NOW) + expect(loadCache(BBC, NOW)).toEqual(ENTRIES) + expect(loadCache(NPR, NOW)).toEqual([ENTRIES[1]]) }) - test('returns saved entries', () => { - saveCache(ENTRIES) - expect(loadCache()).toEqual(ENTRIES) + test('expires after CACHE_MAX_AGE_MS', () => { + saveCache(BBC, ENTRIES, NOW) + expect(loadCache(BBC, NOW + CACHE_MAX_AGE_MS)).toEqual(ENTRIES) + expect(loadCache(BBC, NOW + CACHE_MAX_AGE_MS + 1)).toEqual([]) }) - test('overwrites previous cache', () => { - saveCache(ENTRIES) - saveCache([ENTRIES[0]]) - expect(loadCache()).toEqual([ENTRIES[0]]) + test('ignores the legacy shared cache and removes it on save', () => { + localStorage.setItem( + 'rssStore', + JSON.stringify({ entries: ENTRIES, timestamp: NOW }), + ) + expect(loadCache(BBC, NOW)).toEqual([]) + saveCache(BBC, [ENTRIES[0]], NOW) + expect(localStorageMock.keys()).not.toContain('rssStore') + }) + + test("prunes other feeds' expired caches on save, keeps fresh ones", () => { + saveCache(NPR, ENTRIES, NOW) + saveCache(BBC, ENTRIES, NOW + 1000) + expect(loadCache(NPR, NOW + 1000)).toEqual(ENTRIES) + + saveCache(BBC, ENTRIES, NOW + CACHE_MAX_AGE_MS + 1) + expect(localStorageMock.keys()).toEqual([`rssStore:${BBC}`]) + }) +}) + +describe('cache recovery and validation', () => { + beforeEach(() => { + localStorageMock.clear() + }) + + test('recovers when storage is full of expired caches', () => { + saveCache(NPR, ENTRIES, NOW) + localStorageMock.setQuota(1) + saveCache(BBC, ENTRIES, NOW + CACHE_MAX_AGE_MS + 1) + expect(loadCache(BBC, NOW + CACHE_MAX_AGE_MS + 1)).toEqual(ENTRIES) + }) + + test('treats a cache saved in the future as stale', () => { + // e.g. saved while the player's clock was ahead, before it synced + saveCache(BBC, ENTRIES, NOW + 60_000) + expect(loadCache(BBC, NOW)).toEqual([]) + }) + + test('rejects a cache with any malformed entry', () => { + const store = (entries: unknown) => + localStorage.setItem( + `rssStore:${BBC}`, + JSON.stringify({ feedUrl: BBC, timestamp: NOW, entries }), + ) + store([null]) + expect(loadCache(BBC, NOW)).toEqual([]) + store([ENTRIES[0], { content: 'no title' }]) + expect(loadCache(BBC, NOW)).toEqual([]) + store(['just a string']) + expect(loadCache(BBC, NOW)).toEqual([]) + }) + + test('fills in missing optional fields of a cached entry', () => { + localStorage.setItem( + `rssStore:${BBC}`, + JSON.stringify({ + feedUrl: BBC, + timestamp: NOW, + entries: [{ title: 'T' }], + }), + ) + expect(loadCache(BBC, NOW)).toEqual([ + { title: 'T', content: '', formattedDate: '', imageUrl: '', source: '' }, + ]) }) test('returns empty array for malformed cache', () => { - localStorage.setItem('rssStore', 'not-json') - expect(loadCache()).toEqual([]) + localStorage.setItem(`rssStore:${BBC}`, 'not-json') + expect(loadCache(BBC, NOW)).toEqual([]) + localStorage.setItem(`rssStore:${BBC}`, JSON.stringify({ feedUrl: BBC })) + expect(loadCache(BBC, NOW)).toEqual([]) + localStorage.setItem(`rssStore:${BBC}`, 'null') + expect(loadCache(BBC, NOW)).toEqual([]) + }) +}) + +describe('getRefreshIntervalMs', () => { + test('uses a normal value as is', () => { + expect(getRefreshIntervalMs(1800)).toBe(1_800_000) + expect(getRefreshIntervalMs(60)).toBe(60_000) + }) + + test('raises values that would refetch in a tight loop', () => { + expect(getRefreshIntervalMs(0)).toBe(60_000) + expect(getRefreshIntervalMs(-5)).toBe(60_000) + expect(getRefreshIntervalMs(1)).toBe(60_000) + expect(getRefreshIntervalMs(NaN)).toBe(60_000) + }) + + test('caps values that would overflow setInterval', () => { + // setInterval fires immediately for delays above 2^31 - 1 ms (~24.8 days) + expect(getRefreshIntervalMs(Infinity)).toBe(86_400_000) + expect(getRefreshIntervalMs(10_000_000)).toBe(86_400_000) }) }) diff --git a/src/utils.ts b/src/utils.ts index 0128480..8fdb75f 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -1,4 +1,14 @@ -const CACHE_KEY = 'rssStore' +// One cache per feed URL, so a changed rss_url (or another app on the same +// origin) never shows a different feed's stories +const CACHE_KEY_PREFIX = 'rssStore:' +// The single shared key used before caches were per feed +const LEGACY_CACHE_KEY = 'rssStore' +// Past this, cached stories are too old to show during an outage, and the +// error screen is more honest than yesterday's news +export const CACHE_MAX_AGE_MS = 24 * 60 * 60 * 1000 + +const MIN_REFRESH_SECONDS = 60 +const MAX_REFRESH_SECONDS = 24 * 60 * 60 export interface RssEntry { title: string @@ -9,6 +19,7 @@ export interface RssEntry { } interface AppCache { + feedUrl: string entries: RssEntry[] timestamp: number } @@ -72,24 +83,119 @@ export function stripHtml(html: string): string { .trim() } -export function loadCache(): RssEntry[] { +function cacheKey(feedUrl: string): string { + return `${CACHE_KEY_PREFIX}${feedUrl}` +} + +function readCache(key: string): Partial | null { + const raw = localStorage.getItem(key) + if (!raw) return null try { - const raw = localStorage.getItem(CACHE_KEY) - if (!raw) return [] - const parsed: AppCache = JSON.parse(raw) - // parsed.entries is undefined if the stored value is a legacy plain array; - // falling back to [] causes a fresh fetch on first run after upgrade. - return parsed.entries ?? [] + const parsed: unknown = JSON.parse(raw) + return parsed && typeof parsed === 'object' + ? (parsed as Partial) + : null + } catch { + return null + } +} + +// A cache saved in the future means the clock was wrong when it was saved +// (players often boot with a bad date until they sync), so its real age is +// unknown; treat it as stale rather than letting it outlive the 24 hours +function isFresh(cache: Partial, now: number): boolean { + if (typeof cache.timestamp !== 'number') return false + const age = now - cache.timestamp + return age >= 0 && age <= CACHE_MAX_AGE_MS +} + +// Storage can hold anything (an older build's format, a partial write), and +// rendering trusts these fields, so check every entry. Optional text fields +// default to empty; an entry without a title is unusable. +function toEntry(value: unknown): RssEntry | null { + if (!value || typeof value !== 'object') return null + const v = value as Record + if (typeof v.title !== 'string') return null + const text = (field: unknown) => (typeof field === 'string' ? field : '') + return { + title: v.title, + content: text(v.content), + formattedDate: text(v.formattedDate), + imageUrl: text(v.imageUrl), + source: text(v.source), + } +} + +// Returns the cached entries for this feed, or [] if there are none, they +// belong to another feed, they are older than CACHE_MAX_AGE_MS, or any of +// them is malformed +export function loadCache(feedUrl: string, now = Date.now()): RssEntry[] { + try { + const cache = readCache(cacheKey(feedUrl)) + if ( + !cache || + cache.feedUrl !== feedUrl || + !Array.isArray(cache.entries) || + !isFresh(cache, now) + ) { + return [] + } + const entries = cache.entries.map(toEntry) + return entries.every((entry) => entry !== null) + ? (entries as RssEntry[]) + : [] } catch { return [] } } -export function saveCache(entries: RssEntry[]) { +export function saveCache( + feedUrl: string, + entries: RssEntry[], + now = Date.now(), +) { try { - const data: AppCache = { entries, timestamp: Date.now() } - localStorage.setItem(CACHE_KEY, JSON.stringify(data)) + // Prune first: if storage is full of expired caches, the write below + // would otherwise fail every time and the cache would never recover + pruneCaches(feedUrl, now) + const data: AppCache = { feedUrl, entries, timestamp: now } + localStorage.setItem(cacheKey(feedUrl), JSON.stringify(data)) } catch (err) { console.warn('Failed to save cache:', err) } } + +// Drops the legacy shared cache and other feeds' expired caches, so a screen +// whose rss_url changes over time doesn't collect them forever +function pruneCaches(currentFeedUrl: string, now: number) { + localStorage.removeItem(LEGACY_CACHE_KEY) + const stale: string[] = [] + for (let i = 0; i < localStorage.length; i++) { + const key = localStorage.key(i) + if ( + !key || + !key.startsWith(CACHE_KEY_PREFIX) || + key === cacheKey(currentFeedUrl) + ) { + continue + } + const cache = readCache(key) + if (!cache || !isFresh(cache, now)) stale.push(key) + } + stale.forEach((key) => localStorage.removeItem(key)) +} + +// cache_interval is free text: 0 or a tiny value would refetch the feed in a +// tight loop, and a huge one (or Infinity) overflows setInterval, which then +// fires immediately. Clamp it to a sane range. +export function getRefreshIntervalMs(seconds: number): number { + const clamped = Number.isNaN(seconds) + ? MIN_REFRESH_SECONDS + : Math.min(Math.max(seconds, MIN_REFRESH_SECONDS), MAX_REFRESH_SECONDS) + if (clamped !== seconds) { + console.warn( + `cache_interval ${seconds} is outside ${MIN_REFRESH_SECONDS}-${MAX_REFRESH_SECONDS} seconds; using ${clamped}`, + ) + } + return clamped * 1000 +}