diff --git a/src/google/adk/tools/openapi_tool/openapi_spec_parser/openapi_spec_parser.py b/src/google/adk/tools/openapi_tool/openapi_spec_parser/openapi_spec_parser.py index 2e0e543fa3..1745698e41 100644 --- a/src/google/adk/tools/openapi_tool/openapi_spec_parser/openapi_spec_parser.py +++ b/src/google/adk/tools/openapi_tool/openapi_spec_parser/openapi_spec_parser.py @@ -205,10 +205,21 @@ def _collect_operations( if operation_dict is None: continue - # Append path-level parameters - operation_dict["parameters"] = operation_dict.get( - "parameters", [] - ) + path_item.get("parameters", []) + # Append path-level parameters. An operation-level parameter with the + # same name and location overrides the path-level one, so appending + # both would ask the model for the same value twice. + operation_parameters = operation_dict.get("parameters", []) + overridden = { + (parameter["name"], parameter.get("in")) + for parameter in operation_parameters + if isinstance(parameter, dict) and "name" in parameter + } + operation_dict["parameters"] = operation_parameters + [ + parameter + for parameter in path_item.get("parameters", []) + if not isinstance(parameter, dict) + or (parameter.get("name"), parameter.get("in")) not in overridden + ] # If operation ID is missing, assign an operation id based on path # and method diff --git a/src/google/adk/tools/openapi_tool/openapi_spec_parser/operation_parser.py b/src/google/adk/tools/openapi_tool/openapi_spec_parser/operation_parser.py index 36a2b19402..12214ac313 100644 --- a/src/google/adk/tools/openapi_tool/openapi_spec_parser/operation_parser.py +++ b/src/google/adk/tools/openapi_tool/openapi_spec_parser/operation_parser.py @@ -25,6 +25,7 @@ from fastapi.encoders import jsonable_encoder from fastapi.openapi.models import Operation from fastapi.openapi.models import Parameter +from fastapi.openapi.models import ParameterInType from fastapi.openapi.models import Reference from fastapi.openapi.models import RequestBody from fastapi.openapi.models import Response @@ -139,8 +140,11 @@ def _process_operation_parameters(self) -> None: ) if not schema.description: schema.description = description - # param.required can be None - required = param.required if param.required is not None else False + # OpenAPI requires `required: true` on every path parameter, and the URL + # cannot be built without one, so treat it as required even when the + # spec (e.g. an operation-level override) leaves the flag out. + # param.required can be None. + required = param.in_ == ParameterInType.path or bool(param.required) self._params.append( ApiParameter( diff --git a/tests/unittests/tools/openapi_tool/openapi_spec_parser/test_openapi_spec_parser.py b/tests/unittests/tools/openapi_tool/openapi_spec_parser/test_openapi_spec_parser.py index e5bff337ce..587c41f872 100644 --- a/tests/unittests/tools/openapi_tool/openapi_spec_parser/test_openapi_spec_parser.py +++ b/tests/unittests/tools/openapi_tool/openapi_spec_parser/test_openapi_spec_parser.py @@ -685,6 +685,100 @@ def test_parse_spec_with_path_level_parameters(openapi_spec_generator): assert local_param.type_value is int +def test_parse_spec_operation_parameter_overrides_path_level_parameter( + openapi_spec_generator, +): + """A same-named operation parameter replaces the path-level one.""" + openapi_spec = { + "openapi": "3.1.0", + "info": {"title": "Accounts API", "version": "1.0.0"}, + "paths": { + "/accounts/{accountId}": { + "parameters": [ + { + "name": "accountId", + "in": "path", + "required": True, + "schema": {"type": "string"}, + "description": "Shared account id", + }, + # Same name, different location: not overridden. + { + "name": "accountId", + "in": "header", + "schema": {"type": "string"}, + }, + ], + "get": { + "operationId": "getAccount", + "parameters": [{ + "name": "accountId", + "in": "path", + "required": True, + "schema": {"type": "string"}, + "description": "Account id, e.g. ACC-123", + }], + "responses": {"200": {"description": "ok"}}, + }, + } + }, + } + + operation = openapi_spec_generator.parse(openapi_spec)[0] + + assert [ + (p.original_name, p.param_location) for p in operation.parameters + ] == [ + ("accountId", "path"), + ("accountId", "header"), + ] + assert operation.parameters[0].description == "Account id, e.g. ACC-123" + + +def test_parse_spec_path_parameter_is_required_even_when_override_omits_it( + openapi_spec_generator, +): + """An override that leaves out `required` must not make a path param optional.""" + openapi_spec = { + "openapi": "3.1.0", + "info": {"title": "Accounts API", "version": "1.0.0"}, + "paths": { + "/accounts/{accountId}": { + "parameters": [{ + "name": "accountId", + "in": "path", + "required": True, + "schema": {"type": "string"}, + }], + "get": { + "operationId": "getAccount", + "parameters": [ + { + "name": "accountId", + "in": "path", + "schema": {"type": "string"}, + }, + # Non-path parameters keep the operation's own flag. + { + "name": "expand", + "in": "query", + "schema": {"type": "string"}, + }, + ], + "responses": {"200": {"description": "ok"}}, + }, + } + }, + } + + operation = openapi_spec_generator.parse(openapi_spec)[0] + + assert {p.original_name: p.required for p in operation.parameters} == { + "accountId": True, + "expand": False, + } + + def test_parse_spec_with_invalid_type_any(openapi_spec_generator): """Test that schemas with type='Any' are sanitized for Pydantic 2.11+.