app: Add logic to handle $ref in parameters BNCH-125141 BNCH-125943 (#230)

In December of 2020, when we diverged from the upstream branch, the client generator could not yet handle $ref in the parameters list. They added that feature in August 2022 but we never pulled it in.

In this PR, I try to pull in the same implementation from the upstream while modifying our repo as little as possible. Only the tests are really hand-written.

* Improve configuration for vscode development

- Set ruler to 120 characters, which is the configured value in black
- Set up vscode to use pytest with the same configuration as task unit
- Turn off mypy in tests. They are so noncompliant that it's useless

* Copy ParameterError from upstream

* Copy ClassName from upstream

* Copy parameter reference handling from upstream

This builds a library of parameter references from the openapi spec and
gives a utility for looking them up.

* Copy build_parameters from upstream

This builds the actual Parameters object from the OpenAPI spec, using
the logic we just added to schemas.

* Copy parameter reference logic from upstream

Intead of ignoring reference-typed parameters, look them up in the
Parameters object.

* Update unit tests

* Update version to 1.0.7

* Remove explicit local python interpreter

* Format launch.json

* eng: Update code to be editable BNCH-125859 (#229)

* Update code to be editable

Fix the prod/1.x branch so it's possible to run the client library
generator from a local clone.

#### Changes to make it work at all:

`pyproject.toml`
- We'll use python 3.9 because 3.7 is dead-dead
- PyYAML <6 isn't installable anymore, thanks to PEP 517
- shellingham 1.5.0 was yanked, so we need a later version
- Include type annotations for PyYAML for good measure

#### Changes to make the generated output match what's already published:

`poetry.lock`
- Downgrade black so it tolerates leading and trailing spaces in triple-
quoted strings
- Upgrade autoflake to match the version in aurelia
- Upgrade isort to match the version in aurelia

`openapi_python_client/templates/pyproject.toml`
- Enforce line length of 110 characters
- Configure isort to use the same settings as in aurelia (don't separate
by "from foo import" vs "import"; don't separate constants-style imports
from other imports)

`openapi_python_client/__init__.py`
- Run autoflake from outside the generated client package. This sidesteps
an issue where [autoflake will crash if it runs against a file called
types.py](https://discuss.python.org/t/my-python-dont-work/80726). We
have a file called types.py.

* Run poetry lock

* Fix unit tests

* Fix flake8

* Remove difficult-to-fix checks and add todo's

* Pin the black version

* Minimal changes to enable debugging

Author: peter-goldstein

.vscode/launch.json

@@ -0,0 +1,21 @@
+{
+  // Use IntelliSense to learn about possible attributes.
+  // Hover to view descriptions of existing attributes.
+  // For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387
+  "version": "0.2.0",
+  "configurations": [
+    {
+      "name": "Run openapi-python-client CLI",
+      "type": "debugpy",
+      "request": "launch",
+      "module": "openapi_python_client.cli",
+      "args": [
+        "generate",
+        "--path",
+        "../aurelia/benchling/dev_platform/specs/__dist/la/api/v2/openapi.yaml",
+        "--custom-template-path",
+        "../aurelia/packages/api_client_generation/client_gen/templates"
+      ]
+    }
+  ]
+}

.vscode/settings.json

@@ -0,0 +1,7 @@
+{
+  "editor.rulers": [120],
+  "mypy-type-checker.args": ["--config-file=mypy.ini"],
+  "python.testing.pytestArgs": ["openapi_python_client", "tests"],
+  "python.testing.pytestEnabled": true,
+  "python.testing.unittestEnabled": false
+}

mypy.ini

@@ -3,6 +3,7 @@ disallow_any_generics = True
 disallow_untyped_defs = True
 warn_redundant_casts = True
 strict_equality = True
+exclude = "(^tests/)"
 
 [mypy-importlib_metadata]
 ignore_missing_imports = True
@@ -12,3 +13,9 @@ ignore_missing_imports = True
 
 [mypy-typer]
 ignore_missing_imports = True
+
+[mypy-tests.*]
+ignore_errors = True
+
+[mypy-tests]
+ignore_errors = True

openapi_python_client/cli.py

@@ -142,3 +142,7 @@ def update(
 
     errors = update_existing_client(url=url, path=path, custom_template_path=custom_template_path)
     handle_errors(errors)
+
+
+if __name__ == "__main__":
+    app()

openapi_python_client/parser/errors.py

@@ -41,3 +41,10 @@ class PropertyError(ParseError):
 
 class ValidationError(Exception):
     pass
+
+
+@dataclass
+class ParameterError(ParseError):
+    """Error raised when there's a problem creating a Parameter."""
+
+    header = "Problem creating a Parameter: "

openapi_python_client/parser/openapi.py

@@ -5,10 +5,20 @@
 
 from pydantic import ValidationError
 
+from .properties.schemas import Parameters, parameter_from_reference
+
 from .. import schema as oai
 from .. import utils
 from .errors import GeneratorError, ParseError, PropertyError
-from .properties import EnumProperty, ModelProperty, Property, Schemas, build_schemas, property_from_data
+from .properties import (
+    EnumProperty,
+    ModelProperty,
+    Property,
+    Schemas,
+    build_parameters,
+    build_schemas,
+    property_from_data,
+)
 from .reference import Reference
 from .responses import Response, response_from_data
 
@@ -36,7 +46,7 @@ class EndpointCollection:
 
     @staticmethod
     def from_data(
-        *, data: Dict[str, oai.PathItem], schemas: Schemas
+        *, data: Dict[str, oai.PathItem], schemas: Schemas, parameters: Parameters
     ) -> Tuple[Dict[str, "EndpointCollection"], Schemas]:
         """ Parse the openapi paths data to get EndpointCollections by tag """
         endpoints_by_tag: Dict[str, EndpointCollection] = {}
@@ -51,7 +61,7 @@ def from_data(
                 tag = (operation.tags or ["default"])[0]
                 collection = endpoints_by_tag.setdefault(tag, EndpointCollection(tag=tag))
                 endpoint, schemas = Endpoint.from_data(
-                    data=operation, path=path, method=method, tag=tag, schemas=schemas
+                    data=operation, path=path, method=method, tag=tag, schemas=schemas, parameters=parameters
                 )
                 if isinstance(endpoint, ParseError):
                     endpoint.header = (
@@ -217,14 +227,26 @@ def _add_responses(*, endpoint: "Endpoint", data: oai.Responses, schemas: Schema
 
     @staticmethod
     def _add_parameters(
-        *, endpoint: "Endpoint", data: oai.Operation, schemas: Schemas
+        *,
+        endpoint: "Endpoint",
+        data: oai.Operation,
+        schemas: Schemas,
+        parameters: Parameters,
     ) -> Tuple[Union["Endpoint", ParseError], Schemas]:
         endpoint = deepcopy(endpoint)
         if data.parameters is None:
             return endpoint, schemas
+
         for param in data.parameters:
-            if isinstance(param, oai.Reference) or param.param_schema is None:
+            # Obtain the parameter from the reference or just the parameter itself
+            param_or_error = parameter_from_reference(param=param, parameters=parameters)
+            if isinstance(param_or_error, ParseError):
+                return param_or_error, schemas
+            param = param_or_error  # noqa: PLW2901
+
+            if param.param_schema is None:
                 continue
+
             prop, schemas = property_from_data(
                 name=param.name,
                 required=param.required,
@@ -248,7 +270,7 @@ def _add_parameters(
 
     @staticmethod
     def from_data(
-        *, data: oai.Operation, path: str, method: str, tag: str, schemas: Schemas
+        *, data: oai.Operation, path: str, method: str, tag: str, schemas: Schemas, parameters: Parameters
     ) -> Tuple[Union["Endpoint", ParseError], Schemas]:
         """ Construct an endpoint from the OpenAPI data """
 
@@ -266,7 +288,7 @@ def from_data(
             tag=tag,
         )
 
-        result, schemas = Endpoint._add_parameters(endpoint=endpoint, data=data, schemas=schemas)
+        result, schemas = Endpoint._add_parameters(endpoint=endpoint, data=data, schemas=schemas, parameters=parameters)
         if isinstance(result, ParseError):
             return result, schemas
         result, schemas = Endpoint._add_responses(endpoint=result, data=data.responses, schemas=schemas)
@@ -298,7 +320,13 @@ def from_dict(d: Dict[str, Dict[str, Any]]) -> Union["GeneratorData", GeneratorE
             schemas = Schemas()
         else:
             schemas = build_schemas(components=openapi.components.schemas)
-        endpoint_collections_by_tag, schemas = EndpointCollection.from_data(data=openapi.paths, schemas=schemas)
+        if openapi.components is None or openapi.components.parameters is None:
+            parameters = Parameters()
+        else:
+            parameters = build_parameters(components=openapi.components.parameters)
+        endpoint_collections_by_tag, schemas = EndpointCollection.from_data(
+            data=openapi.paths, schemas=schemas, parameters=parameters
+        )
         enums = schemas.enums
 
         return GeneratorData(

openapi_python_client/parser/properties/__init__.py

@@ -2,18 +2,19 @@
 
 from itertools import chain
 from typing import Any, ClassVar, Dict, Generic, Iterable, Iterator, List, Optional, Set, Tuple, TypeVar, Union
+from xml.etree.ElementTree import ParseError
 
 import attr
 
 from ... import schema as oai
 from ... import utils
-from ..errors import PropertyError, ValidationError
+from ..errors import ParameterError, PropertyError, ValidationError
 from ..reference import Reference
 from .converter import convert, convert_chain
 from .enum_property import EnumProperty
 from .model_property import ModelProperty
 from .property import Property
-from .schemas import Schemas
+from .schemas import Parameters, Schemas, parse_reference_path, update_parameters_with_data
 
 
 @attr.s(auto_attribs=True, frozen=True)
@@ -671,3 +672,44 @@ def build_schemas(*, components: Dict[str, Union[oai.Reference, oai.Schema]]) ->
     schemas.errors.extend(errors)
     schemas.errors.extend(resolve_errors)
     return schemas
+
+
+def build_parameters(
+    *,
+    components: Dict[str, Union[oai.Reference, oai.Parameter]],
+) -> Parameters:
+    """Get a list of Parameters from an OpenAPI dict"""
+    parameters = Parameters()
+    to_process: Iterable[Tuple[str, Union[oai.Reference, oai.Parameter]]] = []
+    if components is not None:
+        to_process = components.items()
+    still_making_progress = True
+    errors: list[ParameterError] = []
+
+    # References could have forward References so keep going as long as we are making progress
+    while still_making_progress:
+        still_making_progress = False
+        errors = []
+        next_round = []
+        # 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(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(ParameterError(detail=ref_path.detail, data=data))
+                continue
+            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)
+                continue
+            parameters = parameters_or_err
+            still_making_progress = True
+        to_process = next_round
+
+    parameters.errors.extend(errors)
+    return parameters

openapi_python_client/parser/properties/schemas.py

@@ -1,13 +1,34 @@
 __all__ = ["Schemas"]
 
-from typing import Dict, List
+from typing import Dict, List, NewType, Tuple, Union, cast
+from urllib.parse import urlparse
 
 import attr
 
-from ..errors import ParseError
+from ... import schema as oai
+from ...schema.parameter import Parameter
+
+from ..errors import ParameterError, ParseError
 from .enum_property import EnumProperty
+from ...utils import ClassName
 from .model_property import ModelProperty
 
+ReferencePath = NewType("ReferencePath", str)
+
+
+def parse_reference_path(ref_path_raw: str) -> Union[ReferencePath, ParseError]:
+    """
+    Takes a raw string provided in a `$ref` and turns it into a validated `_ReferencePath` or a `ParseError` if
+    validation fails.
+
+    See Also:
+        - https://swagger.io/docs/specification/using-ref/
+    """
+    parsed = urlparse(ref_path_raw)
+    if parsed.scheme or parsed.path:
+        return ParseError(detail=f"Remote references such as {ref_path_raw} are not supported yet.")
+    return cast(ReferencePath, parsed.fragment)
+
 
 @attr.s(auto_attribs=True, frozen=True)
 class Schemas:
@@ -16,3 +37,104 @@ class Schemas:
     enums: Dict[str, EnumProperty] = attr.ib(factory=dict)
     models: Dict[str, ModelProperty] = attr.ib(factory=dict)
     errors: List[ParseError] = attr.ib(factory=list)
+
+
+@attr.s(auto_attribs=True, frozen=True)
+class Parameters:
+    """Structure for containing all defined, shareable, and reusable parameters"""
+
+    classes_by_reference: dict[ReferencePath, Parameter] = attr.ib(factory=dict)
+    classes_by_name: dict[ClassName, Parameter] = attr.ib(factory=dict)
+    errors: list[ParseError] = attr.ib(factory=list)
+
+
+def parameter_from_data(
+    *,
+    name: str,
+    required: bool,
+    data: Union[oai.Reference, oai.Parameter],
+    parameters: Parameters,
+) -> Tuple[Union[Parameter, ParameterError], Parameters]:
+    """Generates parameters from an OpenAPI Parameter spec."""
+
+    if isinstance(data, oai.Reference):
+        return ParameterError("Unable to resolve another reference"), parameters
+
+    if data.param_schema is None:
+        return ParameterError("Parameter has no schema"), parameters
+
+    new_param = Parameter(
+        name=name,
+        required=required,
+        explode=data.explode,
+        style=data.style,
+        param_schema=data.param_schema,
+        param_in=data.param_in,
+    )
+    parameters = attr.evolve(parameters, classes_by_name={**parameters.classes_by_name, name: new_param})
+    return new_param, parameters
+
+
+def update_parameters_with_data(
+    *, ref_path: ReferencePath, data: oai.Parameter, parameters: Parameters
+) -> Union[Parameters, ParameterError]:
+    """
+    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.
+        parameters: `Parameters` up until now.
+
+    Returns:
+        Either the updated `parameters` input or a `PropertyError` if something went wrong.
+
+    See Also:
+        - https://swagger.io/docs/specification/using-ref/
+    """
+    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 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. "
+                "See https://github.com/openapi-generators/openapi-python-client/issues/466"
+            )
+        return param
+
+    parameters = attr.evolve(parameters, classes_by_reference={ref_path: param, **parameters.classes_by_reference})
+    return parameters
+
+
+def parameter_from_reference(
+    *,
+    param: Union[oai.Reference, Parameter],
+    parameters: Parameters,
+) -> Union[Parameter, ParameterError]:
+    """
+    Returns a Parameter from a Reference or the Parameter itself if one was provided.
+
+    Args:
+        param: A parameter by `Reference`.
+        parameters: `Parameters` up until now.
+
+    Returns:
+        Either the updated `schemas` input or a `PropertyError` if something went wrong.
+
+    See Also:
+        - https://swagger.io/docs/specification/using-ref/
+    """
+    if isinstance(param, Parameter):
+        return param
+
+    ref_path = parse_reference_path(param.ref)
+
+    if isinstance(ref_path, ParseError):
+        return ParameterError(detail=ref_path.detail)
+
+    _resolved_parameter_class = parameters.classes_by_reference.get(ref_path, None)
+    if _resolved_parameter_class is None:
+        return ParameterError(detail=f"Reference `{ref_path}` not found.")
+    return _resolved_parameter_class