diff --git a/server/providers/mapillary/tiles.js b/server/providers/mapillary/tiles.js index 76971bb66..4c445c90b 100644 --- a/server/providers/mapillary/tiles.js +++ b/server/providers/mapillary/tiles.js @@ -94,9 +94,14 @@ let upstreamActive = 0; /** @type {Array<() => void>} fetches waiting for a slot, oldest first */ const upstreamWaiting = []; -/** Run `task` once an upstream slot is free, unless `signal` aborts first. */ +/** + * Run `task` in an upstream slot, waiting for one unless `signal` aborts + * first. A freed slot passes straight to the oldest waiter, still counted, so + * a request arriving meanwhile cannot take it first. + */ async function withUpstreamSlot(signal, task) { - while (upstreamActive >= TILE_UPSTREAM_CONCURRENCY) + if (upstreamActive < TILE_UPSTREAM_CONCURRENCY) upstreamActive++; + else await new Promise((resolve, reject) => { const wake = () => { signal.removeEventListener('abort', leave); @@ -110,17 +115,14 @@ async function withUpstreamSlot(signal, task) { upstreamWaiting.push(wake); signal.addEventListener('abort', leave, { once: true }); }); - if (signal.aborted) { - // Woken, then left: hand the free slot on. - upstreamWaiting.shift()?.(); - throw signal.reason; - } - upstreamActive++; try { + // Handed a slot after leaving: it goes on to the next waiter. + signal.throwIfAborted(); return await task(); } finally { - upstreamActive--; - upstreamWaiting.shift()?.(); + const next = upstreamWaiting.shift(); + if (next) next(); + else upstreamActive--; } } diff --git a/src/layers/streetLevel/index.js b/src/layers/streetLevel/index.js index 061344ae4..228368388 100644 --- a/src/layers/streetLevel/index.js +++ b/src/layers/streetLevel/index.js @@ -16,6 +16,9 @@ import { export { STREET_LEVEL_LAYER_ID } from './policy.js'; +/** Longest wait for the server's key status; past it, the bundle's token decides. */ +const KEY_STATUS_TIMEOUT_MS = 10_000; + /** Coverage as the panel sees it while the `mapillary` switch is off. */ const COVERAGE_OFF = Object.freeze({ count: 0, @@ -73,15 +76,25 @@ export function createStreetLevelLayer({ } state.notify = notify; + /** The key-status request in flight; overlapping checks share it. */ + let keyCheck = null; + /** Coverage waits for the key status: no tile requests without a key. */ - async function checkKey() { - let configured; - try { - configured = (await source.getStatus())?.configured === true; - } catch { - configured = source.hasToken?.() === true; - } - parts.coverage.setKeyStatus(configured); + function checkKey() { + keyCheck ||= (async () => { + let configured; + try { + // A hung server must not hold LOADING, or the next check, forever. + const signal = AbortSignal.timeout(KEY_STATUS_TIMEOUT_MS); + configured = (await source.getStatus({ signal }))?.configured === true; + } catch { + configured = source.hasToken?.() === true; + } + parts.coverage.setKeyStatus(configured); + })().finally(() => { + keyCheck = null; + }); + return keyCheck; } function startDrawing() { @@ -90,6 +103,9 @@ export function createStreetLevelLayer({ parts.sequences.setVisible(true); parts.coverage.attach(viewer); credit.show(viewer); + // Without a known key, a switch-on asks again: the server may be back, + // or have a key now. + if (['status', 'no-key'].includes(state.coverage.blocked)) checkKey(); } /** Close the photo, clear lines and cones, forget refusals. */ diff --git a/src/layers/streetLevel/index.test.mjs b/src/layers/streetLevel/index.test.mjs index 78fd9a5a4..23c6760c4 100644 --- a/src/layers/streetLevel/index.test.mjs +++ b/src/layers/streetLevel/index.test.mjs @@ -171,6 +171,64 @@ test('coverage asks for no tiles before the key status is known, and none withou } }); +test('a key status that failed is asked again on the next switch-on', async (t) => { + let up = false; + let asked = 0; + const source = fakeMapillarySource({ + hasToken: () => false, + getStatus: async () => { + asked++; + if (!up) throw new TypeError('fetch failed'); + return { configured: true }; + }, + }); + const { layer, viewer } = await startLayer(t, { source, view: true }); + assert.equal(asked, 1, 'one check for init and the first switch-on'); + assert.equal(layer.getUIState().keyRequired, true); + up = true; // the server is reachable again + layer.disable(); + layer.enable(viewer); + await settle(); + assert.equal(asked, 2); + assert.equal(layer.getUIState().keyRequired, false); + assert.ok(source.calls.tiles.length > 0, 'coverage loads'); + // A known key is not asked again. + layer.disable(); + layer.enable(viewer); + await settle(); + assert.equal(asked, 2); +}); + +test('a key status that never answers gives up, so the next switch-on asks again', async (t) => { + // The check's timeout, fired when the test says. + const timeouts = []; + t.mock.method(AbortSignal, 'timeout', (ms) => { + const controller = new AbortController(); + timeouts.push({ ms, fire: () => controller.abort(new Error('timed out')) }); + return controller.signal; + }); + let asked = 0; + const source = fakeMapillarySource({ + hasToken: () => false, + getStatus: ({ signal } = {}) => + new Promise((_, reject) => { + asked++; + signal?.addEventListener('abort', () => reject(signal.reason)); + }), + }); + const { layer, viewer } = await startLayer(t, { source, view: true }); + assert.equal(asked, 1); + assert.equal(timeouts[0].ms, 10_000, 'the check can be cut short'); + assert.equal(layer.getUIState().coverage.loading, true, 'LOADING meanwhile'); + timeouts[0].fire(); + await settle(); + assert.equal(layer.getUIState().coverage.loading, false); + assert.equal(layer.getUIState().keyRequired, true, 'no token: KEY REQUIRED'); + layer.disable(); + layer.enable(viewer); + assert.equal(asked, 2, 'asked again, not stuck on the first'); +}); + test('a rejected key gates the layer as KEY REJECTED until it goes off', async (t) => { const rejected = Object.assign(new Error('Mapillary rejected the token'), { keyRejected: true, diff --git a/src/layers/streetLevel/providers/mapillary/coverage.js b/src/layers/streetLevel/providers/mapillary/coverage.js index 89a31812d..552bfcd84 100644 --- a/src/layers/streetLevel/providers/mapillary/coverage.js +++ b/src/layers/streetLevel/providers/mapillary/coverage.js @@ -407,7 +407,10 @@ export function createCoverage({ state, source }) { const { blocked } = state.coverage; return { count: sequenceCount(), - loading: state.coverage.pending.size > 0, + // Waiting for the key status is loading too, not an empty view. + loading: + state.coverage.pending.size > 0 || + (blocked === 'status' && isActive(state)), hint: state.coverage.hint, keyRequired: blocked === 'no-key' || blocked === 'rejected', keyRejected: blocked === 'rejected', diff --git a/src/layers/streetLevel/providers/mapillary/coverage.test.mjs b/src/layers/streetLevel/providers/mapillary/coverage.test.mjs index 14587059a..082f7f917 100644 --- a/src/layers/streetLevel/providers/mapillary/coverage.test.mjs +++ b/src/layers/streetLevel/providers/mapillary/coverage.test.mjs @@ -220,11 +220,16 @@ test('the key status gates coverage: nothing before it answers, nothing without state.coverage.blocked = 'status'; coverage.refresh(); assert.equal(source.calls.length, 0, 'nothing before the status answers'); + assert.equal(coverage.stats().loading, true, 'waiting reads as loading'); + state.enabled = false; + assert.equal(coverage.stats().loading, false, 'but not while off'); + state.enabled = true; coverage.setKeyStatus(configured); if (configured) assert.equal(source.calls.length, 1, 'coverage loads once it is known'); else { assert.equal(source.calls.length, 0, 'a key-less install never asks'); + assert.equal(coverage.stats().loading, false); assert.equal(coverage.stats().keyRequired, true); assert.equal(coverage.stats().keyRejected, false); coverage.unblock(); diff --git a/src/tooling/mapillaryProvider.test.mjs b/src/tooling/mapillaryProvider.test.mjs index ca0425742..ce894cede 100644 --- a/src/tooling/mapillaryProvider.test.mjs +++ b/src/tooling/mapillaryProvider.test.mjs @@ -405,7 +405,7 @@ test('identical concurrent tile requests share one upstream fetch', async () => test('at most a few upstream fetches run at once; the rest wait their turn', async () => { let release; - const gate = new Promise((resolve) => (release = resolve)); + let gate = new Promise((resolve) => (release = resolve)); await withUpstream( async () => (await gate, ok()), async ({ calls, call }) => { @@ -419,6 +419,17 @@ test('at most a few upstream fetches run at once; the rest wait their turn', asy const done = await Promise.all(pending); assert.ok(done.every((res) => res.statusCode === 200)); assert.equal(calls.length, tiles, 'the waiting ones ran too'); + // Every slot came back: a second burst runs as many at once again. + let reopen; + gate = new Promise((resolve) => (reopen = resolve)); + const again = Array.from({ length: tiles }, (_, i) => + call('/api/mapillary/tiles', `/coverage/14/${i}/4`), + ); + await new Promise((resolve) => setTimeout(resolve, 10)); + assert.equal(calls.length, tiles + TILE_UPSTREAM_CONCURRENCY); + reopen(); + await Promise.all(again); + assert.equal(calls.length, 2 * tiles); }, ); }); diff --git a/src/ui/streetLevelControls.js b/src/ui/streetLevelControls.js index 140ddef72..7fe42a1d3 100644 --- a/src/ui/streetLevelControls.js +++ b/src/ui/streetLevelControls.js @@ -118,18 +118,18 @@ export class StreetLevelControls { this.listen(el.viewerExpand, 'click', () => this._toggleExpanded()); if (this._expandMode === 'fullscreen') this.listen(document, 'fullscreenchange', () => this._syncExpanded()); - else { + else this.listen(el.viewerWrap, 'toggle', (event) => this._onPopoverToggle(event), ); - // Esc shrinks the photo before the panel's own Esc can collapse it. - this.listen(el.viewerWrap, 'keydown', (event) => { - if (event.key !== 'Escape' || !this.isViewerExpanded()) return; - event.preventDefault(); - event.stopPropagation(); - this._exitExpanded(); - }); - } + // Esc shrinks the photo before the panel's own Esc can collapse it, in + // a browser that hands the key to the page while full screen too. + this.listen(el.viewerWrap, 'keydown', (event) => { + if (event.key !== 'Escape' || !this.isViewerExpanded()) return; + event.preventDefault(); + event.stopPropagation(); + this._exitExpanded(); + }); } else setProp(el.viewerExpand, 'hidden', true); for (const button of el.renderButtons) { this.listen(button, 'click', () => { @@ -430,6 +430,10 @@ export class StreetLevelControls { if (this.destroyed) return; this.destroyed = true; this._exitExpanded(); + // Its `toggle` is not heard once destroyed: a popover left on the wrap + // would hide the viewer even inside the panel. + if (this._expandMode === 'popover') + this._elements.viewerWrap?.removeAttribute('popover'); this._watchPanelHidingModes(false); this.listeners.abort(); this._resizeObserver?.disconnect(); diff --git a/src/ui/streetLevelControls.test.mjs b/src/ui/streetLevelControls.test.mjs index 3d04a3bbc..fa1168b02 100644 --- a/src/ui/streetLevelControls.test.mjs +++ b/src/ui/streetLevelControls.test.mjs @@ -185,6 +185,11 @@ function panelDom({ fullscreen = true, popover = false } = {}) { const wrap = add(root, 'div', { id: 'sl-viewer-wrap' }); if (fullscreen) wrap.requestFullscreen = async () => fullscreenTo(wrap); if (popover) { + // `popover` reflects its attribute, as in the DOM. + Object.defineProperty(wrap, 'popover', { + get: () => wrap.getAttribute('popover'), + set: (value) => wrap.setAttribute('popover', value), + }); // The browser fires `toggle` after the popover opens or closes. const toggleTo = (newState) => setTimeout(() => wrap.dispatchEvent({ type: 'toggle', newState }), 0); @@ -559,6 +564,28 @@ test('EXPAND puts the viewer full screen in place, and the button shrinks it aga controls.destroy(); })); +test('Esc in full screen shrinks the photo; the panel never sees it', () => + withDom(async (dom) => { + const { controls } = stubPanel(dom, uiState({ open: true })); + const expand = dom.root.querySelector('#sl-viewer-expand'); + expand.click(); + await settle(); + await settle(); + assert.equal(dom.document.fullscreenElement, dom.wrap); + let panelEsc = 0; + dom.root.addEventListener('keydown', () => panelEsc++); + expand.dispatchEvent({ type: 'keydown', key: 'Escape', bubbles: true }); + await settle(); + await settle(); + assert.equal(dom.document.fullscreenElement, null); + assert.equal(panelEsc, 0, 'the panel stays open'); + assert.deepEqual(expandLabel(dom), ['EXPAND', 'false']); + // Not expanded, Esc is the panel's again. + expand.dispatchEvent({ type: 'keydown', key: 'Escape', bubbles: true }); + assert.equal(panelEsc, 1); + controls.destroy(); + })); + test('an image closed while full screen leaves full screen', () => withDom(async (dom) => { const { layer, controls } = stubPanel(dom, uiState({ open: true })); @@ -622,6 +649,26 @@ test('on iPhone (no element fullscreen) EXPAND opens the viewer as a popover ove { fullscreen: false, popover: true }, )); +test('destroying the panel while the iPhone popover is open leaves no popover behind', () => + withDom( + async (dom) => { + const { controls } = stubPanel(dom, uiState({ open: true })); + dom.root.querySelector('#sl-viewer-expand').click(); + await settle(); + await settle(); + assert.equal(dom.wrap.getAttribute('popover'), 'auto'); + controls.destroy(); + await settle(); + assert.equal(dom.wrap.popoverOpen, false); + assert.equal( + dom.wrap.getAttribute('popover'), + null, + 'its toggle is not heard any more', + ); + }, + { fullscreen: false, popover: true }, + )); + /** A MutationObserver stand-in whose callback the test fires. */ function fakeMutationObserver() { const observers = new Set(); diff --git a/src/ui/streetLevelPresentation.js b/src/ui/streetLevelPresentation.js index 63640d1f4..0ada603f8 100644 --- a/src/ui/streetLevelPresentation.js +++ b/src/ui/streetLevelPresentation.js @@ -103,7 +103,14 @@ function presentMeta(state) { : `${state.sequence.images.toLocaleString()} images in this sequence · Esc clears`; if (state.coverage.count > 0) return `${state.coverage.count.toLocaleString()} sequences in view · click a line for its photos`; - return state.coverage.hint || ''; + if (state.coverage.hint) return state.coverage.hint; + // Still loading, or the error line already says why. + if (state.coverage.loading || state.keyRequired || state.coverage.error) + return ''; + const filter = state.filter || {}; + return (filter.pano || 'all') !== 'all' || Number(filter.sinceDays) > 0 + ? 'No sequences in view match the filter' + : 'No Mapillary coverage in view'; } /** @param {{now?: number}} [options] Clock for the SINCE readout. */ diff --git a/src/ui/streetLevelPresentation.test.mjs b/src/ui/streetLevelPresentation.test.mjs index 69b6a784f..0b4f28631 100644 --- a/src/ui/streetLevelPresentation.test.mjs +++ b/src/ui/streetLevelPresentation.test.mjs @@ -201,6 +201,24 @@ test('the meta line never mixes the visible-sequence count with the selected seq assert.equal(hinted.meta, 'Point the camera at the globe'); }); +test('an empty view says so, and whether the filter emptied it', () => { + const meta = (overrides) => + presentStreetLevelPanel(snapshot({ enabled: true, ...overrides })).meta; + assert.equal(meta({}), 'No Mapillary coverage in view'); + assert.equal( + meta({ filter: { pano: 'flat', sinceDays: 0 } }), + 'No sequences in view match the filter', + ); + assert.equal( + meta({ filter: { pano: 'all', sinceDays: 365 } }), + 'No sequences in view match the filter', + ); + // Nothing to claim while it loads, or when the error line explains it. + assert.equal(meta({ coverage: { loading: true } }), ''); + assert.equal(meta({ keyRequired: true }), ''); + assert.equal(meta({ coverage: { error: 'Tile HTTP 502' } }), ''); +}); + test('viewer caption reads "Image by" left, date right, and links to Mapillary', () => { const view = presentStreetLevelPanel( snapshot({