Skip to content

Allow Unset values to avoid being send during serialization #10

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 1 commit into from
Oct 29, 2020
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
21 changes: 18 additions & 3 deletions openapi_python_client/parser/properties.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,23 +64,38 @@ def get_imports(self, *, prefix: str) -> Set[str]:
back to the root of the generated client.
"""
if self.nullable or not self.required:
return {"from typing import Optional"}
return {"from typing import Optional",
"from typing import cast",
f"from {prefix}types import UNSET"}
return set()

def to_string(self) -> str:
""" How this should be declared in a dataclass """
if self.default:
default = self.default
elif not self.required:
default = "None"
default = "cast(None, UNSET)"
else:
default = None

if default is not None:
return f"{self.python_name}: {self.get_type_string()} = {self.default}"
return f"{self.python_name}: {self.get_type_string()} = {default}"
else:
return f"{self.python_name}: {self.get_type_string()}"

def to_query_method_arg(self) -> str:
""" How this should be declared in a query string """
if self.default:
default = self.default
elif not self.required:
default = "None"
else:
default = None

if default is not None:
return f"{self.python_name}: {self.get_type_string()} = {default}"
else:
return f"{self.python_name}: {self.get_type_string()}"

@dataclass
class StringProperty(Property):
Expand Down
6 changes: 3 additions & 3 deletions openapi_python_client/templates/endpoint_macros.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ client: Client,
{% endif %}
{# path parameters #}
{% for parameter in endpoint.path_parameters %}
{{ parameter.to_string() }},
{{parameter.to_query_method_arg()}},
{% endfor %}
{# Form data if any #}
{% if endpoint.form_body_reference %}
Expand All @@ -96,10 +96,10 @@ json_body: {{ endpoint.json_body.get_type_string() }},
{% endif %}
{# query parameters #}
{% for parameter in endpoint.query_parameters %}
{{ parameter.to_string() }},
{{parameter.to_query_method_arg()}},
{% endfor %}
{% for parameter in endpoint.header_parameters %}
{{ parameter.to_string() }},
{{ parameter.to_query_method_arg() }},
{% endfor %}
{% endmacro %}

Expand Down
16 changes: 11 additions & 5 deletions openapi_python_client/templates/model.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,17 @@ class {{ model.reference.class_name }}:
{% endif %}
{% endfor %}

return {
{% for property in model.required_properties + model.optional_properties %}
"{{ property.name }}": {{ property.python_name }},
{% endfor %}
}
properties: Dict[str, Any] = dict()

{% for property in model.required_properties + model.optional_properties %}
{% if not property.required %}
if {{property.python_name}} is not UNSET:
properties["{{ property.name }}"] = {{ property.python_name }}
{% else %}
properties["{{ property.name }}"] = {{ property.python_name }}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible that a property is both required AND unset, and if so is it desired that it would be exposed in the dictionary directly (instead of, say, erroring)?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a property is required, we shouldn't allow unset - we simply shouldn't allow a default. It could still be nullable and required, in which case we should allow it to be None.

{% endif %}
{% endfor %}
return properties

@staticmethod
def from_dict(d: Dict[str, Any]) -> "{{ model.reference.class_name }}":
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ if {{ source }} is not None:
{% if property.required %}
{{ destination }} = {{ source }}.isoformat()
{% else %}
{{ destination }} = {{ source }}.isoformat() if {{ source }} else None
if {{ source }} is UNSET:
{{ destination }} = UNSET
else:
{{ destination }} = {{ source }}.isoformat() if {{ source }} else None
{% endif %}
{% endmacro %}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ if {{ source }} is not None:
{% if property.required %}
{{ destination }} = {{ source }}.isoformat()
{% else %}
{{ destination }} = {{ source }}.isoformat() if {{ source }} else None
if {{ source }} is UNSET:
{{ destination }} = UNSET
else:
{{ destination }} = {{ source }}.isoformat() if {{ source }} else None
{% endif %}
{% endmacro %}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ if {{ source }} is not None:
{% if property.required %}
{{ destination }} = {{ source }}
{% else %}
{{ destination }} = {{ source }} if {{ source }} else None
if {{ source }} is UNSET:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For my context, where is the macro transform actually called?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's called frequently in the various .pyi templates - you'll find it easily if you search and make sure you aren't filtering to only .py files.

{{ destination }} = UNSET
else:
{{ destination }} = {{ source }} if {{ source }} else None
{% endif %}
{% endmacro %}
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from openapi_python_client.templates.types import UNSET

{% macro construct(property, source) %}
{% if property.required %}
{{ property.python_name }} = {{ property.reference.class_name }}({{ source }})
Expand All @@ -12,6 +14,9 @@ if {{ source }} is not None:
{% if property.required %}
{{ destination }} = {{ source }}.value
{% else %}
{{ destination }} = {{ source }}.value if {{ source }} else None
if {{ source }} is UNSET:
{{ destination }} = UNSET
else:
{{ destination }} = {{ source }}.value if {{ source }} else None
{% endif %}
{% endmacro %}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@
{% if property.required %}
{{ destination }} = {{ source }}.to_tuple()
{% else %}
{{ destination }} = {{ source }}.to_tuple() if {{ source }} else None
if {{ source }} is UNSET:
{{ destination }} = UNSET
else:
{{ destination }} = {{ source }}.to_tuple() if {{ source }} else None
{% endif %}
{% endmacro %}
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from openapi_python_client.templates.types import UNSET

{% macro construct(property, source) %}
{% set inner_property = property.inner_property %}
{% if inner_property.template %}
Expand Down Expand Up @@ -36,6 +38,8 @@ for {{ inner_source }} in {{ source }}:
{% if not property.required %}
if {{ source }} is None:
{{ destination }} = None
elif {{ source }} is UNSET:
{{ destination }} = UNSET
else:
{{ _transform(property, source, destination) | indent(4) }}
{% else %}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ if {{ source }} is not None:
{% if property.required %}
{{ destination }} = {{ source }}.to_dict()
{% else %}
{{ destination }} = {{ source }}.to_dict() if {{ source }} else None
if {{ source }} is UNSET:
{{ destination }} = UNSET
else:
{{ destination }} = {{ source }}.to_dict() if {{ source }} else None
{% endif %}
{% endmacro %}
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ def _parse_{{ property.python_name }}(data: Dict[str, Any]) -> {{ property.get_t
{% if not property.required %}
if {{ source }} is None:
{{ destination }}: {{ property.get_type_string() }} = None
elif {{ source }} is UNSET:
{{ destination }} = UNSET
{% endif %}
{% for inner_property in property.inner_properties %}
{% if loop.first and property.required %}{# No if None statement before this #}
Expand All @@ -36,7 +38,7 @@ else:
{% endif %}
{% if inner_property.template %}
{% from "property_templates/" + inner_property.template import transform %}
{{ transform(inner_property, source, destination) | indent(8) }}
{{ transform(inner_property, source, destination) | indent(4) }}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What caused this indentation level to change?

{% else %}
{{ destination }} = {{ source }}
{% endif %}
Expand Down
5 changes: 4 additions & 1 deletion openapi_python_client/templates/types.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
""" Contains some shared types for properties """
from typing import BinaryIO, Generic, MutableMapping, Optional, TextIO, Tuple, TypeVar, Union
from typing import BinaryIO, Generic, MutableMapping, NewType, Optional, TextIO, Tuple, TypeVar, Union

import attr

Unset = NewType("Unset", object)
UNSET: Unset = Unset(object())


@attr.s(auto_attribs=True)
class File:
Expand Down
23 changes: 21 additions & 2 deletions tests/test_openapi_parser/test_properties.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def test_to_string(self, mocker):

assert p.to_string() == f"{snake_case(name)}: {get_type_string()}"
p.required = False
assert p.to_string() == f"{snake_case(name)}: {get_type_string()} = None"
assert p.to_string() == f"{snake_case(name)}: {get_type_string()} = cast(None, UNSET)"

p.default = "TEST"
assert p.to_string() == f"{snake_case(name)}: {get_type_string()} = TEST"
Expand All @@ -57,7 +57,26 @@ def test_get_imports(self, mocker):
assert p.get_imports(prefix="") == set()

p.required = False
assert p.get_imports(prefix="") == {"from typing import Optional"}
assert p.get_imports(prefix="") == {
"from types import UNSET",
"from typing import Optional",
"from typing import cast",
}

def test_to_query_method_arg(self, mocker):
from openapi_python_client.parser.properties import Property

name = mocker.MagicMock()
snake_case = mocker.patch("openapi_python_client.utils.snake_case")
p = Property(name=name, required=True, default=None, nullable=False)
get_type_string = mocker.patch.object(p, "get_type_string")

assert p.to_query_method_arg() == f"{snake_case(name)}: {get_type_string()}"
p.required = False
assert p.to_query_method_arg() == f"{snake_case(name)}: {get_type_string()} = None"

p.default = "TEST"
assert p.to_query_method_arg() == f"{snake_case(name)}: {get_type_string()} = TEST"

def test__validate_default(self):
from openapi_python_client.parser.properties import Property
Expand Down