Skip to content
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

if constant is optional, don't force to value #952

Merged
merged 7 commits into from
Jun 16, 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
11 changes: 11 additions & 0 deletions ChangeLog.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,16 @@
# Change Log

### 2021-06-16 - 5.8.1

min Autorest core version: 3.3.0

min Modelerfour version: 4.19.1

**Bug Fixes**

- Fix optional properties with constant schemas. Now, properties that have constant schemas but are optional will not have the hardcoded constant value,
but will default to its `x-ms-client-default` or `None` #952

### 2021-05-17 - 5.8.0

min Autorest core version: 3.3.0
Expand Down
1 change: 0 additions & 1 deletion autorest/codegen/models/object_schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,6 @@ def fill_instance_from_yaml(self, namespace: str, yaml_data: Dict[str, Any], **k
# map of discriminator value to child's name
for children_yaml in yaml_data["discriminator"]["immediate"].values():
subtype_map[children_yaml["discriminatorValue"]] = children_yaml["language"]["python"]["name"]

if yaml_data.get("properties"):
properties += [
Property.from_yaml(p, has_additional_properties=len(properties) > 0, **kwargs)
Expand Down
2 changes: 1 addition & 1 deletion autorest/codegen/models/parameter.py
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ def in_method_signature(self) -> bool:

@property
def in_method_code(self) -> bool:
return not (isinstance(self.schema, ConstantSchema) and self.location == ParameterLocation.Other)
return not (self.constant and self.location == ParameterLocation.Other)

@property
def implementation(self) -> str:
Expand Down
30 changes: 20 additions & 10 deletions autorest/codegen/models/property.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,26 +32,36 @@ def __init__(
self.required: bool = yaml_data.get("required", False)
self.readonly: bool = yaml_data.get("readOnly", False)
self.is_discriminator: bool = yaml_data.get("isDiscriminator", False)
# this bool doesn't consider you to be constant if you are a discriminator
self.constant: bool = isinstance(self.schema, ConstantSchema) and not self.is_discriminator

if description:
self.description = description
else:
self.description = yaml_data["language"]["python"]["description"]

validation_map: Dict[str, Union[bool, int, str]] = {}
self.client_default_value = client_default_value

@property
def constant(self) -> bool:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

decided to make constant and validation_map readonly properties to unclutter the main__init__ and because we don't set them later

# this bool doesn't consider you to be constant if you are a discriminator
# you also have to be required to be considered a constant
return (
isinstance(self.schema, ConstantSchema) and
self.required and
not self.is_discriminator
)

@property
def validation_map(self) -> Optional[Dict[str, Union[bool, int, str]]]:
retval: Dict[str, Union[bool, int, str]] = {}
if self.required:
validation_map["required"] = True
retval["required"] = True
if self.readonly:
validation_map["readonly"] = True
retval["readonly"] = True
if self.constant:
validation_map["constant"] = True
retval["constant"] = True
if self.schema.validation_map:
validation_map_from_schema = cast(Dict[str, Union[bool, int, str]], self.schema.validation_map)
validation_map.update(validation_map_from_schema)
self.validation_map = validation_map or None
self.client_default_value = client_default_value
retval.update(validation_map_from_schema)
return retval or None

@property
def escaped_swagger_name(self) -> str:
Expand Down
13 changes: 9 additions & 4 deletions autorest/codegen/serializers/model_base_serializer.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,14 +62,19 @@ def prop_documentation_string(prop: Property) -> str:
description += "."
if prop.name == "tags":
description = "A set of tags. " + description
if prop.required:

if prop.constant:
description += f' Has constant value: {prop.constant_declaration}.'
elif prop.required:
if description:
description = "Required. " + description
else:
description = "Required. "
if prop.constant:
constant_prop = cast(ConstantSchema, prop.schema)
description += f' Default value: "{constant_prop.value}".'
elif isinstance(prop.schema, ConstantSchema):
description += (
f" The only acceptable values to pass in are None and {prop.constant_declaration}. " +
f"The default value is {prop.default_value_declaration}."
)
if prop.is_discriminator:
description += "Constant filled by server. "
if isinstance(prop.schema, EnumSchema):
Expand Down
1 change: 1 addition & 0 deletions dev_requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@ pytest
ptvsd
mypy
black
types-PyYAML
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@autorest/python",
"version": "5.8.0",
"version": "5.8.1",
"description": "The Python extension for generators in AutoRest.",
"scripts": {
"prepare": "node run-python3.js prepare.py",
Expand Down Expand Up @@ -28,7 +28,7 @@
},
"devDependencies": {
"@autorest/autorest": "^3.0.0",
"@microsoft.azure/autorest.testserver": "^3.0.23"
"@microsoft.azure/autorest.testserver": "^3.0.24"
},
"files": [
"autorest/**/*.py",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class Error(msrest.serialization.Model):

:param status:
:type status: int
:ivar constant_id: Required. Default value: "1".
:ivar constant_id: Has constant value: 1.
:vartype constant_id: int
:param message:
:type message: str
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class Error(msrest.serialization.Model):

:param status:
:type status: int
:ivar constant_id: Required. Default value: "1".
:ivar constant_id: Has constant value: 1.
:vartype constant_id: int
:param message:
:type message: str
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ async def test_model_flattening_simple(self, client):
product_id = "123",
description = "product description",
max_product_display_name = "max name",
capacity="Large",
odata_value = "http://foo",
generic_value = "https://generic"
)
Expand All @@ -230,6 +231,7 @@ async def test_model_flattening_with_parameter_flattening(self, client):
product_id = "123",
description = "product description",
max_product_display_name = "max name",
capacity="Large",
odata_value = "http://foo"
)
simple_product.additional_properties = {} # Not the purpose of this test. This enables the ==.
Expand All @@ -238,6 +240,7 @@ async def test_model_flattening_with_parameter_flattening(self, client):
"123", # product_id
"product description", # description
"max name", # max_product_display_name
"Large", # capacity
None, # generic_value
"http://foo", # odata_value
)
Expand All @@ -252,6 +255,7 @@ async def test_model_flattening_with_grouping(self, client):
product_id = "123",
description = "product description",
max_product_display_name = "max name",
capacity="Large",
odata_value = "http://foo"
)
simple_product.additional_properties = {} # Not the purpose of this test. This enables the ==.
Expand All @@ -260,6 +264,7 @@ async def test_model_flattening_with_grouping(self, client):
product_id = "123",
description = "product description",
max_product_display_name="max name",
capacity="Large",
odata_value="http://foo",
name="groupproduct")

Expand Down
5 changes: 5 additions & 0 deletions test/vanilla/AcceptanceTests/test_model_flattening.py
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,7 @@ def test_model_flattening_simple(self, client):
product_id = "123",
description = "product description",
max_product_display_name = "max name",
capacity="Large",
odata_value = "http://foo",
generic_value = "https://generic"
)
Expand All @@ -223,6 +224,7 @@ def test_model_flattening_with_parameter_flattening(self, client):
product_id = "123",
description = "product description",
max_product_display_name = "max name",
capacity="Large",
odata_value = "http://foo"
)
simple_product.additional_properties = {} # Not the purpose of this test. This enables the ==.
Expand All @@ -231,6 +233,7 @@ def test_model_flattening_with_parameter_flattening(self, client):
"123", # product_id
"product description", # description
"max name", # max_product_display_name
"Large", # capacity
None, # generic_value
"http://foo", # odata_value
)
Expand All @@ -244,6 +247,7 @@ def test_model_flattening_with_grouping(self, client):
product_id = "123",
description = "product description",
max_product_display_name = "max name",
capacity="Large",
odata_value = "http://foo"
)
simple_product.additional_properties = {} # Not the purpose of this test. This enables the ==.
Expand All @@ -252,6 +256,7 @@ def test_model_flattening_with_grouping(self, client):
product_id = "123",
description = "product description",
max_product_display_name="max name",
capacity="Large",
odata_value="http://foo",
name="groupproduct")

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,7 @@ class RefColorConstant(msrest.serialization.Model):

All required parameters must be populated in order to send to Azure.

:ivar color_constant: Required. Referenced Color Constant Description. Default value:
"green-color".
:ivar color_constant: Referenced Color Constant Description. Has constant value: "green-color".
:vartype color_constant: str
:param field1: Sample string.
:type field1: str
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,7 @@ class RefColorConstant(msrest.serialization.Model):

All required parameters must be populated in order to send to Azure.

:ivar color_constant: Required. Referenced Color Constant Description. Default value:
"green-color".
:ivar color_constant: Referenced Color Constant Description. Has constant value: "green-color".
:vartype color_constant: str
:param field1: Sample string.
:type field1: str
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,47 +164,35 @@ def __init__(self, **kwargs):
class NoModelAsStringNoRequiredOneValueDefault(msrest.serialization.Model):
"""NoModelAsStringNoRequiredOneValueDefault.

Variables are only populated by the server, and will be ignored when sending a request.

:ivar parameter: Default value: "value1".
:vartype parameter: str
:param parameter: The only acceptable values to pass in are None and "value1". The default
value is "value1".
:type parameter: str
"""

_validation = {
"parameter": {"constant": True},
}

_attribute_map = {
"parameter": {"key": "parameter", "type": "str"},
}

parameter = "value1"

def __init__(self, **kwargs):
super(NoModelAsStringNoRequiredOneValueDefault, self).__init__(**kwargs)
self.parameter = kwargs.get("parameter", "value1")


class NoModelAsStringNoRequiredOneValueNoDefault(msrest.serialization.Model):
"""NoModelAsStringNoRequiredOneValueNoDefault.

Variables are only populated by the server, and will be ignored when sending a request.

:ivar parameter: Default value: "value1".
:vartype parameter: str
:param parameter: The only acceptable values to pass in are None and "value1". The default
value is None.
:type parameter: str
"""

_validation = {
"parameter": {"constant": True},
}

_attribute_map = {
"parameter": {"key": "parameter", "type": "str"},
}

parameter = "value1"

def __init__(self, **kwargs):
super(NoModelAsStringNoRequiredOneValueNoDefault, self).__init__(**kwargs)
self.parameter = kwargs.get("parameter", None)


class NoModelAsStringNoRequiredTwoValueDefault(msrest.serialization.Model):
Expand Down Expand Up @@ -246,7 +234,7 @@ class NoModelAsStringRequiredOneValueDefault(msrest.serialization.Model):

All required parameters must be populated in order to send to Azure.

:ivar parameter: Required. Default value: "value1".
:ivar parameter: Has constant value: "value1".
:vartype parameter: str
"""

Expand All @@ -271,7 +259,7 @@ class NoModelAsStringRequiredOneValueNoDefault(msrest.serialization.Model):

All required parameters must be populated in order to send to Azure.

:ivar parameter: Required. Default value: "value1".
:ivar parameter: Has constant value: "value1".
:vartype parameter: str
"""

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,47 +176,35 @@ def __init__(self, *, parameter: Union[str, "ModelAsStringRequiredTwoValueNoDefa
class NoModelAsStringNoRequiredOneValueDefault(msrest.serialization.Model):
"""NoModelAsStringNoRequiredOneValueDefault.

Variables are only populated by the server, and will be ignored when sending a request.

:ivar parameter: Default value: "value1".
:vartype parameter: str
:param parameter: The only acceptable values to pass in are None and "value1". The default
value is "value1".
:type parameter: str
"""

_validation = {
"parameter": {"constant": True},
}

_attribute_map = {
"parameter": {"key": "parameter", "type": "str"},
}

parameter = "value1"

def __init__(self, **kwargs):
def __init__(self, *, parameter: Optional[str] = "value1", **kwargs):
super(NoModelAsStringNoRequiredOneValueDefault, self).__init__(**kwargs)
self.parameter = parameter


class NoModelAsStringNoRequiredOneValueNoDefault(msrest.serialization.Model):
"""NoModelAsStringNoRequiredOneValueNoDefault.

Variables are only populated by the server, and will be ignored when sending a request.

:ivar parameter: Default value: "value1".
:vartype parameter: str
:param parameter: The only acceptable values to pass in are None and "value1". The default
value is None.
:type parameter: str
"""

_validation = {
"parameter": {"constant": True},
}

_attribute_map = {
"parameter": {"key": "parameter", "type": "str"},
}

parameter = "value1"

def __init__(self, **kwargs):
def __init__(self, *, parameter: Optional[str] = None, **kwargs):
super(NoModelAsStringNoRequiredOneValueNoDefault, self).__init__(**kwargs)
self.parameter = parameter


class NoModelAsStringNoRequiredTwoValueDefault(msrest.serialization.Model):
Expand Down Expand Up @@ -262,7 +250,7 @@ class NoModelAsStringRequiredOneValueDefault(msrest.serialization.Model):

All required parameters must be populated in order to send to Azure.

:ivar parameter: Required. Default value: "value1".
:ivar parameter: Has constant value: "value1".
:vartype parameter: str
"""

Expand All @@ -287,7 +275,7 @@ class NoModelAsStringRequiredOneValueNoDefault(msrest.serialization.Model):

All required parameters must be populated in order to send to Azure.

:ivar parameter: Required. Default value: "value1".
:ivar parameter: Has constant value: "value1".
:vartype parameter: str
"""

Expand Down
Loading