Skip to content

fix(openapi): let operation parameters override path-level ones - #7219

Open
CodeTrainerMan wants to merge 1 commit into
google:mainfrom
CodeTrainerMan:fix/openapi-operation-parameter-override
Open

CodeTrainerMan wants to merge 1 commit into
google:mainfrom
CodeTrainerMan:fix/openapi-operation-parameter-override

Conversation

@CodeTrainerMan

Copy link
Copy Markdown

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

1. Link to an existing issue (if applicable):

Problem:

OpenApiSpecParser._collect_operations() merged the path item's parameters into the operation's with a plain concatenation:

operation_dict["parameters"] = operation_dict.get("parameters", []) + path_item.get("parameters", [])

The OpenAPI 3 Path Item Object says an operation-level parameter overrides a path-level parameter with the same name and in. Declaring a shared path parameter once at the path level and refining it on an operation (a more specific description, pattern or enum) is common, so both declarations ended up collected. OperationParser de-duplicates by name and renames the second one, so the tool asked the model for the same URL segment twice as two required arguments:

model is asked for: ['account_id', 'account_id_0']
args: {'account_id': 'ACC-123', 'account_id_0': 'ACC-999'} -> request: https://crm.example.com/accounts/ACC-999

Both values map to the same {accountId} placeholder; the path-level one wins, so the operation's own more specific parameter is silently dropped from the request.

Solution:

Merge the two lists with a new _merge_parameters() helper that keys each parameter on (name, in) and drops the path-level entries the operation re-declares. Parameters missing name or in are never deduplicated away, so malformed specs behave exactly as before. The operation's parameters keep their leading position, so the ordering of the generated declaration is unchanged except for the removed duplicate.

Testing Plan

Unit Tests:

  • I have added or updated unit tests for my change.
  • All unit tests pass locally.

Three tests added to tests/unittests/tools/openapi_tool/openapi_spec_parser/test_openapi_spec_parser.py:

  • test_operation_parameter_overrides_path_level_parameter - the reproducer from the issue now yields a single account_id parameter carrying the operation-level description.
  • test_path_level_parameters_are_still_collected - a path-level parameter the operation does not redeclare is still collected.
  • test_parameters_with_same_name_different_location_are_both_kept - in participates in the override key, so a same-named query parameter is not deduplicated.
$ pytest tests/unittests/tools/openapi_tool/openapi_spec_parser/ -q
211 passed in 6.57s

That directory had 208 passing tests before the change (28 in test_openapi_spec_parser.py, the rest in test_openapi_toolset.py / test_rest_api_tool.py), so the 3 new tests account for the delta and nothing else regressed. pyink and isort report both changed files unchanged.

Manual End-to-End (E2E) Tests:

Before the fix, parsing the issue's spec printed 2 parameters; after it prints 1:

parameter names      : ['account_id']
  - py_name='account_id' original='accountId' in='path' desc='Account id (operation level).'

No E2E run against a live API was needed - the defect is in tool declaration and request building, which the parser unit tests cover directly.

Checklist

  • I have read the CONTRIBUTING.md document.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.
  • I have manually tested my changes end-to-end.
  • Any dependent changes have been merged and published in downstream modules.

Additional context

The fix is confined to parameter collection at parse time. RestApiTool is untouched, and the style/explode serialization gap in query parameters reported in #7204 is a separate change and is not addressed here.

OpenAPI 3 says an operation-level parameter overrides a path-level one with the same name and in. _collect_operations concatenated both lists instead, so a path parameter declared at the path level and refined on an operation was collected twice. OperationParser renamed the duplicate to account_id_0, the model was asked for the same URL segment as two required arguments, and the path-level value won when both were filled, silently dropping the operation's more specific parameter from the request.

Merge the two lists with _merge_parameters, which keys each parameter on (name, in) and drops the path-level entries the operation re-declares. Parameters missing name or in are never deduplicated away.

Fixes google#7205
@google-cla

google-cla Bot commented Sep 20, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants