[python] avoid etag/match_condition clientName collision with multiple etag headers

.chronus/changes/fix-python-multi-etag-collision-2026-05-27-10-15-00.md

@@ -0,0 +1,7 @@
+---
+changeKind: fix
+packages:
+  - "@typespec/http-client-python"
+---
+
+Fix `etag`/`match_condition` clientName collision when an operation has more than one `Azure.Core.eTag`-typed header (e.g. Storage's `copyFromUrl`, which has both `If-Match`/`If-None-Match` and `x-ms-source-if-match`/`x-ms-source-if-none-match`). The standard `If-Match`/`If-None-Match` pair is now preferred for the `etag`/`match_condition` slot, and any additional etag-typed headers retain their natural client name (e.g. `source_if_match`).

packages/http-client-python/generator/pygen/preprocess/__init__.py

@@ -169,6 +169,23 @@ def _get_etag_role(parameter: dict[str, Any]) -> Optional[str]:
     return parameter.get("etagRole")
 
 
+def _pick_etag_slot(
+    candidates: list[dict[str, Any]], standard_wire_name: str
+) -> Optional[dict[str, Any]]:
+    """Choose which etag-typed header should be promoted to the etag/match_condition slot.
+
+    When more than one etag-typed header is present in an operation, prefer the
+    standard If-Match/If-None-Match header (matched on wireName). Otherwise
+    fall back to the first candidate. Returns None if there are no candidates.
+    """
+    if not candidates:
+        return None
+    for c in candidates:
+        if get_wire_name_lower(c) == standard_wire_name:
+            return c
+    return candidates[0]
+
+
 def headers_convert(yaml_data: dict[str, Any], replace_data: Any) -> None:
     if isinstance(replace_data, dict):
         for k, v in replace_data.items():
@@ -313,18 +330,35 @@ def update_client(self, yaml_data: dict[str, Any]) -> None:
         yaml_data["builderPadName"] = to_snake_case(prop_name)
         for og in yaml_data.get("operationGroups", []):
             for o in og["operations"]:
-                property_if_match = None
-                property_if_none_match = None
+                if_match_candidates: list[dict[str, Any]] = []
+                if_none_match_candidates: list[dict[str, Any]] = []
                 for p in o["parameters"]:
                     wire_name_lower = get_wire_name_lower(p)
                     if p["location"] == "header" and wire_name_lower == "client-request-id":
                         yaml_data["requestIdHeaderName"] = wire_name_lower
                     if self.version_tolerant and p["location"] == "header":
                         role = _get_etag_role(p)
-                        if role == "ifMatch" and not property_if_match:
-                            property_if_match = p
-                        elif role == "ifNoneMatch" and not property_if_none_match:
-                            property_if_none_match = p
+                        if role == "ifMatch":
+                            if_match_candidates.append(p)
+                        elif role == "ifNoneMatch":
+                            if_none_match_candidates.append(p)
+
+                # When an operation has multiple etag-typed headers (e.g. Storage's
+                # copyFromUrl, which has both standard If-Match/If-None-Match and
+                # custom x-ms-source-if-match/x-ms-source-if-none-match), only one
+                # pair can be promoted to the etag/match_condition convention.
+                # Prefer the standard If-Match/If-None-Match pair so the result
+                # matches the pre-PR-10494 behaviour, and strip etagRole from the
+                # rest so they retain their natural clientName.
+                property_if_match = _pick_etag_slot(if_match_candidates, "if-match")
+                property_if_none_match = _pick_etag_slot(if_none_match_candidates, "if-none-match")
+                for c in if_match_candidates:
+                    if c is not property_if_match:
+                        c.pop("etagRole", None)
+                for c in if_none_match_candidates:
+                    if c is not property_if_none_match:
+                        c.pop("etagRole", None)
+
                 # pylint: disable=line-too-long
                 # some service(e.g. https://github.com/Azure/azure-rest-api-specs/blob/main/specification/cosmos-db/data-plane/Microsoft.Tables/preview/2019-02-02/table.json)
                 # only has one, so we need to add "if-none-match" or "if-match" if it's missing

packages/http-client-python/tests/unit/test_preprocess_etag.py

@@ -0,0 +1,203 @@
+# -------------------------------------------------------------------------
+# Copyright (c) Microsoft Corporation. All rights reserved.
+# Licensed under the MIT License. See License.txt in the project root for
+# license information.
+# --------------------------------------------------------------------------
+"""Tests for etag-typed header handling in the preprocess plugin."""
+from pygen.preprocess import PreProcessPlugin
+
+
+def _plugin() -> PreProcessPlugin:
+    return PreProcessPlugin(
+        output_folder="",
+        options={
+            "version-tolerant": True,
+            "models-mode": "dpg",
+            "show-operations": True,
+            "show-send-request": True,
+            "builders-visibility": "public",
+        },
+    )
+
+
+def _header_param(client_name: str, wire_name: str, etag_role: str | None) -> dict:
+    p: dict = {
+        "clientName": client_name,
+        "wireName": wire_name,
+        "location": "header",
+        "optional": True,
+        "implementation": "Method",
+        "type": {"type": "string"},
+    }
+    if etag_role is not None:
+        p["etagRole"] = etag_role
+    return p
+
+
+def _client_yaml(operation_params: list[dict]) -> dict:
+    return {
+        "name": "TestClient",
+        "namespace": "test",
+        "moduleName": "test",
+        "url": "",
+        "description": "test",
+        "parameters": [],
+        "operationGroups": [
+            {
+                "operations": [
+                    {
+                        "name": "copyFromUrl",
+                        "parameters": operation_params,
+                    }
+                ]
+            }
+        ],
+    }
+
+
+def _get_op(client: dict) -> dict:
+    return client["operationGroups"][0]["operations"][0]
+
+
+def test_etag_role_preserved_when_only_standard_pair_present():
+    """Standard If-Match/If-None-Match keep their etagRole."""
+    if_match = _header_param("if_match", "If-Match", "ifMatch")
+    if_none_match = _header_param("if_none_match", "If-None-Match", "ifNoneMatch")
+    client = _client_yaml([if_match, if_none_match])
+
+    _plugin().update_client(client)
+
+    op = _get_op(client)
+    standard_match = next(p for p in op["parameters"] if p["wireName"] == "If-Match")
+    standard_none = next(p for p in op["parameters"] if p["wireName"] == "If-None-Match")
+    assert standard_match.get("etagRole") == "ifMatch"
+    assert standard_none.get("etagRole") == "ifNoneMatch"
+    assert op["hasEtag"] is True
+
+
+def test_etag_role_preserved_when_only_custom_pair_present():
+    """Custom etag headers alone are promoted to the etag/match_condition slot."""
+    source_match = _header_param("source_if_match", "x-ms-source-if-match", "ifMatch")
+    source_none = _header_param(
+        "source_if_none_match", "x-ms-source-if-none-match", "ifNoneMatch"
+    )
+    client = _client_yaml([source_match, source_none])
+
+    _plugin().update_client(client)
+
+    assert source_match.get("etagRole") == "ifMatch"
+    assert source_none.get("etagRole") == "ifNoneMatch"
+
+
+def test_standard_etag_wins_over_custom_when_both_present():
+    """When both standard and custom etag headers are present in the same operation,
+    the standard If-Match/If-None-Match pair takes the etag/match_condition slot and
+    the custom headers have their etagRole stripped so they retain their natural
+    clientName (e.g. source_if_match) instead of colliding with the standard pair.
+
+    Regression test for PR #10494 which caused operations like Storage's copyFromUrl
+    to emit two parameters named "etag" and two named "match_condition".
+    """
+    source_match = _header_param(
+        "source_if_match", "x-ms-source-if-match", "ifMatch"
+    )
+    source_none = _header_param(
+        "source_if_none_match", "x-ms-source-if-none-match", "ifNoneMatch"
+    )
+    if_match = _header_param("if_match", "If-Match", "ifMatch")
+    if_none_match = _header_param("if_none_match", "If-None-Match", "ifNoneMatch")
+
+    # Mirror the parameter ordering in routes.tsp: source headers come first.
+    client = _client_yaml([source_match, source_none, if_match, if_none_match])
+    _plugin().update_client(client)
+
+    # Standard pair keeps etagRole and so will be transformed into etag/match_condition.
+    assert if_match.get("etagRole") == "ifMatch"
+    assert if_none_match.get("etagRole") == "ifNoneMatch"
+    # Custom pair has etagRole stripped, so update_parameter will NOT rename them.
+    assert "etagRole" not in source_match
+    assert "etagRole" not in source_none
+
+    # The selected pair should be at the end of the operation parameters.
+    op = _get_op(client)
+    assert op["parameters"][-2] is if_match
+    assert op["parameters"][-1] is if_none_match
+    assert op["hasEtag"] is True
+
+
+def test_first_custom_pair_chosen_when_multiple_custom_pairs_present():
+    """With multiple custom etag pairs and no standard pair, the first candidate wins."""
+    blob_match = _header_param("blob_if_match", "x-ms-blob-if-match", "ifMatch")
+    blob_none = _header_param(
+        "blob_if_none_match", "x-ms-blob-if-none-match", "ifNoneMatch"
+    )
+    source_match = _header_param(
+        "source_if_match", "x-ms-source-if-match", "ifMatch"
+    )
+    source_none = _header_param(
+        "source_if_none_match", "x-ms-source-if-none-match", "ifNoneMatch"
+    )
+    client = _client_yaml([blob_match, blob_none, source_match, source_none])
+
+    _plugin().update_client(client)
+
+    # First-encountered pair wins; the rest have etagRole stripped.
+    assert blob_match.get("etagRole") == "ifMatch"
+    assert blob_none.get("etagRole") == "ifNoneMatch"
+    assert "etagRole" not in source_match
+    assert "etagRole" not in source_none
+
+
+def test_synthetic_partner_still_works_with_only_one_custom_etag():
+    """When only a single custom etag header is present (no partner), the existing
+    synthetic-partner code path still creates a matching ifNoneMatch (or ifMatch)
+    copy. The fix must not regress this behaviour.
+    """
+    source_match = _header_param(
+        "source_if_match", "x-ms-source-if-match", "ifMatch"
+    )
+    client = _client_yaml([source_match])
+
+    _plugin().update_client(client)
+
+    op = _get_op(client)
+    assert op.get("hasEtag") is True
+    # The original custom param plus a synthetic partner are pushed to the end.
+    last_two = op["parameters"][-2:]
+    assert last_two[0]["etagRole"] == "ifMatch"
+    assert last_two[1]["etagRole"] == "ifNoneMatch"
+
+
+def test_full_update_yaml_does_not_collide_client_names():
+    """End-to-end: after update_client + update_parameter, the four etag headers
+    have distinct clientNames.
+
+    Without the fix, both source_if_match and if_match end up with clientName="etag",
+    and both source_if_none_match and if_none_match end up with clientName="match_condition".
+    """
+    source_match = _header_param(
+        "source_if_match", "x-ms-source-if-match", "ifMatch"
+    )
+    source_none = _header_param(
+        "source_if_none_match", "x-ms-source-if-none-match", "ifNoneMatch"
+    )
+    if_match = _header_param("if_match", "If-Match", "ifMatch")
+    if_none_match = _header_param("if_none_match", "If-None-Match", "ifNoneMatch")
+    client = _client_yaml([source_match, source_none, if_match, if_none_match])
+
+    plugin = _plugin()
+    plugin.update_client(client)
+    # update_client does the slot/strip; update_parameter does the rename.
+    op = _get_op(client)
+    for p in op["parameters"]:
+        plugin.update_parameter(p)
+
+    client_names = [p["clientName"] for p in op["parameters"]]
+    assert len(client_names) == len(set(client_names)), (
+        f"Duplicate clientNames after preprocess: {client_names}"
+    )
+    # The standard pair was promoted; the custom pair retains its natural names.
+    assert "etag" in client_names
+    assert "match_condition" in client_names
+    assert "source_if_match" in client_names
+    assert "source_if_none_match" in client_names