fix(stac): let a declared media type outrank an extension, and fail a refused PMTiles add

Author: clintonlunn

packages/plugins/src/plugins/maplibre-components.ts

@@ -4808,7 +4808,7 @@ export function resolvePMTilesLayerName(layerInfo: PMTilesLayerInfo, id: string)
   const queue = pendingPMTilesNames.get(layerInfo.url);
   const pending = queue?.shift();
   if (queue?.length === 0) pendingPMTilesNames.delete(layerInfo.url);
-  if (pending) return pending.name;
+  if (pending?.name) return pending.name;
   return layerInfo.name || layerNameFromUrl(layerInfo.url, id);
 }
 

packages/plugins/src/plugins/maplibre-stac.ts

@@ -512,7 +512,10 @@ async function visualizeAsset(
     case "pmtiles": {
       if (!appRef) throw new Error(labels.addFailed);
       // The same door the Source Cooperative browser uses, so an archive reaches the map one way.
-      await addPMTilesLayerFromUrl(appRef, asset.href, { fit: false, name });
+      // It answers false when the control will not mount, which is a failure like any other.
+      if (!(await addPMTilesLayerFromUrl(appRef, asset.href, { fit: false, name }))) {
+        throw new Error(labels.addFailed);
+      }
       return;
     }
     case "geojson": {

packages/plugins/src/plugins/stac-api.ts

@@ -554,14 +554,21 @@ export function itemBbox(item: StacItem): [number, number, number, number] | und
  * extension to fall back on when a catalog leaves the type off or writes it as octet-stream.
  * Tested in order, so a format whose extension another could claim comes first.
  */
-const VISUALIZABLE_FORMATS = {
-  pmtiles: { mediaType: "pmtiles", extension: /\.pmtiles($|\?)/i },
-  geojson: { mediaType: "geo+json", extension: /\.geojson($|\?)/i },
-  cog: { mediaType: "geotiff", extension: /\.tiff?($|\?)/i },
-} as const;
-
 /** A format {@link assetFormat} recognizes, and {@link visualizeAsset} knows how to add. */
-export type StacAssetFormat = keyof typeof VISUALIZABLE_FORMATS;
+export type StacAssetFormat = "pmtiles" | "geojson" | "cog";
+
+interface AssetFormatRule {
+  format: StacAssetFormat;
+  /** Matched within the asset's media type, which catalogs write with varying parameters. */
+  mediaType: string;
+  extension: RegExp;
+}
+
+const VISUALIZABLE_FORMATS: readonly AssetFormatRule[] = [
+  { format: "pmtiles", mediaType: "pmtiles", extension: /\.pmtiles($|\?)/i },
+  { format: "geojson", mediaType: "geo+json", extension: /\.geojson($|\?)/i },
+  { format: "cog", mediaType: "geotiff", extension: /\.tiff?($|\?)/i },
+];
 
 /**
  * Which format an asset is, or null when the panel cannot draw it. The single answer both the
@@ -570,12 +577,13 @@ export type StacAssetFormat = keyof typeof VISUALIZABLE_FORMATS;
  */
 export function assetFormat(asset: StacAsset): StacAssetFormat | null {
   const mediaType = (asset.type ?? "").toLowerCase();
-  for (const [format, match] of Object.entries(VISUALIZABLE_FORMATS)) {
-    if (mediaType.includes(match.mediaType) || match.extension.test(asset.href)) {
-      return format as StacAssetFormat;
-    }
-  }
-  return null;
+
+  // Every declared media type outranks every extension: a catalog that calls a `.pmtiles` href
+  // GeoJSON is describing its own asset, and reading the extension first would overrule it.
+  const declared = VISUALIZABLE_FORMATS.find((rule) => mediaType.includes(rule.mediaType));
+  if (declared) return declared.format;
+
+  return VISUALIZABLE_FORMATS.find((rule) => rule.extension.test(asset.href))?.format ?? null;
 }
 
 export function isVisualizableAsset(asset: StacAsset): boolean {

tests/pmtiles-control-layer.test.ts

@@ -16,7 +16,7 @@ function controlLayer(patch: Partial<PMTilesLayerInfo> = {}): PMTilesLayerInfo {
     opacity: 0.8,
     pickable: true,
     ...patch,
-  } as PMTilesLayerInfo;
+  };
 }
 
 describe("the store layer the PMTiles control's layeradd produces", () => {

tests/pmtiles-layer-name.test.ts

@@ -8,10 +8,16 @@ import {
 
 function layerInfo(patch: Partial<PMTilesLayerInfo> = {}): PMTilesLayerInfo {
   return {
+    id: "pmtiles-1",
     url: "https://example.org/warehouse/units.pmtiles",
     name: "",
+    tileType: "vector",
+    sourceLayers: ["units"],
+    layerIds: ["pmtiles-1-units-fill"],
+    opacity: 1,
+    pickable: true,
     ...patch,
-  } as PMTilesLayerInfo;
+  };
 }
 
 describe("naming a PMTiles layer a caller asked for", () => {
@@ -61,6 +67,13 @@ describe("naming a PMTiles layer a caller asked for", () => {
     assert.equal(resolvePMTilesLayerName(info, "layer-1"), "second item");
   });
 
+  it("falls back when a caller queues an empty name, rather than naming the layer nothing", () => {
+    const info = layerInfo({ name: "Named by the panel" });
+    setPendingPMTilesName(info.url, "");
+
+    assert.equal(resolvePMTilesLayerName(info, "layer-1"), "Named by the panel");
+  });
+
   it("keeps the control's own name when the caller supplied none", () => {
     assert.equal(
       resolvePMTilesLayerName(layerInfo({ name: "Named by the panel" }), "layer-1"),

tests/pmtiles-layer-sync.test.ts

@@ -1,29 +1,31 @@
 import assert from "node:assert/strict";
 import { describe, it } from "node:test";
 import { syncLayer } from "../packages/map/src/layer-sync";
-import { createPMTilesStoreLayer } from "../packages/map/src/pmtiles-layer";
+import {
+  createPMTilesStoreLayer,
+  type PMTilesStoreLayerOptions,
+} from "../packages/map/src/pmtiles-layer";
 
-interface MapCall {
-  method: string;
-  args: unknown[];
-}
+/** A MapLibre layer definition as the sync passes it, read by key in the assertions below. */
+type AddedLayer = Record<string, unknown> & { id: string };
 
 /** Enough of a MapLibre map to record what a sync pass adds, starting from an empty style. */
 function makeMapStub() {
-  const calls: MapCall[] = [];
+  const addedSources: { id: string; source: Record<string, unknown> }[] = [];
+  const addedLayers: AddedLayer[] = [];
   const sources = new Set<string>();
   const layers = new Set<string>();
   const map = {
     getStyle: () => ({ layers: [...layers].map((id) => ({ id, type: "fill" })) }),
     getLayer: (id: string) => (layers.has(id) ? { id, type: "fill" } : undefined),
     getSource: (id: string) => (sources.has(id) ? { id } : undefined),
-    addSource: (id: string, source: unknown) => {
+    addSource: (id: string, source: Record<string, unknown>) => {
       sources.add(id);
-      calls.push({ method: "addSource", args: [id, source] });
+      addedSources.push({ id, source });
     },
-    addLayer: (layer: { id: string }) => {
+    addLayer: (layer: AddedLayer) => {
       layers.add(layer.id);
-      calls.push({ method: "addLayer", args: [layer] });
+      addedLayers.push(layer);
     },
     removeLayer: () => {},
     removeSource: () => {},
@@ -32,69 +34,69 @@ function makeMapStub() {
     setPaintProperty: () => {},
     setLayerZoomRange: () => {},
   };
-  const added = (method: string) => calls.filter((call) => call.method === method);
-  return { map, added };
+  return { map, addedSources, addedLayers };
 }
 
-const archive = {
+const archive: PMTilesStoreLayerOptions = {
   id: "layer-1",
   name: "Geologic units",
   url: "https://example.org/units.pmtiles",
-  tileType: "vector" as const,
+  tileType: "vector",
   sourceLayers: ["units"],
 };
 
 describe("syncing a layer from createPMTilesStoreLayer", () => {
   it("adds the archive as a vector source and one MapLibre layer per declared id", () => {
     const layer = createPMTilesStoreLayer(archive);
-    const { map, added } = makeMapStub();
+    const { map, addedSources, addedLayers } = makeMapStub();
 
     syncLayer(map as never, layer);
 
-    assert.deepEqual(added("addSource")[0]?.args, [
-      "layer-1",
-      { type: "vector", url: "pmtiles://https://example.org/units.pmtiles" },
-    ]);
+    assert.deepEqual(addedSources[0], {
+      id: "layer-1",
+      source: { type: "vector", url: "pmtiles://https://example.org/units.pmtiles" },
+    });
     // The ids the builder promised are exactly the ones the sync creates: a layer whose
     // nativeLayerIds name something else renders nothing while claiming it renders.
     assert.deepEqual(
-      added("addLayer").map((call) => (call.args[0] as { id: string }).id),
+      addedLayers.map((added) => added.id),
       layer.metadata.nativeLayerIds,
     );
-    for (const call of added("addLayer")) {
-      assert.equal((call.args[0] as Record<string, unknown>)["source-layer"], "units");
-      assert.equal((call.args[0] as Record<string, unknown>).source, "layer-1");
+    for (const added of addedLayers) {
+      assert.equal(added["source-layer"], "units");
+      assert.equal(added.source, "layer-1");
     }
   });
 
   it("adds a raster archive as a raster source and its single layer", () => {
     const layer = createPMTilesStoreLayer({ ...archive, tileType: "raster", sourceLayers: [] });
-    const { map, added } = makeMapStub();
+    const { map, addedSources, addedLayers } = makeMapStub();
 
     syncLayer(map as never, layer);
 
-    assert.equal((added("addSource")[0]?.args[1] as { type: string }).type, "raster");
+    assert.equal(addedSources[0]?.source.type, "raster");
     assert.deepEqual(
-      added("addLayer").map((call) => (call.args[0] as { id: string }).id),
+      addedLayers.map((added) => added.id),
       ["layer-1-raster"],
     );
   });
 
   it("renders a source layer whose name needs encoding in a layer id", () => {
     const layer = createPMTilesStoreLayer({ ...archive, sourceLayers: ["water lines"] });
-    const { map, added } = makeMapStub();
+    const { map, addedLayers } = makeMapStub();
 
     syncLayer(map as never, layer);
 
     assert.deepEqual(
-      added("addLayer").map((call) => (call.args[0] as { id: string }).id),
+      addedLayers.map((added) => added.id),
       layer.metadata.nativeLayerIds,
     );
     // The source-layer keeps the archive's own name; only the id is encoded.
-    assert.equal(
-      (added("addLayer")[0]?.args[0] as Record<string, unknown>)["source-layer"],
-      "water lines",
-    );
-    assert.equal((layer.metadata.nativeLayerIds as string[])[0], "layer-1-water_20lines-fill");
+    assert.equal(addedLayers[0]?.["source-layer"], "water lines");
+    assert.deepEqual(layer.metadata.nativeLayerIds, [
+      "layer-1-water_20lines-fill",
+      "layer-1-water_20lines-line",
+      "layer-1-water_20lines-circle",
+    ]);
   });
 });

tests/pmtiles-store-layer.test.ts

@@ -1,14 +1,17 @@
 import assert from "node:assert/strict";
 import { describe, it } from "node:test";
 import { createEmptyProject, parseProject, serializeProject } from "@geolibre/core";
-import { createPMTilesStoreLayer } from "../packages/map/src/pmtiles-layer";
+import {
+  createPMTilesStoreLayer,
+  type PMTilesStoreLayerOptions,
+} from "../packages/map/src/pmtiles-layer";
 import { isPlaceholderLayer } from "../packages/map/src/placeholders";
 
-const archive = {
+const archive: PMTilesStoreLayerOptions = {
   id: "layer-1",
   name: "Geologic units",
   url: "https://example.org/units.pmtiles",
-  tileType: "vector" as const,
+  tileType: "vector",
   sourceLayers: ["units"],
 };
 

tests/stac-api.test.ts

@@ -1161,6 +1161,16 @@ test("an asset's format comes from its media type, or its extension when there i
   // what the asset is, not by what the path says.
   assert.equal(assetFormat({ href: "https://example.com/geotiff/a.pmtiles" }), "pmtiles");
 
+  // And a declared media type wins over any extension, whichever format each of them names.
+  assert.equal(
+    assetFormat({ href: "https://example.com/a.pmtiles", type: "application/geo+json" }),
+    "geojson",
+  );
+  assert.equal(
+    assetFormat({ href: "https://example.com/a.geojson", type: "application/vnd.pmtiles" }),
+    "pmtiles",
+  );
+
   // Which is the same answer that decides whether Add is enabled.
   assert.equal(
     isVisualizableAsset({ href: "https://example.com/a.pmtiles", type: "application/vnd.pmtiles" }),