Skip to content

Commit

Permalink
if constant is optional, don't force to value (#952)
Browse files Browse the repository at this point in the history
  • Loading branch information
iscai-msft authored Jun 16, 2021
1 parent 6b12df5 commit fb8ad8b
Show file tree
Hide file tree
Showing 21 changed files with 127 additions and 116 deletions.
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:
# 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

0 comments on commit fb8ad8b

Please sign in to comment.