Skip to content

fix: Allow None in enum properties [#504, #512]. Thanks @juspence! #516

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Oct 17, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from ...client import Client
from ...models.a_model import AModel
from ...models.an_enum import AnEnum
from ...models.an_enum_with_null import AnEnumWithNull
from ...models.http_validation_error import HTTPValidationError
from ...types import UNSET, Response

Expand All @@ -14,6 +15,8 @@ def _get_kwargs(
*,
client: Client,
an_enum_value: List[AnEnum],
an_enum_value_with_null: List[Optional[AnEnumWithNull]],
an_enum_value_with_only_null: List[None],
some_date: Union[datetime.date, datetime.datetime],
) -> Dict[str, Any]:
url = "{}/tests/".format(client.base_url)
Expand All @@ -27,13 +30,25 @@ def _get_kwargs(

json_an_enum_value.append(an_enum_value_item)

json_an_enum_value_with_null = []
for an_enum_value_with_null_item_data in an_enum_value_with_null:
an_enum_value_with_null_item = (
an_enum_value_with_null_item_data.value if an_enum_value_with_null_item_data else None
)

json_an_enum_value_with_null.append(an_enum_value_with_null_item)

json_an_enum_value_with_only_null = an_enum_value_with_only_null

if isinstance(some_date, datetime.date):
json_some_date = some_date.isoformat()
else:
json_some_date = some_date.isoformat()

params: Dict[str, Any] = {
"an_enum_value": json_an_enum_value,
"an_enum_value_with_null": json_an_enum_value_with_null,
"an_enum_value_with_only_null": json_an_enum_value_with_only_null,
"some_date": json_some_date,
}
params = {k: v for k, v in params.items() if v is not UNSET and v is not None}
Expand Down Expand Up @@ -82,11 +97,15 @@ def sync_detailed(
*,
client: Client,
an_enum_value: List[AnEnum],
an_enum_value_with_null: List[Optional[AnEnumWithNull]],
an_enum_value_with_only_null: List[None],
some_date: Union[datetime.date, datetime.datetime],
) -> Response[Union[HTTPValidationError, List[AModel]]]:
kwargs = _get_kwargs(
client=client,
an_enum_value=an_enum_value,
an_enum_value_with_null=an_enum_value_with_null,
an_enum_value_with_only_null=an_enum_value_with_only_null,
some_date=some_date,
)

Expand All @@ -101,13 +120,17 @@ def sync(
*,
client: Client,
an_enum_value: List[AnEnum],
an_enum_value_with_null: List[Optional[AnEnumWithNull]],
an_enum_value_with_only_null: List[None],
some_date: Union[datetime.date, datetime.datetime],
) -> Optional[Union[HTTPValidationError, List[AModel]]]:
"""Get a list of things"""

return sync_detailed(
client=client,
an_enum_value=an_enum_value,
an_enum_value_with_null=an_enum_value_with_null,
an_enum_value_with_only_null=an_enum_value_with_only_null,
some_date=some_date,
).parsed

Expand All @@ -116,11 +139,15 @@ async def asyncio_detailed(
*,
client: Client,
an_enum_value: List[AnEnum],
an_enum_value_with_null: List[Optional[AnEnumWithNull]],
an_enum_value_with_only_null: List[None],
some_date: Union[datetime.date, datetime.datetime],
) -> Response[Union[HTTPValidationError, List[AModel]]]:
kwargs = _get_kwargs(
client=client,
an_enum_value=an_enum_value,
an_enum_value_with_null=an_enum_value_with_null,
an_enum_value_with_only_null=an_enum_value_with_only_null,
some_date=some_date,
)

Expand All @@ -134,6 +161,8 @@ async def asyncio(
*,
client: Client,
an_enum_value: List[AnEnum],
an_enum_value_with_null: List[Optional[AnEnumWithNull]],
an_enum_value_with_only_null: List[None],
some_date: Union[datetime.date, datetime.datetime],
) -> Optional[Union[HTTPValidationError, List[AModel]]]:
"""Get a list of things"""
Expand All @@ -142,6 +171,8 @@ async def asyncio(
await asyncio_detailed(
client=client,
an_enum_value=an_enum_value,
an_enum_value_with_null=an_enum_value_with_null,
an_enum_value_with_only_null=an_enum_value_with_only_null,
some_date=some_date,
)
).parsed
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from .all_of_sub_model_type_enum import AllOfSubModelTypeEnum
from .an_all_of_enum import AnAllOfEnum
from .an_enum import AnEnum
from .an_enum_with_null import AnEnumWithNull
from .an_int_enum import AnIntEnum
from .another_all_of_sub_model import AnotherAllOfSubModel
from .another_all_of_sub_model_type import AnotherAllOfSubModelType
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
from enum import Enum


class AnEnumWithNull(str, Enum):
FIRST_VALUE = "FIRST_VALUE"
SECOND_VALUE = "SECOND_VALUE"

def __str__(self) -> str:
return str(self.value)
40 changes: 40 additions & 0 deletions end_to_end_tests/openapi.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,30 @@
"name": "an_enum_value",
"in": "query"
},
{
"required": true,
"schema": {
"title": "An Enum Value With Null And String Values",
"type": "array",
"items": {
"$ref": "#/components/schemas/AnEnumWithNull"
}
},
"name": "an_enum_value_with_null",
"in": "query"
},
{
"required": true,
"schema": {
"title": "An Enum Value With Only Null Values",
"type": "array",
"items": {
"$ref": "#/components/schemas/AnEnumWithOnlyNull"
}
},
"name": "an_enum_value_with_only_null",
"in": "query"
},
{
"required": true,
"schema": {
Expand Down Expand Up @@ -1164,6 +1188,22 @@
],
"description": "For testing Enums in all the ways they can be used "
},
"AnEnumWithNull": {
"title": "AnEnumWithNull",
"enum": [
"FIRST_VALUE",
"SECOND_VALUE",
null
],
"description": "For testing Enums with mixed string / null values "
},
"AnEnumWithOnlyNull": {
"title": "AnEnumWithOnlyNull",
"enum": [
null
],
"description": "For testing Enums with only null values "
},
"AnAllOfEnum": {
"title": "AnAllOfEnum",
"enum": [
Expand Down
43 changes: 35 additions & 8 deletions openapi_python_client/parser/properties/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,14 @@ class AnyProperty(Property):
template: ClassVar[Optional[str]] = "any_property.py.jinja"


@attr.s(auto_attribs=True, frozen=True)
class NoneProperty(Property):
"""A property that can only be None"""

_type_string: ClassVar[str] = "None"
_json_type_string: ClassVar[str] = "None"


@attr.s(auto_attribs=True, frozen=True)
class StringProperty(Property):
"""A property of type str"""
Expand Down Expand Up @@ -296,10 +304,10 @@ def build_enum_property(
name: str,
required: bool,
schemas: Schemas,
enum: Union[List[str], List[int]],
enum: Union[List[Optional[str]], List[Optional[int]]],
parent_name: Optional[str],
config: Config,
) -> Tuple[Union[EnumProperty, PropertyError], Schemas]:
) -> Tuple[Union[EnumProperty, NoneProperty, PropertyError], Schemas]:
"""
Create an EnumProperty from schema data.

Expand All @@ -316,11 +324,34 @@ def build_enum_property(
A tuple containing either the created property or a PropertyError describing what went wrong AND update schemas.
"""

if len(enum) == 0:
return PropertyError(detail="No values provided for Enum", data=data), schemas

class_name = data.title or name
if parent_name:
class_name = f"{utils.pascal_case(parent_name)}{utils.pascal_case(class_name)}"
class_info = Class.from_string(string=class_name, config=config)
values = EnumProperty.values_from_list(enum)

# OpenAPI allows for null as an enum value, but it doesn't make sense with how enums are constructed in Python.
# So instead, if null is a possible value, make the property nullable.
# Mypy is not smart enough to know that the type is right though
value_list: Union[List[str], List[int]] = [value for value in enum if value is not None] # type: ignore
if len(value_list) < len(enum):
data.nullable = True

# It's legal to have an enum that only contains null as a value, we don't bother constructing an enum in that case
if len(value_list) == 0:
return (
NoneProperty(
name=name,
required=required,
nullable=False,
default="None",
python_name=utils.PythonIdentifier(value=name, prefix=config.field_prefix),
),
schemas,
)
values = EnumProperty.values_from_list(value_list)

if class_info.name in schemas.classes_by_name:
existing = schemas.classes_by_name[class_info.name]
Expand All @@ -332,11 +363,7 @@ def build_enum_property(
schemas,
)

for value in values.values():
value_type = type(value)
break
else:
return PropertyError(data=data, detail="No values provided for Enum"), schemas
value_type = type(next(iter(values.values())))

prop = EnumProperty(
name=name,
Expand Down
2 changes: 1 addition & 1 deletion openapi_python_client/parser/properties/enum_property.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ def get_imports(self, *, prefix: str) -> Set[str]:

@staticmethod
def values_from_list(values: Union[List[str], List[int]]) -> Dict[str, ValueType]:
"""Convert a list of values into dict of {name: value}"""
"""Convert a list of values into dict of {name: value}, where value can sometimes be None"""
output: Dict[str, ValueType] = {}

for i, value in enumerate(values):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class Schema(BaseModel):
maxProperties: Optional[int] = Field(default=None, ge=0)
minProperties: Optional[int] = Field(default=None, ge=0)
required: Optional[List[str]] = Field(default=None, min_items=1)
enum: Union[None, List[StrictInt], List[StrictStr]] = Field(default=None, min_items=1)
enum: Union[None, List[Optional[StrictInt]], List[Optional[StrictStr]]] = Field(default=None, min_items=1)
type: Optional[DataType] = Field(default=None)
allOf: Optional[List[Union[Reference, "Schema"]]] = None
oneOf: List[Union[Reference, "Schema"]] = []
Expand Down
63 changes: 58 additions & 5 deletions tests/test_parser/test_properties/test_init.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
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, Schemas
from openapi_python_client.parser.properties import BooleanProperty, FloatProperty, IntProperty, NoneProperty, Schemas

MODULE_NAME = "openapi_python_client.parser.properties"

Expand Down Expand Up @@ -304,8 +304,58 @@ def test_property_from_data_str_enum(self, enum_property_factory):
"ParentAnEnum": prop,
}

def test_property_from_data_str_enum_with_null(self, enum_property_factory):
from openapi_python_client.parser.properties import Class, Schemas, property_from_data
from openapi_python_client.schema import Schema

existing = enum_property_factory()
data = Schema(title="AnEnum", enum=["A", "B", "C", None], nullable=False, default="B")
name = "my_enum"
required = True

schemas = Schemas(classes_by_name={"AnEnum": existing})

prop, new_schemas = property_from_data(
name=name, required=required, data=data, schemas=schemas, parent_name="parent", config=Config()
)

# None / null is removed from enum, and property is now nullable
assert prop == enum_property_factory(
name=name,
required=required,
nullable=True,
values={"A": "A", "B": "B", "C": "C"},
class_info=Class(name="ParentAnEnum", module_name="parent_an_enum"),
value_type=str,
default="ParentAnEnum.B",
)
assert prop.nullable is True
assert schemas != new_schemas, "Provided Schemas was mutated"
assert new_schemas.classes_by_name == {
"AnEnum": existing,
"ParentAnEnum": prop,
}

def test_property_from_data_null_enum(self, enum_property_factory):
from openapi_python_client.parser.properties import Class, Schemas, property_from_data
from openapi_python_client.schema import Schema

data = Schema(title="AnEnumWithOnlyNull", enum=[None], nullable=False, default=None)
name = "my_enum"
required = True

schemas = Schemas()

prop, new_schemas = property_from_data(
name=name, required=required, data=data, schemas=schemas, parent_name="parent", config=Config()
)

assert prop == NoneProperty(
name="my_enum", required=required, nullable=False, default="None", python_name="my_enum"
)

def test_property_from_data_int_enum(self, enum_property_factory):
from openapi_python_client.parser.properties import Class, EnumProperty, Schemas, property_from_data
from openapi_python_client.parser.properties import Class, Schemas, property_from_data
from openapi_python_client.schema import Schema

name = "my_enum"
Expand Down Expand Up @@ -923,14 +973,17 @@ def test_retries_failing_properties_while_making_progress(self, mocker):
assert result.errors == [PropertyError()]


def test_build_enum_property_conflict(mocker):
def test_build_enum_property_conflict():
from openapi_python_client.parser.properties import Schemas, build_enum_property

data = oai.Schema()
schemas = Schemas(classes_by_name={"Existing": mocker.MagicMock()})
schemas = Schemas()

_, schemas = build_enum_property(
data=data, name="Existing", required=True, schemas=schemas, enum=["a"], parent_name=None, config=Config()
)
err, schemas = build_enum_property(
data=data, name="Existing", required=True, schemas=schemas, enum=[], parent_name=None, config=Config()
data=data, name="Existing", required=True, schemas=schemas, enum=["a", "b"], parent_name=None, config=Config()
)

assert schemas == schemas
Expand Down