diff --git a/src/data/lifecycle.js b/src/data/lifecycle.js index 6c2290c72..c01926451 100644 --- a/src/data/lifecycle.js +++ b/src/data/lifecycle.js @@ -2385,11 +2385,28 @@ export class LayerLifecycle { /** * Ask presentation to repaint layer rows now, for data that lands outside * the manager tick (Transit's proximity polls, Directions' route steps). + * @param {string} [layerId] - The layer whose row changed. */ - refreshLayerStats() { + refreshLayerStats(layerId) { + this._dropRecoveredRefreshError(layerId); this._publishActivity({ type: 'status' }); } + /** + * A tick's failure belongs to the feed it polled. Once the layer's own stats + * stop reporting one (the camera left that feed), the row stops showing it + * instead of waiting for the next tick. + */ + _dropRecoveredRefreshError(layerId) { + const entry = layerId ? this.layers.get(layerId) : null; + if (!entry?.managerRefreshError || entry.refreshing) return; + const failure = refreshFailureFromStats( + this._moduleStats(entry), + entry.module.name || layerId, + ); + if (!failure) entry.managerRefreshError = null; + } + _publishActivity(change) { for (const callback of this._activityListeners) { try { diff --git a/src/data/manager.js b/src/data/manager.js index cf7b31fae..688fb94d2 100644 --- a/src/data/manager.js +++ b/src/data/manager.js @@ -20,8 +20,10 @@ export class DataLayerManager extends LayerLifecycle { * manager tick (camera-driven loads such as Transit's proximity polls), so a * row shows its count when the data lands instead of at the next interval. * One DOM pass; skipped while the document is hidden. + * @param {string} [layerId] - The layer whose row changed. */ - refreshLayerStats() { + refreshLayerStats(layerId) { + this._dropRecoveredRefreshError(layerId); this._refreshTogglePanel(); } diff --git a/src/data/manager.test.mjs b/src/data/manager.test.mjs index 70f705ec6..de3502979 100644 --- a/src/data/manager.test.mjs +++ b/src/data/manager.test.mjs @@ -2677,6 +2677,31 @@ test('refreshLayerStats reaches presentation through the bare lifecycle', async assert.deepEqual(changes, ['status']); }); +test('refreshLayerStats(layerId) drops a tick failure the layer no longer reports', async () => { + const { LayerLifecycle } = await import('./lifecycle.js'); + const lifecycle = new LayerLifecycle({}); + let stats = { count: 0, error: 'Feed unavailable' }; + lifecycle.register({ + id: 'feed', + name: 'Feed', + getStats: () => stats, + async update() {}, + }); + const entry = lifecycle.layers.get('feed'); + entry.enabled = true; + entry.initialized = true; + entry.managerRefreshError = 'Feed unavailable'; + + lifecycle.refreshLayerStats('feed'); + assert.equal(entry.managerRefreshError, 'Feed unavailable'); + + stats = { count: 0 }; + lifecycle.refreshLayerStats(); + assert.equal(entry.managerRefreshError, 'Feed unavailable'); + lifecycle.refreshLayerStats('feed'); + assert.equal(entry.managerRefreshError, null); +}); + test('periodic polls pause hidden, catch up once when due, and dispose visibility listeners', async (t) => { const listeners = new Set(); const doc = { diff --git a/src/layers/bikeshare/controls.js b/src/layers/bikeshare/controls.js index 78edd907b..03bb544ab 100644 --- a/src/layers/bikeshare/controls.js +++ b/src/layers/bikeshare/controls.js @@ -70,17 +70,26 @@ export function createControls({ state: layerState, services, parts, source }) { * @returns {{ count: number, lastUpdate: number|null, loading: boolean, loadingLabel?: string, error?: string, degraded?: boolean, stale?: boolean, status?: string, statusMessage?: string }} */ getStats() { + // A city in range without points is loading even before its fetch + // starts: the row repaints on entry, ahead of beginLoading(). + const loading = + layerState._loading || + [...layerState._activeCityIds].some( + (cityId) => + !layerState._cityRuntime.has(cityId) && + !layerState._failedCityIds.has(cityId), + ); const stats = { count: layerState._count, lastUpdate: layerState._lastUpdate, - loading: layerState._loading, + loading, }; // No system in range is guidance, not an empty feed: the camera is too // high, or no catalogued system is near the view. A city that failed // leaves range empty too, and reports its failure below instead. if ( layerState._enabled && - !layerState._loading && + !loading && layerState._activeCityIds.size === 0 && layerState._failedCityIds.size === 0 ) { @@ -92,7 +101,7 @@ export function createControls({ state: layerState, services, parts, source }) { : `Fly below ${Math.round(ACTIVATION_ENTER_ALTITUDE_M / 1000).toLocaleString('en-US')} km to a covered city`; return { ...stats, status: 'zoom-in', statusMessage: note }; } - if (layerState._loading) { + if (loading) { stats.loadingLabel = layerState._activeCityIds.size > 0 ? t('syncing {count} city feeds...', { diff --git a/src/layers/bikeshare/rowRepaint.test.mjs b/src/layers/bikeshare/rowRepaint.test.mjs new file mode 100644 index 000000000..144ff5502 --- /dev/null +++ b/src/layers/bikeshare/rowRepaint.test.mjs @@ -0,0 +1,295 @@ +// Bike Share row repaints: entering a city, leaving it or nudging the camera +// mid-load, and leaving a failed city. +// Bike Share: after the camera settles, the last row the panel painted must +// match what the layer reports now. Each case drives the real viewport, +// ingestion, rendering and controls; the manager is a spy that records the +// row text (LayerPanel._buildMetaText) at every refreshLayerStats() call. +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import * as Cesium from 'cesium'; +import { CITY_BY_ID } from './registry.js'; +import { createState } from './state.js'; +import { createControls } from './controls.js'; +import { createIngestion } from './ingestion.js'; +import { createModel } from './model.js'; +import { createViewport } from './viewport.js'; +import { createRendering } from './rendering.js'; +import { createLifecycle } from './lifecycle.js'; +import { LayerPanel, layerFeedState } from '../../ui/layerPanel.js'; + +const BOSTON_ID = 'boston-bluebikes'; +const TORONTO_ID = 'toronto-bike-share'; +const BUFFALO_ID = 'buffalo-reddy'; +const LONDON = { lat: 51.5072, lon: -0.1276 }; +const BOSTON = { lat: 42.3601, lon: -71.0589 }; +const BOSTON_NUDGED = { lat: 42.37, lon: -71.05 }; +const BETWEEN = { lat: 43.25, lon: -79.1 }; +const TORONTO_ONLY = { lat: 43.7, lon: -79.6 }; +const LOW_M = 10_000; + +globalThis.document ??= { + addEventListener() {}, + removeEventListener() {}, +}; + +function deferred() { + let resolve; + const promise = new Promise((res) => { + resolve = res; + }); + return { promise, resolve }; +} + +function makeSource() { + const cityOf = (url) => { + for (const [id, city] of CITY_BY_ID) { + if (url === city.stationInformationUrl) return { id, kind: 'info' }; + if (url === city.stationStatusUrl) return { id, kind: 'status' }; + } + throw new Error(`unexpected GBFS url ${url}`); + }; + const source = { + dead: new Set(), + /** `${cityId}:${kind}` → deferred that gates the answer. */ + gates: new Map(), + async getStations(url, { signal } = {}) { + const { id, kind } = cityOf(url); + const gate = source.gates.get(`${id}:${kind}`); + if (gate) { + await new Promise((resolve, reject) => { + gate.promise.then(resolve); + signal?.addEventListener('abort', () => + reject(Object.assign(new Error('aborted'), { name: 'AbortError' })), + ); + }); + } else { + await new Promise((resolve) => setTimeout(resolve, 1)); + } + signal?.throwIfAborted(); + if (source.dead.has(id)) throw new Error('GBFS HTTP 503'); + const city = CITY_BY_ID.get(id); + if (kind === 'info') { + return { + data: { + stations: [ + { + station_id: `${id}-1`, + name: `${id} dock`, + lat: city.centerLat, + lon: city.centerLon, + capacity: 10, + }, + ], + }, + }; + } + return { + data: { + stations: [ + { + station_id: `${id}-1`, + num_bikes_available: 3, + num_docks_available: 7, + is_installed: true, + is_renting: true, + is_returning: true, + }, + ], + }, + }; + }, + }; + return source; +} + +/** The row as the panel draws it: state chip, count, meta text. */ +const rowText = (raw) => { + const stats = { + count: 0, + lastUpdate: null, + ...raw, + loading: raw.loading === true, + }; + const meta = LayerPanel.prototype._buildMetaText({ + id: 'bikeshare', + source: 'GBFS', + enabled: true, + lifecycleState: 'enabled', + stats, + }); + return `${layerFeedState(stats)} | ${stats.count || '—'} | ${meta}`; +}; + +function makeLiveLayer() { + const source = makeSource(); + const services = { + render: { governorRequestRender() {} }, + overlays: { + setOverlayEntries() {}, + setOverlaySourceVisible() {}, + clearOverlaySource() {}, + }, + sprites: { registerSpriteCollection() {}, restoreSpriteOrder() {} }, + picking: { registerPickOwner() {}, unregisterPickOwner() {} }, + }; + const state = createState({ services }); + const parts = {}; + const context = { state, services, parts, source }; + parts.ingestion = createIngestion(context); + parts.model = createModel(context); + parts.viewport = createViewport(context); + parts.rendering = createRendering(context); + parts.selection = { + _installClickHandler() {}, + _clearSelection() {}, + _onKeyDown() {}, + }; + parts.queries = { collectDetectableStations: () => [] }; + parts.lifecycle = createLifecycle(context); + const { methods: controls } = createControls(context); + + const camera = { ...LONDON, height: LOW_M }; + const viewer = { + camera: { + get positionCartographic() { + return Cesium.Cartographic.fromDegrees( + camera.lon, + camera.lat, + camera.height, + ); + }, + computeViewRectangle: () => undefined, + changed: { addEventListener() {}, removeEventListener() {} }, + }, + scene: { globe: {}, primitives: { add() {}, remove() {} } }, + }; + parts.lifecycle.methods.init(viewer); + + const painted = []; + controls.attachDataManager({ + refreshLayerStats() { + painted.push(rowText(controls.getStats())); + }, + }); + + return { + state, + source, + painted, + lastPainted: () => painted.at(-1), + live: () => rowText(controls.getStats()), + moveTo(place, height = LOW_M) { + Object.assign(camera, place, { height }); + return parts.viewport.runProximityCheck(); + }, + async enable() { + parts.lifecycle.methods.enable(viewer); + await settle(); + }, + }; +} + +async function settle() { + for (let i = 0; i < 5; i += 1) + await new Promise((resolve) => setTimeout(resolve, 2)); +} + +test('entering a city paints the loading row, not "never"', async () => { + const layer = makeLiveLayer(); + const info = deferred(); + layer.source.gates.set(`${BOSTON_ID}:info`, info); + await layer.enable(); + const before = layer.painted.length; + const entering = layer.moveTo(BOSTON); + assert.ok(layer.painted.length > before, 'entering repainted'); + const atEntry = layer.lastPainted(); + const liveAfter = layer.live(); + info.resolve(); + await entering; + await settle(); + assert.match(liveAfter, /syncing 1 city feeds/); + assert.match(atEntry, /^loading \|/, 'the entry repaint reads loading'); + assert.doesNotMatch(atEntry, /never/); +}); +test('re-entering a city with cached station info paints loading', async () => { + const layer = makeLiveLayer(); + await layer.enable(); + await layer.moveTo(BOSTON); + await settle(); + await layer.moveTo(LONDON); + await settle(); + const status = deferred(); + layer.source.gates.set(`${BOSTON_ID}:status`, status); + const entering = layer.moveTo(BOSTON); + const atEntry = layer.lastPainted(); + await Promise.resolve(); + await Promise.resolve(); + const liveAfter = layer.live(); + status.resolve(); + await entering; + await settle(); + assert.match(liveAfter, /syncing 1 city feeds/); + assert.match(atEntry, /^loading \|/, 'the entry repaint reads loading'); + assert.doesNotMatch(atEntry, /never/); +}); + +test('leaving a city mid-load does not leave the row on LOADING', async () => { + const layer = makeLiveLayer(); + layer.source.gates.set(`${BOSTON_ID}:info`, deferred()); + await layer.enable(); + const entering = layer.moveTo(BOSTON); + await settle(); + assert.equal(layer.state._loading, true); + + await layer.moveTo(LONDON); + await entering; + await settle(); + assert.equal(layer.state._loading, false); + assert.match(layer.live(), /No bike share here yet/); + assert.equal(layer.lastPainted(), layer.live()); +}); + +test('a camera nudge during the first load still repaints the drawn stations', async () => { + const layer = makeLiveLayer(); + const status = deferred(); + layer.source.gates.set(`${BOSTON_ID}:status`, status); + await layer.enable(); + const entering = layer.moveTo(BOSTON); + await settle(); + assert.ok(layer.state._cityRuntime.has(BOSTON_ID), 'points are drawn'); + + await layer.moveTo(BOSTON_NUDGED); + status.resolve(); + await entering; + await settle(); + assert.equal(layer.state._count, 1, 'one station on the map'); + assert.equal(layer.lastPainted(), layer.live()); +}); + +test('leaving a city whose load failed repaints the error away', async () => { + const layer = makeLiveLayer(); + layer.source.dead.add(BOSTON_ID); + await layer.enable(); + await layer.moveTo(BOSTON); + await settle(); + assert.match(layer.lastPainted(), /GBFS fetch error/i); + + await layer.moveTo(LONDON); + await settle(); + assert.match(layer.live(), /No bike share here yet/); + assert.equal(layer.lastPainted(), layer.live()); +}); + +test('leaving the failed city of two repaints DEGRADED away', async () => { + const layer = makeLiveLayer(); + layer.source.dead.add(BUFFALO_ID); + await layer.enable(); + await layer.moveTo(BETWEEN); + await settle(); + assert.match(layer.lastPainted(), /^degraded/); + + await layer.moveTo(TORONTO_ONLY); + await settle(); + assert.doesNotMatch(layer.live(), /^degraded/); + assert.equal(layer.lastPainted(), layer.live()); +}); diff --git a/src/layers/bikeshare/state.js b/src/layers/bikeshare/state.js index 518c2397a..199c9b5ca 100644 --- a/src/layers/bikeshare/state.js +++ b/src/layers/bikeshare/state.js @@ -19,7 +19,7 @@ export function createState({ services }) { layerState._viewer = null; - /** @type {{ refreshLayerStats?: () => void }|null} Manager that repaints the layer's row. */ + /** @type {{ refreshLayerStats?: (layerId?: string) => void }|null} Manager that repaints the layer's row. */ layerState._dataManager = null; diff --git a/src/layers/bikeshare/viewport.js b/src/layers/bikeshare/viewport.js index dbc328f61..008c01333 100644 --- a/src/layers/bikeshare/viewport.js +++ b/src/layers/bikeshare/viewport.js @@ -121,7 +121,7 @@ export function createViewport({ state: layerState, services, parts, source }) { * prompt, station count), and the manager tick is 60 s. */ function refreshRow() { - layerState._dataManager?.refreshLayerStats?.(); + layerState._dataManager?.refreshLayerStats?.('bikeshare'); } /** Deactivate all currently active cities and clear the active set. */ @@ -232,12 +232,17 @@ export function createViewport({ state: layerState, services, parts, source }) { } } // A failure never outlives its city's range. + let failureCleared = false; for (const cityId of layerState._failedCityIds) { - if (!nextActive.has(cityId)) layerState._failedCityIds.delete(cityId); + if (!nextActive.has(cityId)) { + layerState._failedCityIds.delete(cityId); + failureCleared = true; + } } const changed = !wasGateEnabled || + failureCleared || nextActive.size !== layerState._activeCityIds.size || [...nextActive].some((cityId) => !layerState._activeCityIds.has(cityId)); layerState._activeCityIds = nextActive; @@ -259,7 +264,9 @@ export function createViewport({ state: layerState, services, parts, source }) { await Promise.all( toActivate.map((cityId) => activateCity(cityId, generation)), ); - if (generation === layerState._proximityGeneration) refreshRow(); + // Superseded loads repaint too: a newer check may have painted the row + // while this one was still loading, or found nothing to load itself. + if (layerState._enabled) refreshRow(); } /** Schedule a debounced proximity check after camera movement. */ diff --git a/src/layers/transit/refreshErrorRepaint.test.mjs b/src/layers/transit/refreshErrorRepaint.test.mjs new file mode 100644 index 000000000..d1a5c16b8 --- /dev/null +++ b/src/layers/transit/refreshErrorRepaint.test.mjs @@ -0,0 +1,98 @@ +// Transit: a manager tick that failed over Boston must not keep its error on +// the row the coverage repaint draws after the camera leaves for London. +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { createState } from './state.js'; +import { createViewport } from './viewport.js'; +import { createQueries } from './queries.js'; +import { LayerLifecycle } from '../../data/lifecycle.js'; +import { LayerPanel, layerFeedState } from '../../ui/layerPanel.js'; + +const rad = Math.PI / 180; +const LONDON = { lat: 51.5072, lon: -0.1276 }; +const BOSTON = { lat: 42.3601, lon: -71.0589 }; + +function viewer(at, height) { + return { + camera: { + positionCartographic: { + height, + latitude: at.lat * rad, + longitude: at.lon * rad, + }, + computeViewRectangle: () => undefined, + }, + scene: { globe: {} }, + }; +} + +test('leaving a feed whose tick failed repaints its error away', async () => { + const services = { + overlays: { + setOverlayEntries() {}, + setOverlaySourceVisible() {}, + clearOverlaySource() {}, + }, + render: { governorRequestRender() {} }, + }; + const state = createState({ services }); + const feedStatus = (feedId) => { + if (!state._feedStatus.has(feedId)) + state._feedStatus.set(feedId, { count: 0, error: null, loading: false }); + return state._feedStatus.get(feedId); + }; + const parts = { + ingestion: { + feedStatus, + // As pollFeed's catch does when the proxy answers 503. + pollFeed(feed) { + Object.assign(feedStatus(feed.id), { + error: `${feed.name} feed unavailable`, + loading: false, + }); + return Promise.resolve(); + }, + removeFeedVehicles() {}, + sweepAgedVehicles() {}, + }, + rendering: { requestVisibility() {}, syncRenderHold() {} }, + }; + parts.viewport = createViewport({ state, services, parts }); + const queries = createQueries({ state, parts }).methods; + state._enabled = true; + + const lifecycle = new LayerLifecycle(null); + lifecycle.register({ + id: 'transit', + name: 'Transit', + source: 'GTFS-RT', + getStats: () => queries.getStats(), + async update() {}, + }); + const entry = lifecycle.layers.get('transit'); + entry.enabled = true; + entry.lifecycleState = 'enabled'; + entry.initialized = true; + + const row = () => { + const layer = lifecycle.getAll()[0]; + return `${layerFeedState(layer.stats)} | ${LayerPanel.prototype._buildMetaText(layer)}`; + }; + const painted = []; + state._dataManager = { + refreshLayerStats(layerId) { + lifecycle.refreshLayerStats(layerId); + painted.push(row()); + }, + }; + + state._viewer = viewer(BOSTON, 4_000); + parts.viewport.runProximityCheck(); + await lifecycle.refreshLayer('transit'); + assert.match(row(), /feed unavailable/, 'the tick recorded the failure'); + + state._viewer = viewer(LONDON, 4_000); + parts.viewport.runProximityCheck(); + assert.equal(queries.getStats().status, 'zoom-in'); + assert.doesNotMatch(painted.at(-1), /feed unavailable/); +}); diff --git a/src/layers/transit/state.js b/src/layers/transit/state.js index 75b13d84b..569298e6e 100644 --- a/src/layers/transit/state.js +++ b/src/layers/transit/state.js @@ -112,7 +112,7 @@ export function createState({ services }) { state._cockpitVision = false; /** @type {Array<[string, EventListener]>} window listeners to remove on destroy. */ state._styleListeners = []; - /** @type {{ refreshLayerStats?: () => void }|null} */ + /** @type {{ refreshLayerStats?: (layerId?: string) => void }|null} */ state._dataManager = null; /** @type {number|null} */ state._lastUpdate = null; diff --git a/src/layers/transit/viewport.js b/src/layers/transit/viewport.js index 0e3b317d4..a62d49009 100644 --- a/src/layers/transit/viewport.js +++ b/src/layers/transit/viewport.js @@ -126,7 +126,7 @@ export function createViewport({ state, services, parts }) { governorRequestRender('transit-proximity'); // The row's text follows the camera (coverage prompt, feed list), so it // repaints now instead of at the next 15 s manager tick. - if (changed) state._dataManager?.refreshLayerStats?.(); + if (changed) state._dataManager?.refreshLayerStats?.('transit'); } function onCameraChanged() {