Code cleanup.

Adds missing tests for a number of parameter-related methods.

Author: Jordi Sanchez

openapi_python_client/parser/openapi.py

@@ -521,9 +521,7 @@ def from_dict(data: Dict[str, Any], *, config: Config) -> Union["GeneratorData",
         if openapi.components and openapi.components.schemas:
             schemas = build_schemas(components=openapi.components.schemas, schemas=schemas, config=config)
         if openapi.components and openapi.components.parameters:
-            parameters = build_parameters(
-                components=openapi.components.parameters, schemas=schemas, parameters=parameters, config=config
-            )
+            parameters = build_parameters(components=openapi.components.parameters, parameters=parameters)
         endpoint_collections_by_tag, schemas, parameters = EndpointCollection.from_data(
             data=openapi.paths, schemas=schemas, parameters=parameters, config=config
         )

openapi_python_client/parser/properties/__init__.py

@@ -743,10 +743,8 @@ def build_parameters(
     *,
     components: Dict[str, Union[oai.Reference, oai.Parameter]],
     parameters: Parameters,
-    schemas: Schemas,
-    config: Config,
 ) -> Parameters:
-    """Get a list of Schemas from an OpenAPI dict"""
+    """Get a list of Parameters from an OpenAPI dict"""
     to_process: Iterable[Tuple[str, Union[oai.Reference, oai.Parameter]]] = []
     if components is not None:
         to_process = components.items()
@@ -761,15 +759,13 @@ def build_parameters(
         # Only accumulate errors from the last round, since we might fix some along the way
         for name, data in to_process:
             if isinstance(data, oai.Reference):
-                parameters.errors.append(PropertyError(data=data, detail="Reference schemas are not supported."))
+                parameters.errors.append(ParameterError(data=data, detail="Reference parameters are not supported."))
                 continue
             ref_path = parse_reference_path(f"#/components/parameters/{name}")
             if isinstance(ref_path, ParseError):
-                parameters.errors.append(PropertyError(detail=ref_path.detail, data=data))
+                parameters.errors.append(ParameterError(detail=ref_path.detail, data=data))
                 continue
-            parameters_or_err = update_parameters_with_data(
-                ref_path=ref_path, data=data, schemas=schemas, parameters=parameters, config=config
-            )
+            parameters_or_err = update_parameters_with_data(ref_path=ref_path, data=data, parameters=parameters)
             if isinstance(parameters_or_err, ParameterError):
                 next_round.append((name, data))
                 errors.append(parameters_or_err)

openapi_python_client/parser/properties/schemas.py

@@ -4,7 +4,9 @@
     "Parameters",
     "parse_reference_path",
     "update_schemas_with_data",
+    "update_parameters_with_data",
     "parameter_from_reference",
+    "parameter_from_data",
 ]
 
 from typing import TYPE_CHECKING, Dict, List, NewType, Tuple, Union, cast
@@ -129,27 +131,15 @@ def parameter_from_data(
     name: str,
     required: bool,
     data: Union[oai.Reference, oai.Parameter],
-    schemas: Schemas,
     parameters: Parameters,
-    config: Config,
-) -> Tuple[Union[Parameter, ParameterError], Schemas, Parameters]:
-    """Generates parameters from"""
-    from . import property_from_data
+) -> Tuple[Union[Parameter, ParameterError], Parameters]:
+    """Generates parameters from an OpenAPI Parameter spec."""
 
     if isinstance(data, oai.Reference):
-        return ParameterError("Unable to resolve another reference"), schemas, parameters
+        return ParameterError("Unable to resolve another reference"), parameters
 
     if data.param_schema is None:
-        return ParameterError("Parameter has no schema"), schemas, parameters
-
-    _, new_schemas = property_from_data(
-        name=name,
-        required=required,
-        data=data.param_schema,
-        schemas=schemas,
-        parent_name="",
-        config=config,
-    )
+        return ParameterError("Parameter has no schema"), parameters
 
     new_param = Parameter(
         name=name,
@@ -160,34 +150,32 @@ def parameter_from_data(
         param_in=data.param_in,
     )
     parameters = attr.evolve(parameters, classes_by_name={**parameters.classes_by_name, name: new_param})
-    return new_param, new_schemas, parameters
+    return new_param, parameters
 
 
 def update_parameters_with_data(
-    *, ref_path: _ReferencePath, data: oai.Parameter, parameters: Parameters, schemas: Schemas, config: Config
+    *, ref_path: _ReferencePath, data: oai.Parameter, parameters: Parameters
 ) -> Union[Parameters, ParameterError]:
     """
-    Update a `Schemas` using some new reference.
+    Update a `Parameters` using some new reference.
 
     Args:
         ref_path: The output of `parse_reference_path` (validated $ref).
         data: The schema of the thing to add to Schemas.
-        schemas: `Schemas` up until now.
+        parameters: `Parameters` up until now.
         config: User-provided config for overriding default behavior.
 
     Returns:
-        Either the updated `schemas` input or a `PropertyError` if something went wrong.
+        Either the updated `parameters` input or a `PropertyError` if something went wrong.
 
     See Also:
         - https://swagger.io/docs/specification/using-ref/
     """
-    param, schemas, parameters = parameter_from_data(
-        data=data, name=data.name, parameters=parameters, schemas=schemas, required=True, config=config
-    )
+    param, parameters = parameter_from_data(data=data, name=data.name, parameters=parameters, required=True)
 
     if isinstance(param, ParameterError):
         param.detail = f"{param.header}: {param.detail}"
-        param.header = f"Unable to parse schema {ref_path}"
+        param.header = f"Unable to parse parameter {ref_path}"
         if isinstance(param.data, oai.Reference) and param.data.ref.endswith(ref_path):  # pragma: nocover
             param.detail += (
                 "\n\nRecursive and circular references are not supported. "
@@ -196,6 +184,7 @@ def update_parameters_with_data(
         return param
 
     parameters = attr.evolve(parameters, classes_by_reference={ref_path: param, **parameters.classes_by_reference})
+    print("ref_path is: %s", ref_path)
     return parameters
 
 

tests/test_parser/test_openapi.py

@@ -42,8 +42,6 @@ def test_from_dict(self, mocker, model_property_factory, enum_property_factory):
         build_schemas.assert_called_once_with(components=openapi.components.schemas, config=config, schemas=Schemas())
         build_parameters.assert_called_once_with(
             components=openapi.components.parameters,
-            config=config,
-            schemas=build_schemas.return_value,
             parameters=parameters,
         )
         EndpointCollection.from_data.assert_called_once_with(
@@ -63,10 +61,12 @@ def test_from_dict(self, mocker, model_property_factory, enum_property_factory):
         # Test no components
         openapi.components = None
         build_schemas.reset_mock()
+        build_parameters.reset_mock()
 
         GeneratorData.from_dict(in_dict, config=config)
 
         build_schemas.assert_not_called()
+        build_parameters.assert_not_called()
 
     def test_from_dict_invalid_schema(self, mocker):
         Schemas = mocker.patch(f"{MODULE_NAME}.Schemas")

tests/test_parser/test_properties/test_init.py

@@ -6,15 +6,8 @@
 
 import openapi_python_client.schema as oai
 from openapi_python_client import Config
-from openapi_python_client.parser.errors import PropertyError, ValidationError
-from openapi_python_client.parser.properties import (
-    BooleanProperty,
-    FloatProperty,
-    IntProperty,
-    NoneProperty,
-    Property,
-    Schemas,
-)
+from openapi_python_client.parser.errors import ParameterError, PropertyError, ValidationError
+from openapi_python_client.parser.properties import BooleanProperty, FloatProperty, IntProperty, Property, Schemas
 
 MODULE_NAME = "openapi_python_client.parser.properties"
 
@@ -995,6 +988,84 @@ def test_retries_failing_properties_while_making_progress(self, mocker):
         assert result.errors == [PropertyError()]
 
 
+class TestBuildParameters:
+    def test_skips_references_and_keeps_going(self, mocker):
+        from openapi_python_client.parser.properties import Parameters, build_parameters
+        from openapi_python_client.schema import Parameter, Reference
+
+        parameters = {
+            "reference": Reference(ref="#/components/parameters/another_parameter"),
+            "defined": Parameter(
+                name="page",
+                param_in="query",
+                required=False,
+                style="form",
+                explode=True,
+                schema=oai.Schema(type="integer", default=0),
+            ),
+        }
+
+        update_parameters_with_data = mocker.patch(f"{MODULE_NAME}.update_parameters_with_data")
+        parse_reference_path = mocker.patch(f"{MODULE_NAME}.parse_reference_path")
+
+        result = build_parameters(components=parameters, parameters=Parameters())
+        # Should not even try to parse a path for the Reference
+        parse_reference_path.assert_called_once_with("#/components/parameters/defined")
+        update_parameters_with_data.assert_called_once_with(
+            ref_path=parse_reference_path.return_value,
+            data=parameters["defined"],
+            parameters=Parameters(
+                errors=[ParameterError(detail="Reference parameters are not supported.", data=parameters["reference"])]
+            ),
+        )
+        assert result == update_parameters_with_data.return_value
+
+    def test_records_bad_uris_and_keeps_going(self, mocker):
+        from openapi_python_client.parser.properties import Parameters, build_parameters
+        from openapi_python_client.schema import Parameter
+
+        parameters = {"first": Parameter.construct(), "second": Parameter.construct()}
+        update_parameters_with_data = mocker.patch(f"{MODULE_NAME}.update_parameters_with_data")
+        parse_reference_path = mocker.patch(
+            f"{MODULE_NAME}.parse_reference_path", side_effect=[ParameterError(detail="some details"), "a_path"]
+        )
+
+        result = build_parameters(components=parameters, parameters=Parameters())
+        parse_reference_path.assert_has_calls(
+            [
+                call("#/components/parameters/first"),
+                call("#/components/parameters/second"),
+            ]
+        )
+        update_parameters_with_data.assert_called_once_with(
+            ref_path="a_path",
+            data=parameters["second"],
+            parameters=Parameters(errors=[ParameterError(detail="some details", data=parameters["first"])]),
+        )
+        assert result == update_parameters_with_data.return_value
+
+    def test_retries_failing_parameters_while_making_progress(self, mocker):
+        from openapi_python_client.parser.properties import Parameters, build_parameters
+        from openapi_python_client.schema import Parameter
+
+        parameters = {"first": Parameter.construct(), "second": Parameter.construct()}
+        update_parameters_with_data = mocker.patch(
+            f"{MODULE_NAME}.update_parameters_with_data", side_effect=[ParameterError(), Parameters(), ParameterError()]
+        )
+
+        parse_reference_path = mocker.patch(f"{MODULE_NAME}.parse_reference_path")
+        result = build_parameters(components=parameters, parameters=Parameters())
+        parse_reference_path.assert_has_calls(
+            [
+                call("#/components/parameters/first"),
+                call("#/components/parameters/second"),
+                call("#/components/parameters/first"),
+            ]
+        )
+        assert update_parameters_with_data.call_count == 3
+        assert result.errors == [ParameterError()]
+
+
 def test_build_enum_property_conflict():
     from openapi_python_client.parser.properties import Schemas, build_enum_property
 

tests/test_parser/test_properties/test_schemas.py

@@ -1,5 +1,12 @@
 import pytest
 
+from openapi_python_client.parser.errors import ParameterError
+from openapi_python_client.parser.properties import Class, Parameters
+from openapi_python_client.parser.properties.schemas import parameter_from_reference
+from openapi_python_client.schema import Parameter, Reference
+
+MODULE_NAME = "openapi_python_client.parser.properties.schemas"
+
 
 def test_class_from_string_default_config():
     from openapi_python_client import Config
@@ -32,3 +39,109 @@ def test_class_from_string(class_override, module_override, expected_class, expe
     result = Class.from_string(string=ref, config=config)
     assert result.name == expected_class
     assert result.module_name == expected_module
+
+
+class TestParameterFromData:
+    def test_cannot_parse_parameters_by_reference(self):
+        from openapi_python_client.parser.properties import Parameters
+        from openapi_python_client.parser.properties.schemas import parameter_from_data
+
+        ref = Reference.construct(ref="#/components/parameters/a_param")
+        parameters = Parameters()
+        param_or_error, new_parameters = parameter_from_data(
+            name="a_param", required=True, data=ref, parameters=parameters
+        )
+        assert param_or_error == ParameterError("Unable to resolve another reference")
+        assert new_parameters == parameters
+
+    def test_parameters_without_schema_are_ignored(self):
+        from openapi_python_client.parser.properties import Parameters
+        from openapi_python_client.parser.properties.schemas import parameter_from_data
+        from openapi_python_client.schema import ParameterLocation, Schema
+
+        param = Parameter(name="a_schemaless_param", param_in=ParameterLocation.QUERY)
+        parameters = Parameters()
+        param_or_error, new_parameters = parameter_from_data(
+            name=param.name, required=param.required, data=param, parameters=parameters
+        )
+        assert param_or_error == ParameterError("Parameter has no schema")
+        assert new_parameters == parameters
+
+    def test_registers_new_parameters(self):
+        from openapi_python_client.parser.properties import Parameters
+        from openapi_python_client.parser.properties.schemas import parameter_from_data
+        from openapi_python_client.schema import ParameterLocation, Schema
+
+        param = Parameter.construct(name="a_param", param_in=ParameterLocation.QUERY, param_schema=Schema.construct())
+        parameters = Parameters()
+        param_or_error, new_parameters = parameter_from_data(
+            name=param.name, required=param.required, data=param, parameters=parameters
+        )
+        assert param_or_error == param
+        assert new_parameters.classes_by_name[param.name] == param
+
+
+class TestParameterFromReference:
+    def test_returns_parameter_if_parameter_provided(self):
+        param = Parameter.construct()
+        params = Parameters()
+        param_or_error = parameter_from_reference(param=param, parameters=params)
+        assert param_or_error == param
+
+    def test_errors_out_if_reference_not_in_parameters(self):
+        ref = Reference.construct(ref="#/components/parameters/a_param")
+        class_info = Class(name="a_param", module_name="module_name")
+        existing_param = Parameter.construct(name="a_param")
+        param_by_ref = Reference.construct(ref="#/components/parameters/another_param")
+        params = Parameters(
+            classes_by_name={class_info.name: existing_param}, classes_by_reference={ref.ref: existing_param}
+        )
+        param_or_error = parameter_from_reference(param=param_by_ref, parameters=params)
+        assert param_or_error == ParameterError(
+            detail="Reference `/components/parameters/another_param` not found.",
+        )
+
+    def test_returns_reference_from_registry(self):
+        existing_param = Parameter.construct(name="a_param")
+        class_info = Class(name="MyParameter", module_name="module_name")
+        params = Parameters(
+            classes_by_name={class_info.name: existing_param},
+            classes_by_reference={"/components/parameters/a_param": existing_param},
+        )
+
+        param_by_ref = Reference.construct(ref="#/components/parameters/a_param")
+        param_or_error = parameter_from_reference(param=param_by_ref, parameters=params)
+        assert param_or_error == existing_param
+
+
+class TestUpdateParametersFromData:
+    def test_reports_parameters_with_errors(self, mocker):
+        from openapi_python_client.parser.properties.schemas import update_parameters_with_data
+        from openapi_python_client.schema import ParameterLocation, Schema
+
+        parameters = Parameters()
+        param = Parameter.construct(name="a_param", param_in=ParameterLocation.QUERY, param_schema=Schema.construct())
+        parameter_from_data = mocker.patch(
+            f"{MODULE_NAME}.parameter_from_data", side_effect=[(ParameterError(), parameters)]
+        )
+        ref_path = Reference.construct(ref="#/components/parameters/a_param")
+        new_parameters_or_error = update_parameters_with_data(ref_path=ref_path.ref, data=param, parameters=parameters)
+
+        parameter_from_data.assert_called_once()
+        assert new_parameters_or_error == ParameterError(
+            detail="Unable to parse this part of your OpenAPI document: : None",
+            header="Unable to parse parameter #/components/parameters/a_param",
+        )
+
+    def test_records_references_to_parameters(self, mocker):
+        from openapi_python_client.parser.properties.schemas import update_parameters_with_data
+        from openapi_python_client.schema import ParameterLocation, Schema
+
+        parameters = Parameters()
+        param = Parameter.construct(name="a_param", param_in=ParameterLocation.QUERY, param_schema=Schema.construct())
+        parameter_from_data = mocker.patch(f"{MODULE_NAME}.parameter_from_data", side_effect=[(param, parameters)])
+        ref_path = "#/components/parameters/a_param"
+        new_parameters = update_parameters_with_data(ref_path=ref_path, data=param, parameters=parameters)
+
+        parameter_from_data.assert_called_once()
+        assert new_parameters.classes_by_reference[ref_path] == param