From 43216a37e82e380f89fbc8dc15af97407b51db4c Mon Sep 17 00:00:00 2001 From: Otto Wagner <48657113+isConic@users.noreply.github.com> Date: Wed, 7 Oct 2026 09:48:24 -0500 Subject: [PATCH] fix: version viewer assets to avoid stale inspector scripts --- server/mapcontrol_server/main.py | 29 ++- .../esip-contract-before-inspector.js | 228 ++++++++++++++++++ server/tests/test_inspector_browser.py | 139 +++++++++-- server/tests/test_mcp.py | 4 +- server/tests/test_ui_asset_cache.py | 60 +++++ 5 files changed, 428 insertions(+), 32 deletions(-) create mode 100644 server/tests/fixtures/esip-contract-before-inspector.js create mode 100644 server/tests/test_ui_asset_cache.py diff --git a/server/mapcontrol_server/main.py b/server/mapcontrol_server/main.py index 0a450b3..1397f6f 100644 --- a/server/mapcontrol_server/main.py +++ b/server/mapcontrol_server/main.py @@ -2,6 +2,7 @@ from __future__ import annotations +import hashlib import json import logging import os @@ -11,6 +12,19 @@ _SERVER_START_TIME = time.time() +STATIC_DIR = Path(__file__).parent / "static" + + +def _ui_asset_url(root_path: str, filename: str) -> str: + """Keep a browser's UI scripts/styles in sync with the served map shell. + + Old unversioned URLs can remain fresh in a browser cache across deployments. + A content hash changes the URL when the file changes; unchanged builds keep + their cache hits. These small files are read when serving the HTML, so local + static edits also receive a new version without requiring a process restart. + """ + digest = hashlib.sha256((STATIC_DIR / filename).read_bytes()).hexdigest()[:16] + return f"{root_path}/static/{filename}?v={digest}" import uvicorn from fastapi import FastAPI, WebSocket, WebSocketDisconnect, Request @@ -439,15 +453,15 @@ def _thumbnail_url(entry) -> str: # and publishes window.ESIPMap for bespoke UIs. The default outfit # (esip-embed.css/js) is loaded only when ui == "default". ui_head = ( - f'' + f'' if ui == "default" else "" ) - ui_body = f'' + ui_body = f'' if inspect_enabled: - ui_head += f'' - ui_body += f'' + ui_head += f'' + ui_body += f'' if ui == "default": - ui_body += f'' + ui_body += f'' # Initial camera resolution (server-side). The page used to ALWAYS boot at @@ -3371,7 +3385,9 @@ class DrawToolbarControl {{ """ - return HTMLResponse(content=html) + # The shell selects content-versioned UI assets. Revalidate it on navigation + # so an existing browser also picks up the next deployment's asset URLs. + return HTMLResponse(content=html, headers={"Cache-Control": "no-cache"}) # ─── Screenshot endpoint ───────────────────────────────────────────────────── @@ -4116,7 +4132,6 @@ async def terrain_demo_page(request: Request): # Serve the ESIP default-UI static assets (interaction contract + default # outfit). esip-contract.js is always injected into the served map; # esip-embed.{css,js} only when ?ui=default (or config.map.default_ui). -STATIC_DIR = Path(__file__).parent / "static" if STATIC_DIR.exists(): app.mount("/static", StaticFiles(directory=str(STATIC_DIR)), name="esip-static") diff --git a/server/tests/fixtures/esip-contract-before-inspector.js b/server/tests/fixtures/esip-contract-before-inspector.js new file mode 100644 index 0000000..d999a3b --- /dev/null +++ b/server/tests/fixtures/esip-contract-before-inspector.js @@ -0,0 +1,228 @@ +/* ───────────────────────────────────────────────────────────────────────── + ESIP Interaction Contract — ALWAYS loaded (naked AND dressed). + + This is the genuinely-missing primitive the architecture doc specced but + never implemented: the map emitting `asset_hover` / `asset_hover_end` / + `asset_click` / `map_click` so embedding apps (or the optional default + outfit) can react. It loads in EVERY mode — a naked map fires these events + into the void; the default outfit (esip-embed.js) is just one listener. + + It also publishes `window.ESIPMap`: the public, in-page command surface a + vibe-coder builds their own UI against. Crucially, the action methods here + do NOT poke MapLibre directly — they POST to the public REST events + endpoint, exactly like the Python SDK does. So the built-in UI and a + third-party UI drive the map through the identical contract. If this file + needs something it can't get from `window.__esipInternals` (published by + the inline shell) or REST, that's a contract hole worth fixing. + + Dual delivery, per the architecture spec: + 1. window CustomEvents (`esip:*`) → same-document listeners (the outfit) + 2. window.parent.postMessage(...) → iframe-embedding parents (Svelte) + ───────────────────────────────────────────────────────────────────────── */ +(function () { + "use strict"; + + function start(internals) { + var map = internals.map; + var registry = internals.registry; // asset_id -> { layerIds, visible, srcId, ... } + var basemaps = internals.basemaps; + var handlers = internals.handlers; + var mapId = internals.mapId; + var baseUrl = internals.baseUrl; + var getUserSession = internals.getUserSession; + + // ─── Dual-channel emit ──────────────────────────────────────────────── + function emit(type, detail) { + try { + window.dispatchEvent(new CustomEvent("esip:" + type, { detail: detail })); + } catch (e) { /* no CustomEvent support */ } + try { + window.parent.postMessage({ + source: "mapcontrol", + map_id: mapId, + user_session_id: getUserSession(), + type: type, + data: detail, + }, "*"); + } catch (e) { /* cross-origin parent — ignore */ } + } + + // ─── Which rendered layers belong to user assets ────────────────────── + function assetLayerIds() { + var ids = []; + for (var aid in registry) { + if (!registry.hasOwnProperty(aid)) continue; + var reg = registry[aid]; + if (!reg || !reg.layerIds) continue; + for (var i = 0; i < reg.layerIds.length; i++) { + if (map.getLayer(reg.layerIds[i])) ids.push(reg.layerIds[i]); + } + } + return ids; + } + function assetForLayer(layerId) { + for (var aid in registry) { + if (!registry.hasOwnProperty(aid)) continue; + var reg = registry[aid]; + if (reg && reg.layerIds && reg.layerIds.indexOf(layerId) !== -1) return aid; + } + return null; + } + + // ─── Hover (peek) ───────────────────────────────────────────────────── + // The hit-test runs only after the cursor DWELLS for a beat, never on + // raw mousemove. queryRenderedFeatures over run-sized vector results + // costs ~14ms per call (measured); at 60-120 mousemoves/sec that alone + // saturated the main thread the moment the mouse moved, crawling the + // draw tools and everything else in the iframe. Hover cards are a + // dwell interaction — peek when the pointer settles, stay silent while + // it travels. Also fully suppressed while a draw/delete mode is active + // (a hover card mid-draw is noise, and draws are mousemove-heavy). + var hoverId = null; + var hoverTimer = null; + var HOVER_DWELL_MS = 90; + function clearHover() { + if (hoverId) { + emit("asset_hover_end", { asset_id: hoverId }); + hoverId = null; + map.getCanvas().style.cursor = ""; + } + } + function hoverProbe(point, lngLat) { + if (typeof internals.isDrawing === "function" && internals.isDrawing()) { + clearHover(); + return; + } + var ids = assetLayerIds(); + var feats = ids.length ? map.queryRenderedFeatures(point, { layers: ids }) : []; + if (feats.length === 0) { + clearHover(); + return; + } + var aid = assetForLayer(feats[0].layer.id); + if (!aid) return; + if (aid !== hoverId) { + if (hoverId) emit("asset_hover_end", { asset_id: hoverId }); + hoverId = aid; + map.getCanvas().style.cursor = "pointer"; + var reg = registry[aid] || {}; + emit("asset_hover", { + asset_id: aid, + name: reg.name || null, + asset_type: reg.asset_type || null, + point: { x: point.x, y: point.y }, + lngLat: [lngLat.lng, lngLat.lat], + }); + } + } + map.on("mousemove", function (e) { + if (hoverTimer) clearTimeout(hoverTimer); + var point = e.point; + var lngLat = e.lngLat; + hoverTimer = setTimeout(function () { hoverProbe(point, lngLat); }, HOVER_DWELL_MS); + }); + map.on("mouseout", function () { + if (hoverTimer) clearTimeout(hoverTimer); + clearHover(); + }); + + // ─── Click (commit) ─────────────────────────────────────────────────── + // Additive: this does not interfere with the map's own delete-mode click + // handler; it only emits an event the embedder may act on. + map.on("click", function (e) { + var ids = assetLayerIds(); + var feats = ids.length ? map.queryRenderedFeatures(e.point, { layers: ids }) : []; + if (feats.length) { + var aid = assetForLayer(feats[0].layer.id); + if (aid) { + emit("asset_click", { + asset_id: aid, + point: { x: e.point.x, y: e.point.y }, + lngLat: [e.lngLat.lng, e.lngLat.lat], + }); + return; + } + } + emit("map_click", { lon: e.lngLat.lng, lat: e.lngLat.lat }); + }); + + // ─── Track visibility + signal asset-set changes ────────────────────── + // We wrap the shell's existing event handlers so the contract can keep an + // accurate per-session `visible` flag in the registry, and so any + // listener (the outfit) is told when the layer set changes. We never + // bypass the handlers — we decorate them. + function decorate(name, after) { + var orig = handlers[name]; + if (typeof orig !== "function") return; + handlers[name] = function (data) { + var r = orig.apply(this, arguments); + try { after(data); } catch (e) { /* keep map resilient */ } + return r; + }; + } + var CHANGE_EVENTS = [ + "add_polygon", "add_polygon_url", "add_path", "add_path_url", "add_point", + "add_arc", + "add_geotiff_rgb", "add_geotiff_singleband", "add_tile_layer", + "add_drawn_polygon", "delete_asset", "remove_tile_layer", + "update_style", "set_opacity", "move_layer", + ]; + CHANGE_EVENTS.forEach(function (name) { + decorate(name, function () { + window.dispatchEvent(new CustomEvent("esip:assetschanged")); + }); + }); + decorate("set_visibility", function (data) { + var reg = registry[data.asset_id]; + if (reg) reg.visible = data.visible !== false; + window.dispatchEvent(new CustomEvent("esip:assetschanged")); + }); + + // ─── REST command surface (dogfoods the public events API) ──────────── + function postEvent(type, data) { + return fetch(baseUrl + "/api/maps/" + mapId + "/events", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ type: type, data: data, user_session_id: getUserSession() }), + }); + } + + window.ESIPMap = { + config: { + mapId: mapId, + userSession: getUserSession(), + baseUrl: baseUrl, + theme: document.documentElement.getAttribute("data-theme") || "light", + }, + getBasemaps: function () { return basemaps || {}; }, + getCurrentBasemap: internals.getCurrentBasemap, + getRegistry: function () { return registry; }, + actions: { + setVisibility: function (assetId, visible) { return postEvent("set_visibility", { asset_id: assetId, visible: visible }); }, + deleteAsset: function (assetId) { return postEvent("delete_asset", { asset_id: assetId }); }, + setOpacity: function (assetId, opacity01) { return postEvent("set_opacity", { asset_id: assetId, opacity: opacity01 }); }, + moveLayer: function (assetId, position) { return postEvent("move_layer", { asset_id: assetId, position: position }); }, + zoomToAssets: function (assetIds) { return postEvent("zoom_to_assets", { asset_ids: assetIds }); }, + setBasemap: function (id) { return postEvent("set_basemap", { basemap: id }); }, + emit: emit, // exposed so bespoke UIs can re-broadcast if useful + }, + }; + + // Announce the contract is live. The default outfit (and any bespoke + // consumer) can boot off this. + window.dispatchEvent(new CustomEvent("esip:ready")); + console.log("[ESIP contract] interaction events live (mode:", + document.documentElement.getAttribute("data-esip-ui") || "?", ")"); + } + + // The inline shell publishes internals synchronously at end of its script, + // then fires 'esip:internals-ready'. We may attach before or after that, so + // handle both orders. + if (window.__esipInternals) { + start(window.__esipInternals); + } else { + window.addEventListener("esip:internals-ready", function () { + start(window.__esipInternals); + }, { once: true }); + } +})(); diff --git a/server/tests/test_inspector_browser.py b/server/tests/test_inspector_browser.py index 4e425ec..8e2c71b 100644 --- a/server/tests/test_inspector_browser.py +++ b/server/tests/test_inspector_browser.py @@ -4,34 +4,17 @@ optional browser is absent, so the API suite remains usable without browsers. """ from pathlib import Path +from collections import Counter +from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer +from threading import Thread +from urllib.parse import urlsplit import pytest STATIC = Path(__file__).parents[1] / "mapcontrol_server" / "static" -@pytest.fixture(scope="module") -def browser(): - playwright = pytest.importorskip("playwright.sync_api") - with playwright.sync_playwright() as p: - try: - instance = p.chromium.launch(headless=True) - except playwright.Error as exc: - if "Executable doesn't exist" in str(exc): - pytest.skip("Chromium is not installed; run playwright install chromium") - raise - yield instance - instance.close() - - -@pytest.fixture -def page(browser): - page = browser.new_page(viewport={"width": 900, "height": 650}) - page.route("http://fixture.test/**", lambda route: route.fulfill( - content_type="text/html", body="" - )) - page.set_content('
') - page.evaluate("""() => { +MAP_SETUP_JS = """() => { const container = document.querySelector('#map'); const callbacks = {}; window.drawing = false; @@ -61,7 +44,31 @@ def page(browser): } }; window.clickAsset = id => { window.hitAsset = id; (callbacks.click || []).forEach(fn => fn({ point: { x: 60, y: 90 }, lngLat: { lng: 8, lat: 9 } })); }; window.addEventListener('esip:asset_click', e => window.clickEvents.push(e.detail)); - }""") + }""" + + +@pytest.fixture(scope="module") +def browser(): + playwright = pytest.importorskip("playwright.sync_api") + with playwright.sync_playwright() as p: + try: + instance = p.chromium.launch(headless=True) + except playwright.Error as exc: + if "Executable doesn't exist" in str(exc): + pytest.skip("Chromium is not installed; run playwright install chromium") + raise + yield instance + instance.close() + + +@pytest.fixture +def page(browser): + page = browser.new_page(viewport={"width": 900, "height": 650}) + page.route("http://fixture.test/**", lambda route: route.fulfill( + content_type="text/html", body="" + )) + page.set_content('
') + page.evaluate(MAP_SETUP_JS) page.add_script_tag(path=str(STATIC / "esip-contract.js")) page.add_script_tag(path=str(STATIC / "esip-inspector.js")) yield page @@ -154,3 +161,89 @@ def test_mobile_sheet_overrides_maplibre_top_position(page): assert bounds["y"] + bounds["height"] == pytest.approx(742, abs=1) page.get_by_role("button", name="Expand", exact=True).click() assert popup.bounding_box()["y"] == pytest.approx(8, abs=1) + + +def test_versioned_contract_recovers_inspector_with_warm_browser_cache(browser): + """Reproduce an upgrade in one browser context, without disabling its cache. + + The fixture is the actual contract from 45a2dcf, before inspector support. + A real HTTP server (not Playwright routing, which disables the cache) + serves it as fresh cached content, then deploys the current UI. Only + content-versioned URLs recover the inspector on an ordinary navigation. + """ + from mapcontrol_server.main import _ui_asset_url + + previous = (Path(__file__).parent / "fixtures" / "esip-contract-before-inspector.js").read_bytes() + requests = Counter() + deployed = False + + class Handler(BaseHTTPRequestHandler): + def do_GET(self): + requests[self.path] += 1 + path = urlsplit(self.path).path + if path.startswith("/static/"): + filename = path.rsplit("/", 1)[1] + payload = previous if filename == "esip-contract.js" and not deployed else (STATIC / filename).read_bytes() + content_type = "text/css" if filename.endswith(".css") else "text/javascript" + cache_control = "public, max-age=31536000, immutable" + elif path in ("/before", "/after-bare", "/after-versioned"): + versioned = path == "/after-versioned" + asset_url = lambda filename: _ui_asset_url("", filename) if versioned else f"/static/{filename}" + inspector = "" if path == "/before" else ( + f'' + f'' + ) + payload = ( + '
' + f'' + f'{inspector}' + ).encode() + content_type = "text/html" + cache_control = "no-cache" + else: + self.send_error(404) + return + self.send_response(200) + self.send_header("Content-Type", content_type) + self.send_header("Cache-Control", cache_control) + self.send_header("Content-Length", str(len(payload))) + self.end_headers() + self.wfile.write(payload) + + def log_message(self, *_args): + pass + + server = ThreadingHTTPServer(("127.0.0.1", 0), Handler) + thread = Thread(target=server.serve_forever, daemon=True) + thread.start() + context = browser.new_context() + page = context.new_page() + errors = [] + page.on("pageerror", lambda error: errors.append(str(error))) + base_url = f"http://127.0.0.1:{server.server_port}" + try: + page.goto(base_url + "/before") + assert page.evaluate("typeof ESIPMap.getAssetInfo") == "undefined" + assert requests["/static/esip-contract.js"] == 1 + + deployed = True + page.goto(base_url + "/after-bare") + page.evaluate("clickAsset('a')") + assert page.evaluate("clickEvents.at(-1).asset_id") == "a" + assert page.evaluate("clickEvents.at(-1).inspector_handled") is None + assert page.locator(".esip-inspector-popup").count() == 0 + assert requests["/static/esip-contract.js"] == 1 # Still the old browser cache entry. + + page.goto(base_url + "/after-versioned") + page.evaluate("clickAsset('a')") + assert page.evaluate("clickEvents.at(-1).inspector_handled") is True + assert page.locator(".esip-inspector-popup").count() == 1 + assert page.get_by_role("heading", name="Station A", exact=True).count() == 1 + assert requests[_ui_asset_url("", "esip-contract.js")] == 1 + assert requests["/static/esip-contract.js"] == 1 + assert not errors + finally: + context.close() + server.shutdown() + server.server_close() + thread.join() diff --git a/server/tests/test_mcp.py b/server/tests/test_mcp.py index 48e343a..1da5f98 100644 --- a/server/tests/test_mcp.py +++ b/server/tests/test_mcp.py @@ -591,7 +591,7 @@ def test_served_map_no_root_path(): # WS built from window.location.host with an empty prefix assert "window.location.host + ROOT_PATH + '/ws/'" in html # esip-contract.js is ALWAYS injected; at root it has no prefix - assert '"/static/esip-contract.js"' in html + assert '"/static/esip-contract.js?v=' in html # base_url unprefixed → REST/screenshot URLs resolve at root assert 'const BASE_URL = "http://testserver";' in html @@ -612,7 +612,7 @@ def test_served_map_with_root_path(): assert f'const ROOT_PATH = "{prefix}";' in html assert "window.location.host + ROOT_PATH + '/ws/'" in html # static asset tags carry the prefix - assert f'"{prefix}/static/esip-contract.js"' in html + assert f'"{prefix}/static/esip-contract.js?v=' in html # base_url (and thus every BASE_URL-derived fetch) is auto-prefixed assert f'const BASE_URL = "http://testserver{prefix}";' in html diff --git a/server/tests/test_ui_asset_cache.py b/server/tests/test_ui_asset_cache.py new file mode 100644 index 0000000..b8255d8 --- /dev/null +++ b/server/tests/test_ui_asset_cache.py @@ -0,0 +1,60 @@ +"""Map shells must not mix cached UI code from different deployments.""" + +import hashlib +import re +from urllib.parse import parse_qs, urlsplit + +import pytest +from httpx import ASGITransport, AsyncClient + +from mapcontrol_server import main + + +@pytest.mark.parametrize("root_path", ["", "/services/maps"]) +@pytest.mark.parametrize("query,filenames", [ + ("ui=default", {"esip-contract.js", "esip-embed.js", "esip-embed.css", "esip-inspector.js", "esip-inspector.css"}), + ("ui=controls&inspect=1", {"esip-contract.js", "esip-inspector.js", "esip-inspector.css"}), + ("ui=none", {"esip-contract.js"}), + ("ui=default&inspect=0", {"esip-contract.js", "esip-embed.js", "esip-embed.css"}), +]) +async def test_map_shell_versions_all_owned_assets(tmp_path, monkeypatch, root_path, query, filenames): + monkeypatch.setenv("MAPCONTROL_DB_PATH", str(tmp_path / "test.db")) + monkeypatch.setenv("MAPCONTROL_FILE_DIR", str(tmp_path / "files")) + # Only the database is needed for map HTML/static routes; do not start the + # unrelated single-use MCP session manager for every parameter combination. + config = main.load_config() + monkeypatch.setattr(main, "_config", config) + await main.init_db(config) + try: + transport = ASGITransport(app=main.app, root_path=root_path) + async with AsyncClient(transport=transport, base_url="http://test") as client: + created = await client.post("/api/maps") + map_id = created.json()["map_id"] + response = await client.get(f"/map/{map_id}?{query}") + assert response.status_code == 200 + assert response.headers["cache-control"] == "no-cache" + urls = re.findall(r'(?:src|href)="([^"]*/static/esip-[^"]+)"', response.text) + assert {urlsplit(url).path.rsplit("/", 1)[1] for url in urls} == filenames + assert len(urls) == len(filenames) + for url in urls: + parsed = urlsplit(url) + filename = parsed.path.rsplit("/", 1)[1] + assert parsed.path == f"{root_path}/static/{filename}" + expected = hashlib.sha256((main.STATIC_DIR / filename).read_bytes()).hexdigest()[:16] + assert parse_qs(parsed.query) == {"v": [expected]} + # The version is a cache key, not a different static route. + asset = await client.get(url) + assert asset.status_code == 200 + assert asset.content == (main.STATIC_DIR / filename).read_bytes() + finally: + await main.close_db() + + +def test_asset_version_changes_only_when_content_changes(tmp_path, monkeypatch): + monkeypatch.setattr(main, "STATIC_DIR", tmp_path) + source = tmp_path / "esip-contract.js" + source.write_text("previous contract") + before = main._ui_asset_url("/maps", source.name) + assert main._ui_asset_url("/maps", source.name) == before + source.write_text("current contract") + assert main._ui_asset_url("/maps", source.name) != before