From 41e954ddd0e6ff8690a5992b5ded90fa3c77b7eb Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Thu, 24 Oct 2024 15:44:16 -0700 Subject: [PATCH 1/7] fix: support $ref from endpoint response to components/responses --- end_to_end_tests/baseline_openapi_3.0.json | 26 ++++ end_to_end_tests/baseline_openapi_3.1.yaml | 21 +++ .../api/responses/__init__.py | 9 +- .../api/responses/reference_response.py | 122 ++++++++++++++++++ openapi_python_client/parser/openapi.py | 27 +++- openapi_python_client/parser/responses.py | 24 ++-- tests/test_parser/test_openapi.py | 23 +++- tests/test_parser/test_responses.py | 60 +++++---- 8 files changed, 267 insertions(+), 45 deletions(-) create mode 100644 end_to_end_tests/golden-record/my_test_api_client/api/responses/reference_response.py diff --git a/end_to_end_tests/baseline_openapi_3.0.json b/end_to_end_tests/baseline_openapi_3.0.json index e5bbaf6fc..7e9d27f16 100644 --- a/end_to_end_tests/baseline_openapi_3.0.json +++ b/end_to_end_tests/baseline_openapi_3.0.json @@ -991,6 +991,20 @@ } } }, + "/responses/reference": { + "get": { + "tags": [ + "responses" + ], + "summary": "Endpoint using predefined response", + "operationId": "reference_response", + "responses": { + "200": { + "$ref": "#/components/responses/AResponse" + } + } + } + }, "/auth/token_with_cookie": { "get": { "tags": [ @@ -2930,6 +2944,18 @@ } } } + }, + "responses": { + "AResponse": { + "description": "OK", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/AModel" + } + } + } + } } } } diff --git a/end_to_end_tests/baseline_openapi_3.1.yaml b/end_to_end_tests/baseline_openapi_3.1.yaml index b6a6941e2..f72b36f0d 100644 --- a/end_to_end_tests/baseline_openapi_3.1.yaml +++ b/end_to_end_tests/baseline_openapi_3.1.yaml @@ -983,6 +983,20 @@ info: } } }, + "/responses/reference": { + "get": { + "tags": [ + "responses" + ], + "summary": "Endpoint using predefined response", + "operationId": "reference_response", + "responses": { + "200": { + "$ref": "#/components/responses/AResponse" + } + } + } + }, "/auth/token_with_cookie": { "get": { "tags": [ @@ -2921,3 +2935,10 @@ info: "application/json": "schema": "$ref": "#/components/schemas/AModel" + responses: + AResponse: + description: OK + content: + "application/json": + "schema": + "$ref": "#/components/schemas/AModel" diff --git a/end_to_end_tests/custom-templates-golden-record/my_test_api_client/api/responses/__init__.py b/end_to_end_tests/custom-templates-golden-record/my_test_api_client/api/responses/__init__.py index 6000bd0e7..e09dee3e3 100644 --- a/end_to_end_tests/custom-templates-golden-record/my_test_api_client/api/responses/__init__.py +++ b/end_to_end_tests/custom-templates-golden-record/my_test_api_client/api/responses/__init__.py @@ -2,7 +2,7 @@ import types -from . import post_responses_unions_simple_before_complex, text_response +from . import post_responses_unions_simple_before_complex, reference_response, text_response class ResponsesEndpoints: @@ -19,3 +19,10 @@ def text_response(cls) -> types.ModuleType: Text Response """ return text_response + + @classmethod + def reference_response(cls) -> types.ModuleType: + """ + Endpoint using predefined response + """ + return reference_response diff --git a/end_to_end_tests/golden-record/my_test_api_client/api/responses/reference_response.py b/end_to_end_tests/golden-record/my_test_api_client/api/responses/reference_response.py new file mode 100644 index 000000000..7ab4e3b55 --- /dev/null +++ b/end_to_end_tests/golden-record/my_test_api_client/api/responses/reference_response.py @@ -0,0 +1,122 @@ +from http import HTTPStatus +from typing import Any, Dict, Optional, Union + +import httpx + +from ... import errors +from ...client import AuthenticatedClient, Client +from ...models.a_model import AModel +from ...types import Response + + +def _get_kwargs() -> Dict[str, Any]: + _kwargs: Dict[str, Any] = { + "method": "get", + "url": "/responses/reference", + } + + return _kwargs + + +def _parse_response(*, client: Union[AuthenticatedClient, Client], response: httpx.Response) -> Optional[AModel]: + if response.status_code == 200: + response_200 = AModel.from_dict(response.json()) + + return response_200 + if client.raise_on_unexpected_status: + raise errors.UnexpectedStatus(response.status_code, response.content) + else: + return None + + +def _build_response(*, client: Union[AuthenticatedClient, Client], response: httpx.Response) -> Response[AModel]: + return Response( + status_code=HTTPStatus(response.status_code), + content=response.content, + headers=response.headers, + parsed=_parse_response(client=client, response=response), + ) + + +def sync_detailed( + *, + client: Union[AuthenticatedClient, Client], +) -> Response[AModel]: + """Endpoint using predefined response + + Raises: + errors.UnexpectedStatus: If the server returns an undocumented status code and Client.raise_on_unexpected_status is True. + httpx.TimeoutException: If the request takes longer than Client.timeout. + + Returns: + Response[AModel] + """ + + kwargs = _get_kwargs() + + response = client.get_httpx_client().request( + **kwargs, + ) + + return _build_response(client=client, response=response) + + +def sync( + *, + client: Union[AuthenticatedClient, Client], +) -> Optional[AModel]: + """Endpoint using predefined response + + Raises: + errors.UnexpectedStatus: If the server returns an undocumented status code and Client.raise_on_unexpected_status is True. + httpx.TimeoutException: If the request takes longer than Client.timeout. + + Returns: + AModel + """ + + return sync_detailed( + client=client, + ).parsed + + +async def asyncio_detailed( + *, + client: Union[AuthenticatedClient, Client], +) -> Response[AModel]: + """Endpoint using predefined response + + Raises: + errors.UnexpectedStatus: If the server returns an undocumented status code and Client.raise_on_unexpected_status is True. + httpx.TimeoutException: If the request takes longer than Client.timeout. + + Returns: + Response[AModel] + """ + + kwargs = _get_kwargs() + + response = await client.get_async_httpx_client().request(**kwargs) + + return _build_response(client=client, response=response) + + +async def asyncio( + *, + client: Union[AuthenticatedClient, Client], +) -> Optional[AModel]: + """Endpoint using predefined response + + Raises: + errors.UnexpectedStatus: If the server returns an undocumented status code and Client.raise_on_unexpected_status is True. + httpx.TimeoutException: If the request takes longer than Client.timeout. + + Returns: + AModel + """ + + return ( + await asyncio_detailed( + client=client, + ) + ).parsed diff --git a/openapi_python_client/parser/openapi.py b/openapi_python_client/parser/openapi.py index acc8998cd..2b24b2d9c 100644 --- a/openapi_python_client/parser/openapi.py +++ b/openapi_python_client/parser/openapi.py @@ -50,6 +50,7 @@ def from_data( schemas: Schemas, parameters: Parameters, request_bodies: Dict[str, Union[oai.RequestBody, oai.Reference]], + responses: Dict[str, Union[oai.Response, oai.Reference]], config: Config, ) -> Tuple[Dict[utils.PythonIdentifier, "EndpointCollection"], Schemas, Parameters]: """Parse the openapi paths data to get EndpointCollections by tag""" @@ -72,6 +73,7 @@ def from_data( schemas=schemas, parameters=parameters, request_bodies=request_bodies, + responses=responses, config=config, ) # Add `PathItem` parameters @@ -144,7 +146,12 @@ class Endpoint: @staticmethod def _add_responses( - *, endpoint: "Endpoint", data: oai.Responses, schemas: Schemas, config: Config + *, + endpoint: "Endpoint", + data: oai.Responses, + schemas: Schemas, + responses: Dict[str, Union[oai.Response, oai.Reference]], + config: Config, ) -> Tuple["Endpoint", Schemas]: endpoint = deepcopy(endpoint) for code, response_data in data.items(): @@ -167,6 +174,7 @@ def _add_responses( status_code=status_code, data=response_data, schemas=schemas, + responses=responses, parent_name=endpoint.name, config=config, ) @@ -396,6 +404,7 @@ def from_data( schemas: Schemas, parameters: Parameters, request_bodies: Dict[str, Union[oai.RequestBody, oai.Reference]], + responses: Dict[str, Union[oai.Response, oai.Reference]], config: Config, ) -> Tuple[Union["Endpoint", ParseError], Schemas, Parameters]: """Construct an endpoint from the OpenAPI data""" @@ -424,7 +433,13 @@ def from_data( ) if isinstance(result, ParseError): return result, schemas, parameters - result, schemas = Endpoint._add_responses(endpoint=result, data=data.responses, schemas=schemas, config=config) + result, schemas = Endpoint._add_responses( + endpoint=result, + data=data.responses, + schemas=schemas, + responses=responses, + config=config, + ) if isinstance(result, ParseError): return result, schemas, parameters bodies, schemas = body_from_data( @@ -514,8 +529,14 @@ def from_dict(data: Dict[str, Any], *, config: Config) -> Union["GeneratorData", config=config, ) request_bodies = (openapi.components and openapi.components.requestBodies) or {} + responses = (openapi.components and openapi.components.responses) or {} endpoint_collections_by_tag, schemas, parameters = EndpointCollection.from_data( - data=openapi.paths, schemas=schemas, parameters=parameters, request_bodies=request_bodies, config=config + data=openapi.paths, + schemas=schemas, + parameters=parameters, + request_bodies=request_bodies, + responses=responses, + config=config, ) enums = ( diff --git a/openapi_python_client/parser/responses.py b/openapi_python_client/parser/responses.py index 32412fd35..d41934f6c 100644 --- a/openapi_python_client/parser/responses.py +++ b/openapi_python_client/parser/responses.py @@ -1,11 +1,12 @@ __all__ = ["Response", "response_from_data"] from http import HTTPStatus -from typing import Optional, Tuple, TypedDict, Union +from typing import Dict, Optional, Tuple, TypedDict, Union from attrs import define from openapi_python_client import utils +from openapi_python_client.parser.properties.schemas import parse_reference_path from .. import Config from .. import schema as oai @@ -84,6 +85,7 @@ def response_from_data( status_code: HTTPStatus, data: Union[oai.Response, oai.Reference], schemas: Schemas, + responses: Dict[str, Union[oai.Response, oai.Reference]], parent_name: str, config: Config, ) -> Tuple[Union[Response, ParseError], Schemas]: @@ -91,15 +93,17 @@ def response_from_data( response_name = f"response_{status_code}" if isinstance(data, oai.Reference): - return ( - empty_response( - status_code=status_code, - response_name=response_name, - config=config, - data=data, - ), - schemas, - ) + ref_path = parse_reference_path(data.ref) + if isinstance(ref_path, ParseError): + return ref_path, schemas + if not ref_path.startswith("/components/responses/"): + return ParseError(data=data, detail=f"$ref to {data.ref} not allowed in responses"), schemas + resp_data = responses.get(ref_path.split("/")[-1], None) + if not resp_data: + return ParseError(data=data, detail=f"Could not find reference: {data.ref}"), schemas + if not isinstance(resp_data, oai.Response): + return ParseError(data=data, detail="Top-level $ref inside components/responses is not supported"), schemas + data = resp_data content = data.content if not content: diff --git a/tests/test_parser/test_openapi.py b/tests/test_parser/test_openapi.py index 6eeadcd78..96cf253c5 100644 --- a/tests/test_parser/test_openapi.py +++ b/tests/test_parser/test_openapi.py @@ -85,7 +85,7 @@ def test__add_responses_status_code_error(self, response_status_code, mocker): response_from_data = mocker.patch(f"{MODULE_NAME}.response_from_data", return_value=(parse_error, schemas)) config = MagicMock() - response, schemas = Endpoint._add_responses(endpoint=endpoint, data=data, schemas=schemas, config=config) + response, schemas = Endpoint._add_responses(endpoint=endpoint, data=data, schemas=schemas, responses={}, config=config) assert response.errors == [ ParseError( @@ -110,12 +110,12 @@ def test__add_responses_error(self, mocker): response_from_data = mocker.patch(f"{MODULE_NAME}.response_from_data", return_value=(parse_error, schemas)) config = MagicMock() - response, schemas = Endpoint._add_responses(endpoint=endpoint, data=data, schemas=schemas, config=config) + response, schemas = Endpoint._add_responses(endpoint=endpoint, data=data, schemas=schemas, responses={}, config=config) response_from_data.assert_has_calls( [ - mocker.call(status_code=200, data=response_1_data, schemas=schemas, parent_name="name", config=config), - mocker.call(status_code=404, data=response_2_data, schemas=schemas, parent_name="name", config=config), + mocker.call(status_code=200, data=response_1_data, schemas=schemas, responses={}, parent_name="name", config=config), + mocker.call(status_code=404, data=response_2_data, schemas=schemas, responses={}, parent_name="name", config=config), ] ) assert response.errors == [ @@ -474,6 +474,7 @@ def test_from_data_bad_params(self, mocker, config): method=method, tag="default", schemas=initial_schemas, + responses={}, parameters=parameters, config=config, request_bodies={}, @@ -509,6 +510,7 @@ def test_from_data_bad_responses(self, mocker, config): method=method, tag="default", schemas=initial_schemas, + responses={}, parameters=initial_parameters, config=config, request_bodies={}, @@ -549,6 +551,7 @@ def test_from_data_standard(self, mocker, config): method=method, tag="default", schemas=initial_schemas, + responses={}, parameters=initial_parameters, config=config, request_bodies={}, @@ -570,7 +573,7 @@ def test_from_data_standard(self, mocker, config): config=config, ) _add_responses.assert_called_once_with( - endpoint=param_endpoint, data=data.responses, schemas=param_schemas, config=config + endpoint=param_endpoint, data=data.responses, schemas=param_schemas, responses={}, config=config ) def test_from_data_no_operation_id(self, mocker, config): @@ -600,6 +603,7 @@ def test_from_data_no_operation_id(self, mocker, config): method=method, tag="default", schemas=schemas, + responses={}, parameters=parameters, config=config, request_bodies={}, @@ -624,6 +628,7 @@ def test_from_data_no_operation_id(self, mocker, config): endpoint=add_parameters.return_value[0], data=data.responses, schemas=add_parameters.return_value[1], + responses={}, config=config, ) @@ -654,6 +659,7 @@ def test_from_data_no_security(self, mocker, config): method=method, tag="a", schemas=schemas, + responses={}, parameters=parameters, config=config, request_bodies={}, @@ -678,6 +684,7 @@ def test_from_data_no_security(self, mocker, config): endpoint=add_parameters.return_value[0], data=data.responses, schemas=add_parameters.return_value[1], + responses={}, config=config, ) @@ -693,6 +700,7 @@ def test_from_data_some_bad_bodies(self, config): ), ), schemas=Schemas(), + responses={}, config=config, parameters=Parameters(), tag="tag", @@ -716,6 +724,7 @@ def test_from_data_all_bodies_bad(self, config): ), ), schemas=Schemas(), + responses={}, config=config, parameters=Parameters(), tag="tag", @@ -787,6 +796,7 @@ def test_from_data_overrides_path_item_params_with_operation_params(self, config parameters=Parameters(), config=config, request_bodies={}, + responses={}, ) collection: EndpointCollection = collections["default"] assert isinstance(collection.endpoints[0].query_parameters[0], IntProperty) @@ -825,6 +835,7 @@ def test_from_data_errors(self, mocker, config): config=config, parameters=parameters, request_bodies={}, + responses={}, ) assert result["default"].parse_errors[0].data == "1" @@ -866,7 +877,7 @@ def test_from_data_tags_snake_case_sanitizer(self, mocker, config): parameters = mocker.MagicMock() result = EndpointCollection.from_data( - data=data, schemas=schemas, parameters=parameters, config=config, request_bodies={} + data=data, schemas=schemas, parameters=parameters, config=config, request_bodies={}, responses={} ) assert result == ( diff --git a/tests/test_parser/test_responses.py b/tests/test_parser/test_responses.py index 0ac885764..27cf2cc4c 100644 --- a/tests/test_parser/test_responses.py +++ b/tests/test_parser/test_responses.py @@ -17,6 +17,7 @@ def test_response_from_data_no_content(any_property_factory): status_code=200, data=data, schemas=Schemas(), + responses={}, parent_name="parent", config=MagicMock(), ) @@ -34,31 +35,6 @@ def test_response_from_data_no_content(any_property_factory): ) -def test_response_from_data_reference(any_property_factory): - from openapi_python_client.parser.responses import Response, response_from_data - - data = oai.Reference.model_construct() - - response, schemas = response_from_data( - status_code=200, - data=data, - schemas=Schemas(), - parent_name="parent", - config=MagicMock(), - ) - - assert response == Response( - status_code=200, - prop=any_property_factory( - name="response_200", - default=None, - required=True, - ), - source=NONE_SOURCE, - data=data, - ) - - def test_response_from_data_unsupported_content_type(): from openapi_python_client.parser.responses import response_from_data @@ -69,6 +45,7 @@ def test_response_from_data_unsupported_content_type(): status_code=200, data=data, schemas=Schemas(), + responses={}, parent_name="parent", config=config, ) @@ -89,6 +66,7 @@ def test_response_from_data_no_content_schema(any_property_factory): status_code=200, data=data, schemas=Schemas(), + responses={}, parent_name="parent", config=config, ) @@ -121,6 +99,7 @@ def test_response_from_data_property_error(mocker): status_code=400, data=data, schemas=Schemas(), + responses={}, parent_name="parent", config=config, ) @@ -152,6 +131,7 @@ def test_response_from_data_property(mocker, any_property_factory): status_code=400, data=data, schemas=Schemas(), + responses={}, parent_name="parent", config=config, ) @@ -172,6 +152,35 @@ def test_response_from_data_property(mocker, any_property_factory): ) +def test_response_from_data_reference(mocker, any_property_factory): + from openapi_python_client.parser import responses + + prop = any_property_factory() + mocker.patch.object(responses, "property_from_data", return_value=(prop, Schemas())) + predefined_response_data = oai.Response.model_construct( + description="", + content={"application/json": oai.MediaType.model_construct(media_type_schema="something")}, + ) + config = MagicMock() + config.content_type_overrides = {} + + response, schemas = responses.response_from_data( + status_code=400, + data=oai.Reference.model_construct(ref="#/components/responses/ErrorResponse"), + schemas=Schemas(), + responses={"ErrorResponse": predefined_response_data}, + parent_name="parent", + config=config, + ) + + assert response == responses.Response( + status_code=400, + prop=prop, + source=JSON_SOURCE, + data=predefined_response_data, + ) + + def test_response_from_data_content_type_overrides(any_property_factory): from openapi_python_client.parser.responses import Response, response_from_data @@ -185,6 +194,7 @@ def test_response_from_data_content_type_overrides(any_property_factory): status_code=200, data=data, schemas=Schemas(), + responses={}, parent_name="parent", config=config, ) From fb0182e7affebc3b40c59a5773d26ac60d055067 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Thu, 24 Oct 2024 17:36:30 -0700 Subject: [PATCH 2/7] lint --- tests/test_parser/test_openapi.py | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/tests/test_parser/test_openapi.py b/tests/test_parser/test_openapi.py index 96cf253c5..7f0f7addf 100644 --- a/tests/test_parser/test_openapi.py +++ b/tests/test_parser/test_openapi.py @@ -85,7 +85,9 @@ def test__add_responses_status_code_error(self, response_status_code, mocker): response_from_data = mocker.patch(f"{MODULE_NAME}.response_from_data", return_value=(parse_error, schemas)) config = MagicMock() - response, schemas = Endpoint._add_responses(endpoint=endpoint, data=data, schemas=schemas, responses={}, config=config) + response, schemas = Endpoint._add_responses( + endpoint=endpoint, data=data, schemas=schemas, responses={}, config=config + ) assert response.errors == [ ParseError( @@ -110,12 +112,28 @@ def test__add_responses_error(self, mocker): response_from_data = mocker.patch(f"{MODULE_NAME}.response_from_data", return_value=(parse_error, schemas)) config = MagicMock() - response, schemas = Endpoint._add_responses(endpoint=endpoint, data=data, schemas=schemas, responses={}, config=config) + response, schemas = Endpoint._add_responses( + endpoint=endpoint, data=data, schemas=schemas, responses={}, config=config + ) response_from_data.assert_has_calls( [ - mocker.call(status_code=200, data=response_1_data, schemas=schemas, responses={}, parent_name="name", config=config), - mocker.call(status_code=404, data=response_2_data, schemas=schemas, responses={}, parent_name="name", config=config), + mocker.call( + status_code=200, + data=response_1_data, + schemas=schemas, + responses={}, + parent_name="name", + config=config, + ), + mocker.call( + status_code=404, + data=response_2_data, + schemas=schemas, + responses={}, + parent_name="name", + config=config, + ), ] ) assert response.errors == [ From fe6b7ddb25ad86fdbe9dfe3d092e6b318ecd87e9 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Fri, 25 Oct 2024 09:43:18 -0700 Subject: [PATCH 3/7] lint --- openapi_python_client/parser/responses.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openapi_python_client/parser/responses.py b/openapi_python_client/parser/responses.py index d41934f6c..5f8e467d9 100644 --- a/openapi_python_client/parser/responses.py +++ b/openapi_python_client/parser/responses.py @@ -80,7 +80,7 @@ def empty_response( ) -def response_from_data( +def response_from_data( # noqa: PLR0911 *, status_code: HTTPStatus, data: Union[oai.Response, oai.Reference], From 8b0aca1e5ab0cb5d835a7064caa573019bbd9c7e Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Fri, 25 Oct 2024 10:09:01 -0700 Subject: [PATCH 4/7] test coverage --- tests/test_parser/test_responses.py | 34 +++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/tests/test_parser/test_responses.py b/tests/test_parser/test_responses.py index 27cf2cc4c..081011611 100644 --- a/tests/test_parser/test_responses.py +++ b/tests/test_parser/test_responses.py @@ -1,5 +1,7 @@ from unittest.mock import MagicMock +import pytest + import openapi_python_client.schema as oai from openapi_python_client.parser.errors import ParseError, PropertyError from openapi_python_client.parser.properties import Schemas @@ -181,6 +183,38 @@ def test_response_from_data_reference(mocker, any_property_factory): ) +@pytest.mark.parametrize( + "ref_string", + [ + "#/components/responses/Nonexistent", + "malformed-references-string", + "#/components/something-that-isnt-responses/ErrorResponse", + ], +) +def test_response_from_data_reference_errors(ref_string, mocker, any_property_factory): + from openapi_python_client.parser import responses + + prop = any_property_factory() + mocker.patch.object(responses, "property_from_data", return_value=(prop, Schemas())) + predefined_response_data = oai.Response.model_construct( + description="", + content={"application/json": oai.MediaType.model_construct(media_type_schema="something")}, + ) + config = MagicMock() + config.content_type_overrides = {} + + response, schemas = responses.response_from_data( + status_code=400, + data=oai.Reference.model_construct(ref=ref_string), + schemas=Schemas(), + responses={"ErrorResponse": predefined_response_data}, + parent_name="parent", + config=config, + ) + + assert isinstance(response, ParseError) + + def test_response_from_data_content_type_overrides(any_property_factory): from openapi_python_client.parser.responses import Response, response_from_data From 2edc6fe5e3838722aeb8f9075828f7ec880c3b3b Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Fri, 25 Oct 2024 10:24:49 -0700 Subject: [PATCH 5/7] test coverage --- tests/test_parser/test_responses.py | 42 +++++++++++++++++++++++++---- 1 file changed, 37 insertions(+), 5 deletions(-) diff --git a/tests/test_parser/test_responses.py b/tests/test_parser/test_responses.py index 081011611..c193fbce1 100644 --- a/tests/test_parser/test_responses.py +++ b/tests/test_parser/test_responses.py @@ -184,14 +184,14 @@ def test_response_from_data_reference(mocker, any_property_factory): @pytest.mark.parametrize( - "ref_string", + "ref_string,expected_error_string", [ - "#/components/responses/Nonexistent", - "malformed-references-string", - "#/components/something-that-isnt-responses/ErrorResponse", + ("#/components/responses/Nonexistent", "Could not find"), + ("https://remote-reference", "Remote references"), + ("#/components/something-that-isnt-responses/ErrorResponse", "not allowed in responses"), ], ) -def test_response_from_data_reference_errors(ref_string, mocker, any_property_factory): +def test_response_from_data_invalid_reference(ref_string, expected_error_string, mocker, any_property_factory): from openapi_python_client.parser import responses prop = any_property_factory() @@ -213,6 +213,38 @@ def test_response_from_data_reference_errors(ref_string, mocker, any_property_fa ) assert isinstance(response, ParseError) + assert expected_error_string in response.detail + + +def test_response_from_data_ref_to_response_that_is_a_ref(mocker, any_property_factory): + from openapi_python_client.parser import responses + + prop = any_property_factory() + mocker.patch.object(responses, "property_from_data", return_value=(prop, Schemas())) + predefined_response_base_data = oai.Response.model_construct( + description="", + content={"application/json": oai.MediaType.model_construct(media_type_schema="something")}, + ) + predefined_response_data = oai.Reference.model_construct( + ref="#/components/references/BaseResponse", + ) + config = MagicMock() + config.content_type_overrides = {} + + response, schemas = responses.response_from_data( + status_code=400, + data=oai.Reference.model_construct(ref="#/components/schemas/ErrorResponse"), + schemas=Schemas(), + responses={ + "BaseResponse": predefined_response_base_data, + "ErrorResponse": predefined_response_data, + }, + parent_name="parent", + config=config, + ) + + assert isinstance(response, ParseError) + assert "not allowed in responses" in response.detail def test_response_from_data_content_type_overrides(any_property_factory): From 9df3bb4a2eaf6f98a724774bb08abee53a37eefb Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Fri, 25 Oct 2024 10:38:38 -0700 Subject: [PATCH 6/7] fix test to test the right thing --- tests/test_parser/test_responses.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_parser/test_responses.py b/tests/test_parser/test_responses.py index c193fbce1..24fb94c61 100644 --- a/tests/test_parser/test_responses.py +++ b/tests/test_parser/test_responses.py @@ -233,7 +233,7 @@ def test_response_from_data_ref_to_response_that_is_a_ref(mocker, any_property_f response, schemas = responses.response_from_data( status_code=400, - data=oai.Reference.model_construct(ref="#/components/schemas/ErrorResponse"), + data=oai.Reference.model_construct(ref="#/components/responses/ErrorResponse"), schemas=Schemas(), responses={ "BaseResponse": predefined_response_base_data, @@ -244,7 +244,7 @@ def test_response_from_data_ref_to_response_that_is_a_ref(mocker, any_property_f ) assert isinstance(response, ParseError) - assert "not allowed in responses" in response.detail + assert "Top-level $ref" in response.detail def test_response_from_data_content_type_overrides(any_property_factory): From 78bd8609f670a4703161e3d37cb49d5104a3a330 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 28 Oct 2024 12:22:16 -0700 Subject: [PATCH 7/7] add a helper --- openapi_python_client/parser/bodies.py | 3 ++- openapi_python_client/parser/properties/schemas.py | 9 ++++++++- openapi_python_client/parser/responses.py | 4 ++-- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/openapi_python_client/parser/bodies.py b/openapi_python_client/parser/bodies.py index 8515ad7cc..4e672f702 100644 --- a/openapi_python_client/parser/bodies.py +++ b/openapi_python_client/parser/bodies.py @@ -9,6 +9,7 @@ Schemas, property_from_data, ) +from openapi_python_client.parser.properties.schemas import get_reference_simple_name from .. import schema as oai from ..config import Config @@ -138,7 +139,7 @@ def _resolve_reference( references_seen = [] while isinstance(body, oai.Reference) and body.ref not in references_seen: references_seen.append(body.ref) - body = request_bodies.get(body.ref.split("/")[-1]) + body = request_bodies.get(get_reference_simple_name(body.ref)) if isinstance(body, oai.Reference): return ParseError(detail="Circular $ref in request body", data=body) if body is None and references_seen: diff --git a/openapi_python_client/parser/properties/schemas.py b/openapi_python_client/parser/properties/schemas.py index dad89a572..438400cb3 100644 --- a/openapi_python_client/parser/properties/schemas.py +++ b/openapi_python_client/parser/properties/schemas.py @@ -46,6 +46,13 @@ def parse_reference_path(ref_path_raw: str) -> Union[ReferencePath, ParseError]: return cast(ReferencePath, parsed.fragment) +def get_reference_simple_name(ref_path: str) -> str: + """ + Takes a path like `/components/schemas/NameOfThing` and returns a string like `NameOfThing`. + """ + return ref_path.split("/", 3)[-1] + + @define class Class: """Represents Python class which will be generated from an OpenAPI schema""" @@ -56,7 +63,7 @@ class Class: @staticmethod def from_string(*, string: str, config: Config) -> "Class": """Get a Class from an arbitrary string""" - class_name = string.split("/")[-1] # Get rid of ref path stuff + class_name = get_reference_simple_name(string) # Get rid of ref path stuff class_name = ClassName(class_name, config.field_prefix) override = config.class_overrides.get(class_name) diff --git a/openapi_python_client/parser/responses.py b/openapi_python_client/parser/responses.py index 5f8e467d9..4725ba3c7 100644 --- a/openapi_python_client/parser/responses.py +++ b/openapi_python_client/parser/responses.py @@ -6,7 +6,7 @@ from attrs import define from openapi_python_client import utils -from openapi_python_client.parser.properties.schemas import parse_reference_path +from openapi_python_client.parser.properties.schemas import get_reference_simple_name, parse_reference_path from .. import Config from .. import schema as oai @@ -98,7 +98,7 @@ def response_from_data( # noqa: PLR0911 return ref_path, schemas if not ref_path.startswith("/components/responses/"): return ParseError(data=data, detail=f"$ref to {data.ref} not allowed in responses"), schemas - resp_data = responses.get(ref_path.split("/")[-1], None) + resp_data = responses.get(get_reference_simple_name(ref_path), None) if not resp_data: return ParseError(data=data, detail=f"Could not find reference: {data.ref}"), schemas if not isinstance(resp_data, oai.Response):