Skip to content

Commit

Permalink
feat!: oneOf Union variants use Title names instead of indices
Browse files Browse the repository at this point in the history
Changed behavior from using the term `x_type_0` to `x_type_y` if x is a Schema that defines a Title property string `y`.

This provides an easier interface for understanding what users are importing, as well as less reliance on the ordering of the schema.

BREAKING CHANGE: The name of imports will change if the Schema they were built on previously used Title properties on fields within a oneOf Union.
  • Loading branch information
wallagib committed Feb 19, 2024
1 parent 9600088 commit 5c93054
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 7 deletions.
29 changes: 25 additions & 4 deletions openapi_python_client/parser/properties/union.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,14 @@ class UnionProperty(PropertyProtocol):

@classmethod
def build(
cls, *, data: oai.Schema, name: str, required: bool, schemas: Schemas, parent_name: str, config: Config
cls,
*,
data: oai.Schema,
name: str,
required: bool,
schemas: Schemas,
parent_name: str,
config: Config,
) -> tuple[UnionProperty | PropertyError, Schemas]:
"""
Create a `UnionProperty` the right way.
Expand All @@ -55,16 +62,25 @@ def build(
type_list_data.append(data.model_copy(update={"type": _type, "default": None}))

for i, sub_prop_data in enumerate(chain(data.anyOf, data.oneOf, type_list_data)):
# If a schema has a title property, we can use that to carry forward a descriptive instead of "type_0"
subscript: int | str = i
is_oneOf = i >= len(data.anyOf) and i < (len(data.anyOf) + len(data.oneOf))
if isinstance(sub_prop_data, oai.Schema) and sub_prop_data.title is not None and is_oneOf:
subscript = sub_prop_data.title

sub_prop, schemas = property_from_data(
name=f"{name}_type_{i}",
name=f"{name}_type_{subscript}",
required=True,
data=sub_prop_data,
schemas=schemas,
parent_name=parent_name,
config=config,
)
if isinstance(sub_prop, PropertyError):
return PropertyError(detail=f"Invalid property in union {name}", data=sub_prop_data), schemas
return (
PropertyError(detail=f"Invalid property in union {name}", data=sub_prop_data),
schemas,
)
sub_properties.append(sub_prop)

prop = UnionProperty(
Expand Down Expand Up @@ -97,7 +113,12 @@ def convert_value(self, value: Any) -> Value | None | PropertyError:

def _get_inner_type_strings(self, json: bool, multipart: bool) -> set[str]:
return {
p.get_type_string(no_optional=True, json=json, multipart=multipart, quoted=not p.is_base_type)
p.get_type_string(
no_optional=True,
json=json,
multipart=multipart,
quoted=not p.is_base_type,
)
for p in self.inner_properties
}

Expand Down
4 changes: 2 additions & 2 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

import pytest

from openapi_python_client import Config, MetaType
from openapi_python_client import Config, MetaType, utils
from openapi_python_client import schema as oai
from openapi_python_client.config import ConfigFile
from openapi_python_client.parser.properties import (
Expand Down Expand Up @@ -267,5 +267,5 @@ def _common_kwargs(kwargs: Dict[str, Any]) -> Dict[str, Any]:
**kwargs,
}
if not kwargs.get("python_name"):
kwargs["python_name"] = kwargs["name"]
kwargs["python_name"] = utils.PythonIdentifier(value=kwargs["name"], prefix="")
return kwargs
2 changes: 1 addition & 1 deletion tests/test_parser/test_properties/test_init.py
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,7 @@ def test_property_from_data_str_enum_with_null(
# None / null is removed from enum, and property is now nullable
assert isinstance(prop, UnionProperty), "Enums with None should be converted to UnionProperties"
enum_prop = enum_property_factory(
name="my_enum_type_1",
name="my_enum_type_AnEnum",
required=required,
values={"A": "A", "B": "B", "C": "C"},
class_info=Class(name="ParentAnEnum", module_name="parent_an_enum"),
Expand Down
48 changes: 48 additions & 0 deletions tests/test_parser/test_properties/test_union.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,3 +81,51 @@ def test_not_required_in_path(config):

err = prop.validate_location(ParameterLocation.PATH)
assert isinstance(err, ParseError)


def test_union_oneOf_descriptive_type_name(
union_property_factory, date_time_property_factory, string_property_factory, config
):
from openapi_python_client.parser.properties import Schemas, property_from_data

nested_schema_variant_A = oai.Schema(type=DataType.STRING, title="A")
nested_schema_variant_B = oai.Schema(type=DataType.STRING, title="B")
nested_schema_variant_2 = oai.Schema(type=DataType.STRING)
nested_schema_variant_C = oai.Schema(type=DataType.STRING, title="C")
nested_schema_variant_4 = oai.Schema(type=DataType.STRING)

name = "union_prop"
required = True
data = oai.Schema(
anyOf=[
# AnyOf retains the old naming convention
nested_schema_variant_C,
nested_schema_variant_4,
],
oneOf=[
# OneOf fields that define their own titles will have those titles as their Type names
nested_schema_variant_A,
nested_schema_variant_B,
nested_schema_variant_2,
oai.Schema(type=DataType.STRING, schema_format="date-time"),
],
)
expected = union_property_factory(
name=name,
required=required,
inner_properties=[
string_property_factory(name=f"{name}_type_0"),
string_property_factory(name=f"{name}_type_1"),
string_property_factory(name=f"{name}_type_A"),
string_property_factory(name=f"{name}_type_B"),
string_property_factory(name=f"{name}_type_4"),
date_time_property_factory(name=f"{name}_type_5"),
],
)

p, s = property_from_data(
name=name, required=required, data=data, schemas=Schemas(), parent_name="parent", config=config
)

assert p == expected
assert s == Schemas()

0 comments on commit 5c93054

Please sign in to comment.