From bf44cc0f25f9b08ab4ec7277bb897082b52eda44 Mon Sep 17 00:00:00 2001 From: Huarch Date: Fri, 11 Sep 2026 18:25:41 +0800 Subject: [PATCH] fix: synchronize dynamic GeoServer layer lifecycle Optional sources became dynamic while dependent labels and controller state remained static, allowing stale requests to restore removed layers. Register dependent layers through the same lifecycle and invalidate target request generations before release. --- .../hooks/use-workbench-map-controller.ts | 7 +- .../workbench/hooks/use-workbench-map.ts | 16 +- .../map/workbench-map-controller.test.ts | 149 ++++++++++++++++++ .../workbench/map/workbench-map-controller.ts | 114 +++++++++++++- tests/browser/scada-api-source.e2e.ts | 30 +++- 5 files changed, 303 insertions(+), 13 deletions(-) diff --git a/src/features/workbench/hooks/use-workbench-map-controller.ts b/src/features/workbench/hooks/use-workbench-map-controller.ts index dd9bcc9..3a28a66 100644 --- a/src/features/workbench/hooks/use-workbench-map-controller.ts +++ b/src/features/workbench/hooks/use-workbench-map-controller.ts @@ -67,11 +67,8 @@ export function useWorkbenchMapController({ controller.getSnapshot ); useEffect(() => { - const target = state.target; - if (target && availableSourceIds && !availableSourceIds.includes(target.sourceId)) { - controller.clearHighlight(); - } - }, [availableSourceIds, controller, state.target]); + if (availableSourceIds) controller.syncAvailableSources(availableSourceIds); + }, [availableSourceIds, controller]); useEffect(() => () => controller.destroy(), [controller]); return { controller, state }; } diff --git a/src/features/workbench/hooks/use-workbench-map.ts b/src/features/workbench/hooks/use-workbench-map.ts index 87a9287..576e6b3 100644 --- a/src/features/workbench/hooks/use-workbench-map.ts +++ b/src/features/workbench/hooks/use-workbench-map.ts @@ -276,7 +276,10 @@ export function useWorkbenchMap({ REQUIRED_GEOSERVER_SOURCE_IDS ).forEach((layer) => map.addLayer(layer)); scadaInteractionLayers.forEach((layer) => map.addLayer(layer)); - createValueLabelLayers().forEach((layer) => map.addLayer(layer)); + filterWaterNetworkLayersBySourceIds( + createValueLabelLayers(), + REQUIRED_GEOSERVER_SOURCE_IDS + ).forEach((layer) => map.addLayer(layer)); simulationAnnotationLayers .filter((layer) => layer.type === "symbol") .forEach((layer) => map.addLayer(layer)); @@ -461,6 +464,17 @@ function syncDataDependentGeoServerLayers( availableSourceIdSet, layerVisibility ); + addMissingDataDependentLayers( + map, + createValueLabelLayers(), + [ + ...simulationAnnotationLayers.filter((layer) => layer.type === "symbol"), + ...waterNetworkHitLayers, + ...scadaHitLayers + ].map((layer) => layer.id), + availableSourceIdSet, + layerVisibility + ); addMissingDataDependentLayers( map, waterNetworkHitLayers, diff --git a/src/features/workbench/map/workbench-map-controller.test.ts b/src/features/workbench/map/workbench-map-controller.test.ts index 8492273..c5a3134 100644 --- a/src/features/workbench/map/workbench-map-controller.test.ts +++ b/src/features/workbench/map/workbench-map-controller.test.ts @@ -58,6 +58,155 @@ describe("WorkbenchMapController", () => { expect(controller.getSnapshot().errorCode).toBe("FEATURE_NOT_FOUND"); }); + it("drops cached features and presentation state when a dynamic source disappears", async () => { + const map = createMap(); + map.getLayer.mockReturnValue({} as never); + let collection = pointCollection; + const fetchFeatures = vi.fn(async () => collection); + const controller = new WorkbenchMapController({ + getMap: () => map as unknown as MapLibreMap, + isReady: () => true, + getPadding: () => ({ top: 0, right: 0, bottom: 0, left: 0 }), + fetchFeatures + }); + const target = { sourceId: "valves" as const, featureId: "V-1" }; + await controller.locateAndHighlight(target); + controller.applyLayerGroupStyle("valves", { iconMultiplier: 1.5 }); + await controller.showValueLabel({ + sourceId: "valves", + property: "elevation", + precision: 1, + showValue: true, + showId: false, + textSize: 12, + textColor: "#0F172A" + }); + + controller.syncAvailableSources(["pipes", "junctions", "scada"]); + + expect(controller.getSnapshot()).toMatchObject({ + target: null, + valueLabels: {}, + styledGroups: [] + }); + collection = { + ...pointCollection, + features: [{ + ...pointCollection.features[0], + geometry: { type: "Point", coordinates: [121.9, 29.1] } + }] + }; + map.easeTo.mockClear(); + + await controller.locateAndHighlight(target); + + expect(fetchFeatures).toHaveBeenCalledTimes(2); + expect(map.easeTo).toHaveBeenCalledWith(expect.objectContaining({ center: [121.9, 29.1] })); + }); + + it("ignores an in-flight dynamic-source result from an earlier availability generation", async () => { + const map = createMap(); + let resolveFeatures = (_collection: FeatureCollection) => {}; + const featureResponse = new Promise((resolve) => { + resolveFeatures = resolve; + }); + const controller = new WorkbenchMapController({ + getMap: () => map as unknown as MapLibreMap, + isReady: () => true, + getPadding: () => ({ top: 0, right: 0, bottom: 0, left: 0 }), + fetchFeatures: async () => featureResponse + }); + controller.syncAvailableSources(["pipes", "junctions", "valves", "scada"]); + + const locating = controller.locateAndHighlight({ sourceId: "valves", featureId: "V-1" }); + controller.syncAvailableSources(["pipes", "junctions", "scada"]); + controller.syncAvailableSources(["pipes", "junctions", "valves", "scada"]); + resolveFeatures(pointCollection); + await locating; + + expect(map.easeTo).not.toHaveBeenCalled(); + expect(map.setFeatureState).not.toHaveBeenCalled(); + expect(controller.getSnapshot().target).toBeNull(); + }); + + it("keeps a newer cached target when an older dynamic-source request resolves", async () => { + const map = createMap(); + let resolveDynamicFeatures = (_collection: FeatureCollection) => {}; + const dynamicFeatureResponse = new Promise((resolve) => { + resolveDynamicFeatures = resolve; + }); + const fetchFeatures = vi.fn(async (sourceId: string) => + sourceId === "valves" ? dynamicFeatureResponse : pointCollection + ); + const controller = new WorkbenchMapController({ + getMap: () => map as unknown as MapLibreMap, + isReady: () => true, + getPadding: () => ({ top: 0, right: 0, bottom: 0, left: 0 }), + fetchFeatures + }); + const cachedTarget = { sourceId: "junctions" as const, featureId: "J-1" }; + await controller.locateAndHighlight(cachedTarget); + + const locatingDynamicTarget = controller.locateAndHighlight({ sourceId: "valves", featureId: "V-1" }); + await controller.locateAndHighlight(cachedTarget); + + expect(controller.getSnapshot()).toMatchObject({ target: cachedTarget, pending: false }); + resolveDynamicFeatures(pointCollection); + await locatingDynamicTarget; + + expect(map.easeTo).toHaveBeenCalledTimes(2); + expect(controller.getSnapshot()).toMatchObject({ target: cachedTarget, pending: false }); + }); + + it("does not restore a target after its in-flight request is cleared", async () => { + const map = createMap(); + let resolveFeatures = (_collection: FeatureCollection) => {}; + const featureResponse = new Promise((resolve) => { + resolveFeatures = resolve; + }); + const controller = new WorkbenchMapController({ + getMap: () => map as unknown as MapLibreMap, + isReady: () => true, + getPadding: () => ({ top: 0, right: 0, bottom: 0, left: 0 }), + fetchFeatures: async () => featureResponse + }); + + const locating = controller.locateAndHighlight({ sourceId: "valves", featureId: "V-1" }); + controller.clearHighlight(); + resolveFeatures(pointCollection); + await locating; + + expect(map.easeTo).not.toHaveBeenCalled(); + expect(map.setFeatureState).not.toHaveBeenCalled(); + expect(controller.getSnapshot()).toMatchObject({ target: null, pending: false }); + }); + + it("reports an unavailable target when its source disappears in flight", async () => { + const map = createMap(); + let resolveFeatures = (_collection: FeatureCollection) => {}; + const featureResponse = new Promise((resolve) => { + resolveFeatures = resolve; + }); + const controller = new WorkbenchMapController({ + getMap: () => map as unknown as MapLibreMap, + isReady: () => true, + getPadding: () => ({ top: 0, right: 0, bottom: 0, left: 0 }), + fetchFeatures: async () => featureResponse + }); + controller.syncAvailableSources(["pipes", "junctions", "valves", "scada"]); + + const locating = controller.locateAndHighlight({ sourceId: "valves", featureId: "V-1" }); + controller.syncAvailableSources(["pipes", "junctions", "scada"]); + resolveFeatures(pointCollection); + await locating; + + expect(controller.getSnapshot()).toMatchObject({ + target: null, + pending: false, + errorCode: "WFS_UNAVAILABLE" + }); + }); + it("fits line bounds and uses responsive workbench padding", async () => { const map = createMap(); const padding = { top: 50, right: 420, bottom: 50, left: 320 }; diff --git a/src/features/workbench/map/workbench-map-controller.ts b/src/features/workbench/map/workbench-map-controller.ts index 224920b..1669564 100644 --- a/src/features/workbench/map/workbench-map-controller.ts +++ b/src/features/workbench/map/workbench-map-controller.ts @@ -27,6 +27,7 @@ import { normalizeMapFeatureCollection, parseMapFeatureQuery } from "./map-feature-query"; +import { DATA_DEPENDENT_GEOSERVER_SOURCE_IDS } from "./geoserver-layer-availability"; import { applyValueLabelStyle, clearValueLabels, @@ -110,6 +111,10 @@ export class WorkbenchMapController implements WorkbenchMapCommands { private flowState: FlowOverlayState = createFlowOverlayState(); private flowUpdateRevision = 0; private scadaAnalysisRevision = 0; + private targetRequestRevision = 0; + private pendingTargetSourceId: WaterNetworkSourceId | null = null; + private availableSourceIds: ReadonlySet | null = null; + private sourceAvailabilityGenerations = new Map(); constructor(private readonly options: WorkbenchMapControllerOptions) {} @@ -121,16 +126,20 @@ export class WorkbenchMapController implements WorkbenchMapCommands { getSnapshot = () => this.state; async zoomToFeature(target: FeatureTarget) { + const requestRevision = this.beginTargetCommand(); if (!this.requireTargetMap(target)) return; - const feature = await this.resolveTarget(target); + const feature = await this.resolveTarget(target, requestRevision); + if (requestRevision !== this.targetRequestRevision) return; const map = this.requireTargetMap(target); if (!feature || !map) return; this.moveCameraToFeature(map, feature); } async locateAndHighlight(target: FeatureTarget) { + const requestRevision = this.beginTargetCommand(); if (!this.requireTargetMap(target)) return; - const feature = await this.resolveTarget(target); + const feature = await this.resolveTarget(target, requestRevision); + if (requestRevision !== this.targetRequestRevision) return; const map = this.requireTargetMap(target); if (!feature || !map) return; if (!this.moveCameraToFeature(map, feature)) return; @@ -167,12 +176,15 @@ export class WorkbenchMapController implements WorkbenchMapCommands { } async highlight(target: FeatureTarget) { + const requestRevision = this.beginTargetCommand(); if (!this.requireTargetMap(target)) return; - const feature = await this.resolveTarget(target); + const feature = await this.resolveTarget(target, requestRevision); + if (requestRevision !== this.targetRequestRevision) return; if (feature) this.setHighlight(target); } clearHighlight = () => { + this.beginTargetCommand(); const map = this.options.getMap(); if (map && this.highlightedTarget && map.getSource(this.highlightedTarget.sourceId)) { map.removeFeatureState(toFeatureStateTarget(this.highlightedTarget), "highlighted"); @@ -181,6 +193,73 @@ export class WorkbenchMapController implements WorkbenchMapCommands { this.patchState({ target: null, errorCode: null }); }; + syncAvailableSources(sourceIds: readonly WaterNetworkSourceId[]) { + const availableSourceIds = new Set(sourceIds); + if (this.availableSourceIds) { + DATA_DEPENDENT_GEOSERVER_SOURCE_IDS.forEach((sourceId) => { + if (this.availableSourceIds?.has(sourceId) && !availableSourceIds.has(sourceId)) { + this.sourceAvailabilityGenerations.set( + sourceId, + (this.sourceAvailabilityGenerations.get(sourceId) ?? 0) + 1 + ); + } + }); + } + this.availableSourceIds = availableSourceIds; + + DATA_DEPENDENT_GEOSERVER_SOURCE_IDS.forEach((sourceId) => { + if (availableSourceIds.has(sourceId)) return; + const cachePrefix = `${sourceId}:`; + for (const key of this.cachedFeatures.keys()) { + if (key.startsWith(cachePrefix)) this.cachedFeatures.delete(key); + } + }); + + const targetUnavailable = Boolean( + this.highlightedTarget && !availableSourceIds.has(this.highlightedTarget.sourceId) + ); + const pendingTargetUnavailable = Boolean( + this.pendingTargetSourceId && !availableSourceIds.has(this.pendingTargetSourceId) + ); + if (pendingTargetUnavailable) { + this.targetRequestRevision += 1; + this.pendingTargetSourceId = null; + } + if (targetUnavailable && this.highlightedTarget) { + const map = this.options.getMap(); + if (map?.getSource(this.highlightedTarget.sourceId)) { + map.removeFeatureState(toFeatureStateTarget(this.highlightedTarget), "highlighted"); + } + this.highlightedTarget = null; + } + + const valueLabelEntries = Object.entries(this.state.valueLabels); + const availableValueLabelEntries = valueLabelEntries.filter(([sourceId]) => + availableSourceIds.has(sourceId as WaterNetworkSourceId) + ); + const valueLabelsChanged = availableValueLabelEntries.length !== valueLabelEntries.length; + const styledGroups = this.state.styledGroups.filter((groupId) => + availableSourceIds.has(groupId) + ); + const styledGroupsChanged = styledGroups.length !== this.state.styledGroups.length; + if ( + !targetUnavailable && + !pendingTargetUnavailable && + !valueLabelsChanged && + !styledGroupsChanged + ) return; + + this.patchState({ + target: targetUnavailable ? null : this.state.target, + pending: pendingTargetUnavailable ? false : this.state.pending, + errorCode: pendingTargetUnavailable ? "WFS_UNAVAILABLE" : this.state.errorCode, + valueLabels: valueLabelsChanged + ? Object.fromEntries(availableValueLabelEntries) as WorkbenchMapControllerState["valueLabels"] + : this.state.valueLabels, + styledGroups + }); + } + async setFlowVisible(visible: boolean) { const map = this.requireMap(); if (!map) return; @@ -445,29 +524,52 @@ export class WorkbenchMapController implements WorkbenchMapCommands { destroy() { this.flowUpdateRevision += 1; + this.targetRequestRevision += 1; + this.pendingTargetSourceId = null; const map = this.options.getMap(); if (map) hideFlowOverlay(map, this.flowState); this.listeners.clear(); } - private async resolveTarget(target: FeatureTarget) { + private beginTargetCommand() { + const requestRevision = ++this.targetRequestRevision; + if (this.pendingTargetSourceId) { + this.pendingTargetSourceId = null; + this.patchState({ pending: false }); + } + return requestRevision; + } + + private async resolveTarget(target: FeatureTarget, requestRevision: number) { const map = this.requireMap(); if (!map) return null; const key = `${target.sourceId}:${target.featureId}`; - const cached = this.cachedFeatures.get(key); + const dataDependent = DATA_DEPENDENT_GEOSERVER_SOURCE_IDS.includes( + target.sourceId as (typeof DATA_DEPENDENT_GEOSERVER_SOURCE_IDS)[number] + ); + const cached = dataDependent ? undefined : this.cachedFeatures.get(key); if (cached) return cached; + const sourceAvailabilityGeneration = this.sourceAvailabilityGenerations.get(target.sourceId) ?? 0; + this.pendingTargetSourceId = target.sourceId; this.patchState({ pending: true, errorCode: null }); try { const collection = await this.fetchFeatures(target.sourceId, [target.featureId]); + if ( + requestRevision !== this.targetRequestRevision || + sourceAvailabilityGeneration !== (this.sourceAvailabilityGenerations.get(target.sourceId) ?? 0) + ) return null; + this.pendingTargetSourceId = null; const feature = collection.features[0]; if (!feature) { this.patchState({ pending: false, errorCode: "FEATURE_NOT_FOUND" }); return null; } - this.cachedFeatures.set(key, feature); + if (!dataDependent) this.cachedFeatures.set(key, feature); this.patchState({ pending: false }); return feature; } catch { + if (requestRevision !== this.targetRequestRevision) return null; + this.pendingTargetSourceId = null; this.patchState({ pending: false, errorCode: target.sourceId === "scada" ? "SCADA_API_UNAVAILABLE" : "WFS_UNAVAILABLE" diff --git a/tests/browser/scada-api-source.e2e.ts b/tests/browser/scada-api-source.e2e.ts index 97cb22f..03387b8 100644 --- a/tests/browser/scada-api-source.e2e.ts +++ b/tests/browser/scada-api-source.e2e.ts @@ -61,10 +61,38 @@ test("shows data-dependent layer controls only when GeoServer reports features", await expect(page.getByRole("button", { name: /^水箱/ })).toBeVisible(); await expect.poll(() => page.evaluate(() => ({ valve: Boolean(globalThis.__waterNetworkMap?.getLayer("valves-symbol")), + valveLabel: Boolean(globalThis.__waterNetworkMap?.getLayer("workbench-value-label-valves")), reservoir: Boolean(globalThis.__waterNetworkMap?.getLayer("reservoirs-symbol")), + reservoirLabel: Boolean(globalThis.__waterNetworkMap?.getLayer("workbench-value-label-reservoirs")), pump: Boolean(globalThis.__waterNetworkMap?.getLayer("pumps-symbol")), tank: Boolean(globalThis.__waterNetworkMap?.getLayer("tanks-symbol")) - }))).toEqual({ valve: true, reservoir: true, pump: true, tank: true }); + }))).toEqual({ + valve: true, + valveLabel: true, + reservoir: true, + reservoirLabel: true, + pump: true, + tank: true + }); + await expect.poll(() => page.evaluate(() => { + const layerIds = globalThis.__waterNetworkMap?.getStyle().layers.map((layer) => layer.id) ?? []; + return { + valveLabel: layerIds.indexOf("workbench-value-label-valves"), + simulationLabel: layerIds.indexOf("simulation-burst-label") + }; + })).toMatchObject({ + valveLabel: expect.any(Number), + simulationLabel: expect.any(Number) + }); + const layerOrder = await page.evaluate(() => { + const layerIds = globalThis.__waterNetworkMap?.getStyle().layers.map((layer) => layer.id) ?? []; + return [ + layerIds.indexOf("workbench-value-label-valves"), + layerIds.indexOf("simulation-burst-label") + ]; + }); + expect(layerOrder[0]).toBeGreaterThanOrEqual(0); + expect(layerOrder[0]).toBeLessThan(layerOrder[1]); }); test("loads required map layers while optional layer probes are still pending", async ({ page }) => {