diff --git a/test_mcp/test_per_source_type_layer_tools.py b/test_mcp/test_per_source_type_layer_tools.py index 75123d6..88e9cdb 100644 --- a/test_mcp/test_per_source_type_layer_tools.py +++ b/test_mcp/test_per_source_type_layer_tools.py @@ -1523,7 +1523,14 @@ def test_popup_options_omit_persist_at_omitted_popup_attributes(self): assert layer["omittedPopupAttributes"] == {"Cities": ["sensitive_field"]} def test_advanced_dicts_accepted_as_json_strings(self): - """LLM providers may serialize dict args as JSON strings — boundary coerces.""" + """LLM providers may serialize dict args as JSON strings — boundary coerces. + + Per the 2026-05-21 WMS attr_key fix, the popup_options outer key is + the wms_layers value (here ``"ws:layer"``), not the display name — + regardless of what the LLM emits as the outer key. PR #11's + single-entry outer-key normalization rewrites the mismatched + display-name key to the canonical attr_key. + """ result = add_wms_layer( map_uuid=MAP_UUID, name="WMS JSON-String Dicts", @@ -1536,8 +1543,11 @@ def test_advanced_dicts_accepted_as_json_strings(self): config = _get_configuration(result) assert config["props"]["minZoom"] == 3 assert config["props"]["source"]["props"]["projection"] == "EPSG:4326" + # Outer key normalized to wms_layers value ("ws:layer"), not the + # display name. This is what React's getImageWMSLayerAttributes + # queries by. assert result["layer_update"]["layer"]["omittedPopupAttributes"] == { - "WMS JSON-String Dicts": ["x"] + "ws:layer": ["x"] } @@ -2223,3 +2233,85 @@ def test_consuming_types_still_accept_params(self): params={"STYLES": "default"}, ) assert "layer_update" in result, result + + +# --------------------------------------------------------------------------- +# WMS attribute-variables / popup-options outer-key resolution +# (debug audit 2026-05-21: Class A bug — same shape as ESRI Image pre-PR-#12) +# --------------------------------------------------------------------------- + + +class TestAddWmsLayerAttrKeyFromWmsLayers: + """Server keys attributeVariables / popup_options.aliases by the WMS + LAYERS param value (e.g., "topp:states"), not the user-supplied + display name. React's getImageWMSLayerAttributes queries by the + LAYERS value — mismatch causes silent click-time lookup failures + just like the pre-PR-#12 ESRI Image bug. + """ + + URL = "https://example.com/geoserver/wms" + + def test_attribute_variables_outer_key_is_wms_layers_value(self): + """The motivating bug: outer key uses LAYERS (topp:states), not display name.""" + result = add_wms_layer( + map_uuid=MAP_UUID, + name="US States", + url=self.URL, + wms_layers="topp:states", + attribute_variables={"STATE_NAME": "selected_state"}, + ) + layer = result["layer_update"]["layer"] + assert layer["attributeVariables"] == { + "topp:states": {"STATE_NAME": "selected_state"} + }, "outer key should be wms_layers value, not the display name 'US States'" + assert "US States" not in layer["attributeVariables"] + + def test_comma_separated_wms_layers_picks_first(self): + """For multi-layer WMS calls, the first layer is the conservative pick.""" + result = add_wms_layer( + map_uuid=MAP_UUID, + name="Multi Layer", + url=self.URL, + wms_layers="topp:states,topp:counties", + attribute_variables={"STATE_NAME": "selected_state"}, + ) + layer = result["layer_update"]["layer"] + assert "topp:states" in layer["attributeVariables"] + assert "topp:counties" not in layer["attributeVariables"] + + def test_popup_options_aliases_also_uses_wms_layers_value(self): + """popup_options.aliases gets the same outer-key normalization as attributeVariables.""" + result = add_wms_layer( + map_uuid=MAP_UUID, + name="US States", + url=self.URL, + wms_layers="topp:states", + popup_options={ + "aliases": {"topp:states": {"STATE_NAME": "State Name"}}, + }, + ) + layer = result["layer_update"]["layer"] + # PR #11's outer-key normalize logic also has to use wms_layers + # to land the right key; verify the persisted shape matches what + # the React popup-table render path will query. + assert "topp:states" in layer["attributeAliases"] + assert layer["attributeAliases"]["topp:states"] == { + "STATE_NAME": "State Name" + } + + def test_no_attribute_variables_no_regression(self): + """When neither attribute_variables nor popup_options is provided, + the regular WMS layer-add flow is unchanged. + """ + result = add_wms_layer( + map_uuid=MAP_UUID, + name="Plain WMS", + url=self.URL, + wms_layers="topp:states", + ) + assert "layer_update" in result + layer = result["layer_update"]["layer"] + # No attributeVariables / attributeAliases when not provided (build() + # deletes the empty dicts). + assert "attributeVariables" not in layer + assert "attributeAliases" not in layer diff --git a/tethysdash_mcp/mcp_server.py b/tethysdash_mcp/mcp_server.py index b419e79..408b1d2 100644 --- a/tethysdash_mcp/mcp_server.py +++ b/tethysdash_mcp/mcp_server.py @@ -1675,6 +1675,28 @@ def _resolve_esri_layer_name(url: str, layer_id: Optional[str]) -> Optional[str] return None +def _resolve_wms_attr_key(wms_layers: str, fallback_name: str) -> str: + """Return the canonical layer-name key for WMS ``attributeVariables``. + + The React popup-render path (``reactapp/components/map/utilities.js`` + ``getImageWMSLayerAttributes``) keys alias maps by the WMS ``LAYERS`` + param value (e.g., ``"topp:states"``), not by the user-supplied display + name. Storing ``attributeVariables`` keyed by the display name causes + the click-time lookup to silently miss. + + This helper picks the first comma-separated entry from ``wms_layers`` + (stripped of whitespace). Multi-layer WMS calls are rare in practice + and the React attribute-config UI is per-layer anyway; the first layer + is the conservative pick. If ``wms_layers`` is empty or missing, + falls back to ``fallback_name`` so behavior never breaks for + misconfigured inputs. + """ + if not wms_layers or not isinstance(wms_layers, str): + return fallback_name + first = wms_layers.split(",", 1)[0].strip() + return first or fallback_name + + # --------------------------------------------------------------------------- # Per-source-type map-layer tools. # @@ -1940,11 +1962,18 @@ def add_wms_layer( builder.set_source_properties(**flat_source_props) if source_props: builder.set_source_properties(**source_props) + # WMS attr_key normalization: React's getImageWMSLayerAttributes + # keys alias maps by the LAYERS param value (e.g., "topp:states"), + # not by the user-supplied display name. Resolve attr_key to the + # first comma-separated entry of wms_layers so the persisted + # attributeVariables / popup_options keys match what the popup- + # render path queries. See _resolve_wms_attr_key for full rationale. + attr_key = _resolve_wms_attr_key(wms_layers, name) _apply_common_layer_options( builder, opacity=opacity, min_zoom=min_zoom, max_zoom=max_zoom, visible=visible, queryable=queryable, legend=legend, style=style, - attribute_variables=attribute_variables, attr_key=name, + attribute_variables=attribute_variables, attr_key=attr_key, layer_props=layer_props, popup_options=popup_options, ) layer_config = builder.build()