From 41641cd91a1fbf090554bd274b09a8097a1e60ee Mon Sep 17 00:00:00 2001 From: Sameh Khamis Date: Tue, 10 Nov 2026 13:40:40 -0800 Subject: [PATCH] fix(shipwrecks): the page clears shown wrecks as a whole, once even the newest source is too old Removing one source's wrecks on the page split merged records: a wreck both sources list is one Wikidata record carrying EMODnet facts, so expiring Wikidata dropped wrecks EMODnet still had, and expiring EMODnet kept its facts. The page now keeps every shown wreck, marked stale, until the newest source passes the limit, then clears them all. Adds a source test for the reported ages and limit. --- docs/CURRENT-STATE.md | 7 ++-- scripts/format-scope.json | 1 + src/layers/shipwrecks/index.js | 52 +++++++++++---------------- src/layers/shipwrecks/index.test.mjs | 30 ++++++++-------- src/layers/shipwrecks/source.test.mjs | 27 ++++++++++++++ 5 files changed, 67 insertions(+), 50 deletions(-) create mode 100644 src/layers/shipwrecks/source.test.mjs diff --git a/docs/CURRENT-STATE.md b/docs/CURRENT-STATE.md index c7b57785e..444512b5a 100644 --- a/docs/CURRENT-STATE.md +++ b/docs/CURRENT-STATE.md @@ -164,9 +164,10 @@ one that failed is asked again after 15 minutes. Each source keeps its own last good records: a source that fails, answers with nothing usable, or answers with less than half of its last good set keeps serving its earlier records as stale for up to seven days, and never removes the other source's wrecks. When the -page cannot refresh, the wrecks it shows are marked stale, and it removes each -source's wrecks once they pass seven days, counted from the age the proxy -reported (so the page and server clocks are never compared). EMODnet rows the portal took from +page cannot refresh, the wrecks it shows are marked stale, and it removes them +once even the newest source is past seven days, counted from the ages the +proxy reported (so the page and server clocks are never compared; a wreck +both sources list is one record, so nothing is removed per source). EMODnet rows the portal took from wrecksite.eu are skipped (that site is not openly licensed). A Wikidata wreck with the same name within 3 km of an EMODnet wreck is merged into one record that keeps the article link and takes EMODnet's year, sinking text, depth and diff --git a/scripts/format-scope.json b/scripts/format-scope.json index b6d08d904..10ffd7283 100644 --- a/scripts/format-scope.json +++ b/scripts/format-scope.json @@ -1280,6 +1280,7 @@ "src/layers/shipwrecks/eras.test.mjs", "src/layers/shipwrecks/period.test.mjs", "src/layers/shipwrecks/index.test.mjs", + "src/layers/shipwrecks/source.test.mjs", "src/data/shipwrecksProxy.test.mjs", "src/app/sourceComposition.js", "src/app/sourceComposition.test.mjs", diff --git a/src/layers/shipwrecks/index.js b/src/layers/shipwrecks/index.js index a16a728e1..a28a324c7 100644 --- a/src/layers/shipwrecks/index.js +++ b/src/layers/shipwrecks/index.js @@ -33,19 +33,20 @@ const MAX_CARD_LINES = 14; const MAX_SHOWN_AGE_MS = 7 * 24 * 60 * 60 * 1000; /** - * When each source's wrecks leave the map, on this page's clock: the age the - * proxy reports (its own clock) is subtracted from the limit, so the two - * clocks are never compared. With no age reported, the snapshot counts as - * fresh when it arrives. + * When the shown wrecks leave the map if the page cannot refresh, on this + * page's clock: the age the proxy reports (its own clock) is subtracted from + * the limit, so the two clocks are never compared, and the newest source + * decides. A wreck both sources list is one merged record, so the snapshot + * goes as a whole, not per source. With no age reported, the snapshot counts + * as fresh when it arrives. */ -export function sourceDeadlines(snapshot, at) { +export function snapshotDeadline(snapshot, at) { const limit = snapshot.maxAgeMs ?? MAX_SHOWN_AGE_MS; - const deadlines = {}; - for (const name of ['emodnet', 'wikidata']) { - if (snapshot.sources?.[name] === false) continue; - deadlines[name] = at + limit - (snapshot.ages?.[name] ?? 0); - } - return deadlines; + const ages = ['emodnet', 'wikidata'] + .filter((name) => snapshot.sources?.[name] !== false) + .map((name) => snapshot.ages?.[name] ?? 0); + const newest = ages.length ? Math.min(...ages) : 0; + return at + limit - newest; } /** * Marker size in pixels: grows with the death toll, from 16 px (no deaths known) to 36 px (about a thousand or more). @@ -101,7 +102,7 @@ export function createShipwrecksLayer({ let _records = new Map(); let _pins = []; let _snapshot = null; - let _deadlines = {}; + let _deadline = null; let _lastError = null; let _clickHandler = null; let _rowListener = null; @@ -402,7 +403,7 @@ export function createShipwrecksLayer({ snapshot.version === _snapshot?.version && _pins.length > 0; _snapshot = snapshot; - _deadlines = sourceDeadlines(snapshot, now()); + _deadline = snapshotDeadline(snapshot, now()); _lastError = snapshot.sources.emodnet && snapshot.sources.wikidata ? null @@ -417,27 +418,14 @@ export function createShipwrecksLayer({ return false; console.warn('[Data:Shipwrecks] Fetch error:', error); _lastError = error?.message || msg('Shipwrecks unavailable'); - // What is on the map was not refreshed: say so, and drop each - // source's wrecks once they are older than the proxy would serve. + // What is on the map was not refreshed: say so, and drop it once + // even its newest source is older than the proxy would serve. if (_snapshot) { - const at = now(); - const expired = Object.keys(_deadlines).filter( - (name) => _deadlines[name] <= at, - ); - for (const name of expired) delete _deadlines[name]; - if (!Object.keys(_deadlines).length) { + if (now() >= _deadline) { _snapshot = null; + _deadline = null; rebuild([]); - } else { - _snapshot = { ..._snapshot, stale: true }; - if (expired.length) { - _snapshot.records = _snapshot.records.filter( - (record) => !expired.includes(record.source), - ); - _snapshot.version = null; - rebuild(_snapshot.records); - } - } + } else _snapshot = { ..._snapshot, stale: true }; } notifyRow(); return false; @@ -458,7 +446,7 @@ export function createShipwrecksLayer({ _records = new Map(); _pins = []; _snapshot = null; - _deadlines = {}; + _deadline = null; _viewer = null; _rowListener = null; }, diff --git a/src/layers/shipwrecks/index.test.mjs b/src/layers/shipwrecks/index.test.mjs index ce2fe992c..56cf34fde 100644 --- a/src/layers/shipwrecks/index.test.mjs +++ b/src/layers/shipwrecks/index.test.mjs @@ -292,10 +292,10 @@ test('a failed refresh marks the shown wrecks stale, and clears them after seven assert.equal(h.markers.billboards.length, 0); }); -test('expiry runs on the page clock, per source, even without timestamps', async () => { +test('expiry runs on the page clock and keeps every wreck until the newest source is too old', async () => { const DAY = 24 * 60 * 60 * 1000; const down = () => new Error('Shipwrecks unavailable (HTTP 502)'); - // A page clock far from the proxy's: only the reported ages count. + // The page clock is far from the proxy's fetchedAt: only reported ages count. let clock = 5e12; const h = harness({ now: () => clock, @@ -303,33 +303,33 @@ test('expiry runs on the page clock, per source, even without timestamps', async { ...snapshot([ wreck('fresh', { source: 'emodnet' }), - wreck('old', { source: 'wikidata' }), + wreck('merged', { source: 'wikidata', alsoIn: 'emodnet' }), ]), - fetchedAt: null, + fetchedAt: 1, ages: { emodnet: 0, wikidata: 6.5 * DAY }, maxAgeMs: 7 * DAY, }, down(), down(), - down(), ], }); h.layer.enable(); await h.layer.update(); - clock += 0.25 * DAY; + clock += 1 * DAY; await h.layer.update(); - assert.equal(h.layer.getStats().count, 2, 'both still inside their limit'); - clock += 0.5 * DAY; - await h.layer.update(); - assert.deepEqual( - h.markers.billboards.map((billboard) => billboard.id), - ['shipwreck:fresh'], - 'only the source past its limit leaves', + assert.equal( + h.layer.getStats().count, + 2, + 'nothing leaves while EMODnet is in date', ); assert.equal(h.layer.getStats().stale, true); - clock += 7 * DAY; + clock += 6.5 * DAY; await h.layer.update(); - assert.equal(h.layer.getStats().count, 0); + assert.equal( + h.layer.getStats().count, + 0, + 'all leave once the newest is too old', + ); }); test('a snapshot with no timestamp or ages still expires seven days after it arrived', async () => { diff --git a/src/layers/shipwrecks/source.test.mjs b/src/layers/shipwrecks/source.test.mjs new file mode 100644 index 000000000..c82045951 --- /dev/null +++ b/src/layers/shipwrecks/source.test.mjs @@ -0,0 +1,27 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { createShipwrecksSource } from './source.js'; + +const respond = (payload) => + createShipwrecksSource({ fetchImpl: async () => Response.json(payload) }); + +test('the snapshot keeps valid source ages and drops malformed ones', async () => { + const record = { id: 'a', source: 'emodnet', lat: 50, lon: -1 }; + const good = await respond({ + records: [record], + version: 'v', + ages: { emodnet: 0, wikidata: 3600 }, + maxAgeMs: 604800000, + }).getSnapshot(); + assert.deepEqual(good.ages, { emodnet: 0, wikidata: 3600 }); + assert.equal(good.maxAgeMs, 604800000); + assert.equal(good.version, 'v'); + const bad = await respond({ + records: [record], + ages: { emodnet: -5, wikidata: 'x' }, + maxAgeMs: Number.NaN, + }).getSnapshot(); + assert.deepEqual(bad.ages, { emodnet: null, wikidata: null }); + assert.equal(bad.maxAgeMs, null); + assert.equal(bad.version, null); +});