From a8da821c82b1c527585e8eeac0956b51f5c3aef0 Mon Sep 17 00:00:00 2001 From: Corey Krewson Date: Mon, 24 Aug 2026 15:07:51 -0700 Subject: [PATCH 01/18] feat(map): register coordinate reference systems layers name by code Adds a projection table for CRSes OpenLayers cannot resolve on its own, registered at module evaluation so it is in place before layers construct. Layers are built concurrently, so a registration that waited on anything async would race them. Registration is scoped deliberately. `register` builds pairwise transforms across every registered code, so its cost is quadratic -- 99ms for two definitions on top of proj4's built-ins, and hundreds of milliseconds at a State-Plane-sized set. This module is a static import, so an unbounded init set would be a first-render regression on every dashboard. Only codes a layer actually names are registered up front; `ensureProjection` handles the rest on demand. Definitions, extents and control points come from PROJ's EPSG database rather than being hand-derived, and the control points sit away from each projection's origin so they exercise the standard parallels and scale factor. proj4 agrees with PROJ on both to sub-millimetre, which makes the round-trip test a cross-implementation check rather than a self-consistency one. Three behaviors worth naming, each found by measurement: - `register` cannot supply an extent, so extents are applied afterward. - A definition is validated before being registered, not after: `register` constructs a transform for every pair of registered codes, so one unusable definition makes it throw and takes working projections down with it. The probe uses the definition's own centre, since a fixed point is outside many projections' domains. - proj4 implements the ESRI spelling of Albers but not the OGC one, which fails silently with non-finite coordinates. Now reported rather than rendering features nowhere. A layer's own WKT never overwrites a definition that already resolves. The registry is global to the browser session, so letting one layer's parameters replace a code others resolve through would make rendering depend on which dashboard was opened first. Unresolvable WKT registers under a synthetic code, never under a claimed authority code. The raster auto-fit will not adopt a registered-but-not-native projection as the view projection. Adoption calls setView and publishes the adopted code into the map-extent variable other visualizations read; widening it is a separate change with its own verification. Such a raster still renders, by reprojection. Dependencies for the whole feature land here rather than across three commits: proj4, shapefile, fflate, @mapbox/geojson-rewind, wkt-parser. All exact-pinned; fflate matches the version already resolved transitively to avoid a duplicate install. wkt-parser is declared directly because proj4 does not re-export its parser and the outermost AUTHORITY node is needed. Co-Authored-By: Claude Opus 5 (1M context) --- package-lock.json | 112 +++++++- package.json | 7 +- reactapp/__tests__/components/map/Map.test.js | 98 ++++++- .../components/map/projections.test.js | 196 +++++++++++++ reactapp/components/map/Map.js | 26 +- reactapp/components/map/projections.js | 264 ++++++++++++++++++ 6 files changed, 692 insertions(+), 11 deletions(-) create mode 100644 reactapp/__tests__/components/map/projections.test.js create mode 100644 reactapp/components/map/projections.js diff --git a/package-lock.json b/package-lock.json index 28f92993..de5a1b20 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9,6 +9,7 @@ "version": "0.16.10", "license": "ISC", "dependencies": { + "@mapbox/geojson-rewind": "0.5.2", "@mapbox/vector-tile": "^1.3.1", "@tiptap/extension-color": "^2.12.0", "@tiptap/extension-font-family": "^2.12.0", @@ -32,6 +33,7 @@ "date-fns": "^4.1.0", "dompurify": "^3.1.6", "dotenv": "^16.0.1", + "fflate": "0.8.2", "file-loader": "^6.2.0", "geotiff": "2.1.3", "html-react-parser": "^5.1.18", @@ -42,6 +44,7 @@ "ol-pmtiles": "^2.0.2", "plotly.js-strict-dist-min": "^2.35.2", "prismjs": "^1.28.0", + "proj4": "2.21.0", "rc-slider": "^11.1.8", "react": "^18.3.1", "react-bootstrap": "^2.10.2", @@ -62,13 +65,15 @@ "react-use-websocket": "^4.13.0", "sass": "^1.49.0", "sass-loader": "^12.3.0", + "shapefile": "0.6.6", "simple-xml-to-json": "^1.2.3", "style-loader": "^3.3.1", "styled-components": "^6.3.9", "swiper": "^11.2.1", "uuid": "^8.3.2", "webpack": "^5.64.4", - "webpack-dev-server": "^4.6.0" + "webpack-dev-server": "^4.6.0", + "wkt-parser": "1.5.6" }, "devDependencies": { "@babel/core": "^7.18.2", @@ -3579,7 +3584,6 @@ "resolved": "https://registry.npmjs.org/@mapbox/geojson-rewind/-/geojson-rewind-0.5.2.tgz", "integrity": "sha512-tJaT+RbYGJYStt7wI3cq4Nl4SXxG8W7JDG5DMJu97V25RnbNg3QtQtf+KD+VLjNpWKYsRvXDNmNrBgEETr1ifA==", "license": "ISC", - "peer": true, "dependencies": { "get-stream": "^6.0.1", "minimist": "^1.2.6" @@ -7027,6 +7031,12 @@ "license": "MIT", "peer": true }, + "node_modules/array-source": { + "version": "0.0.4", + "resolved": "https://registry.npmjs.org/array-source/-/array-source-0.0.4.tgz", + "integrity": "sha512-frNdc+zBn80vipY+GdcJkLEbMWj3xmzArYApmUGxoiV8uAu/ygcs9icPdsGdA26h0MkHUMW6EN2piIvVx+M5Mw==", + "license": "BSD-3-Clause" + }, "node_modules/array-union": { "version": "2.1.0", "resolved": "https://registry.npmjs.org/array-union/-/array-union-2.1.0.tgz", @@ -11503,6 +11513,15 @@ "url": "https://opencollective.com/webpack" } }, + "node_modules/file-source": { + "version": "0.6.1", + "resolved": "https://registry.npmjs.org/file-source/-/file-source-0.6.1.tgz", + "integrity": "sha512-1R1KneL7eTXmXfKxC10V/9NeGOdbsAXJ+lQ//fvvcHUgtaZcZDWNJNblxAoVOyV1cj45pOtUrR3vZTBwqcW8XA==", + "license": "BSD-3-Clause", + "dependencies": { + "stream-source": "0.3" + } + }, "node_modules/fill-range": { "version": "7.1.1", "resolved": "https://registry.npmjs.org/fill-range/-/fill-range-7.1.1.tgz", @@ -16809,6 +16828,12 @@ "node": ">= 0.6" } }, + "node_modules/mgrs": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/mgrs/-/mgrs-1.0.0.tgz", + "integrity": "sha512-awNbTOqCxK1DBGjalK3xqWIstBZgN6fxsMSiXLs9/spqWkF2pAhb2rrYCFSsr1/tT7PhcDGjZndG8SWYn0byYA==", + "license": "MIT" + }, "node_modules/micromatch": { "version": "4.0.8", "resolved": "https://registry.npmjs.org/micromatch/-/micromatch-4.0.8.tgz", @@ -17895,6 +17920,16 @@ "integrity": "sha512-LDJzPVEEEPR+y48z93A0Ed0yXb8pAByGWo/k5YYdYgpY2/2EsOsksJrq7lOHxryrVOn1ejG6oAp8ahvOIQD8sw==", "license": "MIT" }, + "node_modules/path-source": { + "version": "0.1.3", + "resolved": "https://registry.npmjs.org/path-source/-/path-source-0.1.3.tgz", + "integrity": "sha512-dWRHm5mIw5kw0cs3QZLNmpUWty48f5+5v9nWD2dw3Y0Hf+s01Ag8iJEWV0Sm0kocE8kK27DrIowha03e1YR+Qw==", + "license": "BSD-3-Clause", + "dependencies": { + "array-source": "0.0", + "file-source": "0.6" + } + }, "node_modules/path-to-regexp": { "version": "6.3.0", "resolved": "https://registry.npmjs.org/path-to-regexp/-/path-to-regexp-6.3.0.tgz", @@ -18380,6 +18415,27 @@ "integrity": "sha512-3ouUOpQhtgrbOa17J7+uxOTpITYWaGP7/AhoR3+A+/1e9skrzelGi/dXzEYyvbxubEF6Wn2ypscTKiKJFFn1ag==", "license": "MIT" }, + "node_modules/proj4": { + "version": "2.21.0", + "resolved": "https://registry.npmjs.org/proj4/-/proj4-2.21.0.tgz", + "integrity": "sha512-33HfDftqw8kY+Cl1dcL16SJuqTSzYxmz4re7Nmhk+fs1/N1fFrAkkF579msQTR/4fLeJVSNne8/gpoCz0fk1uw==", + "license": "MIT", + "dependencies": { + "mgrs": "1.0.0", + "wkt-parser": "^1.5.5" + }, + "funding": { + "url": "https://github.com/sponsors/ahocevar" + }, + "peerDependencies": { + "geotiff": "*" + }, + "peerDependenciesMeta": { + "geotiff": { + "optional": true + } + } + }, "node_modules/promise": { "version": "8.3.0", "resolved": "https://registry.npmjs.org/promise/-/promise-8.3.0.tgz", @@ -20399,6 +20455,30 @@ "license": "MIT", "peer": true }, + "node_modules/shapefile": { + "version": "0.6.6", + "resolved": "https://registry.npmjs.org/shapefile/-/shapefile-0.6.6.tgz", + "integrity": "sha512-rLGSWeK2ufzCVx05wYd+xrWnOOdSV7xNUW5/XFgx3Bc02hBkpMlrd2F1dDII7/jhWzv0MSyBFh5uJIy9hLdfuw==", + "license": "BSD-3-Clause", + "dependencies": { + "array-source": "0.0", + "commander": "2", + "path-source": "0.1", + "slice-source": "0.4", + "stream-source": "0.3", + "text-encoding": "^0.6.4" + }, + "bin": { + "dbf2json": "bin/dbf2json", + "shp2json": "bin/shp2json" + } + }, + "node_modules/shapefile/node_modules/commander": { + "version": "2.20.3", + "resolved": "https://registry.npmjs.org/commander/-/commander-2.20.3.tgz", + "integrity": "sha512-GpVkmM8vF2vQUkj2LvZmD35JxeJOLCwJ9cUkugyk2nuhbv3+mJvpLYYt+0+USMxE+oj+ey/lJEnhZw75x/OMcQ==", + "license": "MIT" + }, "node_modules/shebang-command": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/shebang-command/-/shebang-command-2.0.0.tgz", @@ -20558,6 +20638,12 @@ "node": ">=8" } }, + "node_modules/slice-source": { + "version": "0.4.1", + "resolved": "https://registry.npmjs.org/slice-source/-/slice-source-0.4.1.tgz", + "integrity": "sha512-YiuPbxpCj4hD9Qs06hGAz/OZhQ0eDuALN0lRWJez0eD/RevzKqGdUx1IOMUnXgpr+sXZLq3g8ERwbAH0bCb8vg==", + "license": "BSD-3-Clause" + }, "node_modules/sockjs": { "version": "0.3.24", "resolved": "https://registry.npmjs.org/sockjs/-/sockjs-0.3.24.tgz", @@ -20769,6 +20855,12 @@ "license": "MIT", "peer": true }, + "node_modules/stream-source": { + "version": "0.3.5", + "resolved": "https://registry.npmjs.org/stream-source/-/stream-source-0.3.5.tgz", + "integrity": "sha512-ZuEDP9sgjiAwUVoDModftG0JtYiLUV8K4ljYD1VyUMRWtbVf92474o4kuuul43iZ8t/hRuiDAx1dIJSvirrK/g==", + "license": "BSD-3-Clause" + }, "node_modules/strict-event-emitter": { "version": "0.4.6", "resolved": "https://registry.npmjs.org/strict-event-emitter/-/strict-event-emitter-0.4.6.tgz", @@ -21462,6 +21554,13 @@ "node": ">=8" } }, + "node_modules/text-encoding": { + "version": "0.6.4", + "resolved": "https://registry.npmjs.org/text-encoding/-/text-encoding-0.6.4.tgz", + "integrity": "sha512-hJnc6Qg3dWoOMkqP53F0dzRIgtmsAge09kxUIqGrEUS4qr5rWLckGYaQAVr+opBrIMRErGgy6f5aPnyPpyGRfg==", + "deprecated": "no longer maintained", + "license": "Unlicense" + }, "node_modules/text-segmentation": { "version": "1.0.3", "resolved": "https://registry.npmjs.org/text-segmentation/-/text-segmentation-1.0.3.tgz", @@ -23155,6 +23254,15 @@ "dev": true, "license": "MIT" }, + "node_modules/wkt-parser": { + "version": "1.5.6", + "resolved": "https://registry.npmjs.org/wkt-parser/-/wkt-parser-1.5.6.tgz", + "integrity": "sha512-cqHU3lzGt/gt2OqIORP0uVy5yeOX43WABmgFkmJsmcqH3HhQo9PiJG6ftytwGKmpQUbegeX7+pQc2BuNCRfcrw==", + "license": "MIT", + "funding": { + "url": "https://github.com/sponsors/ahocevar" + } + }, "node_modules/word-wrap": { "version": "1.2.5", "resolved": "https://registry.npmjs.org/word-wrap/-/word-wrap-1.2.5.tgz", diff --git a/package.json b/package.json index 198f2531..92fc0705 100644 --- a/package.json +++ b/package.json @@ -19,6 +19,7 @@ "author": "", "license": "ISC", "dependencies": { + "@mapbox/geojson-rewind": "0.5.2", "@mapbox/vector-tile": "^1.3.1", "@tiptap/extension-color": "^2.12.0", "@tiptap/extension-font-family": "^2.12.0", @@ -42,6 +43,7 @@ "date-fns": "^4.1.0", "dompurify": "^3.1.6", "dotenv": "^16.0.1", + "fflate": "0.8.2", "file-loader": "^6.2.0", "geotiff": "2.1.3", "html-react-parser": "^5.1.18", @@ -52,6 +54,7 @@ "ol-pmtiles": "^2.0.2", "plotly.js-strict-dist-min": "^2.35.2", "prismjs": "^1.28.0", + "proj4": "2.21.0", "rc-slider": "^11.1.8", "react": "^18.3.1", "react-bootstrap": "^2.10.2", @@ -72,13 +75,15 @@ "react-use-websocket": "^4.13.0", "sass": "^1.49.0", "sass-loader": "^12.3.0", + "shapefile": "0.6.6", "simple-xml-to-json": "^1.2.3", "style-loader": "^3.3.1", "styled-components": "^6.3.9", "swiper": "^11.2.1", "uuid": "^8.3.2", "webpack": "^5.64.4", - "webpack-dev-server": "^4.6.0" + "webpack-dev-server": "^4.6.0", + "wkt-parser": "1.5.6" }, "devDependencies": { "@babel/core": "^7.18.2", diff --git a/reactapp/__tests__/components/map/Map.test.js b/reactapp/__tests__/components/map/Map.test.js index 837274d3..8dff0255 100644 --- a/reactapp/__tests__/components/map/Map.test.js +++ b/reactapp/__tests__/components/map/Map.test.js @@ -30,15 +30,20 @@ jest.mock("ol/source/GeoTIFF.js", () => { } getView() { getViewSpy(); - return Promise.resolve({ - projection: "EPSG:4326", - extent: [-180, -90, 180, 90], - center: [0, 0], - zoom: 2, - }); + // Overridable so a test can drive a raster whose projection resolves from + // a registered definition rather than natively. + return Promise.resolve( + MockGeoTIFFSource.viewOptions ?? { + projection: "EPSG:4326", + extent: [-180, -90, 180, 90], + center: [0, 0], + zoom: 2, + }, + ); } } MockGeoTIFFSource.getViewSpy = getViewSpy; + MockGeoTIFFSource.viewOptions = null; return { __esModule: true, default: MockGeoTIFFSource, @@ -1509,6 +1514,87 @@ describe("WebGLTile ramp-style render path (Unit 7)", () => { }); }); + test("auto-fit does not adopt a registered-but-not-native projection as the view projection", async () => { + // EPSG:5041 resolves because the projection table registers it, which is + // what makes such a raster render at all. It must not also become the view + // projection: adoption calls setView and publishes the adopted code into the + // map-extent variable other visualizations read. Widening that is a separate + // change, so the view has to stay put while the layer still renders. + const warn = jest.spyOn(console, "warn").mockImplementation(() => {}); + GeoTIFFSource.viewOptions = { + projection: "EPSG:5041", + extent: [-1405881, -1405881, 5405881, 5405881], + center: [2000000, 2000000], + zoom: 2, + }; + + let capturedRef; + const RefCapture = ({ mapProps }) => { + const ref = useRef(); + capturedRef = ref; + return ( +
+ +

{useMapContext()?.mapReady ? "Map Ready" : "Map Not Ready"}

+
+ ); + }; + RefCapture.propTypes = { mapProps: PropTypes.object }; + + const layers = [ + { + type: "WebGLTile", + props: { + source: { + type: "GeoTIFF", + props: { url: "https://example.com/polar.tif" }, + }, + name: "Polar GeoTIFF Layer", + zIndex: 0, + }, + }, + ]; + + try { + render( + + + + + , + ); + + expect(await screen.findByText("Map Ready")).toBeInTheDocument(); + await waitFor(() => { + expect(GeoTIFFSource.getViewSpy).toHaveBeenCalled(); + }); + + // The layer is still added -- rendering by reprojection is the point. + await waitFor(() => { + const names = capturedRef.current + .getLayers() + .getArray() + .map((l) => l.get("name")); + expect(names).toContain("Polar GeoTIFF Layer"); + }); + + // ...but the view did not move off the default. + expect(capturedRef.current.getView().getProjection().getCode()).toBe( + "EPSG:3857", + ); + await waitFor(() => { + expect(warn).toHaveBeenCalledWith( + expect.stringContaining('Not adopting "EPSG:5041"'), + ); + }); + } finally { + GeoTIFFSource.viewOptions = null; + warn.mockRestore(); + } + }); + test("GeoTIFF layer triggers auto-fit: map view's projection switches to the TIF's", async () => { // The auto-fit's contract is: when a GeoTIFF layer is added, the map's // view projection switches to the TIF's so tiles can render. The mock diff --git a/reactapp/__tests__/components/map/projections.test.js b/reactapp/__tests__/components/map/projections.test.js new file mode 100644 index 00000000..560daeaa --- /dev/null +++ b/reactapp/__tests__/components/map/projections.test.js @@ -0,0 +1,196 @@ +import proj4 from "proj4"; +import { get as getProjection } from "ol/proj.js"; +import { + PROJECTION_TABLE, + INITIAL_CODES, + ensureProjection, + registerProjectionFromWkt, +} from "components/map/projections"; + +// A projected CRS with no AUTHORITY node, which is how ESRI writes .prj files. +const ESRI_ALBERS_NO_AUTHORITY = `PROJCS["NAD_1983_Albers",GEOGCS["GCS_North_American_1983",DATUM["D_North_American_1983",SPHEROID["GRS_1980",6378137.0,298.257222101]],PRIMEM["Greenwich",0.0],UNIT["Degree",0.0174532925199433]],PROJECTION["Albers"],PARAMETER["False_Easting",0.0],PARAMETER["False_Northing",0.0],PARAMETER["Central_Meridian",-96.0],PARAMETER["Standard_Parallel_1",29.5],PARAMETER["Standard_Parallel_2",45.5],PARAMETER["Latitude_Of_Origin",23.0],UNIT["Meter",1.0]]`; + +// Append an AUTHORITY node to the outermost PROJCS. It has to go before the +// final bracket: a string replace on "]]" lands inside GEOGCS instead, which +// wkt-parser then reports as a nested authority and the module correctly ignores +// -- making any test built that way pass without testing anything. +function withAuthority(wkt, code) { + const [name, id] = code.split(":"); + return `${wkt.slice(0, -1)},AUTHORITY["${name}","${id}"]]`; +} + +// The same body, claiming EPSG:5070 -- which the table already covers -- and +// with a deliberately wrong standard parallel, to prove the seeded definition is +// not replaced by a layer's copy. +const ALBERS_5070_WRONG_PARAMS = withAuthority( + ESRI_ALBERS_NO_AUTHORITY.replace( + '"Standard_Parallel_1",29.5', + '"Standard_Parallel_1",20', + ), + "EPSG:5070", +); + +// The OGC spelling of the same projection. proj4 does not implement this +// variant: it yields non-finite coordinates even at the projection's own centre, +// so registration reports it rather than rendering features nowhere. +const OGC_ALBERS = `PROJCS["NAD83 / Conus Albers",GEOGCS["NAD83",DATUM["North_American_Datum_1983",SPHEROID["GRS 1980",6378137,298.257222101]],PRIMEM["Greenwich",0],UNIT["degree",0.0174532925199433]],PROJECTION["Albers_Conic_Equal_Area"],PARAMETER["latitude_of_center",23],PARAMETER["longitude_of_center",-96],PARAMETER["standard_parallel_1",29.5],PARAMETER["standard_parallel_2",45.5],UNIT["metre",1]]`; + +const UNSUPPORTED_METHOD = `PROJCS["nonsense",GEOGCS["g",DATUM["d",SPHEROID["s",6378137,298.257222101]],PRIMEM["Greenwich",0],UNIT["Degree",0.0174532925199433]],PROJECTION["Totally_Not_A_Real_Projection"],UNIT["Meter",1.0]]`; + +describe("projection table", () => { + it("resolves every code registered at init and gives it an extent", () => { + INITIAL_CODES.forEach((code) => { + const projection = getProjection(code); + expect(projection).toBeTruthy(); + // register() cannot supply an extent, so a null here means the + // post-registration extent application did not run. + expect(projection.getExtent()).toEqual(PROJECTION_TABLE[code].extent); + }); + }); + + // Cross-implementation check: the expected values come from PROJ's EPSG + // database, and the control points sit away from each projection's origin so a + // wrong standard parallel, scale factor or linear unit moves the result. A + // point at the origin would return the false easting regardless. + it.each(INITIAL_CODES)( + "%s transforms its control point to the coordinate PROJ gives", + (code) => { + const { lonLat, projected } = PROJECTION_TABLE[code].controlPoint; + const [x, y] = proj4("EPSG:4326", code, lonLat); + expect(x).toBeCloseTo(projected[0], 2); + expect(y).toBeCloseTo(projected[1], 2); + }, + ); + + it("registers a bounded number of codes at init rather than the whole table", () => { + // register() is quadratic in registered-definition count, and this module is + // a static import, so an unbounded init set is a first-render regression on + // every dashboard. + expect(INITIAL_CODES.length).toBeLessThanOrEqual(4); + }); + + it("leaves the native projections untouched", () => { + expect(getProjection("EPSG:4326").getUnits()).toBe("degrees"); + expect(getProjection("EPSG:3857").getUnits()).toBe("m"); + expect(getProjection("EPSG:3857").getExtent()).toBeTruthy(); + // OpenLayers' own UTM factory still answers for zone codes. + expect(getProjection("EPSG:32615")).toBeTruthy(); + }); +}); + +describe("ensureProjection", () => { + it("returns null for a code that is neither native nor in the table", () => { + expect(ensureProjection("EPSG:99999")).toBeNull(); + }); + + it("returns null for an empty code without throwing", () => { + expect(ensureProjection(undefined)).toBeNull(); + expect(ensureProjection("")).toBeNull(); + }); + + it("resolves a native code without needing a table entry", () => { + expect(ensureProjection("EPSG:4326")).toBeTruthy(); + }); + + it("registers a table entry on demand and applies its extent", () => { + const projection = ensureProjection("EPSG:5070"); + expect(projection).toBeTruthy(); + expect(projection.getExtent()).toEqual( + PROJECTION_TABLE["EPSG:5070"].extent, + ); + }); +}); + +describe("registerProjectionFromWkt", () => { + it("registers WKT with no authority node and transforms with it", () => { + const result = registerProjectionFromWkt(ESRI_ALBERS_NO_AUTHORITY); + expect(result.error).toBeUndefined(); + expect(result.code).toMatch(/^WKT:/); + expect(getProjection(result.code)).toBeTruthy(); + + // Assert the resolved parameters, not merely that nothing threw: proj4 has a + // history of parsing ESRI WKT variants and silently producing wrong ones. + const [x, y] = proj4("EPSG:4326", result.code, [-105, 40]); + expect(x).toBeCloseTo(-760465.745, 2); + expect(y).toBeCloseTo(1923013.98, 2); + }); + + it("reuses an already-resolvable code instead of registering the layer's copy", () => { + const before = proj4("EPSG:4326", "EPSG:5070", [-105, 40]); + + const result = registerProjectionFromWkt(ALBERS_5070_WRONG_PARAMS); + expect(result.code).toBe("EPSG:5070"); + + // The seeded definition survives and the layer's differing parameters are + // discarded. Without this, one shapefile changes how every other layer on + // every dashboard in this session transforms. + const after = proj4("EPSG:4326", "EPSG:5070", [-105, 40]); + expect(after[0]).toBeCloseTo(before[0], 6); + expect(after[1]).toBeCloseTo(before[1], 6); + }); + + it("never registers under a claimed authority code", () => { + const claimed = "EPSG:26985"; + expect(getProjection(claimed)).toBeFalsy(); + const wkt = withAuthority(ESRI_ALBERS_NO_AUTHORITY, claimed); + + const result = registerProjectionFromWkt(wkt); + + // Guard against this passing vacuously: the fixture must actually carry a + // top-level authority, or the module would fall through to a synthetic code + // for the wrong reason and the assertions below would prove nothing. + expect(wkt).toContain('AUTHORITY["EPSG","26985"]]'); + expect(result.code).toMatch(/^WKT:/); + expect(result.code).not.toBe(claimed); + // The claimed code stays unresolvable, so a later layer naming it by code + // does not silently inherit this layer's parameters. + expect(getProjection(claimed)).toBeFalsy(); + }); + + it("returns the same code for the same WKT without re-registering", () => { + const first = registerProjectionFromWkt(ESRI_ALBERS_NO_AUTHORITY); + const second = registerProjectionFromWkt(ESRI_ALBERS_NO_AUTHORITY); + expect(second.code).toBe(first.code); + }); + + it("reports an unparsable definition and names what failed", () => { + const result = registerProjectionFromWkt('PROJCS["broken",GARBAGE['); + expect(result.code).toBeUndefined(); + expect(result.error.reason).toBe("unparsable"); + expect(result.error.detail).toMatch(/could not be parsed/); + }); + + it("reports an unsupported projection method by name", () => { + // This one parses cleanly and only fails at transform time, so the message + // has to come from validating the transform rather than from the parser. + const result = registerProjectionFromWkt(UNSUPPORTED_METHOD); + expect(result.code).toBeUndefined(); + expect(result.error.reason).toBe("unsupported"); + expect(result.error.detail).toContain("Totally_Not_A_Real_Projection"); + }); + + it("reports the OGC Albers variant as unsupported rather than rendering nowhere", () => { + // proj4 implements the ESRI spelling of Albers but not this one, and the + // failure is silent: non-finite coordinates, not an exception. Caught by + // probing the definition at its own centre before registering it. + const result = registerProjectionFromWkt(OGC_ALBERS); + expect(result.code).toBeUndefined(); + expect(result.error.reason).toBe("unsupported"); + expect(result.error.detail).toContain("Albers_Conic_Equal_Area"); + }); + + it("leaves the registry usable after rejecting a definition", () => { + // A rejected definition must not stay in proj4's registry: register() + // constructs a transform for every pair of registered codes, so one unusable + // definition would make it throw and take working projections down with it. + registerProjectionFromWkt(UNSUPPORTED_METHOD); + const after = registerProjectionFromWkt(ESRI_ALBERS_NO_AUTHORITY); + expect(after.error).toBeUndefined(); + expect(getProjection("EPSG:5070")).toBeTruthy(); + }); + + it("reports an empty definition rather than throwing", () => { + expect(registerProjectionFromWkt("").error.reason).toBe("empty"); + expect(registerProjectionFromWkt(undefined).error.reason).toBe("empty"); + }); +}); diff --git a/reactapp/components/map/Map.js b/reactapp/components/map/Map.js index 9d0cebf7..a374c48d 100644 --- a/reactapp/components/map/Map.js +++ b/reactapp/components/map/Map.js @@ -4,6 +4,12 @@ import moduleLoader, { applyAutoRamp, createJsonStyleFunction, } from "components/map/ModuleLoader"; +// Importing this registers the coordinate reference systems that layers name by +// code. Module evaluation completes before any render, so registration is in +// place before the layer effect below constructs a single source -- which +// matters, because layers are constructed concurrently and a registration that +// waited on anything async would race them. +import { isNativelyResolvable } from "components/map/projections"; import LayersControl from "components/map/LayersControl"; import FloatingMapControl from "components/map/FloatingMapControl"; import LegendControl from "components/map/LegendControl"; @@ -546,8 +552,24 @@ const MapComponent = ({ // feature count. Move them with the view. const previousCode = prevProjection.getCode(); const adoptedCode = newView.getProjection().getCode(); - map.setView(newView); - reprojectVectorFeatures(map, previousCode, adoptedCode); + + // Adopt the raster's projection as the view projection only when + // OpenLayers resolves it on its own. Registering a definition + // makes a previously-unresolvable raster render, but it must not + // also start changing the view: setView publishes the adopted + // code into the map-extent variable other visualizations consume, + // and saved center/zoom values would be reinterpreted in the new + // projection's units. Widening this is its own change, verified + // against live dashboards. Such a raster still renders here -- + // by reprojection rather than natively. + if (!isNativelyResolvable(adoptedCode)) { + console.warn( + `Not adopting "${adoptedCode}" as the view projection for layer "${name}": it resolves from a registered definition rather than natively. The layer renders by reprojection.`, + ); + } else { + map.setView(newView); + reprojectVectorFeatures(map, previousCode, adoptedCode); + } } catch (err) { console.warn( `GeoTIFF auto-fit failed for layer "${name}":`, diff --git a/reactapp/components/map/projections.js b/reactapp/components/map/projections.js new file mode 100644 index 00000000..569e309e --- /dev/null +++ b/reactapp/components/map/projections.js @@ -0,0 +1,264 @@ +import proj4 from "proj4"; +import { register } from "ol/proj/proj4.js"; +import { get as getProjection } from "ol/proj.js"; +import wktParser from "wkt-parser"; + +// Coordinate reference systems the map can resolve, beyond the ones OpenLayers +// ships with. OL natively handles EPSG:4326, EPSG:3857 and every WGS84 UTM zone +// via its own projection factory; everything else -- State Plane, Albers, the +// polar stereographics -- resolves only if a definition is registered here. +// +// Two things are registered, from two different places, and the split matters: +// +// 1. Codes named by a layer that carries no definition of its own. A WMS or +// GeoTIFF layer says "EPSG:5041" and nothing more, so the definition has to +// already be on hand. That is what the table below is for. +// +// 2. Definitions a layer brings with it. A shapefile carries its CRS as WKT in +// its .prj, so it needs no table entry -- see registerProjectionFromWkt. +// +// The table therefore only has to cover case 1, which is why it is short. A +// survey of the live dashboards found exactly one layer naming a non-native code +// (a WMS layer requesting EPSG:5041); EPSG:5070 is included because US national +// hydrology datasets commonly name Conus Albers by code. Adding a zone is a +// table entry plus a control point, and `ensureProjection` registers it on +// demand rather than at startup. +// +// Registration is deliberately *not* done for the whole table at load time. +// `register` builds pairwise transforms across every registered code, so its +// cost is quadratic: measured at 99ms for two definitions on top of proj4's +// built-ins, and hundreds of milliseconds once a State-Plane-sized set is in +// play. This module is imported statically by the map, so that cost would land +// before first render on every dashboard, including the ones with no layer that +// needs it. + +// Definitions, extents and control points are taken from PROJ's EPSG database +// rather than hand-derived. The control points sit away from each projection's +// origin so they exercise the standard parallels and scale factor -- a point at +// the origin would return the false easting no matter how wrong the rest of the +// definition was. proj4 agrees with PROJ on both to sub-millimetre, so the test +// that round-trips them is a cross-implementation check, not a self-consistency +// one. +export const PROJECTION_TABLE = { + "EPSG:5041": { + name: "WGS 84 / UPS North (E,N)", + definition: + "+proj=stere +lat_0=90 +lon_0=0 +k=0.994 +x_0=2000000 +y_0=2000000 +datum=WGS84 +units=m +no_defs", + extent: [-1405881, -1405881, 5405881, 5405881], + controlPoint: { lonLat: [-45, 70], projected: [414390.988, 414390.988] }, + }, + "EPSG:5070": { + name: "NAD83 / Conus Albers", + definition: + "+proj=aea +lat_0=23 +lon_0=-96 +lat_1=29.5 +lat_2=45.5 +x_0=0 +y_0=0 +datum=NAD83 +units=m +no_defs", + extent: [-2916311, 153629, 2945750, 3255275], + controlPoint: { lonLat: [-105, 40], projected: [-760465.745, 1923013.98] }, + }, +}; + +// Registered when this module is evaluated. Keep this list to codes a layer +// actually names today; the rest of the table is reachable through +// `ensureProjection` at the point of use. +export const INITIAL_CODES = ["EPSG:5041", "EPSG:5070"]; + +// Prefix for projections registered from a layer's own WKT. Kept distinct from +// any authority namespace so a synthetic code can never be mistaken for -- or +// collide with -- a real EPSG code. +const WKT_CODE_PREFIX = "WKT:"; + +/** + * Whether OpenLayers resolves this code on its own, without anything registered + * here. + * + * Asked by code rather than by registry lookup, because once a definition is + * registered the two are indistinguishable through the registry -- which is the + * whole point of the question. Used to keep the raster auto-fit from adopting a + * newly-registered projection as the map's *view* projection: adoption calls + * setView and publishes the adopted code into the map-extent variable other + * visualizations read, so widening it is a separate change with its own + * verification. Registered projections still serve as data projections, so a + * raster in one renders by reprojection instead. + * + * @param {string} code Projection code. + * @returns {boolean} + */ +export function isNativelyResolvable(code) { + if (typeof code !== "string") return false; + const match = /^EPSG:(\d+)$/.exec(code.trim()); + if (!match) return false; + const id = Number(match[1]); + if ([4326, 3857, 900913, 102100].includes(id)) return true; + // OpenLayers ships a UTM projection factory covering the WGS84 zones. + return (id > 32600 && id < 32661) || (id > 32700 && id < 32761); +} + +// Registering a definition does not give the resulting projection an extent: +// `register` builds it from the proj4 definition, and a proj4 definition has +// nowhere to carry one. Verified against the installed versions -- the extent +// reads null until it is set explicitly. OpenLayers uses projection extent for +// view clamping, so a projection that ever becomes the view projection without +// one degrades silently. +function applyExtent(code) { + const entry = PROJECTION_TABLE[code]; + const projection = getProjection(code); + if (entry?.extent && projection && !projection.getExtent()) { + projection.setExtent(entry.extent); + } +} + +// Definitions have to be declared and registered together. `register` iterates +// everything already in proj4's registry, so declaring the whole table and then +// registering a subset is not possible -- the subset is chosen by what gets +// declared. +function registerCodes(codes) { + const pending = codes.filter( + (code) => PROJECTION_TABLE[code] && !getProjection(code), + ); + if (pending.length === 0) return; + pending.forEach((code) => { + proj4.defs(code, PROJECTION_TABLE[code].definition); + }); + register(proj4); + pending.forEach(applyExtent); +} + +/** + * Resolve a projection by code, registering its table entry if it has not been + * registered yet. + * + * Re-registering is safe: OpenLayers skips any code already in its projection + * cache, so previously registered projections keep their identity and their + * applied extent. + * + * @param {string} code Projection code, e.g. "EPSG:5070". + * @returns {import("ol/proj/Projection.js").default|null} The projection, or + * null when the code is neither native nor in the table. + */ +export function ensureProjection(code) { + if (!code) return null; + const existing = getProjection(code); + if (existing) return existing; + if (!PROJECTION_TABLE[code]) return null; + registerCodes([code]); + return getProjection(code); +} + +// Stable, dependency-free hash of the normalized WKT. Two textually different +// but semantically equivalent definitions hash differently and so register +// separately; that costs a duplicate registration and nothing else, which is +// cheaper than trying to canonicalise WKT. +function wktCode(wkt) { + const normalized = wkt.replace(/\s+/g, ""); + let hash = 5381; + for (let i = 0; i < normalized.length; i += 1) { + hash = ((hash << 5) + hash + normalized.charCodeAt(i)) | 0; + } + return `${WKT_CODE_PREFIX}${(hash >>> 0).toString(36)}`; +} + +// The outermost AUTHORITY node, which is the one belonging to the projected CRS +// itself. A WKT string carries several -- the datum and the geographic CRS have +// their own -- so reading the last one out of the raw text would pick the wrong +// node. wkt-parser hands back only the top-level one. +function claimedCode(parsed) { + const authority = parsed?.AUTHORITY; + if (!authority) return null; + const [name] = Object.keys(authority); + if (!name) return null; + return `${name}:${authority[name]}`; +} + +// Where to probe a candidate definition. It has to be a point the projection +// actually covers: an Albers centred on -96 returns nothing usable at [0, 0], so +// probing there would report a perfectly good definition as unsupported. The +// parsed definition carries its own centre in radians, which is always inside +// the domain. +function probePoint(parsed) { + const toDegrees = 180 / Math.PI; + const lon = parsed?.long0 ?? parsed?.longc; + const lat = parsed?.lat0 ?? parsed?.lat_ts; + return [ + typeof lon === "number" ? lon * toDegrees : 0, + typeof lat === "number" ? lat * toDegrees : 0, + ]; +} + +// A WKT whose projection method proj4 does not implement parses cleanly, and +// declaring it does not fail either -- it only goes wrong at transform time, and +// then with an internal error that names nothing. The only way to tell is to +// transform something, which is why every candidate is probed. +// +// This runs before `register`, deliberately. `register` constructs a transform +// for every pair of registered codes, so an unusable definition sitting in the +// registry makes it throw -- taking down projections that were working. A +// candidate that fails is removed again before anything else sees it. +function definitionUsable(code, parsed) { + try { + const [x, y] = proj4("EPSG:4326", code, probePoint(parsed)); + return Number.isFinite(x) && Number.isFinite(y); + } catch { + return false; + } +} + +/** + * Register a coordinate reference system from a layer's own WKT definition. + * + * Never overwrites a definition that already resolves. A layer's WKT is + * authoritative for that layer's own features, but the projection registry is + * global to the browser session -- so letting one layer's parameters replace a + * code every other layer resolves through would make rendering depend on which + * dashboard was opened first. When the WKT claims a code that already resolves, + * the existing definition is reused and nothing is written. Otherwise the + * definition is registered under a synthetic code, never under the claimed one. + * + * @param {string} wkt WKT definition, typically the contents of a .prj. + * @returns {{code: string}|{error: {reason: string, detail: string}}} The code to + * read coordinates with, or a failure describing what could not be resolved. + */ +export function registerProjectionFromWkt(wkt) { + if (typeof wkt !== "string" || wkt.trim() === "") { + return { + error: { reason: "empty", detail: "No projection definition was found." }, + }; + } + + let parsed; + try { + parsed = wktParser(wkt); + } catch (error) { + return { + error: { + reason: "unparsable", + detail: `The projection definition could not be parsed: ${error.message}`, + }, + }; + } + + const claimed = claimedCode(parsed); + if (claimed && (getProjection(claimed) || ensureProjection(claimed))) { + return { code: claimed }; + } + + const code = wktCode(wkt); + if (getProjection(code)) return { code }; + + proj4.defs(code, wkt); + if (!definitionUsable(code, parsed)) { + delete proj4.defs[code]; + const method = parsed?.projName ?? "an unnamed projection method"; + return { + error: { + reason: "unsupported", + detail: `The projection "${method}"${ + claimed ? ` (${claimed})` : "" + } could not be resolved.`, + }, + }; + } + + register(proj4); + return { code }; +} + +registerCodes(INITIAL_CODES); From c1cee018a161c6201d05cac99f892677dbdf8edd Mon Sep 17 00:00:00 2001 From: Corey Krewson Date: Mon, 24 Aug 2026 15:13:41 -0700 Subject: [PATCH 02/18] feat(map): fetch and decompress shapefile components under a size ceiling A map-free acquisition step: URL validation, sibling derivation, fetching, decompression and a bounded component cache. It returns raw buffers and knows nothing about what a shapefile means, so the parser choice -- which carries the real risk here -- can change without disturbing any of it. The size ceiling binds on each member's *declared* size, read from the local header before any data flows, and refuses by never starting that member. Summing bytes as they arrive does not work: an 8 MiB expansion arrives in a single callback, so a running total only notices once the payload is already allocated and inflated, which is the cost the ceiling exists to prevent. A member declaring no size falls back to counting. Only shapefile components are ever started, so a bomb parked in an unrelated member costs nothing -- covered by a test. fflate's Unzip carries only a pass-through decoder, so without registering the inflate decoder every member of a real archive throws on start. That is a total failure rather than a degradation, and it now has a regression test. Bodies are read whole rather than streamed. Aborting rejects the read and terminates the transfer, which is what cancellation needs, and the configured test environment exposes no response stream at all -- so a stream-reader implementation could not have been exercised. Absence and failure stay distinguishable on the sibling path: a 404 on an optional component means absent, any other status is reported. A transient 403 routed into the absent path would fall back to the author-supplied projection and draw features somewhere else entirely, with no error. Two failure modes get accurate messages rather than misleading ones. A response is checked for markup, and the buffer for the zip magic number, because a portal returning an HTML error page with a 200 would otherwise be reported as an archive containing no .shp entry. And a fetch-stage failure names cross-origin policy, an unreachable host, a missing file and an expired signature together, since a browser cannot tell them apart. The cache holds component buffers keyed on resolved URL, not parsed features: buffers are already under the ceiling by construction so a small entry count has an exact memory bound, while parsed GeoJSON runs several times the archive size. A hit skips the network hop and the decompression and still re-parses. Failures are never cached, so a retry retries. Co-Authored-By: Claude Opus 5 (1M context) --- .../components/map/shapefile/acquire.test.js | 316 ++++++++++++++++++ .../components/map/shapefile/siblings.test.js | 101 ++++++ .../components/map/shapefile/unzip.test.js | 192 +++++++++++ reactapp/components/map/shapefile/acquire.js | 200 +++++++++++ reactapp/components/map/shapefile/cache.js | 64 ++++ reactapp/components/map/shapefile/siblings.js | 99 ++++++ reactapp/components/map/shapefile/unzip.js | 194 +++++++++++ 7 files changed, 1166 insertions(+) create mode 100644 reactapp/__tests__/components/map/shapefile/acquire.test.js create mode 100644 reactapp/__tests__/components/map/shapefile/siblings.test.js create mode 100644 reactapp/__tests__/components/map/shapefile/unzip.test.js create mode 100644 reactapp/components/map/shapefile/acquire.js create mode 100644 reactapp/components/map/shapefile/cache.js create mode 100644 reactapp/components/map/shapefile/siblings.js create mode 100644 reactapp/components/map/shapefile/unzip.js diff --git a/reactapp/__tests__/components/map/shapefile/acquire.test.js b/reactapp/__tests__/components/map/shapefile/acquire.test.js new file mode 100644 index 00000000..da580371 --- /dev/null +++ b/reactapp/__tests__/components/map/shapefile/acquire.test.js @@ -0,0 +1,316 @@ +import { zipSync, strToU8 } from "fflate"; +import { + acquireComponents, + DEFAULT_MAX_BYTES, +} from "components/map/shapefile/acquire"; +import { + clearComponentCache, + cachedComponentCount, + CACHE_MAX_ENTRIES, +} from "components/map/shapefile/cache"; + +const MB = 1024 * 1024; +const ARCHIVE = zipSync({ + "basins.shp": strToU8("SHPBODY"), + "basins.dbf": strToU8("DBFBODY"), + "basins.prj": strToU8('PROJCS["NAD_1983_Albers"]'), + "basins.shx": strToU8("SHXBODY"), +}); + +// Minimal Response stand-in. The configured environment exposes no response +// stream, so the implementation reads whole bodies and only arrayBuffer is +// needed here. +function respond({ status = 200, contentType = "application/zip", body } = {}) { + return { + ok: status >= 200 && status < 300, + status, + headers: { get: (name) => (name === "content-type" ? contentType : null) }, + arrayBuffer: async () => (body ?? new Uint8Array()).buffer, + }; +} + +let fetchMock; + +beforeEach(() => { + clearComponentCache(); + fetchMock = jest.fn(); + global.fetch = fetchMock; +}); + +describe("acquireComponents — validation happens before any request", () => { + it.each([ + "data:application/zip;base64,UEsDBA==", + "blob:https://example.org/8f3c", + "file:///tmp/basins.zip", + ])("rejects %s without fetching", async (url) => { + const result = await acquireComponents(url); + expect(result.error.reason).toBe("unsupported_scheme"); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it("rejects an unsupported path without fetching", async () => { + const result = await acquireComponents( + "https://example.org/basins.geojson", + ); + expect(result.error.reason).toBe("unsupported_path"); + expect(fetchMock).not.toHaveBeenCalled(); + }); +}); + +describe("acquireComponents — archive form", () => { + it("returns the four components from one request", async () => { + fetchMock.mockResolvedValue(respond({ body: ARCHIVE })); + + const result = await acquireComponents("https://example.org/basins.zip"); + + expect(result.error).toBeUndefined(); + expect(Object.keys(result.components).sort()).toEqual([ + "dbf", + "prj", + "shp", + "shx", + ]); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + it("reports a markup response as the wrong content type, naming what came back", async () => { + fetchMock.mockResolvedValue( + respond({ + contentType: "text/html; charset=utf-8", + body: strToU8(""), + }), + ); + + const result = await acquireComponents("https://example.org/basins.zip"); + + expect(result.error.stage).toBe("parse"); + expect(result.error.reason).toBe("wrong_content_type"); + expect(result.error.detail).toContain("text/html"); + }); + + it("reports a non-success status and names the candidate causes together", async () => { + fetchMock.mockResolvedValue(respond({ status: 403 })); + + const result = await acquireComponents("https://example.org/basins.zip"); + + expect(result.error.stage).toBe("fetch"); + expect(result.error.status).toBe(403); + // A browser cannot distinguish these, so the message must not claim one. + expect(result.error.detail).toMatch(/cross-origin/i); + expect(result.error.detail).toMatch(/expired signature/i); + }); + + it("reports a network rejection as unreachable rather than throwing", async () => { + fetchMock.mockRejectedValue(new TypeError("Failed to fetch")); + + const result = await acquireComponents("https://example.org/basins.zip"); + + expect(result.error.reason).toBe("unreachable"); + expect(result.error.detail).toMatch(/cross-origin/i); + }); + + it("refuses an archive that expands past the ceiling", async () => { + const bomb = zipSync({ + "basins.shp": new Uint8Array(8 * MB), + "basins.prj": strToU8('PROJCS["x"]'), + }); + fetchMock.mockResolvedValue(respond({ body: bomb })); + + const result = await acquireComponents("https://example.org/basins.zip", { + maxBytes: 1 * MB, + }); + + expect(result.error.reason).toBe("too_large"); + expect(result.error.permitted).toBe(1 * MB); + }); +}); + +describe("acquireComponents — sibling form", () => { + function siblingResponder(overrides = {}) { + return (url) => { + const extension = url.split("?")[0].split(".").pop(); + if (overrides[extension]) return Promise.resolve(overrides[extension]); + return Promise.resolve( + respond({ + contentType: "application/octet-stream", + body: strToU8(`${extension.toUpperCase()}BODY`), + }), + ); + }; + } + + it("derives and fetches every component", async () => { + fetchMock.mockImplementation(siblingResponder()); + + const result = await acquireComponents("https://example.org/basins.shp"); + + expect(result.error).toBeUndefined(); + expect(Object.keys(result.components).sort()).toEqual([ + "dbf", + "prj", + "shp", + "shx", + ]); + const requested = fetchMock.mock.calls.map(([u]) => u); + expect(requested).toEqual([ + "https://example.org/basins.shp", + "https://example.org/basins.dbf", + "https://example.org/basins.prj", + "https://example.org/basins.shx", + ]); + }); + + it("preserves a signed query string on every derived request", async () => { + fetchMock.mockImplementation(siblingResponder()); + + await acquireComponents( + "https://bucket.s3.amazonaws.com/basins.shp?X-Amz-Signature=abc", + ); + + fetchMock.mock.calls.forEach(([url]) => { + expect(url).toContain("X-Amz-Signature=abc"); + }); + }); + + it("treats a 404 on an optional component as absent", async () => { + fetchMock.mockImplementation( + siblingResponder({ dbf: respond({ status: 404 }) }), + ); + + const result = await acquireComponents("https://example.org/basins.shp"); + + expect(result.error).toBeUndefined(); + expect(result.components.dbf).toBeUndefined(); + expect(result.components.shp).toBeTruthy(); + }); + + it("treats any other status on an optional component as a reported failure", async () => { + // This is the distinction that matters: a transient 403 routed into the + // absent path would fall back to the author-supplied projection and draw the + // features somewhere else entirely, with no error. + fetchMock.mockImplementation( + siblingResponder({ prj: respond({ status: 403 }) }), + ); + + const result = await acquireComponents("https://example.org/basins.shp"); + + expect(result.error.reason).toBe("component_status"); + expect(result.error.component).toBe("prj"); + expect(result.error.status).toBe(403); + }); + + it("reports a missing .shp rather than continuing", async () => { + fetchMock.mockImplementation( + siblingResponder({ shp: respond({ status: 404 }) }), + ); + + const result = await acquireComponents("https://example.org/basins.shp"); + + expect(result.error.stage).toBe("fetch"); + expect(result.error.status).toBe(404); + // Stops at the first component rather than paying for the other three. + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + it("refuses once the components total past the ceiling", async () => { + const half = new Uint8Array(600 * 1024); + fetchMock.mockImplementation( + siblingResponder({ + shp: respond({ body: half }), + dbf: respond({ body: half }), + }), + ); + + const result = await acquireComponents("https://example.org/basins.shp", { + maxBytes: 1 * MB, + }); + + expect(result.error.reason).toBe("too_large"); + }); +}); + +describe("acquireComponents — cancellation", () => { + it("resolves as cancelled when the signal is already aborted", async () => { + const controller = new AbortController(); + controller.abort(); + + const result = await acquireComponents("https://example.org/basins.zip", { + signal: controller.signal, + }); + + expect(result.cancelled).toBe(true); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it("resolves as cancelled with no failure message when the body read aborts", async () => { + const controller = new AbortController(); + fetchMock.mockImplementation(() => { + controller.abort(); + const error = new Error("aborted"); + error.name = "AbortError"; + return Promise.reject(error); + }); + + const result = await acquireComponents("https://example.org/basins.zip", { + signal: controller.signal, + }); + + expect(result.cancelled).toBe(true); + expect(result.error).toBeUndefined(); + }); +}); + +describe("acquireComponents — caching", () => { + it("serves a repeat of the same resolved URL without fetching", async () => { + fetchMock.mockResolvedValue(respond({ body: ARCHIVE })); + + const first = await acquireComponents("https://example.org/basins.zip"); + const second = await acquireComponents("https://example.org/basins.zip"); + + expect(first.fromCache).toBeUndefined(); + expect(second.fromCache).toBe(true); + expect(second.components.shp).toEqual(first.components.shp); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + it("fetches a different resolved URL", async () => { + fetchMock.mockResolvedValue(respond({ body: ARCHIVE })); + + await acquireComponents("https://example.org/a.zip"); + await acquireComponents("https://example.org/b.zip"); + + expect(fetchMock).toHaveBeenCalledTimes(2); + }); + + it("evicts the least recently used entry once full", async () => { + fetchMock.mockResolvedValue(respond({ body: ARCHIVE })); + + for (let i = 0; i <= CACHE_MAX_ENTRIES; i += 1) { + await acquireComponents(`https://example.org/${i}.zip`); + } + expect(cachedComponentCount()).toBe(CACHE_MAX_ENTRIES); + + // The first URL was evicted, so it fetches again. + fetchMock.mockClear(); + await acquireComponents("https://example.org/0.zip"); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + it("does not cache a failure, so a retry retries", async () => { + fetchMock.mockResolvedValueOnce(respond({ status: 503 })); + const failed = await acquireComponents("https://example.org/basins.zip"); + expect(failed.error).toBeTruthy(); + + fetchMock.mockResolvedValueOnce(respond({ body: ARCHIVE })); + const retried = await acquireComponents("https://example.org/basins.zip"); + + expect(retried.error).toBeUndefined(); + expect(fetchMock).toHaveBeenCalledTimes(2); + }); +}); + +describe("acquireComponents — defaults", () => { + it("defaults the ceiling to 25 MB", () => { + expect(DEFAULT_MAX_BYTES).toBe(25 * MB); + }); +}); diff --git a/reactapp/__tests__/components/map/shapefile/siblings.test.js b/reactapp/__tests__/components/map/shapefile/siblings.test.js new file mode 100644 index 00000000..3e515bc9 --- /dev/null +++ b/reactapp/__tests__/components/map/shapefile/siblings.test.js @@ -0,0 +1,101 @@ +import { + validateSourceUrl, + deriveSiblingUrls, +} from "components/map/shapefile/siblings"; + +describe("validateSourceUrl", () => { + it.each([ + ["https://example.org/data/basins.zip", "archive"], + ["http://example.org/data/basins.ZIP", "archive"], + ["https://example.org/data/basins.shp", "components"], + ["https://example.org/data/basins.SHP", "components"], + ])("accepts %s as %s", (url, form) => { + expect(validateSourceUrl(url)).toEqual({ form, url }); + }); + + it.each([ + "data:application/zip;base64,UEsDBA==", + "blob:https://example.org/8f3c", + "file:///home/user/basins.zip", + "//example.org/basins.zip", + "ftp://example.org/basins.zip", + ])("rejects %s before any fetch", (url) => { + const { error } = validateSourceUrl(url); + expect(error.reason).toBe("unsupported_scheme"); + // The message has to name what is accepted, since the author's next action + // is to supply a different URL. + expect(error.detail).toMatch(/https?/); + }); + + it("rejects a path ending in neither .zip nor .shp and names both forms", () => { + const { error } = validateSourceUrl( + "https://example.org/data/basins.geojson", + ); + expect(error.reason).toBe("unsupported_path"); + expect(error.detail).toContain(".zip"); + expect(error.detail).toContain(".shp"); + }); + + it("classifies by the path, not the query string", () => { + // A download endpoint whose query says "shp" is still not a .shp path, and a + // .zip path with an unrelated query still is an archive. + expect( + validateSourceUrl("https://example.org/download?format=shp").error, + ).toBeTruthy(); + expect( + validateSourceUrl("https://example.org/basins.zip?token=abc").form, + ).toBe("archive"); + }); + + it("rejects an empty or non-string url", () => { + expect(validateSourceUrl("").error.reason).toBe("empty"); + expect(validateSourceUrl(undefined).error.reason).toBe("empty"); + }); + + it("rejects a malformed url rather than throwing", () => { + expect(validateSourceUrl("https://").error).toBeTruthy(); + }); +}); + +describe("deriveSiblingUrls", () => { + it("replaces the extension for each component", () => { + expect(deriveSiblingUrls("https://example.org/data/basins.shp")).toEqual({ + shp: "https://example.org/data/basins.shp", + dbf: "https://example.org/data/basins.dbf", + prj: "https://example.org/data/basins.prj", + shx: "https://example.org/data/basins.shx", + }); + }); + + it("preserves a query string and fragment untouched", () => { + // Presigned links carry a signature computed over the object key, and portal + // links carry cache tokens. Replacing the extension across the whole URL + // would corrupt both. + const derived = deriveSiblingUrls( + "https://bucket.s3.amazonaws.com/w/basins.shp?X-Amz-Signature=abc123&X-Amz-Expires=3600#frag", + ); + expect(derived.dbf).toBe( + "https://bucket.s3.amazonaws.com/w/basins.dbf?X-Amz-Signature=abc123&X-Amz-Expires=3600#frag", + ); + expect(derived.prj).toContain("?X-Amz-Signature=abc123"); + expect(derived.prj).toContain("#frag"); + }); + + it("only replaces the final path segment's extension", () => { + // A directory named like a component must not be rewritten. + const derived = deriveSiblingUrls( + "https://example.org/shp.archive/basins.shp", + ); + expect(derived.dbf).toBe("https://example.org/shp.archive/basins.dbf"); + }); + + it("normalizes the derived extension to lower case from an upper-case source", () => { + const derived = deriveSiblingUrls("https://example.org/BASINS.SHP"); + expect(derived.dbf).toBe("https://example.org/BASINS.dbf"); + }); + + it("handles a filename containing dots", () => { + const derived = deriveSiblingUrls("https://example.org/wbd.huc8.v2.shp"); + expect(derived.prj).toBe("https://example.org/wbd.huc8.v2.prj"); + }); +}); diff --git a/reactapp/__tests__/components/map/shapefile/unzip.test.js b/reactapp/__tests__/components/map/shapefile/unzip.test.js new file mode 100644 index 00000000..d73cca39 --- /dev/null +++ b/reactapp/__tests__/components/map/shapefile/unzip.test.js @@ -0,0 +1,192 @@ +import { zipSync, strToU8, strFromU8 } from "fflate"; +import { + unzipShapefileComponents, + createByteBudget, +} from "components/map/shapefile/unzip"; + +const MB = 1024 * 1024; + +function archive(entries) { + return zipSync(entries); +} + +const MINIMAL = { + "basins.shp": strToU8("SHPBODY"), + "basins.dbf": strToU8("DBFBODY"), + "basins.prj": strToU8('PROJCS["NAD_1983_Albers"]'), + "basins.shx": strToU8("SHXBODY"), +}; + +describe("createByteBudget", () => { + // The accounting is tested directly because the archives fflate can build + // always declare their sizes. A streamed archive that declares none takes the + // byte-counting path instead, and this is where that arithmetic lives. + it("accepts a total exactly at the ceiling", () => { + const budget = createByteBudget(100); + expect(budget.add(60)).toBe(true); + expect(budget.add(40)).toBe(true); + expect(budget.exceeded).toBe(false); + expect(budget.observed).toBe(100); + }); + + it("rejects the byte that crosses the ceiling and stays rejected", () => { + const budget = createByteBudget(100); + expect(budget.add(60)).toBe(true); + expect(budget.add(41)).toBe(false); + expect(budget.exceeded).toBe(true); + expect(budget.observed).toBe(101); + // Once over, it does not recover even if nothing more is added. + expect(budget.add(0)).toBe(false); + }); + + it("rejects a single addition larger than the whole ceiling", () => { + const budget = createByteBudget(100); + expect(budget.add(1000)).toBe(false); + expect(budget.observed).toBe(1000); + }); + + it("treats an unknown size as zero rather than NaN", () => { + const budget = createByteBudget(100); + expect(budget.add(undefined)).toBe(true); + expect(budget.observed).toBe(0); + }); +}); + +describe("unzipShapefileComponents", () => { + it("extracts the four components and decodes nothing", () => { + const result = unzipShapefileComponents(archive(MINIMAL), { + maxBytes: 10 * MB, + }); + expect(result.error).toBeUndefined(); + expect(Object.keys(result.components).sort()).toEqual([ + "dbf", + "prj", + "shp", + "shx", + ]); + // Buffers come back raw; decoding the .prj is the interpretation step's job. + expect(result.components.shp).toBeInstanceOf(Uint8Array); + expect(strFromU8(result.components.prj)).toContain("PROJCS"); + }); + + it("decompresses a deflate-compressed archive at all", () => { + // Without registering the inflate decoder, fflate's Unzip carries only a + // pass-through and every member of a real archive throws on start. This is + // the regression guard for that. + const compressible = { + "basins.shp": new Uint8Array(64 * 1024), + "basins.prj": strToU8('PROJCS["x"]'), + }; + const zipped = archive(compressible); + expect(zipped.length).toBeLessThan(64 * 1024); + + const result = unzipShapefileComponents(zipped, { maxBytes: 10 * MB }); + expect(result.error).toBeUndefined(); + expect(result.components.shp.length).toBe(64 * 1024); + }); + + it("tolerates components nested in a directory", () => { + const result = unzipShapefileComponents( + archive({ + "wbd/basins.shp": strToU8("SHPBODY"), + "wbd/basins.prj": strToU8('PROJCS["x"]'), + }), + { maxBytes: 10 * MB }, + ); + expect(result.error).toBeUndefined(); + expect(result.components.shp).toBeTruthy(); + }); + + it("ignores members that are not shapefile components", () => { + const result = unzipShapefileComponents( + archive({ + ...MINIMAL, + "readme.txt": strToU8("notes"), + "metadata.xml": strToU8(""), + }), + { maxBytes: 10 * MB }, + ); + expect(result.error).toBeUndefined(); + expect(Object.keys(result.components).sort()).toEqual([ + "dbf", + "prj", + "shp", + "shx", + ]); + }); + + it("refuses an archive whose declared component size exceeds the ceiling, before expanding it", () => { + // 8 MiB of zeros compresses to a few KB, so this is a real bomb ratio: the + // ceiling has to bind on the declared expansion, not on the transfer. + const zipped = archive({ + "basins.shp": new Uint8Array(8 * MB), + "basins.prj": strToU8('PROJCS["x"]'), + }); + expect(zipped.length).toBeLessThan(64 * 1024); + + const result = unzipShapefileComponents(zipped, { maxBytes: 1 * MB }); + + expect(result.components).toBeUndefined(); + expect(result.error.reason).toBe("too_large"); + expect(result.error.observed).toBeGreaterThanOrEqual(8 * MB); + expect(result.error.permitted).toBe(1 * MB); + // The message states both numbers, since the author's next move depends on + // how far over the source is. + expect(result.error.detail).toMatch(/8|permitted|MB/i); + }); + + it("refuses on the sum of components, not on any single one", () => { + const half = 600 * 1024; + const zipped = archive({ + "basins.shp": new Uint8Array(half), + "basins.dbf": new Uint8Array(half), + "basins.prj": strToU8('PROJCS["x"]'), + }); + const result = unzipShapefileComponents(zipped, { maxBytes: 1 * MB }); + expect(result.error.reason).toBe("too_large"); + }); + + it("does not expand a bomb hidden in an irrelevant member", () => { + // Members that are not shapefile components are never started, so a bomb + // parked in one costs nothing and must not fail the archive either. + const zipped = archive({ + ...MINIMAL, + "bomb.bin": new Uint8Array(64 * MB), + }); + const result = unzipShapefileComponents(zipped, { maxBytes: 1 * MB }); + expect(result.error).toBeUndefined(); + expect(result.components.shp).toBeTruthy(); + }); + + it("reports an archive carrying more than one .shp rather than choosing", () => { + const result = unzipShapefileComponents( + archive({ + "basins.shp": strToU8("A"), + "gages.shp": strToU8("B"), + "basins.prj": strToU8('PROJCS["x"]'), + }), + { maxBytes: 10 * MB }, + ); + expect(result.components).toBeUndefined(); + expect(result.error.reason).toBe("ambiguous_archive"); + expect(result.error.detail).toContain("basins.shp"); + expect(result.error.detail).toContain("gages.shp"); + }); + + it("reports an archive with no .shp at all", () => { + const result = unzipShapefileComponents( + archive({ "readme.txt": strToU8("nothing here") }), + { maxBytes: 10 * MB }, + ); + expect(result.error.reason).toBe("no_shapefile"); + }); + + it("reports a buffer that is not an archive rather than throwing", () => { + const result = unzipShapefileComponents(strToU8("404"), { + maxBytes: 10 * MB, + }); + expect(result.components).toBeUndefined(); + expect(result.error.reason).toBe("unreadable_archive"); + expect(result.error.stage).toBe("parse"); + }); +}); diff --git a/reactapp/components/map/shapefile/acquire.js b/reactapp/components/map/shapefile/acquire.js new file mode 100644 index 00000000..5097613b --- /dev/null +++ b/reactapp/components/map/shapefile/acquire.js @@ -0,0 +1,200 @@ +import { + validateSourceUrl, + deriveSiblingUrls, +} from "components/map/shapefile/siblings"; +import { + unzipShapefileComponents, + createByteBudget, +} from "components/map/shapefile/unzip"; +import { + getCachedComponents, + setCachedComponents, +} from "components/map/shapefile/cache"; + +// The ceiling on how much a shapefile is allowed to expand to. Applied +// identically here and at view time, because the two paths share this module -- +// a single number is what keeps an author from saving a layer viewers cannot +// load. +export const DEFAULT_MAX_BYTES = 25 * 1024 * 1024; + +// A browser cannot tell these apart: a cross-origin refusal, an unreachable +// host, a missing file and an expired signature all surface as the same opaque +// rejection, with no status and no body. So the message names them together +// rather than picking one and being wrong. +const FETCH_STAGE_CAUSES = + "The likely causes are missing cross-origin headers on the host, an unreachable host, a URL that no longer exists, or an expired signature on a signed URL."; + +function fetchFailure(reason, detail, extra = {}) { + return { error: { stage: "fetch", reason, detail, ...extra } }; +} + +// A host returning an HTML error page with a success status is common on the +// portal class this feature targets, and it would otherwise reach the parser as +// geometry. Only markup is rejected: .prj is legitimately text, and archives are +// served as everything from application/zip to octet-stream to nothing at all, +// so allow-listing would break more hosts than it protects. +function isMarkup(contentType) { + if (!contentType) return false; + return /^\s*(text\/html|application\/xhtml)/i.test(contentType); +} + +function wasAborted(error, signal) { + return signal?.aborted || error?.name === "AbortError"; +} + +async function fetchBytes(url, signal) { + let response; + try { + response = await fetch(url, { signal }); + } catch (error) { + if (wasAborted(error, signal)) return { cancelled: true }; + return fetchFailure( + "unreachable", + `The shapefile could not be fetched. ${FETCH_STAGE_CAUSES}`, + ); + } + + const contentType = response.headers?.get?.("content-type") ?? ""; + if (response.ok && isMarkup(contentType)) { + return { + error: { + stage: "parse", + reason: "wrong_content_type", + detail: `The host returned "${contentType}" rather than shapefile data. A portal error page served with a success status is the usual cause.`, + }, + }; + } + + if (!response.ok) { + return { status: response.status, response }; + } + + try { + // Read the whole body rather than streaming it. Aborting rejects this and + // terminates the transfer, which is what cancellation needs, and the + // configured test environment exposes no response stream at all -- so a + // stream-reader implementation could not be exercised. + const buffer = new Uint8Array(await response.arrayBuffer()); + return { status: response.status, bytes: buffer }; + } catch (error) { + if (wasAborted(error, signal)) return { cancelled: true }; + return fetchFailure( + "unreachable", + `The shapefile transfer did not complete. ${FETCH_STAGE_CAUSES}`, + ); + } +} + +async function acquireArchive(url, signal, maxBytes) { + const fetched = await fetchBytes(url, signal); + if (fetched.cancelled || fetched.error) return fetched; + if (fetched.status && fetched.status >= 400) { + return fetchFailure( + "unreachable", + `The shapefile request returned ${fetched.status}. ${FETCH_STAGE_CAUSES}`, + { status: fetched.status }, + ); + } + return unzipShapefileComponents(fetched.bytes, { maxBytes }); +} + +async function acquireSiblings(url, signal, maxBytes) { + const derived = deriveSiblingUrls(url); + const budget = createByteBudget(maxBytes); + const components = {}; + + // Sequential rather than concurrent: the .shp is required, so there is no + // point paying for the other three before knowing it exists, and a shared + // budget is simpler to reason about when only one request is in flight. + for (const extension of ["shp", "dbf", "prj", "shx"]) { + const fetched = await fetchBytes(derived[extension], signal); + if (fetched.cancelled) return fetched; + if (fetched.error) { + // A missing optional component is not an error; a malformed one is. + if (fetched.error.stage === "parse" && extension !== "shp") continue; + return fetched; + } + + if (fetched.status === 404) { + // Absence is only meaningful for the optional components. What matters is + // that absence and failure stay distinguishable: a transient 403 routed + // into the "no projection supplied" fallback would render features at the + // wrong location with no error at all. + if (extension === "shp") { + return fetchFailure( + "unreachable", + `No shapefile was found at ${derived.shp}. ${FETCH_STAGE_CAUSES}`, + { status: 404 }, + ); + } + continue; + } + + if (fetched.status >= 400) { + return { + error: { + stage: "fetch", + reason: "component_status", + component: extension, + status: fetched.status, + detail: `The .${extension} component returned ${fetched.status}. It is not being treated as absent, because that would silently change how the layer is drawn.`, + }, + }; + } + + if (!budget.add(fetched.bytes.length)) { + const mb = (bytes) => (bytes / (1024 * 1024)).toFixed(1); + return { + error: { + stage: "fetch", + reason: "too_large", + observed: budget.observed, + permitted: budget.permitted, + detail: `The shapefile components total at least ${mb( + budget.observed, + )} MB, above the ${mb(budget.permitted)} MB permitted.`, + }, + }; + } + + components[extension] = fetched.bytes; + } + + return { components }; +} + +/** + * Fetch a shapefile's component bytes, from either a zipped archive or an + * unzipped set of siblings. + * + * Knows nothing about what a shapefile means -- it returns raw buffers, and + * interpreting them is a separate step. That split is deliberate: the parser + * choice carries real risk, and the byte accounting and cancellation contract + * here survive a parser swap intact. + * + * @param {string} rawUrl The author-supplied URL, already interpolated. + * @param {{signal?: AbortSignal, maxBytes?: number}} [options] + * @returns {Promise<{components: Record, fromCache?: boolean} + * |{error: object}|{cancelled: true}>} + */ +export async function acquireComponents( + rawUrl, + { signal, maxBytes = DEFAULT_MAX_BYTES } = {}, +) { + const validated = validateSourceUrl(rawUrl); + if (validated.error) return validated; + + const cached = getCachedComponents(validated.url); + if (cached) return { components: cached, fromCache: true }; + + if (signal?.aborted) return { cancelled: true }; + + const result = + validated.form === "archive" + ? await acquireArchive(validated.url, signal, maxBytes) + : await acquireSiblings(validated.url, signal, maxBytes); + + // A failed acquisition is never cached, so a retry actually retries. + if (result.components) setCachedComponents(validated.url, result.components); + return result; +} diff --git a/reactapp/components/map/shapefile/cache.js b/reactapp/components/map/shapefile/cache.js new file mode 100644 index 00000000..7903ba87 --- /dev/null +++ b/reactapp/components/map/shapefile/cache.js @@ -0,0 +1,64 @@ +// Cache of decompressed shapefile components, keyed on resolved URL. +// +// Layer preservation keeps a layer from refetching when nothing about it +// changed, but it only helps while the resolved URL stays the same. A variable +// input driving the URL refetches the whole archive even when toggling back to a +// value loaded seconds earlier, which is a common interaction on a +// variable-input dashboard -- and that is what this covers. +// +// It caches component buffers rather than parsed features deliberately. Buffers +// are already under the size ceiling by construction, so a small entry count has +// an exact memory bound; parsed GeoJSON runs several times the archive size, and +// any honest byte cap on that would hold about one entry. A hit skips the +// network hop and the decompression -- the slow, failure-prone part -- and still +// re-parses, which is fast and deterministic. +// +// Scope is the browser session. Entries persist across dashboards visited in one +// tab, which is correct: the bytes at a URL do not depend on which dashboard +// asked for them. A host that changes content mid-session serves the cached copy +// until eviction. +export const CACHE_MAX_ENTRIES = 3; + +// Insertion-ordered, so the first key is the least recently used. +const entries = new Map(); + +/** + * Look up cached components, marking the entry as most recently used. + * + * @param {string} key Resolved source URL. + * @returns {Record|null} + */ +export function getCachedComponents(key) { + if (!entries.has(key)) return null; + const components = entries.get(key); + // Re-insert to move it to the end of the eviction order. + entries.delete(key); + entries.set(key, components); + return components; +} + +/** + * Store components against a resolved URL, evicting the least recently used + * entry once the cache is full. + * + * @param {string} key Resolved source URL. + * @param {Record} components + */ +export function setCachedComponents(key, components) { + if (entries.has(key)) entries.delete(key); + entries.set(key, components); + while (entries.size > CACHE_MAX_ENTRIES) { + const oldest = entries.keys().next().value; + entries.delete(oldest); + } +} + +/** Empty the cache. Exists for tests; nothing in the app needs it. */ +export function clearComponentCache() { + entries.clear(); +} + +/** Current entry count. Exists for tests. */ +export function cachedComponentCount() { + return entries.size; +} diff --git a/reactapp/components/map/shapefile/siblings.js b/reactapp/components/map/shapefile/siblings.js new file mode 100644 index 00000000..07d2653c --- /dev/null +++ b/reactapp/components/map/shapefile/siblings.js @@ -0,0 +1,99 @@ +// The components of an unzipped shapefile. `shp` carries geometry, `dbf` +// attributes, `prj` the coordinate reference system as WKT, and `shx` the record +// index. +export const COMPONENT_EXTENSIONS = ["shp", "dbf", "prj", "shx"]; + +const ALLOWED_PROTOCOLS = ["http:", "https:"]; + +function failure(reason, detail) { + return { error: { stage: "fetch", reason, detail } }; +} + +// The extension of the final path segment, lower-cased, or "" when there is +// none. Read from the path alone: a download endpoint whose query string says +// `format=shp` is not a .shp path, and a .zip path carrying a cache token still +// is an archive. +function pathExtension(url) { + const segments = url.pathname.split("/"); + const last = segments[segments.length - 1]; + const dot = last.lastIndexOf("."); + return dot === -1 ? "" : last.slice(dot + 1).toLowerCase(); +} + +/** + * Decide whether a source URL is usable, and which of the two accepted forms it + * is. + * + * Runs before any fetch. The scheme restriction is the point: an author-supplied + * `data:` URI would carry an entire base64 archive into the saved layer + * configuration, which is exactly the storage accumulation that referencing a + * remote URL exists to avoid. + * + * @param {string} rawUrl The author-supplied URL. + * @returns {{form: "archive"|"components", url: string}|{error: object}} + */ +export function validateSourceUrl(rawUrl) { + if (typeof rawUrl !== "string" || rawUrl.trim() === "") { + return failure("empty", "No shapefile URL was supplied."); + } + + const trimmed = rawUrl.trim(); + + // Checked before parsing, because a protocol-relative URL has no protocol to + // report and would otherwise surface as an unhelpful malformed-URL error. + if (trimmed.startsWith("//")) { + return failure( + "unsupported_scheme", + "A protocol-relative URL is not accepted. Use an http:// or https:// URL.", + ); + } + + let url; + try { + url = new URL(trimmed); + } catch { + return failure("malformed_url", `"${trimmed}" is not a valid URL.`); + } + + if (!ALLOWED_PROTOCOLS.includes(url.protocol)) { + return failure( + "unsupported_scheme", + `The scheme "${url.protocol}" is not accepted. Use an http:// or https:// URL.`, + ); + } + + const extension = pathExtension(url); + if (extension === "zip") return { form: "archive", url: trimmed }; + if (extension === "shp") return { form: "components", url: trimmed }; + + return failure( + "unsupported_path", + "The URL path must end in .zip for a zipped shapefile, or .shp for an unzipped one.", + ); +} + +/** + * Derive the sibling component URLs from a `.shp` URL. + * + * Only the final path segment's extension is replaced; the query string and + * fragment are carried through untouched. Presigned links compute their + * signature over the object key and portal links carry cache tokens, so + * rewriting anything outside the path corrupts the request. + * + * @param {string} shpUrl A validated `.shp` URL. + * @returns {Record} One URL per component extension. + */ +export function deriveSiblingUrls(shpUrl) { + const url = new URL(shpUrl); + const segments = url.pathname.split("/"); + const last = segments[segments.length - 1]; + const stem = last.slice(0, last.lastIndexOf(".")); + + return COMPONENT_EXTENSIONS.reduce((derived, extension) => { + const rebuilt = new URL(url.toString()); + rebuilt.pathname = [...segments.slice(0, -1), `${stem}.${extension}`].join( + "/", + ); + return { ...derived, [extension]: rebuilt.toString() }; + }, {}); +} diff --git a/reactapp/components/map/shapefile/unzip.js b/reactapp/components/map/shapefile/unzip.js new file mode 100644 index 00000000..a12a2428 --- /dev/null +++ b/reactapp/components/map/shapefile/unzip.js @@ -0,0 +1,194 @@ +import { Unzip, UnzipInflate } from "fflate"; +import { COMPONENT_EXTENSIONS } from "components/map/shapefile/siblings"; + +/** + * Running total against a ceiling, for bounding how much a source is allowed to + * expand to. + * + * Separate from the unzip loop because the archives available to a test always + * declare their member sizes, while a streamed archive declares none and takes + * the byte-counting path instead. Keeping the arithmetic here makes both + * testable. + * + * @param {number} maxBytes The ceiling, in bytes. + */ +export function createByteBudget(maxBytes) { + return { + observed: 0, + exceeded: false, + permitted: maxBytes, + add(bytes) { + this.observed += Number.isFinite(bytes) ? bytes : 0; + if (this.observed > maxBytes) this.exceeded = true; + return !this.exceeded; + }, + }; +} + +function componentExtension(name) { + const base = name.split("/").pop() ?? ""; + const dot = base.lastIndexOf("."); + if (dot === -1) return null; + const extension = base.slice(dot + 1).toLowerCase(); + return COMPONENT_EXTENSIONS.includes(extension) ? extension : null; +} + +function tooLarge(budget) { + const mb = (bytes) => (bytes / (1024 * 1024)).toFixed(1); + return { + error: { + stage: "fetch", + reason: "too_large", + observed: budget.observed, + permitted: budget.permitted, + detail: `The shapefile expands to at least ${mb( + budget.observed, + )} MB, above the ${mb(budget.permitted)} MB permitted.`, + }, + }; +} + +/** + * Extract the shapefile components from a zipped archive, bounded by how much + * they are allowed to expand to. + * + * The ceiling is applied to each member's *declared* size, read from the local + * header before any data flows, and refused by simply never starting that + * member. Summing bytes as they arrive does not work: a 200 MB expansion can + * arrive in a single callback, so a running total notices it only once the whole + * payload is already allocated and inflated -- which is the cost the ceiling + * exists to prevent. Members with no declared size fall back to counting. + * + * Only shapefile components are ever started, so a bomb parked in an unrelated + * member costs nothing. + * + * @param {Uint8Array} buffer The archive bytes. + * @param {{maxBytes: number}} options + * @returns {{components: Record}|{error: object}} + */ +export function unzipShapefileComponents(buffer, { maxBytes }) { + // Every zip starts "PK" -- 0x03 0x04 for a normal entry, 0x05 0x06 for an + // empty archive, 0x07 0x08 for a spanned one. Checked up front because pushing + // something else into Unzip does not fail: it simply finds no entries, and the + // author would be told the archive has no .shp entry when the real answer is + // that a portal returned an HTML error page with a 200. + if (!(buffer?.length >= 4 && buffer[0] === 0x50 && buffer[1] === 0x4b)) { + return { + error: { + stage: "parse", + reason: "unreadable_archive", + detail: + "The source is not a zip archive. A portal returning an error page with a success status is the usual cause.", + }, + }; + } + + const components = {}; + const shpMembers = []; + const budget = createByteBudget(maxBytes); + let failure = null; + + const unzip = new Unzip(); + // Without this, Unzip carries only a pass-through decoder and every member of + // a deflate-compressed archive -- which is to say every real archive -- throws + // on start. + unzip.register(UnzipInflate); + + unzip.onfile = (file) => { + const extension = componentExtension(file.name); + if (!extension || failure) return; + if (extension === "shp") shpMembers.push(file.name); + + const declared = file.originalSize; + if (Number.isFinite(declared) && declared > 0) { + if (!budget.add(declared)) { + failure = tooLarge(budget); + return; + } + } + + const chunks = []; + const declaredKnown = Number.isFinite(declared) && declared > 0; + file.ondata = (error, chunk, final) => { + if (error) { + failure = failure ?? { + error: { + stage: "parse", + reason: "unreadable_component", + detail: `The "${file.name}" entry could not be read: ${error.message}`, + }, + }; + return; + } + // Counted only when the header declared nothing, so a declared member is + // not charged twice. + if (!declaredKnown) { + if (!budget.add(chunk.length)) { + failure = tooLarge(budget); + file.terminate?.(); + return; + } + } + chunks.push(chunk); + if (final) { + const total = chunks.reduce((sum, part) => sum + part.length, 0); + const merged = new Uint8Array(total); + chunks.reduce((offset, part) => { + merged.set(part, offset); + return offset + part.length; + }, 0); + components[extension] = merged; + } + }; + + try { + file.start(); + } catch (error) { + failure = failure ?? { + error: { + stage: "parse", + reason: "unreadable_component", + detail: `The "${file.name}" entry could not be decompressed: ${error.message}`, + }, + }; + } + }; + + try { + unzip.push(buffer, true); + } catch (error) { + return { + error: { + stage: "parse", + reason: "unreadable_archive", + detail: `The source could not be read as a zip archive: ${error.message}`, + }, + }; + } + + if (failure) return failure; + + if (shpMembers.length > 1) { + return { + error: { + stage: "parse", + reason: "ambiguous_archive", + detail: `The archive contains more than one shapefile (${shpMembers.join( + ", ", + )}). Point the URL at a single shapefile instead.`, + }, + }; + } + + if (!components.shp) { + return { + error: { + stage: "parse", + reason: "no_shapefile", + detail: "The archive contains no .shp entry.", + }, + }; + } + + return { components }; +} From 9a81177ce596e27bd186b61335fbc24d9b4db3e4 Mon Sep 17 00:00:00 2001 From: Corey Krewson Date: Tue, 25 Aug 2026 10:16:28 -0700 Subject: [PATCH 03/18] feat(map): interpret shapefile components into a georeferenced collection Parses the geometry, resolves the coordinate reference, normalizes ring winding, and returns a plain GeoJSON feature collection whose `crs` names its projection -- the same payload shape the existing vector-swap path already consumes, so the two vector paths stay interchangeable and no OpenLayers object is built here. The parser choice is now settled rather than assumed. Fixtures are real shapefile bytes generated by pyshp, and the expectations come from pyshp reading them back through its own independent ring-nesting implementation -- so the fidelity test is a cross-implementation check. It passes: a polygon with an interior ring reads as one polygon with a hole, and a multi-part record as one MultiPolygon, both matching pyshp coordinate for coordinate. That was the largest risk in the plan. The browser build is imported by name. The package resolves to a Node build under the test runner and a browser build under the bundler, and a fidelity guarantee measured against an artifact that never ships is worth nothing. That needs TextDecoder, which jsdom lacks, so setupTests now supplies it -- which also moves tests closer to a real browser than the Node build's bundled decoder. Absence and failure stay distinct. A genuinely missing .prj falls back to the author-supplied projection; a .prj that is present but unresolvable is reported, never quietly replaced by the fallback, because the file said what it was and drawing it with a guessed projection would put the features somewhere else with no error. One test-harness trap worth recording: with a native TextEncoder available, fflate's strToU8 returns a Uint8Array from the Node realm, so fflate's own instanceof check fails and zipSync recurses into the byte indices, producing archive members named "basins.shp/0/" instead of a file. Fixtures now build their bytes in the test realm. Only fixture construction was affected -- nothing in the application builds archives. Co-Authored-By: Claude Opus 5 (1M context) --- .../components/map/shapefile/acquire.test.js | 17 +- .../components/map/shapefile/index.test.js | 209 ++++++++++++++++++ .../components/map/shapefile/unzip.test.js | 37 ++-- reactapp/__tests__/setupTests.js | 10 + reactapp/__tests__/utilities/bytes.js | 19 ++ .../utilities/fixtures/shapefile/generate.py | 80 +++++++ .../utilities/fixtures/shapefile/holes.dbf | Bin 0 -> 150 bytes .../utilities/fixtures/shapefile/holes.prj | 1 + .../utilities/fixtures/shapefile/holes.shp | Bin 0 -> 320 bytes .../utilities/fixtures/shapefile/holes.shx | Bin 0 -> 108 bytes .../fixtures/shapefile/multipart.dbf | Bin 0 -> 106 bytes .../fixtures/shapefile/multipart.prj | 1 + .../fixtures/shapefile/multipart.shp | Bin 0 -> 320 bytes .../fixtures/shapefile/multipart.shx | Bin 0 -> 108 bytes .../utilities/fixtures/shapefile/points.dbf | Bin 0 -> 139 bytes .../utilities/fixtures/shapefile/points.prj | 1 + .../utilities/fixtures/shapefile/points.shp | Bin 0 -> 156 bytes .../utilities/fixtures/shapefile/points.shx | Bin 0 -> 116 bytes reactapp/components/map/shapefile/index.js | 134 +++++++++++ 19 files changed, 483 insertions(+), 26 deletions(-) create mode 100644 reactapp/__tests__/components/map/shapefile/index.test.js create mode 100644 reactapp/__tests__/utilities/bytes.js create mode 100644 reactapp/__tests__/utilities/fixtures/shapefile/generate.py create mode 100644 reactapp/__tests__/utilities/fixtures/shapefile/holes.dbf create mode 100644 reactapp/__tests__/utilities/fixtures/shapefile/holes.prj create mode 100644 reactapp/__tests__/utilities/fixtures/shapefile/holes.shp create mode 100644 reactapp/__tests__/utilities/fixtures/shapefile/holes.shx create mode 100644 reactapp/__tests__/utilities/fixtures/shapefile/multipart.dbf create mode 100644 reactapp/__tests__/utilities/fixtures/shapefile/multipart.prj create mode 100644 reactapp/__tests__/utilities/fixtures/shapefile/multipart.shp create mode 100644 reactapp/__tests__/utilities/fixtures/shapefile/multipart.shx create mode 100644 reactapp/__tests__/utilities/fixtures/shapefile/points.dbf create mode 100644 reactapp/__tests__/utilities/fixtures/shapefile/points.prj create mode 100644 reactapp/__tests__/utilities/fixtures/shapefile/points.shp create mode 100644 reactapp/__tests__/utilities/fixtures/shapefile/points.shx create mode 100644 reactapp/components/map/shapefile/index.js diff --git a/reactapp/__tests__/components/map/shapefile/acquire.test.js b/reactapp/__tests__/components/map/shapefile/acquire.test.js index da580371..f165f7f2 100644 --- a/reactapp/__tests__/components/map/shapefile/acquire.test.js +++ b/reactapp/__tests__/components/map/shapefile/acquire.test.js @@ -1,4 +1,5 @@ -import { zipSync, strToU8 } from "fflate"; +import { zipSync } from "fflate"; +import { bytes } from "../../../utilities/bytes"; import { acquireComponents, DEFAULT_MAX_BYTES, @@ -11,10 +12,10 @@ import { const MB = 1024 * 1024; const ARCHIVE = zipSync({ - "basins.shp": strToU8("SHPBODY"), - "basins.dbf": strToU8("DBFBODY"), - "basins.prj": strToU8('PROJCS["NAD_1983_Albers"]'), - "basins.shx": strToU8("SHXBODY"), + "basins.shp": bytes("SHPBODY"), + "basins.dbf": bytes("DBFBODY"), + "basins.prj": bytes('PROJCS["NAD_1983_Albers"]'), + "basins.shx": bytes("SHXBODY"), }); // Minimal Response stand-in. The configured environment exposes no response @@ -77,7 +78,7 @@ describe("acquireComponents — archive form", () => { fetchMock.mockResolvedValue( respond({ contentType: "text/html; charset=utf-8", - body: strToU8(""), + body: bytes(""), }), ); @@ -112,7 +113,7 @@ describe("acquireComponents — archive form", () => { it("refuses an archive that expands past the ceiling", async () => { const bomb = zipSync({ "basins.shp": new Uint8Array(8 * MB), - "basins.prj": strToU8('PROJCS["x"]'), + "basins.prj": bytes('PROJCS["x"]'), }); fetchMock.mockResolvedValue(respond({ body: bomb })); @@ -133,7 +134,7 @@ describe("acquireComponents — sibling form", () => { return Promise.resolve( respond({ contentType: "application/octet-stream", - body: strToU8(`${extension.toUpperCase()}BODY`), + body: bytes(`${extension.toUpperCase()}BODY`), }), ); }; diff --git a/reactapp/__tests__/components/map/shapefile/index.test.js b/reactapp/__tests__/components/map/shapefile/index.test.js new file mode 100644 index 00000000..82d85fa6 --- /dev/null +++ b/reactapp/__tests__/components/map/shapefile/index.test.js @@ -0,0 +1,209 @@ +import fs from "fs"; +import path from "path"; +import { bytes } from "../../../utilities/bytes"; +import { interpretShapefile } from "components/map/shapefile/index"; + +// Read from disk rather than imported: the asset transform turns a non-JS import +// into its filename string, so a binary fixture cannot be `import`ed. +const FIXTURES = path.join(__dirname, "../../../utilities/fixtures/shapefile"); + +function load(name) { + const components = {}; + ["shp", "dbf", "prj", "shx"].forEach((extension) => { + const file = path.join(FIXTURES, `${name}.${extension}`); + if (fs.existsSync(file)) { + components[extension] = new Uint8Array(fs.readFileSync(file)); + } + }); + return components; +} + +describe("interpretShapefile — geometry fidelity", () => { + // These expectations come from pyshp reading the same files back through its + // own ring-nesting implementation, which makes this a cross-implementation + // check. Shapefile encodes interior rings by winding direction with no parent + // pointer, so a parser that re-derives containment wrongly draws a basin's + // holes as filled polygons on top of it -- and the better-known JS parser has + // a filed bug for exactly that. This is the gate on the parser choice. + it("reads a polygon with an interior ring as one polygon with a hole", async () => { + const result = await interpretShapefile(load("holes")); + + expect(result.error).toBeUndefined(); + const [feature] = result.featureCollection.features; + expect(feature.geometry.type).toBe("Polygon"); + // Two rings on one polygon -- not two separate polygons, and not a + // MultiPolygon. + expect(feature.geometry.coordinates).toHaveLength(2); + const [exterior, interior] = feature.geometry.coordinates; + expect(exterior).toHaveLength(5); + expect(interior).toHaveLength(5); + // The interior ring is the inner square, whatever winding it ended up with. + const interiorXs = interior.map(([x]) => x).sort((a, b) => a - b); + expect(interiorXs[0]).toBe(3); + expect(interiorXs[interiorXs.length - 1]).toBe(7); + }); + + it("reads a multi-part record as one multi-geometry feature", async () => { + const result = await interpretShapefile(load("multipart")); + + expect(result.error).toBeUndefined(); + // One record in, one feature out -- not two features. + expect(result.featureCollection.features).toHaveLength(1); + const [feature] = result.featureCollection.features; + expect(feature.geometry.type).toBe("MultiPolygon"); + expect(feature.geometry.coordinates).toHaveLength(2); + expect(feature.properties.NAME).toBe("Two islands, one record"); + }); + + it("normalizes ring winding to the GeoJSON spec", async () => { + // Shapefile writes exterior rings clockwise; the GeoJSON spec wants them + // counter-clockwise. Signed area is positive for a counter-clockwise ring. + const result = await interpretShapefile(load("holes")); + const [exterior] = + result.featureCollection.features[0].geometry.coordinates; + const area = exterior.reduce((sum, [x1, y1], index) => { + const [x2, y2] = exterior[(index + 1) % exterior.length]; + return sum + (x1 * y2 - x2 * y1); + }, 0); + expect(area).toBeGreaterThan(0); + }); + + it("reads points with their attributes", async () => { + const result = await interpretShapefile(load("points")); + + expect(result.featureCollection.features).toHaveLength(2); + expect(result.featureCollection.features[0].geometry.type).toBe("Point"); + expect(result.featureCollection.features[0].properties).toEqual({ + GAGE_ID: "06730200", + STAGE_FT: 4.25, + }); + }); +}); + +describe("interpretShapefile — payload contract", () => { + it("names the resolved projection on the collection's crs", async () => { + const result = await interpretShapefile(load("holes")); + // The existing vector-swap path reads dataProjection from exactly here, so + // both vector paths produce interchangeable payloads. + expect(result.featureCollection.crs.properties.name).toBe( + result.projectionCode, + ); + expect(result.projectionCode).toMatch(/^WKT:/); + }); + + it("returns plain GeoJSON with no OpenLayers objects", async () => { + const result = await interpretShapefile(load("holes")); + expect(result.featureCollection.type).toBe("FeatureCollection"); + // Round-trips through JSON, which an OpenLayers feature would not. + expect(() => JSON.stringify(result.featureCollection)).not.toThrow(); + }); +}); + +describe("interpretShapefile — attributes", () => { + it("reads geometry with no attributes when the .dbf is absent", async () => { + const components = load("holes"); + delete components.dbf; + + const result = await interpretShapefile(components); + + expect(result.error).toBeUndefined(); + expect(result.featureCollection.features[0].geometry.type).toBe("Polygon"); + expect(result.featureCollection.features[0].properties).toEqual({}); + }); +}); + +describe("interpretShapefile — projection resolution", () => { + it("registers the .prj and uses it", async () => { + const result = await interpretShapefile(load("holes")); + expect(result.error).toBeUndefined(); + expect(result.projectionCode).toBeTruthy(); + }); + + it("falls back to the supplied projection when there is no .prj", async () => { + const components = load("holes"); + delete components.prj; + + const result = await interpretShapefile(components, { + fallbackProjection: "EPSG:5070", + }); + + expect(result.error).toBeUndefined(); + expect(result.projectionCode).toBe("EPSG:5070"); + expect(result.featureCollection.crs.properties.name).toBe("EPSG:5070"); + }); + + it("reports a missing projection with no fallback rather than guessing", async () => { + const components = load("holes"); + delete components.prj; + + const result = await interpretShapefile(components); + + expect(result.featureCollection).toBeUndefined(); + expect(result.error.reason).toBe("missing_projection"); + expect(result.error.stage).toBe("parse"); + }); + + it("reports an unresolvable fallback projection by name", async () => { + const components = load("holes"); + delete components.prj; + + const result = await interpretShapefile(components, { + fallbackProjection: "EPSG:99999", + }); + + expect(result.error.reason).toBe("unresolvable_projection"); + expect(result.error.detail).toContain("EPSG:99999"); + }); + + it("reports an unresolvable .prj naming the projection method", async () => { + const components = load("holes"); + components.prj = bytes( + 'PROJCS["x",GEOGCS["g",DATUM["d",SPHEROID["s",6378137,298.257222101]],PRIMEM["Greenwich",0],UNIT["Degree",0.0174532925199433]],PROJECTION["Totally_Not_A_Real_Projection"],UNIT["Meter",1.0]]', + ); + + const result = await interpretShapefile(components); + + expect(result.error.reason).toBe("unresolvable_projection"); + expect(result.error.detail).toContain("Totally_Not_A_Real_Projection"); + }); + + it("does not fall back when a .prj is present but unresolvable", async () => { + // A present-but-broken .prj must not quietly become the fallback's problem: + // the file said what it was and the answer is to report it, not to draw the + // features using a projection the author guessed. + const components = load("holes"); + components.prj = bytes("not wkt at all"); + + const result = await interpretShapefile(components, { + fallbackProjection: "EPSG:5070", + }); + + expect(result.featureCollection).toBeUndefined(); + expect(result.error.reason).toBe("unresolvable_projection"); + }); +}); + +describe("interpretShapefile — failure paths", () => { + it("reports missing geometry", async () => { + const result = await interpretShapefile({ prj: bytes('PROJCS["x"]') }); + expect(result.error.reason).toBe("no_geometry"); + }); + + it("reports unreadable geometry rather than throwing", async () => { + const components = load("holes"); + components.shp = new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8]); + + const result = await interpretShapefile(components); + + expect(result.featureCollection).toBeUndefined(); + expect(result.error.reason).toBe("unreadable_geometry"); + expect(result.error.stage).toBe("parse"); + }); + + it("reports an empty components object", async () => { + expect((await interpretShapefile({})).error.reason).toBe("no_geometry"); + expect((await interpretShapefile(undefined)).error.reason).toBe( + "no_geometry", + ); + }); +}); diff --git a/reactapp/__tests__/components/map/shapefile/unzip.test.js b/reactapp/__tests__/components/map/shapefile/unzip.test.js index d73cca39..5ba977aa 100644 --- a/reactapp/__tests__/components/map/shapefile/unzip.test.js +++ b/reactapp/__tests__/components/map/shapefile/unzip.test.js @@ -1,4 +1,5 @@ -import { zipSync, strToU8, strFromU8 } from "fflate"; +import { zipSync } from "fflate"; +import { bytes, text } from "../../../utilities/bytes"; import { unzipShapefileComponents, createByteBudget, @@ -11,10 +12,10 @@ function archive(entries) { } const MINIMAL = { - "basins.shp": strToU8("SHPBODY"), - "basins.dbf": strToU8("DBFBODY"), - "basins.prj": strToU8('PROJCS["NAD_1983_Albers"]'), - "basins.shx": strToU8("SHXBODY"), + "basins.shp": bytes("SHPBODY"), + "basins.dbf": bytes("DBFBODY"), + "basins.prj": bytes('PROJCS["NAD_1983_Albers"]'), + "basins.shx": bytes("SHXBODY"), }; describe("createByteBudget", () => { @@ -66,7 +67,7 @@ describe("unzipShapefileComponents", () => { ]); // Buffers come back raw; decoding the .prj is the interpretation step's job. expect(result.components.shp).toBeInstanceOf(Uint8Array); - expect(strFromU8(result.components.prj)).toContain("PROJCS"); + expect(text(result.components.prj)).toContain("PROJCS"); }); it("decompresses a deflate-compressed archive at all", () => { @@ -75,7 +76,7 @@ describe("unzipShapefileComponents", () => { // the regression guard for that. const compressible = { "basins.shp": new Uint8Array(64 * 1024), - "basins.prj": strToU8('PROJCS["x"]'), + "basins.prj": bytes('PROJCS["x"]'), }; const zipped = archive(compressible); expect(zipped.length).toBeLessThan(64 * 1024); @@ -88,8 +89,8 @@ describe("unzipShapefileComponents", () => { it("tolerates components nested in a directory", () => { const result = unzipShapefileComponents( archive({ - "wbd/basins.shp": strToU8("SHPBODY"), - "wbd/basins.prj": strToU8('PROJCS["x"]'), + "wbd/basins.shp": bytes("SHPBODY"), + "wbd/basins.prj": bytes('PROJCS["x"]'), }), { maxBytes: 10 * MB }, ); @@ -101,8 +102,8 @@ describe("unzipShapefileComponents", () => { const result = unzipShapefileComponents( archive({ ...MINIMAL, - "readme.txt": strToU8("notes"), - "metadata.xml": strToU8(""), + "readme.txt": bytes("notes"), + "metadata.xml": bytes(""), }), { maxBytes: 10 * MB }, ); @@ -120,7 +121,7 @@ describe("unzipShapefileComponents", () => { // ceiling has to bind on the declared expansion, not on the transfer. const zipped = archive({ "basins.shp": new Uint8Array(8 * MB), - "basins.prj": strToU8('PROJCS["x"]'), + "basins.prj": bytes('PROJCS["x"]'), }); expect(zipped.length).toBeLessThan(64 * 1024); @@ -140,7 +141,7 @@ describe("unzipShapefileComponents", () => { const zipped = archive({ "basins.shp": new Uint8Array(half), "basins.dbf": new Uint8Array(half), - "basins.prj": strToU8('PROJCS["x"]'), + "basins.prj": bytes('PROJCS["x"]'), }); const result = unzipShapefileComponents(zipped, { maxBytes: 1 * MB }); expect(result.error.reason).toBe("too_large"); @@ -161,9 +162,9 @@ describe("unzipShapefileComponents", () => { it("reports an archive carrying more than one .shp rather than choosing", () => { const result = unzipShapefileComponents( archive({ - "basins.shp": strToU8("A"), - "gages.shp": strToU8("B"), - "basins.prj": strToU8('PROJCS["x"]'), + "basins.shp": bytes("A"), + "gages.shp": bytes("B"), + "basins.prj": bytes('PROJCS["x"]'), }), { maxBytes: 10 * MB }, ); @@ -175,14 +176,14 @@ describe("unzipShapefileComponents", () => { it("reports an archive with no .shp at all", () => { const result = unzipShapefileComponents( - archive({ "readme.txt": strToU8("nothing here") }), + archive({ "readme.txt": bytes("nothing here") }), { maxBytes: 10 * MB }, ); expect(result.error.reason).toBe("no_shapefile"); }); it("reports a buffer that is not an archive rather than throwing", () => { - const result = unzipShapefileComponents(strToU8("404"), { + const result = unzipShapefileComponents(bytes("404"), { maxBytes: 10 * MB, }); expect(result.components).toBeUndefined(); diff --git a/reactapp/__tests__/setupTests.js b/reactapp/__tests__/setupTests.js index 65d03009..de0beeac 100644 --- a/reactapp/__tests__/setupTests.js +++ b/reactapp/__tests__/setupTests.js @@ -95,3 +95,13 @@ HTMLCanvasElement.prototype.getContext = function () { jest.mock("uuid", () => ({ v4: () => 12345678, })); + +// jsdom ships no TextDecoder/TextEncoder, but every browser does. Without these, +// any dependency's browser build that decodes text -- the shapefile parser +// reading a .dbf, for one -- fails only under test. Supplying them lets tests +// exercise the same build the bundle ships rather than a Node-only fallback. +if (typeof global.TextDecoder === "undefined") { + const { TextDecoder, TextEncoder } = require("util"); + global.TextDecoder = TextDecoder; + global.TextEncoder = TextEncoder; +} diff --git a/reactapp/__tests__/utilities/bytes.js b/reactapp/__tests__/utilities/bytes.js new file mode 100644 index 00000000..d1065d1f --- /dev/null +++ b/reactapp/__tests__/utilities/bytes.js @@ -0,0 +1,19 @@ +// Build a Uint8Array in the test realm. +// +// fflate's `strToU8` uses the native TextEncoder when one is available, and +// under jsdom that returns a Uint8Array belonging to the Node realm. fflate's +// own `instanceof Uint8Array` check then fails, so `zipSync` treats the value as +// a plain object and recurses into its byte indices -- producing archive members +// named "basins.shp/0/" instead of a file. Constructing here keeps the array in +// the realm the code under test compares against. +// +// Only fixture construction is affected; reading bytes back with `strFromU8` +// works across realms, and nothing in the application builds archives. +export function bytes(text) { + return Uint8Array.from(text, (character) => character.charCodeAt(0)); +} + +/** Decode ASCII bytes back to a string, realm-independently. */ +export function text(byteArray) { + return Array.from(byteArray, (byte) => String.fromCharCode(byte)).join(""); +} diff --git a/reactapp/__tests__/utilities/fixtures/shapefile/generate.py b/reactapp/__tests__/utilities/fixtures/shapefile/generate.py new file mode 100644 index 00000000..f266af23 --- /dev/null +++ b/reactapp/__tests__/utilities/fixtures/shapefile/generate.py @@ -0,0 +1,80 @@ +"""Regenerate the shapefile test fixtures. + +Run from the repo root: python3 reactapp/__tests__/utilities/fixtures/shapefile/generate.py + +These are real shapefile bytes rather than hand-crafted ones, because the +behavior under test is how a parser interprets the format's own conventions -- +notably that polygon interior rings are encoded by winding direction with no +parent pointer. pyshp writes the files and also reads them back through its own +independent ring-nesting implementation, which is what makes the expected +GeoJSON in index.test.js a cross-implementation oracle rather than a +self-consistency check. +""" + +import json +import os + +import shapefile + +HERE = os.path.dirname(os.path.abspath(__file__)) + +ESRI_ALBERS_PRJ = ( + 'PROJCS["NAD_1983_Albers",GEOGCS["GCS_North_American_1983",' + 'DATUM["D_North_American_1983",SPHEROID["GRS_1980",6378137.0,298.257222101]],' + 'PRIMEM["Greenwich",0.0],UNIT["Degree",0.0174532925199433]],' + 'PROJECTION["Albers"],PARAMETER["False_Easting",0.0],' + 'PARAMETER["False_Northing",0.0],PARAMETER["Central_Meridian",-96.0],' + 'PARAMETER["Standard_Parallel_1",29.5],PARAMETER["Standard_Parallel_2",45.5],' + 'PARAMETER["Latitude_Of_Origin",23.0],UNIT["Meter",1.0]]' +) + +# Shapefile convention: exterior rings clockwise, interior rings counter-clockwise. +OUTER_CW = [(0, 0), (0, 10), (10, 10), (10, 0), (0, 0)] +HOLE_CCW = [(3, 3), (7, 3), (7, 7), (3, 7), (3, 3)] +ISLAND_A_CW = [(0, 0), (0, 4), (4, 4), (4, 0), (0, 0)] +ISLAND_B_CW = [(20, 20), (20, 24), (24, 24), (24, 20), (20, 20)] + + +def write(name, build): + path = os.path.join(HERE, name) + writer = shapefile.Writer(path) + build(writer) + writer.close() + with open(path + ".prj", "w") as handle: + handle.write(ESRI_ALBERS_PRJ) + with shapefile.Reader(path) as reader: + return { + "shapeType": reader.shapeTypeName, + "fields": [f[0] for f in reader.fields if f[0] != "DeletionFlag"], + "features": [s.__geo_interface__ for s in reader.shapeRecords()], + } + + +def holes(writer): + writer.field("NAME", "C", 40) + writer.field("AREASQKM", "N", 12, 3) + writer.poly([OUTER_CW, HOLE_CCW]) + writer.record("Basin with hole", 91.0) + + +def multipart(writer): + writer.field("NAME", "C", 40) + writer.poly([ISLAND_A_CW, ISLAND_B_CW]) + writer.record("Two islands, one record") + + +def points(writer): + writer.field("GAGE_ID", "C", 12) + writer.field("STAGE_FT", "N", 8, 2) + writer.point(-105.0, 40.0) + writer.record("06730200", 4.25) + writer.point(-104.5, 39.5) + writer.record("06730500", 2.5) + + +oracle = { + "holes": write("holes", holes), + "multipart": write("multipart", multipart), + "points": write("points", points), +} +print(json.dumps(oracle, indent=2, sort_keys=True)) diff --git a/reactapp/__tests__/utilities/fixtures/shapefile/holes.dbf b/reactapp/__tests__/utilities/fixtures/shapefile/holes.dbf new file mode 100644 index 0000000000000000000000000000000000000000..121a3ec248d0f012b3c26f398937397719e13cf1 GIT binary patch literal 150 zcmZRsjOFa?sBz|Yaw6)NfsqBYQzI0m^o1_yfk0)_oRQasG43V0Qq5{onQ X6v{J8G88iMb5a%X14~0a0|Ns9nN1Kr literal 0 HcmV?d00001 diff --git a/reactapp/__tests__/utilities/fixtures/shapefile/holes.prj b/reactapp/__tests__/utilities/fixtures/shapefile/holes.prj new file mode 100644 index 00000000..8cf77785 --- /dev/null +++ b/reactapp/__tests__/utilities/fixtures/shapefile/holes.prj @@ -0,0 +1 @@ +PROJCS["NAD_1983_Albers",GEOGCS["GCS_North_American_1983",DATUM["D_North_American_1983",SPHEROID["GRS_1980",6378137.0,298.257222101]],PRIMEM["Greenwich",0.0],UNIT["Degree",0.0174532925199433]],PROJECTION["Albers"],PARAMETER["False_Easting",0.0],PARAMETER["False_Northing",0.0],PARAMETER["Central_Meridian",-96.0],PARAMETER["Standard_Parallel_1",29.5],PARAMETER["Standard_Parallel_2",45.5],PARAMETER["Latitude_Of_Origin",23.0],UNIT["Meter",1.0]] \ No newline at end of file diff --git a/reactapp/__tests__/utilities/fixtures/shapefile/holes.shp b/reactapp/__tests__/utilities/fixtures/shapefile/holes.shp new file mode 100644 index 0000000000000000000000000000000000000000..b1b6f66c3301487f6838adf69233245d96520dd1 GIT binary patch literal 320 zcmZQzQ0HR64i>y%W?*2&E(a7|;Z_ea5(Hp&y%W?*2&E(a8~aDYg`Xq*Z`5{y8cMT}WYK!q>|;Z_ea5(ESsz!Xjz VB8y3yK=q;1Fu&lU(bbzc002p|3Hty5 literal 0 HcmV?d00001 diff --git a/reactapp/__tests__/utilities/fixtures/shapefile/multipart.shx b/reactapp/__tests__/utilities/fixtures/shapefile/multipart.shx new file mode 100644 index 0000000000000000000000000000000000000000..3dee151992e81ef0b3f25003e1087559e2b54002 GIT binary patch literal 108 lcmZQzQ0HR64$NLKGcd4XmjjAgI6$OeG){#e2_qoR0sx$^0^|Sy literal 0 HcmV?d00001 diff --git a/reactapp/__tests__/utilities/fixtures/shapefile/points.dbf b/reactapp/__tests__/utilities/fixtures/shapefile/points.dbf new file mode 100644 index 0000000000000000000000000000000000000000..db10e7e27cd832e294cb4738598a5345f2b8b895 GIT binary patch literal 139 zcmZRsjO5CxK$z}?Z^HQv(&B;gDqct8Xa2o3=$a0>wn`GJHvK!za!UIha) Zb7KP|0|NypFwrwIg$SA=1da4e4FKO34KV-! literal 0 HcmV?d00001 diff --git a/reactapp/__tests__/utilities/fixtures/shapefile/points.prj b/reactapp/__tests__/utilities/fixtures/shapefile/points.prj new file mode 100644 index 00000000..8cf77785 --- /dev/null +++ b/reactapp/__tests__/utilities/fixtures/shapefile/points.prj @@ -0,0 +1 @@ +PROJCS["NAD_1983_Albers",GEOGCS["GCS_North_American_1983",DATUM["D_North_American_1983",SPHEROID["GRS_1980",6378137.0,298.257222101]],PRIMEM["Greenwich",0.0],UNIT["Degree",0.0174532925199433]],PROJECTION["Albers"],PARAMETER["False_Easting",0.0],PARAMETER["False_Northing",0.0],PARAMETER["Central_Meridian",-96.0],PARAMETER["Standard_Parallel_1",29.5],PARAMETER["Standard_Parallel_2",45.5],PARAMETER["Latitude_Of_Origin",23.0],UNIT["Meter",1.0]] \ No newline at end of file diff --git a/reactapp/__tests__/utilities/fixtures/shapefile/points.shp b/reactapp/__tests__/utilities/fixtures/shapefile/points.shp new file mode 100644 index 0000000000000000000000000000000000000000..329a6cce14de48c7f027f9a880ac3fce9f6462b6 GIT binary patch literal 156 zcmZQzQ0HR64*Xs)GcYj1} components Buffers keyed by extension. + * @param {{fallbackProjection?: string}} [options] `fallbackProjection` is the + * author-supplied projection, used only when the source carries no .prj. + * @returns {Promise<{featureCollection: object, projectionCode: string} + * |{error: {stage: string, reason: string, detail: string}}>} + */ +export async function interpretShapefile( + components, + { fallbackProjection } = {}, +) { + if (!components?.shp) { + return { + error: { + stage: "parse", + reason: "no_geometry", + detail: "The source contained no .shp geometry to read.", + }, + }; + } + + const projection = resolveProjection(components.prj, fallbackProjection); + if (projection.error) return projection; + + // Loaded lazily so the parser stays out of the main bundle, matching how the + // GeoTIFF reader is pulled in. The browser build is named explicitly: the + // package resolves to a Node build under the test runner and a browser build + // under the bundler, and the fidelity guarantees below are only worth anything + // if they were measured against the artifact that actually ships. + const { read } = await import("shapefile/dist/shapefile.js"); + + let collection; + try { + collection = await read( + toArrayBuffer(components.shp), + components.dbf ? toArrayBuffer(components.dbf) : undefined, + ); + } catch (error) { + return { + error: { + stage: "parse", + reason: "unreadable_geometry", + detail: `The shapefile geometry could not be read: ${error.message}`, + }, + }; + } + + // Shapefile encodes a polygon's interior rings by winding direction with no + // parent pointer, so the ring order a parser produces is the only record of + // which ring is a hole. Normalizing to the GeoJSON spec's winding is cheap + // insurance: rendering keys off ring order rather than direction, but anything + // downstream that reads the geometry as spec GeoJSON gets what it expects. + const { default: rewind } = await import("@mapbox/geojson-rewind"); + rewind(collection); + + collection.crs = { + type: "name", + properties: { name: projection.code }, + }; + + return { featureCollection: collection, projectionCode: projection.code }; +} + +// The parser reads from an ArrayBuffer. A component buffer may be a view over a +// larger allocation, so slice to its own bounds rather than handing over the +// whole backing store. +function toArrayBuffer(bytes) { + return bytes.buffer.slice( + bytes.byteOffset, + bytes.byteOffset + bytes.byteLength, + ); +} + +// Absence and failure are different inputs here, and keeping them apart is the +// point. A .prj that is genuinely missing falls back to what the author +// supplied; a .prj that failed to arrive was already reported upstream and never +// reaches this function, because silently substituting a fallback for it would +// draw the features somewhere else with no error at all. +function resolveProjection(prjBytes, fallbackProjection) { + if (prjBytes) { + // Decoded with fflate rather than TextDecoder: this runs in the browser and + // under the test runner, and one of those has no TextDecoder. + const wkt = strFromU8(prjBytes).trim(); + const registered = registerProjectionFromWkt(wkt); + if (registered.error) { + return { + error: { + stage: "parse", + reason: "unresolvable_projection", + detail: registered.error.detail, + }, + }; + } + return { code: registered.code }; + } + + if (fallbackProjection) { + const resolved = ensureProjection(fallbackProjection); + if (!resolved) { + return { + error: { + stage: "parse", + reason: "unresolvable_projection", + detail: `The projection "${fallbackProjection}" could not be resolved. Supply a coordinate system this map recognises, or a WKT definition.`, + }, + }; + } + return { code: fallbackProjection }; + } + + return { + error: { + stage: "parse", + reason: "missing_projection", + detail: + "The shapefile carries no .prj and no projection was supplied, so its coordinates cannot be placed.", + }, + }; +} From aaf438367a4a49d3dc7e1711408daced5e1e7019 Mon Sep 17 00:00:00 2001 From: Corey Krewson Date: Tue, 25 Aug 2026 10:18:32 -0700 Subject: [PATCH 04/18] refactor(map): share the feature-collection read and the load-status vocabulary Extracts `readFeatureCollection` from `swapVectorLayerFeatures` so the shapefile source's loader can read a collection the same way, and adds one module for the names the two async vector paths share. The loader cannot call the swap function itself: that clears every feature already on the source, whereas a loader is additive inside its success callback. Worth recording alongside it -- clearing a source does *not* reset OpenLayers' loaded-extent bookkeeping, so it is not a usable retry primitive. `refresh()` is the one that works. Taking the projection as a per-call argument rather than baking it in when the source is built is what lets a loader read features against the view as it stands at insertion, instead of as it stood when its fetch began. Covered by a test that reads the same collection into two projections. The status vocabulary carries one judgment: retry is offered only for fetch-stage failures. A missing projection, an unresolvable coordinate system, a malformed component and a source over the size ceiling all need the author to change something, so a retry button for them invites a viewer to re-download megabytes and fail identically. The plugin path already gates its own retry by failure kind for the same reason. The two paths stay separate by design -- one pushes on its own schedule, the other is pulled by OpenLayers when a layer renders -- so this shares their names rather than their lifecycle. Co-Authored-By: Claude Opus 5 (1M context) --- .../components/map/layerStatus.test.js | 73 +++++++++++++++++++ .../components/map/utilities.test.js | 59 +++++++++++++++ reactapp/components/map/layerStatus.js | 72 ++++++++++++++++++ reactapp/components/map/utilities.js | 27 ++++++- 4 files changed, 227 insertions(+), 4 deletions(-) create mode 100644 reactapp/__tests__/components/map/layerStatus.test.js create mode 100644 reactapp/components/map/layerStatus.js diff --git a/reactapp/__tests__/components/map/layerStatus.test.js b/reactapp/__tests__/components/map/layerStatus.test.js new file mode 100644 index 00000000..3ab324b4 --- /dev/null +++ b/reactapp/__tests__/components/map/layerStatus.test.js @@ -0,0 +1,73 @@ +import { + CANCEL_REASON, + ERROR_KIND, + isRetryable, + errorKindFor, +} from "components/map/layerStatus"; + +describe("cancel reasons", () => { + it("names the three reasons a load stops", () => { + expect(Object.values(CANCEL_REASON).sort()).toEqual([ + "removed", + "superseded", + "unmount", + ]); + }); +}); + +describe("isRetryable", () => { + it("offers retry only for a fetch-stage failure", () => { + // Re-running the same request can only help when the request itself was the + // problem. Offering it for the others invites a viewer to re-download + // megabytes and fail identically. + expect(isRetryable(ERROR_KIND.FETCH)).toBe(true); + expect(isRetryable(ERROR_KIND.PARSE)).toBe(false); + expect(isRetryable(ERROR_KIND.TOO_LARGE)).toBe(false); + expect(isRetryable(ERROR_KIND.PROJECTION)).toBe(false); + expect(isRetryable(ERROR_KIND.UNAVAILABLE)).toBe(false); + }); + + it("does not offer retry for an unknown kind", () => { + expect(isRetryable(undefined)).toBe(false); + expect(isRetryable("something-else")).toBe(false); + }); +}); + +describe("errorKindFor", () => { + it.each([ + [{ stage: "fetch", reason: "unreachable" }, ERROR_KIND.FETCH], + [{ stage: "fetch", reason: "unsupported_scheme" }, ERROR_KIND.FETCH], + [{ stage: "fetch", reason: "component_status" }, ERROR_KIND.FETCH], + [{ stage: "parse", reason: "unreadable_archive" }, ERROR_KIND.PARSE], + [{ stage: "parse", reason: "wrong_content_type" }, ERROR_KIND.PARSE], + [{ stage: "parse", reason: "unreadable_geometry" }, ERROR_KIND.PARSE], + [{ stage: "parse", reason: "ambiguous_archive" }, ERROR_KIND.PARSE], + ])("maps %o to %s", (failure, expected) => { + expect(errorKindFor(failure)).toBe(expected); + }); + + it("gives the size ceiling its own kind regardless of stage", () => { + // The pipeline reports this on the fetch stage, but a viewer must not be + // offered a retry for it. + expect(errorKindFor({ stage: "fetch", reason: "too_large" })).toBe( + ERROR_KIND.TOO_LARGE, + ); + expect( + isRetryable(errorKindFor({ stage: "fetch", reason: "too_large" })), + ).toBe(false); + }); + + it.each(["missing_projection", "unresolvable_projection"])( + "gives %s the projection kind so retry is withheld", + (reason) => { + const kind = errorKindFor({ stage: "parse", reason }); + expect(kind).toBe(ERROR_KIND.PROJECTION); + expect(isRetryable(kind)).toBe(false); + }, + ); + + it("defaults to the fetch kind for an unrecognised failure", () => { + expect(errorKindFor(undefined)).toBe(ERROR_KIND.FETCH); + expect(errorKindFor({})).toBe(ERROR_KIND.FETCH); + }); +}); diff --git a/reactapp/__tests__/components/map/utilities.test.js b/reactapp/__tests__/components/map/utilities.test.js index 4528bad6..6d4887f1 100644 --- a/reactapp/__tests__/components/map/utilities.test.js +++ b/reactapp/__tests__/components/map/utilities.test.js @@ -1,4 +1,5 @@ import { + readFeatureCollection, reprojectVectorFeatures, createMarkerLayer, createHighlightLayer, @@ -4188,3 +4189,61 @@ describe("reprojectVectorFeatures", () => { expect(Math.abs(y - SANTA_INES_3857[1])).toBeLessThan(1); }); }); + +describe("readFeatureCollection", () => { + const collection = { + type: "FeatureCollection", + crs: { type: "name", properties: { name: "EPSG:4326" } }, + features: [ + { + type: "Feature", + properties: { NAME: "one" }, + geometry: { type: "Point", coordinates: [-105, 40] }, + }, + ], + }; + + it("reads a collection into the projection the caller names", () => { + const features = readFeatureCollection(collection, "EPSG:3857"); + expect(features).toHaveLength(1); + const [x, y] = features[0].getGeometry().getCoordinates(); + // Web Mercator metres, not degrees. + expect(Math.abs(x)).toBeGreaterThan(1e6); + expect(Math.abs(y)).toBeGreaterThan(1e6); + expect(features[0].get("NAME")).toBe("one"); + }); + + it("reads into a different projection on a later call", () => { + // The projection is a per-call argument, not baked in when the source was + // built -- which is what lets a loader read against the view as it stands at + // insertion rather than when its fetch began. + const asDegrees = readFeatureCollection(collection, "EPSG:4326"); + const [x] = asDegrees[0].getGeometry().getCoordinates(); + expect(x).toBeCloseTo(-105, 6); + }); + + it("defaults the data projection to EPSG:4326 when the collection names none", () => { + const { crs, ...withoutCrs } = collection; + expect(crs).toBeTruthy(); + const features = readFeatureCollection(withoutCrs, "EPSG:4326"); + expect(features[0].getGeometry().getCoordinates()[0]).toBeCloseTo(-105, 6); + }); + + it("honours a non-default crs on the collection", () => { + const mercator = { + ...collection, + crs: { type: "name", properties: { name: "EPSG:3857" } }, + features: [ + { + type: "Feature", + properties: {}, + geometry: { type: "Point", coordinates: [-11688546.53, 4865942.28] }, + }, + ], + }; + const features = readFeatureCollection(mercator, "EPSG:4326"); + const [x, y] = features[0].getGeometry().getCoordinates(); + expect(x).toBeCloseTo(-105, 1); + expect(y).toBeCloseTo(40, 1); + }); +}); diff --git a/reactapp/components/map/layerStatus.js b/reactapp/components/map/layerStatus.js new file mode 100644 index 00000000..8a57eba9 --- /dev/null +++ b/reactapp/components/map/layerStatus.js @@ -0,0 +1,72 @@ +// Vocabulary shared by the two vector paths that load features asynchronously: +// the plugin-layer fetcher and the shapefile source's loader. +// +// They are deliberately not unified -- one pushes on its own schedule and paints +// into a preserved layer, the other is pulled by OpenLayers only when a layer is +// mounted and rendering, and forcing one abstraction over both would mean +// parameterizing five axes for two implementations. What they do share is these +// names. Keeping them in one place turns a future divergence into a visible edit +// rather than two string literals drifting apart. + +/** Why an in-flight load stopped. */ +export const CANCEL_REASON = { + // A newer load for the same layer started. + SUPERSEDED: "superseded", + // The layer was removed from the map. + REMOVED: "removed", + // The map itself went away. + UNMOUNT: "unmount", +}; + +/** + * What kind of failure a layer is in. + * + * The distinction that carries weight is whether re-running the same request + * could succeed. A fetch-stage failure might: the host could come back, a + * signature could be refreshed. The rest cannot -- a missing projection, an + * unresolvable coordinate system, a malformed component and a source over the + * size ceiling all need the author to change something, so offering a viewer a + * retry button for them invites them to re-download megabytes to fail the same + * way. + */ +export const ERROR_KIND = { + FETCH: "fetch", + PARSE: "parse", + TOO_LARGE: "too_large", + PROJECTION: "projection", + // The plugin path's own kind, for a layer whose plugin is not installed. + UNAVAILABLE: "unavailable", +}; + +/** Failure kinds where re-running the same request could plausibly succeed. */ +const RETRYABLE = [ERROR_KIND.FETCH]; + +/** + * Whether a retry affordance should be offered for a failure of this kind. + * + * @param {string} kind One of ERROR_KIND. + * @returns {boolean} + */ +export function isRetryable(kind) { + return RETRYABLE.includes(kind); +} + +/** + * Map a typed failure from the shapefile pipeline onto a status error kind. + * + * The pipeline reports a stage and a reason; the status surface cares about + * whether the failure is worth retrying and what to call it. + * + * @param {{stage?: string, reason?: string}} failure + * @returns {string} One of ERROR_KIND. + */ +export function errorKindFor(failure) { + if (failure?.reason === "too_large") return ERROR_KIND.TOO_LARGE; + if ( + failure?.reason === "missing_projection" || + failure?.reason === "unresolvable_projection" + ) { + return ERROR_KIND.PROJECTION; + } + return failure?.stage === "parse" ? ERROR_KIND.PARSE : ERROR_KIND.FETCH; +} diff --git a/reactapp/components/map/utilities.js b/reactapp/components/map/utilities.js index ef331413..1ca7b86c 100644 --- a/reactapp/components/map/utilities.js +++ b/reactapp/components/map/utilities.js @@ -333,13 +333,32 @@ export function swapVectorLayerFeatures( return; } + source.addFeatures(readFeatureCollection(featureCollection, mapProjection)); +} + +/** + * Parse a GeoJSON FeatureCollection into OpenLayers features. + * + * `dataProjection` comes from the collection's own `crs`, defaulting to + * EPSG:4326 when it carries none, and `featureProjection` is the map's -- so the + * caller decides which projection the features land in, at the moment it calls. + * + * Shared by the swap above and by the shapefile source's loader. The loader must + * not call `swapVectorLayerFeatures` itself: that clears every feature already + * on the source, whereas a loader is additive inside its success callback. + * (Clearing a source does not reset OpenLayers' loaded-extent bookkeeping + * either, so it is not a retry primitive -- `refresh()` is.) + * + * @param {object} featureCollection A GeoJSON FeatureCollection. + * @param {string} mapProjection Projection code to read the features into. + * @returns {Array} + */ +export function readFeatureCollection(featureCollection, mapProjection) { const crsName = featureCollection?.crs?.properties?.name; - const dataProjection = crsName || "EPSG:4326"; - const features = new GeoJSONFormat().readFeatures(featureCollection, { - dataProjection, + return new GeoJSONFormat().readFeatures(featureCollection, { + dataProjection: crsName || "EPSG:4326", featureProjection: mapProjection, }); - source.addFeatures(features); } /** From 8480564c4d9ed0f7d4926b2e421829830aa00531 Mon Sep 17 00:00:00 2001 From: Corey Krewson Date: Tue, 25 Aug 2026 10:28:53 -0700 Subject: [PATCH 05/18] feat(map): add Shapefile as a selectable map layer source Registers the type end to end: the authoring registry, the module mapping, both dispatch branches in moduleLoader, and an explicit layer-type guard. Features load through OpenLayers' own loader hook rather than being fetched ahead of construction. That is what makes the projection requirement expressible at all -- moduleLoader receives a projection string captured at task start, but the loader is handed the live one when it runs, and its success/failure callbacks drive the load-event triple. It also means the loader is not called until the layer is actually mounted and rendering. The projection is read again at the moment features are inserted, not when the load began. A shapefile is the slowest-loading vector source in the app, so it is the one most exposed to a sibling raster's auto-fit changing the view mid-load -- features parsed into the outgoing projection are drawn thousands of kilometres off screen while still reporting the right feature count. Covered by a test that deliberately makes the construction-time and invocation-time projections both wrong. The dispatch for client-loading types is duplicated across the module-cache path and the post-import path, so both branches are wired and a test builds two shapefile sources to exercise each. One documented controller object on the source carries abort, status, error and reset, rather than three loose properties two modules discover by reaching into each other. Reset goes through `refresh()`, and the test asserts the loader ran a second time -- clearing the loaded extent alone leaves it un-invoked, which is how a retry button ends up doing nothing while its test passes. No layerId is assigned. Status lives on the source object, so a torn-down layer takes its status with it and a rebuilt one starts idle -- there is no external keyspace to invalidate, and none of the reused-identity hazards apply. The projection field takes a WKT or proj4 definition as well as a code, since a .prj-less shapefile in a CRS the table does not cover has no other authorable path. Handling that turned up a real gap: wkt-parser cannot read a proj4 string, so the definition is only parsed as WKT when it is shaped like WKT, and the probe reads the definition back from proj4 instead. `registerProjectionFromWkt` is renamed `registerProjectionDefinition` to match what it now accepts. Also fixes a latent flake of my own making: acquire.test.js assigned global.fetch without restoring it, and Jest reuses a worker across files. Co-Authored-By: Claude Opus 5 (1M context) --- .../components/map/projections.test.js | 28 +- .../components/map/shapefile/acquire.test.js | 10 + .../components/map/shapefile/index.test.js | 54 ++++ .../components/map/shapefileSource.test.js | 292 ++++++++++++++++++ .../components/map/utilities.test.js | 26 ++ .../modals/MapLayer/MapLayer.test.js | 15 + reactapp/__tests__/utilities/constants.js | 20 ++ reactapp/components/map/Map.js | 12 +- reactapp/components/map/ModuleLoader.js | 118 ++++++- reactapp/components/map/projections.js | 81 +++-- reactapp/components/map/shapefile/index.js | 30 +- reactapp/components/map/utilities.js | 15 + .../components/modals/MapLayer/MapLayer.js | 6 + 13 files changed, 660 insertions(+), 47 deletions(-) create mode 100644 reactapp/__tests__/components/map/shapefileSource.test.js diff --git a/reactapp/__tests__/components/map/projections.test.js b/reactapp/__tests__/components/map/projections.test.js index 560daeaa..04eba8ec 100644 --- a/reactapp/__tests__/components/map/projections.test.js +++ b/reactapp/__tests__/components/map/projections.test.js @@ -4,7 +4,7 @@ import { PROJECTION_TABLE, INITIAL_CODES, ensureProjection, - registerProjectionFromWkt, + registerProjectionDefinition, } from "components/map/projections"; // A projected CRS with no AUTHORITY node, which is how ESRI writes .prj files. @@ -101,9 +101,9 @@ describe("ensureProjection", () => { }); }); -describe("registerProjectionFromWkt", () => { +describe("registerProjectionDefinition", () => { it("registers WKT with no authority node and transforms with it", () => { - const result = registerProjectionFromWkt(ESRI_ALBERS_NO_AUTHORITY); + const result = registerProjectionDefinition(ESRI_ALBERS_NO_AUTHORITY); expect(result.error).toBeUndefined(); expect(result.code).toMatch(/^WKT:/); expect(getProjection(result.code)).toBeTruthy(); @@ -118,7 +118,7 @@ describe("registerProjectionFromWkt", () => { it("reuses an already-resolvable code instead of registering the layer's copy", () => { const before = proj4("EPSG:4326", "EPSG:5070", [-105, 40]); - const result = registerProjectionFromWkt(ALBERS_5070_WRONG_PARAMS); + const result = registerProjectionDefinition(ALBERS_5070_WRONG_PARAMS); expect(result.code).toBe("EPSG:5070"); // The seeded definition survives and the layer's differing parameters are @@ -134,7 +134,7 @@ describe("registerProjectionFromWkt", () => { expect(getProjection(claimed)).toBeFalsy(); const wkt = withAuthority(ESRI_ALBERS_NO_AUTHORITY, claimed); - const result = registerProjectionFromWkt(wkt); + const result = registerProjectionDefinition(wkt); // Guard against this passing vacuously: the fixture must actually carry a // top-level authority, or the module would fall through to a synthetic code @@ -148,13 +148,13 @@ describe("registerProjectionFromWkt", () => { }); it("returns the same code for the same WKT without re-registering", () => { - const first = registerProjectionFromWkt(ESRI_ALBERS_NO_AUTHORITY); - const second = registerProjectionFromWkt(ESRI_ALBERS_NO_AUTHORITY); + const first = registerProjectionDefinition(ESRI_ALBERS_NO_AUTHORITY); + const second = registerProjectionDefinition(ESRI_ALBERS_NO_AUTHORITY); expect(second.code).toBe(first.code); }); it("reports an unparsable definition and names what failed", () => { - const result = registerProjectionFromWkt('PROJCS["broken",GARBAGE['); + const result = registerProjectionDefinition('PROJCS["broken",GARBAGE['); expect(result.code).toBeUndefined(); expect(result.error.reason).toBe("unparsable"); expect(result.error.detail).toMatch(/could not be parsed/); @@ -163,7 +163,7 @@ describe("registerProjectionFromWkt", () => { it("reports an unsupported projection method by name", () => { // This one parses cleanly and only fails at transform time, so the message // has to come from validating the transform rather than from the parser. - const result = registerProjectionFromWkt(UNSUPPORTED_METHOD); + const result = registerProjectionDefinition(UNSUPPORTED_METHOD); expect(result.code).toBeUndefined(); expect(result.error.reason).toBe("unsupported"); expect(result.error.detail).toContain("Totally_Not_A_Real_Projection"); @@ -173,7 +173,7 @@ describe("registerProjectionFromWkt", () => { // proj4 implements the ESRI spelling of Albers but not this one, and the // failure is silent: non-finite coordinates, not an exception. Caught by // probing the definition at its own centre before registering it. - const result = registerProjectionFromWkt(OGC_ALBERS); + const result = registerProjectionDefinition(OGC_ALBERS); expect(result.code).toBeUndefined(); expect(result.error.reason).toBe("unsupported"); expect(result.error.detail).toContain("Albers_Conic_Equal_Area"); @@ -183,14 +183,14 @@ describe("registerProjectionFromWkt", () => { // A rejected definition must not stay in proj4's registry: register() // constructs a transform for every pair of registered codes, so one unusable // definition would make it throw and take working projections down with it. - registerProjectionFromWkt(UNSUPPORTED_METHOD); - const after = registerProjectionFromWkt(ESRI_ALBERS_NO_AUTHORITY); + registerProjectionDefinition(UNSUPPORTED_METHOD); + const after = registerProjectionDefinition(ESRI_ALBERS_NO_AUTHORITY); expect(after.error).toBeUndefined(); expect(getProjection("EPSG:5070")).toBeTruthy(); }); it("reports an empty definition rather than throwing", () => { - expect(registerProjectionFromWkt("").error.reason).toBe("empty"); - expect(registerProjectionFromWkt(undefined).error.reason).toBe("empty"); + expect(registerProjectionDefinition("").error.reason).toBe("empty"); + expect(registerProjectionDefinition(undefined).error.reason).toBe("empty"); }); }); diff --git a/reactapp/__tests__/components/map/shapefile/acquire.test.js b/reactapp/__tests__/components/map/shapefile/acquire.test.js index f165f7f2..922c78c4 100644 --- a/reactapp/__tests__/components/map/shapefile/acquire.test.js +++ b/reactapp/__tests__/components/map/shapefile/acquire.test.js @@ -31,13 +31,23 @@ function respond({ status = 200, contentType = "application/zip", body } = {}) { } let fetchMock; +let originalFetch; beforeEach(() => { clearComponentCache(); + originalFetch = global.fetch; fetchMock = jest.fn(); global.fetch = fetchMock; }); +// Restored rather than left assigned. Jest reuses a worker process across test +// files, so a fetch mock left on the global leaks into whichever file that +// worker picks up next -- which shows up as an unrelated suite failing only in +// certain run orders. +afterEach(() => { + global.fetch = originalFetch; +}); + describe("acquireComponents — validation happens before any request", () => { it.each([ "data:application/zip;base64,UEsDBA==", diff --git a/reactapp/__tests__/components/map/shapefile/index.test.js b/reactapp/__tests__/components/map/shapefile/index.test.js index 82d85fa6..13e291f3 100644 --- a/reactapp/__tests__/components/map/shapefile/index.test.js +++ b/reactapp/__tests__/components/map/shapefile/index.test.js @@ -207,3 +207,57 @@ describe("interpretShapefile — failure paths", () => { ); }); }); + +describe("interpretShapefile — projection field accepts a definition", () => { + const ESRI_ALBERS = `PROJCS["NAD_1983_Albers",GEOGCS["GCS_North_American_1983",DATUM["D_North_American_1983",SPHEROID["GRS_1980",6378137.0,298.257222101]],PRIMEM["Greenwich",0.0],UNIT["Degree",0.0174532925199433]],PROJECTION["Albers"],PARAMETER["False_Easting",0.0],PARAMETER["False_Northing",0.0],PARAMETER["Central_Meridian",-96.0],PARAMETER["Standard_Parallel_1",29.5],PARAMETER["Standard_Parallel_2",45.5],PARAMETER["Latitude_Of_Origin",23.0],UNIT["Meter",1.0]]`; + + it("accepts WKT as the fallback when there is no .prj", async () => { + // Without this, a .prj-less shapefile in a CRS the table does not cover has + // no authorable path at all -- there is no code to type that resolves. + const components = load("holes"); + delete components.prj; + + const result = await interpretShapefile(components, { + fallbackProjection: ESRI_ALBERS, + }); + + expect(result.error).toBeUndefined(); + expect(result.projectionCode).toMatch(/^WKT:/); + }); + + it("accepts a proj4 definition as the fallback", async () => { + const components = load("holes"); + delete components.prj; + + const result = await interpretShapefile(components, { + fallbackProjection: + "+proj=aea +lat_0=23 +lon_0=-96 +lat_1=29.5 +lat_2=45.5 +x_0=0 +y_0=0 +datum=NAD83 +units=m +no_defs", + }); + + expect(result.error).toBeUndefined(); + expect(result.projectionCode).toBeTruthy(); + }); + + it("reports a malformed definition as unparsable rather than as an unknown code", async () => { + const components = load("holes"); + delete components.prj; + + const result = await interpretShapefile(components, { + fallbackProjection: 'PROJCS["broken",GARBAGE[', + }); + + expect(result.error.reason).toBe("unresolvable_projection"); + expect(result.error.detail).toMatch(/could not be parsed/); + }); + + it("still treats a short token as a code", async () => { + const components = load("holes"); + delete components.prj; + + const result = await interpretShapefile(components, { + fallbackProjection: "EPSG:5070", + }); + + expect(result.projectionCode).toBe("EPSG:5070"); + }); +}); diff --git a/reactapp/__tests__/components/map/shapefileSource.test.js b/reactapp/__tests__/components/map/shapefileSource.test.js new file mode 100644 index 00000000..019de6f7 --- /dev/null +++ b/reactapp/__tests__/components/map/shapefileSource.test.js @@ -0,0 +1,292 @@ +import { get as getProjection } from "ol/proj.js"; +import VectorSource from "ol/source/Vector.js"; +import moduleLoader, { loadShapefile } from "components/map/ModuleLoader"; +import { acquireComponents } from "components/map/shapefile/acquire"; +import { interpretShapefile } from "components/map/shapefile/index"; + +// The pipeline is covered by its own suites; what matters here is the wiring -- +// which projection features land in, what drives the load events, and what the +// controller exposes. +jest.mock("components/map/shapefile/acquire", () => ({ + acquireComponents: jest.fn(), +})); +jest.mock("components/map/shapefile/index", () => ({ + interpretShapefile: jest.fn(), +})); + +const FULL_EXTENT = [-Infinity, -Infinity, Infinity, Infinity]; + +// A single point at -105, 40 in degrees, so the projection features are read +// into is observable from the resulting coordinates. +const COLLECTION = { + type: "FeatureCollection", + crs: { type: "name", properties: { name: "EPSG:4326" } }, + features: [ + { + type: "Feature", + properties: { NAME: "basin" }, + geometry: { type: "Point", coordinates: [-105, 40] }, + }, + ], +}; + +function config(props = {}) { + return { + type: "Shapefile", + props: { url: "https://example.org/basins.zip", ...props }, + }; +} + +function drive(source, projectionCode = "EPSG:3857") { + source.loadFeatures(FULL_EXTENT, 1, getProjection(projectionCode)); +} + +beforeEach(() => { + acquireComponents.mockReset(); + interpretShapefile.mockReset(); + acquireComponents.mockResolvedValue({ + components: { shp: new Uint8Array() }, + }); + interpretShapefile.mockResolvedValue({ + featureCollection: COLLECTION, + projectionCode: "EPSG:4326", + }); +}); + +describe("loadShapefile — construction", () => { + it("builds a vector source that loads through a loader, not a url", () => { + const source = loadShapefile(config(), "EPSG:3857"); + expect(source).toBeInstanceOf(VectorSource); + // A `url` would hand fetching to OpenLayers, which cannot decompress an + // archive or enforce a byte ceiling. + expect(source.getUrl()).toBeUndefined(); + }); + + it("throws the empty sentinel for a source with no url", () => { + // Mirrors the GeoTIFF sentinel: a half-authored source stays silent rather + // than painting a failure after every keystroke. + expect(() => loadShapefile({ type: "Shapefile", props: {} })).toThrow( + "ShapefileEmptySources", + ); + expect(() => loadShapefile({ type: "Shapefile" })).toThrow( + "ShapefileEmptySources", + ); + }); + + it("exposes one controller carrying abort, status, error and reset", () => { + const controller = loadShapefile(config(), "EPSG:3857").get( + "shapefileController", + ); + expect(typeof controller.abort).toBe("function"); + expect(typeof controller.getStatus).toBe("function"); + expect(typeof controller.getError).toBe("function"); + expect(typeof controller.reset).toBe("function"); + expect(controller.getStatus()).toBe("idle"); + }); +}); + +describe("loadShapefile — projection at insertion", () => { + it("reads features against the projection supplied when they are inserted", async () => { + // The construction-time projection is deliberately wrong here. A shapefile + // is the slowest-loading vector source in the app, so it is the one most + // exposed to a sibling raster's auto-fit changing the view mid-load -- + // features parsed into the outgoing projection are drawn far off screen + // while still reporting the right count. + let current = "EPSG:4326"; + const source = loadShapefile(config(), "EPSG:3857", () => current); + + current = "EPSG:3857"; + drive(source, "EPSG:4326"); + await new Promise(process.nextTick); + + const [x, y] = source.getFeatures()[0].getGeometry().getCoordinates(); + // Web Mercator metres, from the getter -- not the degrees either the + // construction argument or the loader argument would have given. + expect(Math.abs(x)).toBeGreaterThan(1e6); + expect(Math.abs(y)).toBeGreaterThan(1e6); + }); + + it("falls back to the loader's own projection when no getter is supplied", async () => { + const source = loadShapefile(config(), "EPSG:3857"); + drive(source, "EPSG:4326"); + await new Promise(process.nextTick); + + const [x] = source.getFeatures()[0].getGeometry().getCoordinates(); + expect(x).toBeCloseTo(-105, 6); + }); +}); + +describe("loadShapefile — load events and status", () => { + it("fires featuresloadend and reports ready on success", async () => { + const source = loadShapefile(config(), "EPSG:3857"); + const started = jest.fn(); + const ended = jest.fn(); + source.on("featuresloadstart", started); + source.on("featuresloadend", ended); + + drive(source); + await new Promise(process.nextTick); + + expect(started).toHaveBeenCalled(); + expect(ended).toHaveBeenCalled(); + expect(source.get("shapefileController").getStatus()).toBe("ready"); + expect(source.getFeatures()).toHaveLength(1); + }); + + it("fires featuresloaderror and keeps the typed failure on an acquisition failure", async () => { + acquireComponents.mockResolvedValue({ + error: { stage: "fetch", reason: "unreachable", detail: "no host" }, + }); + const source = loadShapefile(config(), "EPSG:3857"); + const errored = jest.fn(); + source.on("featuresloaderror", errored); + + drive(source); + await new Promise(process.nextTick); + + expect(errored).toHaveBeenCalled(); + const controller = source.get("shapefileController"); + expect(controller.getStatus()).toBe("error"); + // featuresloaderror carries no payload, so the typed failure has to travel + // on the source for anything to report a real message. + expect(controller.getError()).toEqual({ + stage: "fetch", + reason: "unreachable", + detail: "no host", + }); + expect(source.getFeatures()).toHaveLength(0); + }); + + it("keeps the typed failure on an interpretation failure", async () => { + interpretShapefile.mockResolvedValue({ + error: { + stage: "parse", + reason: "missing_projection", + detail: "no prj", + }, + }); + const source = loadShapefile(config(), "EPSG:3857"); + + drive(source); + await new Promise(process.nextTick); + + expect(source.get("shapefileController").getError().reason).toBe( + "missing_projection", + ); + }); + + it("returns to idle with no failure when the load is cancelled", async () => { + acquireComponents.mockResolvedValue({ cancelled: true }); + const source = loadShapefile(config(), "EPSG:3857"); + + drive(source); + await new Promise(process.nextTick); + + const controller = source.get("shapefileController"); + expect(controller.getStatus()).toBe("idle"); + expect(controller.getError()).toBeNull(); + }); + + it("passes the author's projection through as the fallback", async () => { + const source = loadShapefile( + config({ projection: "EPSG:5070" }), + "EPSG:3857", + ); + drive(source); + await new Promise(process.nextTick); + + expect(interpretShapefile).toHaveBeenCalledWith(expect.anything(), { + fallbackProjection: "EPSG:5070", + }); + }); +}); + +describe("loadShapefile — controller actions", () => { + it("aborts an in-flight load through the controller", async () => { + let capturedSignal; + acquireComponents.mockImplementation((url, { signal }) => { + capturedSignal = signal; + return new Promise(() => {}); + }); + const source = loadShapefile(config(), "EPSG:3857"); + + drive(source); + await new Promise(process.nextTick); + expect(capturedSignal.aborted).toBe(false); + + source.get("shapefileController").abort("removed"); + + expect(capturedSignal.aborted).toBe(true); + expect(source.get("shapefileController").getStatus()).toBe("idle"); + }); + + it("aborting when nothing is in flight is a no-op", () => { + const source = loadShapefile(config(), "EPSG:3857"); + expect(() => + source.get("shapefileController").abort("removed"), + ).not.toThrow(); + }); + + it("reset re-invokes the loader", async () => { + // Asserted by invocation count rather than by whether the loaded extent was + // cleared: clearing the extent alone leaves the loader un-invoked, so an + // extent assertion passes while retry is broken. + const source = loadShapefile(config(), "EPSG:3857"); + drive(source); + await new Promise(process.nextTick); + expect(acquireComponents).toHaveBeenCalledTimes(1); + + source.get("shapefileController").reset(); + drive(source); + await new Promise(process.nextTick); + + expect(acquireComponents).toHaveBeenCalledTimes(2); + expect(source.get("shapefileController").getStatus()).toBe("ready"); + }); + + it("reset clears a previous failure before reloading", async () => { + acquireComponents.mockResolvedValueOnce({ + error: { stage: "fetch", reason: "unreachable", detail: "no host" }, + }); + const source = loadShapefile(config(), "EPSG:3857"); + drive(source); + await new Promise(process.nextTick); + expect(source.get("shapefileController").getError()).toBeTruthy(); + + source.get("shapefileController").reset(); + + expect(source.get("shapefileController").getError()).toBeNull(); + expect(source.get("shapefileController").getStatus()).toBe("idle"); + }); +}); + +describe("moduleLoader dispatch", () => { + it("builds a shapefile source on the first call and on the second", async () => { + // The dispatch for client-loading types is duplicated across the + // module-cache path and the post-import path. Missing one means the first + // layer of a type works and the second breaks. + const first = await moduleLoader(config(), "EPSG:3857"); + const second = await moduleLoader(config(), "EPSG:3857"); + + expect(first).toBeInstanceOf(VectorSource); + expect(second).toBeInstanceOf(VectorSource); + expect(first.get("shapefileController")).toBeTruthy(); + expect(second.get("shapefileController")).toBeTruthy(); + expect(first).not.toBe(second); + }); + + it("forwards the live projection getter through moduleLoader", async () => { + const source = await moduleLoader(config(), "EPSG:3857", () => "EPSG:3857"); + drive(source, "EPSG:4326"); + await new Promise(process.nextTick); + + const [x] = source.getFeatures()[0].getGeometry().getCoordinates(); + expect(Math.abs(x)).toBeGreaterThan(1e6); + }); + + it("propagates the empty sentinel out of moduleLoader", async () => { + await expect( + moduleLoader({ type: "Shapefile", props: {} }, "EPSG:3857"), + ).rejects.toThrow("ShapefileEmptySources"); + }); +}); diff --git a/reactapp/__tests__/components/map/utilities.test.js b/reactapp/__tests__/components/map/utilities.test.js index 6d4887f1..c88d623c 100644 --- a/reactapp/__tests__/components/map/utilities.test.js +++ b/reactapp/__tests__/components/map/utilities.test.js @@ -1,4 +1,5 @@ import { + sourcePropertiesOptions, readFeatureCollection, reprojectVectorFeatures, createMarkerLayer, @@ -4247,3 +4248,28 @@ describe("readFeatureCollection", () => { expect(y).toBeCloseTo(40, 1); }); }); + +describe("sourcePropertiesOptions — Shapefile", () => { + it("is offered as a source type with url required", () => { + // This single object drives the source-type dropdown, the properties table, + // and the required-key validation at save. + const entry = sourcePropertiesOptions.Shapefile; + expect(entry).toBeTruthy(); + expect(Object.keys(entry.required)).toEqual(["url"]); + expect(entry.required.url.placeholder).toMatch(/\.zip|\.shp/); + }); + + it("offers projection and attributions as optional", () => { + const optional = Object.keys(sourcePropertiesOptions.Shapefile.optional); + expect(optional).toContain("projection"); + expect(optional).toContain("attributions"); + }); + + it("tells the author the projection field takes a definition as well as a code", () => { + // A .prj-less shapefile in an uncommon CRS has no table entry to name, so + // the field has to accept WKT for that case to be authorable at all. + expect( + sourcePropertiesOptions.Shapefile.optional.projection.placeholder, + ).toMatch(/WKT|proj4/i); + }); +}); diff --git a/reactapp/__tests__/components/modals/MapLayer/MapLayer.test.js b/reactapp/__tests__/components/modals/MapLayer/MapLayer.test.js index 7ce90204..834eb4e4 100644 --- a/reactapp/__tests__/components/modals/MapLayer/MapLayer.test.js +++ b/reactapp/__tests__/components/modals/MapLayer/MapLayer.test.js @@ -3640,3 +3640,18 @@ ExtentTestComponent.propTypes = { layerInfo: PropTypes.object, visualizationRefOverride: PropTypes.object, }; + +describe("getLayerType — Shapefile", () => { + it("routes Shapefile to a vector layer", () => { + expect(getLayerType("Shapefile")).toBe("VectorLayer"); + }); + + it("pins the exact label", () => { + // getLayerType routes by substring, so a label variant would silently pick a + // different layer class with no error. This documents that the exact string + // is load-bearing -- and it is persisted user data besides, so renaming it + // would cost a migration over every dashboard. + expect(getLayerType("Shapefile Tile")).toBe("TileLayer"); + expect(getLayerType("Shapefile Vector")).toBe("VectorTileLayer"); + }); +}); diff --git a/reactapp/__tests__/utilities/constants.js b/reactapp/__tests__/utilities/constants.js index 8547ccb9..b59970cd 100644 --- a/reactapp/__tests__/utilities/constants.js +++ b/reactapp/__tests__/utilities/constants.js @@ -1360,6 +1360,26 @@ export const layerConfigKML = { }, }; +// Neither the GeoTIFF nor the Zarr source type added a fixture here, which made +// both harder to test than they needed to be. The URL lives at +// source.props.url, like every other URL-based source -- GeoJSON's storage +// outside props is exactly why it needs a special case in nearly every dispatch +// function. +export const layerConfigShapefile = { + configuration: { + type: "VectorLayer", + props: { + name: "Shapefile Layer", + source: { + type: "Shapefile", + props: { + url: "https://example.org/basins.zip", + }, + }, + }, + }, +}; + export const layerConfigGeoJSON = { configuration: { type: "VectorLayer", diff --git a/reactapp/components/map/Map.js b/reactapp/components/map/Map.js index a374c48d..69e127fc 100644 --- a/reactapp/components/map/Map.js +++ b/reactapp/components/map/Map.js @@ -367,6 +367,12 @@ const MapComponent = ({ const newLayer = await moduleLoader( layerConfig, map.getView().getProjection().getCode(), + // Read again when features are actually inserted. A source with a + // long async load -- a shapefile -- can finish after a sibling + // raster's auto-fit has already changed the view, and features + // parsed into the outgoing projection are drawn far off screen + // while still reporting the right count. + () => map.getView().getProjection().getCode(), ); newLayer.set("name", name); @@ -607,7 +613,11 @@ const MapComponent = ({ } } } catch (err) { - if (err && err.message === "GeoTIFFEmptySources") { + if ( + err && + (err.message === "GeoTIFFEmptySources" || + err.message === "ShapefileEmptySources") + ) { return; } console.log(err); diff --git a/reactapp/components/map/ModuleLoader.js b/reactapp/components/map/ModuleLoader.js index b34c5c14..8fac355c 100644 --- a/reactapp/components/map/ModuleLoader.js +++ b/reactapp/components/map/ModuleLoader.js @@ -26,7 +26,12 @@ import { defaultDotSpacing, defaultDotRadius, } from "components/inputs/RuleEditor.js"; -import { rewriteArcGISExportUrlForAntimeridian } from "components/map/utilities"; +import { + rewriteArcGISExportUrlForAntimeridian, + readFeatureCollection, +} from "components/map/utilities"; +import { acquireComponents } from "components/map/shapefile/acquire"; +import { interpretShapefile } from "components/map/shapefile/index"; import { buildGeoTIFFStyleColor, buildCategoricalStyleColor, @@ -313,7 +318,7 @@ export async function applyAutoRamp(layerConfig) { return layerConfig; } -const moduleLoader = async (config, mapProjection) => { +const moduleLoader = async (config, mapProjection, getMapProjection) => { if (config.type === "Zarr") { // Already yields OL's `sources` shape, so it skips the GeoTIFF branch below. config = zarrSourceToGeoTIFF(config); @@ -349,6 +354,8 @@ const moduleLoader = async (config, mapProjection) => { if (moduleCache[type]) { if (type === "GeoJSON") { return loadGeoJSON(config, mapProjection); + } else if (type === "Shapefile") { + return loadShapefile(config, mapProjection, getMapProjection); } else if (type === "ESRI Feature Service") { return loadESRIJSON(config); } else { @@ -390,6 +397,8 @@ const moduleLoader = async (config, mapProjection) => { if (type === "GeoJSON") { return loadGeoJSON(config, mapProjection); + } else if (type === "Shapefile") { + return loadShapefile(config, mapProjection, getMapProjection); } else if (type === "ESRI Feature Service") { return loadESRIJSON(config); } else { @@ -506,6 +515,7 @@ const getModuleImporter = (type) => { WMS: "ol/source/ImageWMS.js", Raster: "ol/source/Raster.js", GeoJSON: "ol/format/GeoJSON.js", + Shapefile: "ol/source/Vector.js", KML: "ol/source/Vector.js", Style: "ol/style/Style.js", Stroke: "ol/style/Stroke.js", @@ -535,6 +545,110 @@ const getModuleImporter = (type) => { return importer; }; +/** + * Build the vector source for a `Shapefile` layer. + * + * Features load through OpenLayers' own loader hook rather than being fetched + * ahead of construction, which buys three things: the loader is handed the live + * view projection when it runs, its success/failure callbacks drive the + * `featuresloadstart` / `featuresloadend` / `featuresloaderror` events, and it + * is not called at all until the layer is actually mounted and rendering. + * + * `getMapProjection`, when supplied, is read at the moment features are inserted + * rather than when the load began. A shapefile is the slowest-loading vector + * source in the app, so it is the one most exposed to a sibling raster's auto-fit + * changing the view mid-load -- and features parsed into a projection the map has + * already left are drawn thousands of kilometres off screen while still reporting + * the right feature count. + * + * The single `shapefileController` set on the source is the whole channel between + * this module and the map: abort, status, error and reset. Hanging those on the + * source as loose properties would give two modules an undocumented surface each + * discovered by reaching into the other's object. + */ +export const loadShapefile = (config, mapProjection, getMapProjection) => { + const { url, projection: fallbackProjection } = config.props ?? {}; + // Mirrors the GeoTIFF sentinel: a half-authored source is silent rather than + // an error, so typing a URL does not paint a failure after every keystroke. + if (!url) throw new Error("ShapefileEmptySources"); + + let abortController = null; + let status = "idle"; + let failure = null; + + const source = new VectorSource(); + + source.setLoader(async (extent, resolution, projection, success, onError) => { + abortController = new AbortController(); + status = "loading"; + failure = null; + + const finish = (nextStatus, nextFailure) => { + status = nextStatus; + failure = nextFailure ?? null; + abortController = null; + }; + + const acquired = await acquireComponents(url, { + signal: abortController.signal, + }); + if (acquired.cancelled) { + finish("idle"); + onError?.(); + return; + } + if (acquired.error) { + finish("error", acquired.error); + onError?.(); + return; + } + + const interpreted = await interpretShapefile(acquired.components, { + fallbackProjection, + }); + if (interpreted.error) { + finish("error", interpreted.error); + onError?.(); + return; + } + + // Read against the view as it stands now, not as it stood when the fetch + // was issued. + const targetProjection = + getMapProjection?.() ?? projection?.getCode?.() ?? mapProjection; + const features = readFeatureCollection( + interpreted.featureCollection, + targetProjection, + ); + source.addFeatures(features); + finish("ready"); + success?.(features); + }); + + source.set("shapefileController", { + getStatus: () => status, + getError: () => failure, + abort: (reason) => { + if (abortController) { + abortController.abort(reason); + abortController = null; + status = "idle"; + } + }, + // `refresh` is the only primitive that actually causes the loader to run + // again. Removing the loaded extent alone leaves it un-invoked, because the + // renderer short-circuits its frame on an unchanged layer revision -- which + // is how a retry button ends up doing nothing while its test passes. + reset: () => { + status = "idle"; + failure = null; + source.refresh(); + }, + }); + + return source; +}; + const loadGeoJSON = (config, mapProjection) => { const geojson = config.geojson; diff --git a/reactapp/components/map/projections.js b/reactapp/components/map/projections.js index 569e309e..b3d544b2 100644 --- a/reactapp/components/map/projections.js +++ b/reactapp/components/map/projections.js @@ -15,7 +15,7 @@ import wktParser from "wkt-parser"; // already be on hand. That is what the table below is for. // // 2. Definitions a layer brings with it. A shapefile carries its CRS as WKT in -// its .prj, so it needs no table entry -- see registerProjectionFromWkt. +// its .prj, so it needs no table entry -- see registerProjectionDefinition. // // The table therefore only has to cover case 1, which is why it is short. A // survey of the live dashboards found exactly one layer naming a non-native code @@ -143,12 +143,12 @@ export function ensureProjection(code) { return getProjection(code); } -// Stable, dependency-free hash of the normalized WKT. Two textually different -// but semantically equivalent definitions hash differently and so register -// separately; that costs a duplicate registration and nothing else, which is -// cheaper than trying to canonicalise WKT. -function wktCode(wkt) { - const normalized = wkt.replace(/\s+/g, ""); +// Stable, dependency-free hash of the normalized definition. Two textually +// different but semantically equivalent definitions hash differently and so +// register separately; that costs a duplicate registration and nothing else, +// which is cheaper than trying to canonicalise WKT. +function definitionCode(definition) { + const normalized = definition.replace(/\s+/g, ""); let hash = 5381; for (let i = 0; i < normalized.length; i += 1) { hash = ((hash << 5) + hash + normalized.charCodeAt(i)) | 0; @@ -201,31 +201,61 @@ function definitionUsable(code, parsed) { } } +// WKT names its projected or geographic CRS with a bracketed keyword; a proj4 +// string is a run of `+key=value` tokens. Only the former carries an AUTHORITY +// node worth reading, and only the former can be handed to the WKT parser. +function isWkt(definition) { + return /\b(PROJCS|GEOGCS|PROJCRS|GEOGCRS|GEODCRS)\s*\[/i.test(definition); +} + /** - * Register a coordinate reference system from a layer's own WKT definition. + * Register a coordinate reference system from a definition a layer supplies -- + * WKT, as found in a shapefile's .prj, or a proj4 string. * - * Never overwrites a definition that already resolves. A layer's WKT is + * Never overwrites a definition that already resolves. A layer's definition is * authoritative for that layer's own features, but the projection registry is * global to the browser session -- so letting one layer's parameters replace a * code every other layer resolves through would make rendering depend on which - * dashboard was opened first. When the WKT claims a code that already resolves, - * the existing definition is reused and nothing is written. Otherwise the - * definition is registered under a synthetic code, never under the claimed one. + * dashboard was opened first. When the definition claims a code that already + * resolves, the existing one is reused and nothing is written. Otherwise it is + * registered under a synthetic code, never under the claimed one. * - * @param {string} wkt WKT definition, typically the contents of a .prj. + * @param {string} definition WKT or proj4 definition. * @returns {{code: string}|{error: {reason: string, detail: string}}} The code to * read coordinates with, or a failure describing what could not be resolved. */ -export function registerProjectionFromWkt(wkt) { - if (typeof wkt !== "string" || wkt.trim() === "") { +export function registerProjectionDefinition(definition) { + if (typeof definition !== "string" || definition.trim() === "") { return { error: { reason: "empty", detail: "No projection definition was found." }, }; } - let parsed; + let claimed = null; + if (isWkt(definition)) { + let parsed; + try { + parsed = wktParser(definition); + } catch (error) { + return { + error: { + reason: "unparsable", + detail: `The projection definition could not be parsed: ${error.message}`, + }, + }; + } + claimed = claimedCode(parsed); + } + + if (claimed && (getProjection(claimed) || ensureProjection(claimed))) { + return { code: claimed }; + } + + const code = definitionCode(definition); + if (getProjection(code)) return { code }; + try { - parsed = wktParser(wkt); + proj4.defs(code, definition); } catch (error) { return { error: { @@ -235,18 +265,13 @@ export function registerProjectionFromWkt(wkt) { }; } - const claimed = claimedCode(parsed); - if (claimed && (getProjection(claimed) || ensureProjection(claimed))) { - return { code: claimed }; - } - - const code = wktCode(wkt); - if (getProjection(code)) return { code }; - - proj4.defs(code, wkt); - if (!definitionUsable(code, parsed)) { + // Read the definition back rather than reusing the WKT parse: this is the only + // shape available for a proj4 string, and it is what proj4 will actually + // transform with either way. + const registered = proj4.defs(code); + if (!definitionUsable(code, registered)) { delete proj4.defs[code]; - const method = parsed?.projName ?? "an unnamed projection method"; + const method = registered?.projName ?? "an unnamed projection method"; return { error: { reason: "unsupported", diff --git a/reactapp/components/map/shapefile/index.js b/reactapp/components/map/shapefile/index.js index bb8a86c4..00c7e962 100644 --- a/reactapp/components/map/shapefile/index.js +++ b/reactapp/components/map/shapefile/index.js @@ -1,7 +1,7 @@ import { strFromU8 } from "fflate"; import { ensureProjection, - registerProjectionFromWkt, + registerProjectionDefinition, } from "components/map/projections"; /** @@ -86,6 +86,15 @@ function toArrayBuffer(bytes) { ); } +// A code is a short token like "EPSG:5070"; a definition is WKT or a proj4 +// string. Detected by shape rather than by trying one and falling back, so a +// malformed definition is reported as such instead of as an unknown code. +function looksLikeDefinition(value) { + return /^\s*(\+proj=|[A-Z_]*(PROJCS|GEOGCS|PROJCRS|GEOGCRS|GEODCRS)\s*\[)/i.test( + value, + ); +} + // Absence and failure are different inputs here, and keeping them apart is the // point. A .prj that is genuinely missing falls back to what the author // supplied; a .prj that failed to arrive was already reported upstream and never @@ -96,7 +105,7 @@ function resolveProjection(prjBytes, fallbackProjection) { // Decoded with fflate rather than TextDecoder: this runs in the browser and // under the test runner, and one of those has no TextDecoder. const wkt = strFromU8(prjBytes).trim(); - const registered = registerProjectionFromWkt(wkt); + const registered = registerProjectionDefinition(wkt); if (registered.error) { return { error: { @@ -110,6 +119,23 @@ function resolveProjection(prjBytes, fallbackProjection) { } if (fallbackProjection) { + // The field takes a definition as well as a code. A shapefile with no .prj + // in an uncommon CRS has no other way to be placed: there is no table entry + // to name, and the registration helper already accepts exactly this input. + if (looksLikeDefinition(fallbackProjection)) { + const registered = registerProjectionDefinition(fallbackProjection); + if (registered.error) { + return { + error: { + stage: "parse", + reason: "unresolvable_projection", + detail: registered.error.detail, + }, + }; + } + return { code: registered.code }; + } + const resolved = ensureProjection(fallbackProjection); if (!resolved) { return { diff --git a/reactapp/components/map/utilities.js b/reactapp/components/map/utilities.js index 1ca7b86c..d5d124ae 100644 --- a/reactapp/components/map/utilities.js +++ b/reactapp/components/map/utilities.js @@ -122,6 +122,21 @@ export const sourcePropertiesOptions = { required: {}, optional: {}, }, + Shapefile: { + required: { + url: { + placeholder: + "URL of a zipped shapefile (.zip) or of its .shp component", + }, + }, + optional: { + // Used only when the source carries no .prj. Accepts a WKT or proj4 + // definition as well as a code, because a .prj-less shapefile in an + // uncommon CRS has no other way to be placed. + projection: { placeholder: "EPSG:, or a WKT/proj4 definition" }, + attributions: { placeholder: "Attributions" }, + }, + }, GeoTIFF: { required: { url: { placeholder: "Cloud Optimized GeoTIFF URL" }, diff --git a/reactapp/components/modals/MapLayer/MapLayer.js b/reactapp/components/modals/MapLayer/MapLayer.js index 54664b42..63468d97 100644 --- a/reactapp/components/modals/MapLayer/MapLayer.js +++ b/reactapp/components/modals/MapLayer/MapLayer.js @@ -133,6 +133,12 @@ export function renameLayerInAttributeProps(attributeProps, oldName, newName) { export const getLayerType = (sourceType) => { if (sourceType === "GeoTIFF" || sourceType === "Zarr") return "WebGLTile"; + // Explicit rather than left to the fallthrough below. "Shapefile" happens to + // match none of the substring tests, so it would reach VectorLayer anyway -- + // but a label variant like "Zipped Shapefile Tile" would silently route to the + // wrong layer class with no error. The label is load-bearing, and it is also + // persisted user data: renaming one costs a migration over every dashboard. + if (sourceType === "Shapefile") return "VectorLayer"; if (sourceType.includes("Vector")) return "VectorTileLayer"; if (sourceType.includes("Raster")) return "WebGLTile"; if (sourceType.includes("Tile")) return "TileLayer"; From 6089eb4383a1a601da32542df243cf74ccb4b534 Mon Sep 17 00:00:00 2001 From: Corey Krewson Date: Tue, 25 Aug 2026 10:38:33 -0700 Subject: [PATCH 06/18] feat(map): register Shapefile at every source-type dispatch point The sites were re-derived by searching for the existing type-name literals rather than taken from a list, because that is the only way to find them: the capability is encoded as strings scattered across modules, and the failure mode of missing one is silent. Five needed changing; a sixth comes free. - The client-vector list gates both click queries and snapping. A type absent from it does not error -- the snap path falls through to the feature-service query and returns nothing. - Attribute discovery gains a branch that reads field names from the .dbf. - Style-field discovery routes through that same branch rather than getting a second implementation. The two are otherwise independent trees with different logic, and registering in only one gives working fields in one pane and an empty list in the other -- so a test asserts they agree. - The Style pane's supported-type list is a hard gate: absent from it, the tab renders a dead-end panel and styling the layer is impossible no matter what discovery returned. An existing test hardcoded that list in its expected message and has been updated. - The layer-property help text for clickTolerance and snapToFeatures enumerates eligible types and is user-visible. Snapping needs nothing further: it reads the shared client-vector list, and features arriving through a loading strategy become snappable as they load. The service-legend branch is deliberately untouched -- a shapefile layer takes the style-derived legend path, as a rule-styled vector should. Discovery reads the source, which acquisition caches against the resolved URL, so the style pane and the attributes pane reading in turn cost one fetch between them. When the source cannot be read, discovery returns an empty field list rather than throwing, so a failure surfaces through the layer's error path instead of breaking the editor. Co-Authored-By: Claude Opus 5 (1M context) --- .../components/map/shapefileDispatch.test.js | 224 ++++++++++++++++++ .../modals/MapLayer/StylePane.test.js | 43 +++- reactapp/components/map/utilities.js | 61 ++++- .../components/modals/MapLayer/StylePane.js | 10 +- 4 files changed, 332 insertions(+), 6 deletions(-) create mode 100644 reactapp/__tests__/components/map/shapefileDispatch.test.js diff --git a/reactapp/__tests__/components/map/shapefileDispatch.test.js b/reactapp/__tests__/components/map/shapefileDispatch.test.js new file mode 100644 index 00000000..90dd898c --- /dev/null +++ b/reactapp/__tests__/components/map/shapefileDispatch.test.js @@ -0,0 +1,224 @@ +import { + CLIENT_VECTOR_SOURCE_TYPES, + layerPropertiesOptions, + getLayerAttributes, + getStyleFields, + queryLayerFeatures, +} from "components/map/utilities"; +import { acquireComponents } from "components/map/shapefile/acquire"; +import { interpretShapefile } from "components/map/shapefile/index"; + +jest.mock("components/map/shapefile/acquire", () => ({ + acquireComponents: jest.fn(), +})); +jest.mock("components/map/shapefile/index", () => ({ + interpretShapefile: jest.fn(), +})); + +const SOURCE_PROPS = { + type: "Shapefile", + props: { url: "https://example.org/basins.zip" }, +}; + +const WITH_ATTRIBUTES = { + featureCollection: { + type: "FeatureCollection", + features: [ + { + type: "Feature", + properties: { HUC8: "10190005", AREASQKM: 91, NAME: "Upper" }, + geometry: { type: "Point", coordinates: [0, 0] }, + }, + // A second feature carrying one field the first lacks, so the union rather + // than the first record decides the field list. + { + type: "Feature", + properties: { HUC8: "10190006", STATES: "CO" }, + geometry: { type: "Point", coordinates: [1, 1] }, + }, + ], + }, + projectionCode: "EPSG:4326", +}; + +beforeEach(() => { + acquireComponents.mockReset(); + interpretShapefile.mockReset(); + acquireComponents.mockResolvedValue({ + components: { shp: new Uint8Array() }, + }); + interpretShapefile.mockResolvedValue(WITH_ATTRIBUTES); +}); + +describe("client-vector source types", () => { + it("includes Shapefile, so clicks and snapping read from the map", () => { + // Absent from this list the snap path falls through to the feature-service + // query and returns nothing -- a silent failure, not an error. + expect(CLIENT_VECTOR_SOURCE_TYPES).toContain("Shapefile"); + }); +}); + +describe("queryLayerFeatures", () => { + it("reaches the client-vector branch for a shapefile layer instead of throwing", async () => { + // Without Shapefile in the client-vector list this throws "is not currently + // configured to be queried" -- it does not fall through to anything. The + // empty result here is the point: dispatch arrived, found no matching + // feature, and returned normally. + const mockMap = { + getView: jest.fn(() => ({ + getResolution: jest.fn(), + getZoom: jest.fn(() => 10), + })), + forEachFeatureAtPixel: jest.fn((pixel, callback) => { + callback(null, { + get: jest.fn(() => "Some Other Layer"), + getProperties: () => ({ name: "Some Other Layer" }), + }); + }), + }; + + const features = await queryLayerFeatures( + { + configuration: { props: { name: "Basins", source: SOURCE_PROPS } }, + }, + mockMap, + [0, 0], + [639, 366], + ); + + expect(features).toStrictEqual([]); + expect(mockMap.forEachFeatureAtPixel).toHaveBeenCalled(); + }); +}); + +describe("getLayerAttributes — Shapefile", () => { + it("returns the union of .dbf field names", async () => { + const attributes = await getLayerAttributes({ + sourceProps: SOURCE_PROPS, + layerName: "Basins", + dashboard_uuid: "uuid", + }); + + expect(attributes.Basins.map((f) => f.name).sort()).toEqual([ + "AREASQKM", + "HUC8", + "NAME", + "STATES", + ]); + // No alias source for a shapefile, so each field aliases to itself. + expect(attributes.Basins.every((f) => f.alias === f.name)).toBe(true); + }); + + it("passes the author's projection through as the fallback", async () => { + await getLayerAttributes({ + sourceProps: { + ...SOURCE_PROPS, + props: { ...SOURCE_PROPS.props, projection: "EPSG:5070" }, + }, + layerName: "Basins", + }); + + expect(interpretShapefile).toHaveBeenCalledWith(expect.anything(), { + fallbackProjection: "EPSG:5070", + }); + }); + + it("returns an empty list rather than throwing when the source cannot be read", async () => { + acquireComponents.mockResolvedValue({ + error: { stage: "fetch", reason: "unreachable", detail: "no host" }, + }); + + const attributes = await getLayerAttributes({ + sourceProps: SOURCE_PROPS, + layerName: "Basins", + }); + + expect(attributes).toEqual({ Basins: [] }); + }); + + it("returns an empty list when interpretation fails", async () => { + interpretShapefile.mockResolvedValue({ + error: { stage: "parse", reason: "missing_projection", detail: "no prj" }, + }); + + const attributes = await getLayerAttributes({ + sourceProps: SOURCE_PROPS, + layerName: "Basins", + }); + + expect(attributes).toEqual({ Basins: [] }); + }); + + it("returns an empty list for geometry with no attributes", async () => { + // Covers the no-.dbf case: the layer still renders and is styleable by + // geometry-independent rules, but offers no fields. + interpretShapefile.mockResolvedValue({ + featureCollection: { + type: "FeatureCollection", + features: [ + { + type: "Feature", + properties: {}, + geometry: { type: "Point", coordinates: [0, 0] }, + }, + ], + }, + projectionCode: "EPSG:4326", + }); + + const attributes = await getLayerAttributes({ + sourceProps: SOURCE_PROPS, + layerName: "Basins", + }); + + expect(attributes.Basins).toEqual([]); + }); +}); + +describe("getStyleFields — Shapefile", () => { + it("returns the same field list attribute discovery does", async () => { + // Registering in only one of the two discovery trees gives working fields in + // one pane and an empty list in the other, so this asserts they agree. + const styleFields = await getStyleFields({ + sourceProps: SOURCE_PROPS, + layerProps: { name: "Basins" }, + dashboard_uuid: "uuid", + }); + const attributes = await getLayerAttributes({ + sourceProps: SOURCE_PROPS, + layerName: "Basins", + dashboard_uuid: "uuid", + }); + + expect(styleFields.sort()).toEqual( + attributes.Basins.map((f) => f.name).sort(), + ); + }); + + it("returns an empty list rather than throwing when the source cannot be read", async () => { + acquireComponents.mockResolvedValue({ cancelled: true }); + + const fields = await getStyleFields({ + sourceProps: SOURCE_PROPS, + layerProps: { name: "Basins" }, + }); + + expect(fields).toEqual([]); + }); +}); + +describe("layerPropertiesOptions help text", () => { + it("names Shapefile among the types clickTolerance applies to", () => { + // This registry drives the editor's Layer Properties table, so the text is + // user-visible. + expect(layerPropertiesOptions.clickTolerance.placeholder).toContain( + "Shapefile", + ); + }); + + it("names Shapefile among the types snapToFeatures applies to", () => { + expect(layerPropertiesOptions.snapToFeatures.placeholder).toContain( + "Shapefile", + ); + }); +}); diff --git a/reactapp/__tests__/components/modals/MapLayer/StylePane.test.js b/reactapp/__tests__/components/modals/MapLayer/StylePane.test.js index ffef912b..ef38c162 100644 --- a/reactapp/__tests__/components/modals/MapLayer/StylePane.test.js +++ b/reactapp/__tests__/components/modals/MapLayer/StylePane.test.js @@ -317,7 +317,12 @@ test("StylePane Updating Existing GeoJSON", async () => { test("StylePane Styling not available", async () => { render(); - const supportedTypes = ["GeoJSON", "ESRI Feature Service", "PMTiles Vector"]; + const supportedTypes = [ + "GeoJSON", + "ESRI Feature Service", + "PMTiles Vector", + "Shapefile", + ]; expect( await screen.findByText( `Custom Styling is only available for ${supportedTypes.join(", ")} layers.`, @@ -325,6 +330,42 @@ test("StylePane Styling not available", async () => { ).toBeInTheDocument(); }); +test("StylePane offers the style editor for a Shapefile source", async () => { + // The gate this exercises is separate from field discovery: absent from the + // supported list, the tab renders a dead-end panel and styling the layer is + // impossible no matter what fields were found. Asserted on the absence of that + // panel, which is decided at render rather than after discovery resolves. + // + // Discovery is stubbed so the effect does not reach the network for a URL that + // does not exist; what it returns is covered by its own suite. + const styleFieldsSpy = jest + .spyOn(utilities, "getStyleFields") + .mockResolvedValue(["HUC8", "AREASQKM"]); + + render( + , + ); + + expect( + screen.queryByText(/Custom Styling is only available for/), + ).not.toBeInTheDocument(); + // And discovery is reached rather than skipped, so the rule editor has fields + // to offer once it resolves. + await waitFor(() => { + expect(styleFieldsSpy).toHaveBeenCalledWith( + expect.objectContaining({ + sourceProps: expect.objectContaining({ type: "Shapefile" }), + }), + ); + }); + styleFieldsSpy.mockRestore(); +}); + test("StylePane switches to rules mode and syncs rules/defaultStyle from JSON", async () => { render(); // Switch to rules mode diff --git a/reactapp/components/map/utilities.js b/reactapp/components/map/utilities.js index d5d124ae..0564e3c2 100644 --- a/reactapp/components/map/utilities.js +++ b/reactapp/components/map/utilities.js @@ -1,4 +1,6 @@ import PropTypes from "prop-types"; +import { acquireComponents } from "components/map/shapefile/acquire"; +import { interpretShapefile } from "components/map/shapefile/index"; import { convertXML } from "simple-xml-to-json"; import { transform } from "ol/proj"; import Feature from "ol/Feature"; @@ -18,7 +20,15 @@ import Protobuf from "pbf"; // Source types whose features live in a client-side OL VectorSource (vs // server-rendered services queried remotely). -export const CLIENT_VECTOR_SOURCE_TYPES = ["GeoJSON", "ESRI Feature Service"]; +// Source types whose features live in a client-side vector source, so a click or +// a snap can read them straight off the map rather than querying a service. +// A type missing from here does not error -- the snap path falls through to the +// feature-service query, which returns nothing -- so the failure is silent. +export const CLIENT_VECTOR_SOURCE_TYPES = [ + "GeoJSON", + "ESRI Feature Service", + "Shapefile", +]; // Coerce an optional numeric layer prop: GUI inputs emit strings, so accept // any numeric value but treat null/undefined/blank/non-numeric as unset. @@ -264,12 +274,12 @@ export const layerPropertiesOptions = { clickTolerance: { type: "number", placeholder: - "Pixel tolerance for ESRI Image and Map Service identify (click) requests (default 10) and for GeoJSON / ESRI Feature Service feature queries (default 0). Also sets the snap radius when Snap To Features is on (default 15).", + "Pixel tolerance for ESRI Image and Map Service identify (click) requests (default 10) and for GeoJSON / ESRI Feature Service / Shapefile feature queries (default 0). Also sets the snap radius when Snap To Features is on (default 15).", }, snapToFeatures: { type: "checkbox", placeholder: - "Snap hover/click to the nearest feature of this layer (ESRI Map Service, GeoJSON, or ESRI Feature Service).", + "Snap hover/click to the nearest feature of this layer (ESRI Map Service, GeoJSON, ESRI Feature Service, or Shapefile).", }, snapSublayer: { type: "number", @@ -1012,7 +1022,15 @@ export async function getStyleFields({ isDynamicMapLayer = false, }) { let fields = []; - if (isDynamicMapLayer || sourceProps.type === "PMTiles Vector") { + // Shapefile joins the delegating branch rather than getting a second + // implementation. Attribute discovery and style-field discovery are otherwise + // independent trees, and registering in only one gives working fields in one + // pane and an empty list in the other. + if ( + isDynamicMapLayer || + sourceProps.type === "PMTiles Vector" || + sourceProps.type === "Shapefile" + ) { const attributes = await getLayerAttributes({ sourceProps, layerName: layerProps?.name ?? "", @@ -1125,6 +1143,12 @@ export async function getLayerAttributes({ attributes = await getKMLLayerAttributes(sourceUrl, layerName); } else if (sourceType === "PMTiles Vector") { attributes = await getPMTilesVectorLayerAttributes(sourceUrl); + } else if (sourceType === "Shapefile") { + attributes = await getShapefileLayerAttributes( + sourceUrl, + sourceProps?.props?.projection, + layerName, + ); } else { throw Error(`${sourceType} is not currently configured to be queried`); } @@ -1132,6 +1156,35 @@ export async function getLayerAttributes({ return attributes; } +// Field names come from the .dbf, which means reading the source. Acquisition is +// cached against the resolved URL, so the style pane and the attributes pane +// reading in turn cost one fetch between them. +async function getShapefileLayerAttributes( + sourceUrl, + fallbackProjection, + layerName, +) { + const acquired = await acquireComponents(sourceUrl); + if (acquired.error || acquired.cancelled) return { [layerName]: [] }; + + const interpreted = await interpretShapefile(acquired.components, { + fallbackProjection, + }); + if (interpreted.error) return { [layerName]: [] }; + + const fieldNames = new Set( + (interpreted.featureCollection.features ?? []).flatMap((feature) => + Object.keys(feature.properties ?? {}), + ), + ); + return { + [layerName]: Array.from(fieldNames).map((field) => ({ + name: field, + alias: field, + })), + }; +} + async function getPMTilesVectorLayerAttributes(sourceUrl) { // Default to tile 0/0/0 if not specified, or allow passing tile coordinates as needed const z = 0, diff --git a/reactapp/components/modals/MapLayer/StylePane.js b/reactapp/components/modals/MapLayer/StylePane.js index 3f38f3df..bea34222 100644 --- a/reactapp/components/modals/MapLayer/StylePane.js +++ b/reactapp/components/modals/MapLayer/StylePane.js @@ -428,7 +428,15 @@ const StylePane = ({ ); } - const supportedTypes = ["GeoJSON", "ESRI Feature Service", "PMTiles Vector"]; + // Absent from this list, the Style tab renders a dead-end "not available for + // this source type" panel instead of the rule editor -- so styling a shapefile + // layer would be impossible regardless of what field discovery returned. + const supportedTypes = [ + "GeoJSON", + "ESRI Feature Service", + "PMTiles Vector", + "Shapefile", + ]; const isDynamicMapLayer = findSelectOptionByValue( dynamicMapLayers, sourceProps.type, From 61165278b4f51c1ddad699271562a8768e6e0bea Mon Sep 17 00:00:00 2001 From: Corey Krewson Date: Tue, 25 Aug 2026 10:52:59 -0700 Subject: [PATCH 07/18] feat(map): preserve shapefile layers across unrelated layer changes The reconciliation sweep rebuilds every vector layer on any change to the layer array, so without this an opacity edit on an unrelated layer -- or one frame of a raster time-slider -- costs a full refetch, decompress and reparse of the whole archive. Preservation keys on the layer's name plus its resolved source URL. The keep predicate gains a branch rather than being generalized. The plugin-provenance check every existing plugin layer depends on is left exactly as it was, so preservation for those is untouched by this change. Preservation has a cost the add path was hiding: style is applied only when a layer is constructed, and the cosmetic prop sync carries only the props OpenLayers has first-class setters for. A preserved layer would therefore ignore a style-rule edit entirely -- which would contradict styling working on a shapefile layer at all. The style application is now factored out of the add path and re-applied to preserved layers when it differs from what was last applied. Two duplicate-layer hazards are closed. A run that has been superseded no longer adds its layer: it sits in no newer run's removal snapshot, so it would never be collected, leaving features drawn twice and every clicked feature reported twice in the popup. And the removal sweep now runs whenever the map actually holds layers rather than only when reconciliation state was recorded, because a run starting while a previous one is still loading sees no recorded state -- and gating removal on it let both runs' layers sit on the map. With no recorded state nothing is kept, so that case rebuilds rather than duplicates. In-flight loads are aborted when the layer is removed, when a run is superseded, and on unmount, so a fetch and decompression do not keep running for a layer nobody will see. The tests drive the loader directly, because OpenLayers pulls it only when a layer renders and a jsdom map has no size. That makes the assertion the right one anyway: a preserved layer keeps its source and its loaded-extent bookkeeping, so driving it again is a no-op, while a rebuilt layer loads from scratch. They wait on observable post-conditions rather than fixed delays. Co-Authored-By: Claude Opus 5 (1M context) --- .../map/shapefilePreservation.test.js | 245 ++++++++++++++++++ reactapp/components/map/Map.js | 136 ++++++++-- 2 files changed, 353 insertions(+), 28 deletions(-) create mode 100644 reactapp/__tests__/components/map/shapefilePreservation.test.js diff --git a/reactapp/__tests__/components/map/shapefilePreservation.test.js b/reactapp/__tests__/components/map/shapefilePreservation.test.js new file mode 100644 index 00000000..44c21cff --- /dev/null +++ b/reactapp/__tests__/components/map/shapefilePreservation.test.js @@ -0,0 +1,245 @@ +import { useRef, useState } from "react"; +import { render, screen, waitFor } from "@testing-library/react"; +import PropTypes from "prop-types"; +import { get as getProjection } from "ol/proj.js"; +import MapComponent from "components/map/Map"; +import MapContextProvider, { + useMapContext, +} from "components/contexts/MapContext"; +import { VariableInputsContext } from "components/contexts/Contexts"; +import { acquireComponents } from "components/map/shapefile/acquire"; +import { interpretShapefile } from "components/map/shapefile/index"; + +global.ResizeObserver = require("resize-observer-polyfill"); + +// Acquisition stands in for "did this layer load again". The pipeline itself is +// covered by its own suites. +jest.mock("components/map/shapefile/acquire", () => ({ + acquireComponents: jest.fn(), +})); +jest.mock("components/map/shapefile/index", () => ({ + interpretShapefile: jest.fn(), +})); + +const FULL_EXTENT = [-Infinity, -Infinity, Infinity, Infinity]; + +const COLLECTION = { + type: "FeatureCollection", + crs: { type: "name", properties: { name: "EPSG:4326" } }, + features: [ + { + type: "Feature", + properties: { HUC8: "10190005" }, + geometry: { type: "Point", coordinates: [-105, 40] }, + }, + ], +}; + +function shapefileLayer({ + url = "https://example.org/basins.zip", + style, +} = {}) { + return { + type: "VectorLayer", + props: { + name: "Basins", + source: { type: "Shapefile", props: { url } }, + }, + ...(style === undefined ? {} : { style }), + }; +} + +function otherLayer({ opacity = 1 } = {}) { + return { + type: "TileLayer", + props: { + name: "Basemap", + opacity, + source: { type: "Image Tile", props: { url: "https://example.org/{z}" } }, + }, + }; +} + +let mapRef; +let setLayers; + +const Harness = ({ initialLayers }) => { + const visualizationRef = useRef(); + const [layers, setLayersState] = useState(initialLayers); + const { mapReady } = useMapContext(); + mapRef = visualizationRef; + setLayers = setLayersState; + return ( +
+ +

{mapReady ? "Map Ready" : "Map Not Ready"}

+
+ ); +}; +Harness.propTypes = { initialLayers: PropTypes.array }; + +async function mount(initialLayers) { + render( + + + + + , + ); + expect(await screen.findByText("Map Ready")).toBeInTheDocument(); + await waitFor(() => expect(shapefileLayers()).toHaveLength(1)); +} + +function shapefileLayers() { + return (mapRef?.current?.getLayers?.().getArray() ?? []).filter( + (layer) => !!layer.getSource?.()?.get?.("shapefileController"), + ); +} + +function layerNamed(name) { + return (mapRef?.current?.getLayers?.().getArray() ?? []).find( + (layer) => layer.get("name") === name, + ); +} + +// The loader is pulled by OpenLayers only when a layer renders, and a jsdom map +// has no size -- so drive it directly. This also makes the assertion the right +// one: a preserved layer keeps its source and its loaded-extent bookkeeping, so +// driving it again is a no-op, while a rebuilt layer has a fresh source that +// loads from scratch. +async function drive() { + shapefileLayers().forEach((layer) => { + layer.getSource().loadFeatures(FULL_EXTENT, 1, getProjection("EPSG:3857")); + }); + await new Promise((resolve) => setTimeout(resolve, 0)); +} + +// Wait on an observable post-condition rather than a fixed delay, so a slow +// machine cannot turn these into flakes. Reconciliation is asynchronous, so +// something it did has to be visible before the assertions run. +async function reconciled(condition) { + await waitFor(condition); +} + +beforeEach(() => { + mapRef = undefined; + setLayers = undefined; + acquireComponents.mockReset(); + interpretShapefile.mockReset(); + acquireComponents.mockResolvedValue({ + components: { shp: new Uint8Array() }, + }); + interpretShapefile.mockResolvedValue({ + featureCollection: COLLECTION, + projectionCode: "EPSG:4326", + }); +}); + +describe("shapefile layer preservation", () => { + it("does not load again when an unrelated layer's opacity changes", async () => { + // The reconciliation sweep rebuilds every vector layer on any change to the + // layer array, so without preservation an opacity edit elsewhere -- or one + // frame of a raster time-slider -- costs a full refetch and reparse. + await mount([shapefileLayer(), otherLayer({ opacity: 1 })]); + await drive(); + expect(acquireComponents).toHaveBeenCalledTimes(1); + const original = layerNamed("Basins"); + + setLayers([shapefileLayer(), otherLayer({ opacity: 0.4 })]); + await reconciled(() => + expect(layerNamed("Basemap").getOpacity()).toBeCloseTo(0.4), + ); + await drive(); + + expect(acquireComponents).toHaveBeenCalledTimes(1); + // Same instance, so its features and loaded-extent bookkeeping survived. + expect(layerNamed("Basins")).toBe(original); + }); + + it("loads again when the resolved url changes", async () => { + await mount([shapefileLayer()]); + await drive(); + expect(acquireComponents).toHaveBeenCalledTimes(1); + + const original = shapefileLayers()[0]; + setLayers([shapefileLayer({ url: "https://example.org/gages.zip" })]); + await reconciled(() => expect(shapefileLayers()[0]).not.toBe(original)); + await drive(); + + expect(acquireComponents.mock.calls.length).toBeGreaterThan(1); + expect(acquireComponents).toHaveBeenLastCalledWith( + "https://example.org/gages.zip", + expect.anything(), + ); + }); + + it("does not load again when a re-render resolves to the same url", async () => { + await mount([shapefileLayer()]); + await drive(); + expect(acquireComponents).toHaveBeenCalledTimes(1); + + // A new config object carrying an identical resolved url. Nothing observable + // changes when a layer is preserved, so the instance check is the assertion + // and the load count corroborates it. + const original = shapefileLayers()[0]; + setLayers([shapefileLayer()]); + await reconciled(() => expect(shapefileLayers()).toHaveLength(1)); + await drive(); + + expect(shapefileLayers()[0]).toBe(original); + expect(acquireComponents).toHaveBeenCalledTimes(1); + }); + + it("repaints a style edit on a preserved layer without loading again", async () => { + // Style is otherwise applied only when a layer is constructed, so a + // preserved layer would silently ignore a style edit -- which would + // contradict styling working on a shapefile layer at all. + await mount([shapefileLayer({ style: { a: 1 } })]); + await drive(); + const original = layerNamed("Basins"); + expect(acquireComponents).toHaveBeenCalledTimes(1); + + setLayers([shapefileLayer({ style: { a: 2 } })]); + await waitFor(() => { + expect(layerNamed("Basins").get("appliedStyle")).toEqual({ a: 2 }); + }); + await drive(); + + expect(acquireComponents).toHaveBeenCalledTimes(1); + expect(layerNamed("Basins")).toBe(original); + }); + + it("keeps exactly one instance of the layer across a change", async () => { + await mount([shapefileLayer(), otherLayer()]); + + setLayers([shapefileLayer(), otherLayer({ opacity: 0.5 })]); + await reconciled(() => + expect(layerNamed("Basemap").getOpacity()).toBeCloseTo(0.5), + ); + + expect(shapefileLayers()).toHaveLength(1); + }); +}); + +describe("shapefile load cancellation", () => { + it("aborts an in-flight load when the layer is removed", async () => { + let capturedSignal; + acquireComponents.mockImplementation((url, options) => { + capturedSignal = options?.signal; + return new Promise(() => {}); + }); + + await mount([shapefileLayer()]); + await drive(); + expect(capturedSignal).toBeDefined(); + expect(capturedSignal.aborted).toBe(false); + + setLayers([otherLayer()]); + + await waitFor(() => { + expect(capturedSignal.aborted).toBe(true); + }); + }); +}); diff --git a/reactapp/components/map/Map.js b/reactapp/components/map/Map.js index 69e127fc..4234c6ca 100644 --- a/reactapp/components/map/Map.js +++ b/reactapp/components/map/Map.js @@ -10,6 +10,7 @@ import moduleLoader, { // matters, because layers are constructed concurrently and a registration that // waited on anything async would race them. import { isNativelyResolvable } from "components/map/projections"; +import { CANCEL_REASON } from "components/map/layerStatus"; import LayersControl from "components/map/LayersControl"; import FloatingMapControl from "components/map/FloatingMapControl"; import LegendControl from "components/map/LegendControl"; @@ -59,6 +60,45 @@ const InfoDiv = styled.div` z-index: 1000; `; +// Apply a layer config's style to an OL layer. +// +// Extracted from the add path so a *preserved* layer can be restyled too. +// Preservation keeps the layer instance, and the cosmetic prop sync handles only +// the props OL has first-class setters for -- so without this, editing a +// preserved layer's style rules would change nothing on the map. +async function applyLayerStyle(olLayer, layerConfig) { + if (!layerConfig.style) return; + + const isWebGLTileRampStyle = + layerConfig.type === "WebGLTile" && + layerConfig.style && + typeof layerConfig.style === "object" && + !Array.isArray(layerConfig.style) && + "color" in layerConfig.style; + + if (isWebGLTileRampStyle) { + olLayer.setStyle(layerConfig.style); + return; + } + + try { + await applyStyle(olLayer, layerConfig.style); + } catch (err) { + if (err.message !== "Cannot read properties of undefined (reading 'crs')") { + const styleFunction = createJsonStyleFunction(layerConfig.style); + if (typeof olLayer.setStyle === "function") { + olLayer.setStyle(styleFunction); + } + } + } +} + +// Stop an in-flight shapefile load. Called when the layer is going away, so the +// fetch and decompression do not keep running for a layer nobody will see. +function abortShapefileLoad(olLayer, reason) { + olLayer?.getSource?.()?.get?.("shapefileController")?.abort?.(reason); +} + const MapComponent = ({ mapConfig, mapExtent, @@ -164,6 +204,10 @@ const MapComponent = ({ // istanbul ignore next if (visualizationRef.current) { if (activeFadeRef.current) activeFadeRef.current(); + visualizationRef.current + .getLayers() + .getArray() + .forEach((layer) => abortShapefileLoad(layer, CANCEL_REASON.UNMOUNT)); visualizationRef.current.setTarget(undefined); visualizationRef.current = null; } @@ -251,6 +295,11 @@ const MapComponent = ({ // decision. Collect those here and apply after the loop so the in-place // update doesn't interfere with layersToKeep membership checks. const runtimeLayerUpdates = []; + // Preserved shapefile layers, collected the same way. Identity is the + // layer's name plus its resolved source URL: rebuilding refetches and + // reparses the whole archive, which an unrelated edit -- an opacity change + // on another layer, one frame of a raster time-slider -- should not cost. + const shapefileLayerUpdates = []; if (currentLayers.current.length) { const newLayerProps = (layers ?? []).map((l) => l.props); @@ -310,6 +359,31 @@ const MapComponent = ({ // layerId) fall through and let the layer be torn down + rebuilt. } + // Additive branch: the plugin-provenance check above is left exactly + // as it was rather than generalized, so preservation for plugin layers + // is untouched by this. + if ( + currentLayer?.props?.source?.type === "Shapefile" && + currentLayer.type === "VectorLayer" + ) { + const incoming = (layers ?? []).find( + (candidate) => + candidate?.props?.source?.type === "Shapefile" && + candidate?.props?.name === currentLayer.props.name && + candidate?.props?.source?.props?.url === + currentLayer.props.source?.props?.url, + ); + if (incoming) { + layersToKeep.push(incoming.props.name); + shapefileLayerUpdates.push({ + name: incoming.props.name, + newProps: incoming.props, + config: incoming, + }); + return; + } + } + const shouldKeep = newLayerProps.some((newProps) => valuesEqual(newProps, currentLayer.props), @@ -318,7 +392,15 @@ const MapComponent = ({ layersToKeep.push(currentLayer.props.name); } }); + } + // The removal sweep runs whenever the map actually holds layers, not only + // when reconciliation state was recorded. A run that starts while a + // previous one is still loading sees no recorded state, and gating removal + // on it would let both runs' layers sit on the map -- features drawn twice, + // and every clicked feature reported twice in the popup. With no recorded + // state nothing is kept, so this rebuilds rather than duplicates. + if (currentMapLayers.length) { const keptRuntimeLayerIds = new Set( runtimeLayerUpdates.map((u) => u.layerId), ); @@ -329,6 +411,8 @@ const MapComponent = ({ return; } if (!layersToKeep.includes(layerName)) { + // Stop any load still running for a layer that is going away. + abortShapefileLoad(layer, CANCEL_REASON.REMOVED); layersToRemove.push(layer); } }); @@ -342,6 +426,20 @@ const MapComponent = ({ updateOlLayerProps(olLayer, newProps); } }); + + // Same for preserved shapefile layers -- plus the style, which the + // cosmetic sync does not carry. Without this a style-rule edit on a + // preserved layer would change nothing, since the style is otherwise + // only applied when a layer is constructed. + shapefileLayerUpdates.forEach(({ name, newProps, config }) => { + const olLayer = currentMapLayers.find((l) => l.get("name") === name); + if (!olLayer) return; + updateOlLayerProps(olLayer, newProps); + if (!valuesEqual(olLayer.get("appliedStyle"), config.style)) { + olLayer.set("appliedStyle", config.style); + applyLayerStyle(olLayer, config); + } + }); } // setup constants for handling new layers @@ -438,6 +536,15 @@ const MapComponent = ({ } } + // A run that has already been superseded must not add its layer: + // it is in no newer run's removal snapshot, so it would never be + // collected -- leaving features drawn twice and every clicked + // feature reported twice in the popup. + if (myToken !== layerSyncToken.current) { + abortShapefileLoad(newLayer, CANCEL_REASON.SUPERSEDED); + return; + } + newLayer.set("appliedStyle", layerConfig.style); map.addLayer(newLayer); if ( @@ -584,34 +691,7 @@ const MapComponent = ({ } } - if (layerConfig.style) { - const isWebGLTileRampStyle = - layerConfig.type === "WebGLTile" && - layerConfig.style && - typeof layerConfig.style === "object" && - !Array.isArray(layerConfig.style) && - "color" in layerConfig.style; - - if (isWebGLTileRampStyle) { - newLayer.setStyle(layerConfig.style); - } else { - try { - await applyStyle(newLayer, layerConfig.style); - } catch (err) { - if ( - err.message !== - "Cannot read properties of undefined (reading 'crs')" - ) { - const styleFunction = createJsonStyleFunction( - layerConfig.style, - ); - if (typeof newLayer.setStyle === "function") { - newLayer.setStyle(styleFunction); - } - } - } - } - } + await applyLayerStyle(newLayer, layerConfig); } catch (err) { if ( err && From 0d08b69213843abdf8917279ac284725631b9b00 Mon Sep 17 00:00:00 2001 From: Corey Krewson Date: Tue, 25 Aug 2026 11:22:09 -0700 Subject: [PATCH 08/18] feat(map): surface shapefile load state and failures Failures and an in-flight indication go to the existing map-level alert, which is not gated on the author's layers-control toggle. That control is opt-in per dashboard and collapsed to an icon by default, so routing status only there would leave a viewer with nothing at all on any dashboard whose author disabled it -- and a failure rendering as a blank layer is the one outcome this must avoid. The control still carries the richer per-layer detail when enabled. Every failure class reaches the user with its own message: the observed and permitted size when a source is refused, the coordinate system when one cannot be resolved, the component and its status when one fails, and for a fetch-stage failure the candidate causes named together, since a browser cannot tell them apart. Retry is offered only where re-running the same request could succeed. A missing projection, an unresolvable coordinate system, a malformed component and a source over the size ceiling all need the author to change something, so a button for them would invite a viewer to re-download megabytes and fail identically. The plugin path already gates its own retry this way. Status is read from the source's controller rather than from a request id -- there is no backend request behind a client-parsed source, so the existing progress channel has nothing to report for one. It is mirrored into component state only so it can be rendered, and pruned to the layers actually on the map after each reconciliation: a rebuilt layer must not inherit the previous instance's failure, and a stale error must not suppress the replacement's loading indication. Retry goes through the source's refresh, which is the only primitive that causes the loader to run again, and a test drives the loader afterward to prove it did rather than asserting on internal state that would pass either way. R19 and R21 -- the author-facing remedy text and the elapsed-time escalation -- move to the editor unit, where the load action they attach to is built. Co-Authored-By: Claude Opus 5 (1M context) --- .../components/map/shapefileStatus.test.js | 312 ++++++++++++++++++ reactapp/components/map/LayersControl.js | 62 +++- reactapp/components/map/Map.js | 98 +++++- 3 files changed, 470 insertions(+), 2 deletions(-) create mode 100644 reactapp/__tests__/components/map/shapefileStatus.test.js diff --git a/reactapp/__tests__/components/map/shapefileStatus.test.js b/reactapp/__tests__/components/map/shapefileStatus.test.js new file mode 100644 index 00000000..7c7151ac --- /dev/null +++ b/reactapp/__tests__/components/map/shapefileStatus.test.js @@ -0,0 +1,312 @@ +import { useRef, useState } from "react"; +import { render, screen, fireEvent, waitFor } from "@testing-library/react"; +import PropTypes from "prop-types"; +import { get as getProjection } from "ol/proj.js"; +import MapComponent from "components/map/Map"; +import LayersControl from "components/map/LayersControl"; +import MapContextProvider, { + useMapContext, +} from "components/contexts/MapContext"; +import { VariableInputsContext } from "components/contexts/Contexts"; +import { ERROR_KIND } from "components/map/layerStatus"; +import { acquireComponents } from "components/map/shapefile/acquire"; +import { interpretShapefile } from "components/map/shapefile/index"; + +global.ResizeObserver = require("resize-observer-polyfill"); + +jest.mock("components/map/shapefile/acquire", () => ({ + acquireComponents: jest.fn(), +})); +jest.mock("components/map/shapefile/index", () => ({ + interpretShapefile: jest.fn(), +})); + +const FULL_EXTENT = [-Infinity, -Infinity, Infinity, Infinity]; + +const COLLECTION = { + type: "FeatureCollection", + crs: { type: "name", properties: { name: "EPSG:4326" } }, + features: [ + { + type: "Feature", + properties: {}, + geometry: { type: "Point", coordinates: [-105, 40] }, + }, + ], +}; + +const SHAPEFILE_LAYER = { + type: "VectorLayer", + props: { + name: "Basins", + source: { + type: "Shapefile", + props: { url: "https://example.org/basins.zip" }, + }, + }, +}; + +let mapRef; + +let setLayers; + +const Harness = ({ layers: initialLayers, layerControl }) => { + const visualizationRef = useRef(); + const [layers, setLayersState] = useState(initialLayers); + const { mapReady } = useMapContext(); + mapRef = visualizationRef; + setLayers = setLayersState; + return ( +
+ +

{mapReady ? "Map Ready" : "Map Not Ready"}

+
+ ); +}; +Harness.propTypes = { layers: PropTypes.array, layerControl: PropTypes.bool }; + +async function mount({ layerControl = false } = {}) { + render( + + + + + , + ); + expect(await screen.findByText("Map Ready")).toBeInTheDocument(); + await waitFor(() => expect(shapefileSource()).toBeDefined()); +} + +function shapefileSource() { + return (mapRef?.current?.getLayers?.().getArray() ?? []) + .map((layer) => layer.getSource?.()) + .find((source) => !!source?.get?.("shapefileController")); +} + +// The loader is pulled by OpenLayers only when a layer renders, and a jsdom map +// has no size. +function drive() { + shapefileSource().loadFeatures(FULL_EXTENT, 1, getProjection("EPSG:3857")); +} + +beforeEach(() => { + mapRef = undefined; + acquireComponents.mockReset(); + interpretShapefile.mockReset(); + acquireComponents.mockResolvedValue({ + components: { shp: new Uint8Array() }, + }); + interpretShapefile.mockResolvedValue({ + featureCollection: COLLECTION, + projectionCode: "EPSG:4326", + }); +}); + +describe("map-level surfacing", () => { + it("reports a failure even with the layers control disabled", async () => { + // The layers control is opt-in per dashboard and collapsed by default, so a + // dashboard with it off must still not render a failure as a blank layer. + acquireComponents.mockResolvedValue({ + error: { + stage: "fetch", + reason: "unreachable", + detail: "The shapefile could not be fetched.", + }, + }); + + await mount({ layerControl: false }); + drive(); + + const alert = await screen.findByRole("alert"); + expect(alert).toHaveTextContent("Basins"); + expect(alert).toHaveTextContent("could not be fetched"); + }); + + it("reports a load in flight even with the layers control disabled", async () => { + acquireComponents.mockImplementation(() => new Promise(() => {})); + + await mount({ layerControl: false }); + drive(); + + const status = await screen.findByRole("status"); + expect(status).toHaveTextContent("Loading Basins"); + }); + + it("clears the in-flight indication once the load succeeds", async () => { + await mount({ layerControl: false }); + drive(); + + await waitFor(() => { + expect(screen.queryByRole("status")).not.toBeInTheDocument(); + }); + expect(screen.queryByRole("alert")).not.toBeInTheDocument(); + }); + + it("names the size ceiling and the observed size when a source is refused", async () => { + acquireComponents.mockResolvedValue({ + error: { + stage: "fetch", + reason: "too_large", + observed: 90 * 1024 * 1024, + permitted: 25 * 1024 * 1024, + detail: + "The shapefile expands to at least 90.0 MB, above the 25.0 MB permitted.", + }, + }); + + await mount({ layerControl: false }); + drive(); + + const alert = await screen.findByRole("alert"); + expect(alert).toHaveTextContent("90.0 MB"); + expect(alert).toHaveTextContent("25.0 MB"); + }); + + it("names the unresolvable coordinate system", async () => { + interpretShapefile.mockResolvedValue({ + error: { + stage: "parse", + reason: "unresolvable_projection", + detail: 'The projection "Totally_Not_Real" could not be resolved.', + }, + }); + + await mount({ layerControl: false }); + drive(); + + expect(await screen.findByRole("alert")).toHaveTextContent( + "Totally_Not_Real", + ); + }); +}); + +describe("retry wiring and teardown", () => { + it("retry from the layers control re-invokes the loader", async () => { + acquireComponents.mockResolvedValue({ + error: { stage: "fetch", reason: "unreachable", detail: "no host" }, + }); + + await mount({ layerControl: true }); + drive(); + await screen.findAllByRole("alert"); + expect(acquireComponents).toHaveBeenCalledTimes(1); + + fireEvent.click(await screen.findByLabelText("Show Layers Control")); + fireEvent.click(await screen.findByLabelText("Retry Basins")); + + // Reset goes through the source's refresh, which is the only primitive that + // causes the loader to run again -- so driving it once more loads afresh. + drive(); + await waitFor(() => { + expect(acquireComponents.mock.calls.length).toBeGreaterThan(1); + }); + }); + + it("discards a layer's status when the layer is removed", async () => { + acquireComponents.mockResolvedValue({ + error: { stage: "fetch", reason: "unreachable", detail: "no host" }, + }); + + await mount({ layerControl: false }); + drive(); + expect(await screen.findByRole("alert")).toBeInTheDocument(); + + setLayers([]); + + // Status lives keyed on layer name, so a removed layer's failure must not + // linger -- and must not suppress a replacement's loading indication. + await waitFor(() => { + expect(screen.queryByRole("alert")).not.toBeInTheDocument(); + }); + }); +}); + +describe("per-layer rows in the layers control", () => { + function renderControl(shapefileStatus, onRetryShapefile) { + const layer = { + get: jest.fn((key) => (key === "name" ? "Basins" : undefined)), + getVisible: jest.fn(() => true), + setVisible: jest.fn(), + }; + render( + ({ getArray: () => [layer] }) }, + }} + shapefileStatus={shapefileStatus} + onRetryShapefile={onRetryShapefile} + />, + ); + return screen.findByLabelText("Show Layers Control").then((button) => { + fireEvent.click(button); + }); + } + + it("shows an in-flight indication for a loading layer", async () => { + await renderControl({ Basins: { state: "loading" } }); + expect(await screen.findByLabelText("Basins loading")).toBeInTheDocument(); + }); + + it("shows the failure message for a failed layer", async () => { + await renderControl({ + Basins: { + state: "error", + message: "The shapefile could not be fetched.", + kind: ERROR_KIND.FETCH, + }, + }); + expect( + await screen.findByText("The shapefile could not be fetched."), + ).toBeInTheDocument(); + }); + + it("offers retry for a fetch-stage failure and calls back with the layer name", async () => { + const onRetry = jest.fn(); + await renderControl( + { + Basins: { + state: "error", + message: "unreachable", + kind: ERROR_KIND.FETCH, + }, + }, + onRetry, + ); + + fireEvent.click(await screen.findByLabelText("Retry Basins")); + + expect(onRetry).toHaveBeenCalledWith("Basins"); + }); + + it.each([ + [ERROR_KIND.PROJECTION, "a missing or unresolvable coordinate system"], + [ERROR_KIND.PARSE, "a malformed component"], + [ERROR_KIND.TOO_LARGE, "a source over the size ceiling"], + ])("withholds retry for %s (%s)", async (kind) => { + // Re-running the same request cannot fix any of these -- they need the + // author to change something -- so a retry button would invite a viewer to + // re-download megabytes and fail identically. + await renderControl( + { Basins: { state: "error", message: "nope", kind } }, + jest.fn(), + ); + + expect(await screen.findByText("nope")).toBeInTheDocument(); + expect(screen.queryByLabelText("Retry Basins")).not.toBeInTheDocument(); + }); + + it("renders nothing extra for a layer with no status", async () => { + await renderControl({}); + expect( + await screen.findByLabelText("Basins Set Visible"), + ).toBeInTheDocument(); + expect(screen.queryByRole("alert")).not.toBeInTheDocument(); + }); +}); diff --git a/reactapp/components/map/LayersControl.js b/reactapp/components/map/LayersControl.js index da86d35c..d7d7e78e 100644 --- a/reactapp/components/map/LayersControl.js +++ b/reactapp/components/map/LayersControl.js @@ -1,5 +1,6 @@ import { useContext, useEffect, useState } from "react"; import PropTypes from "prop-types"; +import { isRetryable } from "components/map/layerStatus"; import styled from "styled-components"; import { FaLayerGroup, @@ -110,7 +111,13 @@ const CloseButton = styled.button` right: 5px; `; -const LayersControl = ({ updater, visualizationRef, runtimeLayerState }) => { +const LayersControl = ({ + updater, + visualizationRef, + runtimeLayerState, + shapefileStatus, + onRetryShapefile, +}) => { const [layers, setLayers] = useState([]); // [], controls what is shown in the layer controls const [isexpanded, setisexpanded] = useState(false); // bool, controls layer conrol menu expansion const [layerVisibility, setLayerVisibility] = useState({}); // {layerName: layerVisibility, ...}, controls checkbox checked value based on layer visibility @@ -198,6 +205,14 @@ const LayersControl = ({ updater, visualizationRef, runtimeLayerState }) => { : null; const progressPct = parseProgress(progressMessage); const error = isRuntime ? errorsByLayerId[layerId] : undefined; + // Client-parsed sources carry their own status, read from the + // source rather than from a request id -- there is no backend + // request behind them, so the progress channel above never has + // anything to report for one. + const shapefile = shapefileStatus?.[layerName]; + const shapefileLoading = shapefile?.state === "loading"; + const shapefileError = + shapefile?.state === "error" ? shapefile : null; // Hide progress bar once an error is set (error supersedes // any stale in-progress message) or when it has completed. const showProgress = @@ -240,6 +255,41 @@ const LayersControl = ({ updater, visualizationRef, runtimeLayerState }) => { )} + {shapefileLoading && ( +
+ + + +
+ )} + {shapefileError && ( + + + )} {error && (