[python] avoid etag/match_condition clientName collision with multiple etag headers#10816
Draft
l0lawrence wants to merge 1 commit into
Draft
[python] avoid etag/match_condition clientName collision with multiple etag headers#10816l0lawrence wants to merge 1 commit into
l0lawrence wants to merge 1 commit into
Conversation
…ion with multiple etag headers When an operation has more than one Azure.Core.eTag-typed header (e.g. Storage's copyFromUrl, which carries both standard If-Match/If-None-Match AND custom x-ms-source-if-match/x-ms-source-if-none-match), PR microsoft#10494's per-parameter conversion renamed every etag-typed header to clientName='etag' or 'match_condition', producing two parameters with the same name on the generated method. Fix: in preprocess update_client, collect all ifMatch/ifNoneMatch candidates per operation, prefer the standard If-Match/If-None-Match wire names for the etag/match_condition slot, and strip etagRole from the non-selected candidates so they retain their natural clientName (e.g. source_if_match). Adds tests/unit/test_preprocess_etag.py covering the standard-only, custom-only, mixed (regression), multi-custom-pair, synthetic-partner, and end-to-end clientName uniqueness scenarios. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
commit: |
Contributor
|
All changed packages have been documented.
Show changes
|
|
You can try these changes here
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
PR #10494 (which added support for custom etag wire names) broke operations that have more than one
Azure.Core.eTag-typed header. The Azure Storage BlobcopyFromUrloperation is a concrete example — it carries:If-Match/If-None-Matchx-ms-source-if-match/x-ms-source-if-none-matchThe emitter in
http.tsstampsetagRole = "ifMatch"/"ifNoneMatch"onto every one of those headers. Then inpreprocess/__init__.py,update_parameterblindly appliesheaders_convert(ETAG_MATCH_DATA)/ETAG_NONE_MATCH_DATAto every parameter with that role — overwritingclientNameto"etag"and"match_condition"on both pairs. The operation ends up with two parameters namedetagand two namedmatch_condition.The slot-picker in
update_clientalready chose one ifMatch/ifNoneMatch slot per operation, but it did nothing to prevent the per-parameter rename on the others.Fix
In
preprocess/__init__.pyupdate_client:ifMatchandifNoneMatchcandidates per operation, not just the first._pick_etag_slothelper prefers the standardIf-Match/If-None-Matchwire names over custom etag headers (matches pre-PR-10494 behaviour when both are present).etagRolefrom non-selected candidates soupdate_parameterleaves their naturalclientNameintact (e.g.source_if_match,source_if_none_match).Result
For
copyFromUrl:If-Match/If-None-Match→ promoted to theetag/match_conditionpair (unchanged behaviour vs. pre-PR-10494).x-ms-source-if-match/x-ms-source-if-none-match→ keep their naturalsource_if_match/source_if_none_matchclient names.No more clientName collisions.
Tests
Adds
tests/unit/test_preprocess_etag.pywith six tests:test_etag_role_preserved_when_only_standard_pair_presenttest_etag_role_preserved_when_only_custom_pair_presenttest_standard_etag_wins_over_custom_when_both_present(regression)test_first_custom_pair_chosen_when_multiple_custom_pairs_presenttest_synthetic_partner_still_works_with_only_one_custom_etagtest_full_update_yaml_does_not_collide_client_names(end-to-end)Verified the three multi-etag tests fail on
upstream/mainand pass with this change. All existing unit tests still pass.Spec referenced
The original repro is
copyFromUrlinspecification/storage/Microsoft.BlobStorage/routes.tspofAzure/azure-rest-api-specs, which composesSourceIfMatchParameter,SourceIfNoneMatchParameter,IfMatchParameter, andIfNoneMatchParameter(allAzure.Core.eTag).