Skip to content

Commit

Permalink
fix content type issue in multiapi and clean up code (#1232)
Browse files Browse the repository at this point in the history
  • Loading branch information
iscai-msft authored Apr 18, 2022
1 parent 7a82803 commit ae129f5
Show file tree
Hide file tree
Showing 52 changed files with 560 additions and 382 deletions.
3 changes: 2 additions & 1 deletion ChangeLog.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Change Log

### 2022-xx-xx - 5.16.0
### 2022-04-15 - 5.16.0

| Library | Min Version |
| ----------------------------------------------------------------------- | ----------- |
Expand All @@ -17,6 +17,7 @@
**Bug Fixes**

- Drop package dependency on "@azure-tools/extension", switch to "@autorest/system-requirements" #1229
- Fix `content_type` generation in multiapi SDKs with multiple content types for bodies #1232

### 2022-04-07 - 5.15.0

Expand Down
8 changes: 7 additions & 1 deletion autorest/codegen/models/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,12 @@
from .imports import FileImport, ImportType, TypingSection
from .lro_operation import LROOperation
from .paging_operation import PagingOperation
from .parameter import Parameter, ParameterStyle, ParameterLocation
from .parameter import (
Parameter,
ParameterStyle,
ParameterLocation,
ParameterMethodLocation,
)
from .operation import Operation
from .property import Property
from .operation_group import OperationGroup
Expand Down Expand Up @@ -69,6 +74,7 @@
"ParameterStyle",
"IOSchema",
"GlobalParameterList",
"ParameterMethodLocation",
]


Expand Down
9 changes: 7 additions & 2 deletions autorest/codegen/models/operation.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,12 @@
from .base_builder import BaseBuilder, create_parameters
from .imports import FileImport, ImportType, TypingSection
from .schema_response import SchemaResponse
from .parameter import Parameter, get_parameter, ParameterLocation
from .parameter import (
Parameter,
ParameterMethodLocation,
get_parameter,
ParameterLocation,
)
from .parameter_list import ParameterList
from .base_schema import BaseSchema
from .object_schema import ObjectSchema
Expand Down Expand Up @@ -434,7 +439,7 @@ def from_yaml(
if len(parameter_list.content_types) > 1:
for p in parameter_list.parameters:
if p.rest_api_name == "Content-Type":
p.is_keyword_only = True
p.method_location = ParameterMethodLocation.KEYWORD_ONLY
request_builder = code_model.lookup_request_builder(id(yaml_data))

return cls(
Expand Down
104 changes: 62 additions & 42 deletions autorest/codegen/models/parameter.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,13 @@
_HIDDEN_KWARGS = ["content_type"]


class ParameterMethodLocation(str, Enum):
POSITIONAL = "positional"
KEYWORD_ONLY = "keyword_only"
KWARG = "kwarg"
HIDDEN_KWARG = "hidden_kwarg"


class ParameterLocation(Enum):
Path = "path"
Body = "body"
Expand Down Expand Up @@ -84,7 +91,6 @@ def __init__(
grouped_by: Optional["Parameter"] = None,
original_parameter: Optional["Parameter"] = None,
client_default_value: Optional[Any] = None,
keyword_only: Optional[bool] = None,
content_types: Optional[List[str]] = None,
) -> None:
super().__init__(yaml_data, code_model)
Expand All @@ -108,7 +114,6 @@ def __init__(
self.has_multiple_content_types: bool = False
self.multiple_content_types_type_annot: Optional[str] = None
self.multiple_content_types_docstring_type: Optional[str] = None
self._keyword_only = keyword_only
self.is_multipart = (
yaml_data.get("language", {}).get("python", {}).get("multipart", False)
)
Expand All @@ -118,10 +123,8 @@ def __init__(
self.content_types = content_types or []
self.body_kwargs: List[Parameter] = []
self.is_body_kwarg = False
self.is_kwarg = self.rest_api_name == "Content-Type" or (
self.constant and self.inputtable_by_user
)
self.need_import = True
self._method_location: Optional[ParameterMethodLocation] = None

def __hash__(self) -> int:
return hash(self.serialized_name)
Expand Down Expand Up @@ -302,11 +305,29 @@ def _default_value(self) -> Tuple[Optional[Any], str, str]:

@property
def description_keyword(self) -> str:
return "keyword" if self.is_kwarg or self.is_keyword_only else "param"
return (
"keyword"
if self.method_location
in (
ParameterMethodLocation.KWARG,
ParameterMethodLocation.HIDDEN_KWARG,
ParameterMethodLocation.KEYWORD_ONLY,
)
else "param"
)

@property
def docstring_type_keyword(self) -> str:
return "paramtype" if self.is_kwarg or self.is_keyword_only else "type"
return (
"paramtype"
if self.method_location
in (
ParameterMethodLocation.KWARG,
ParameterMethodLocation.HIDDEN_KWARG,
ParameterMethodLocation.KEYWORD_ONLY,
)
else "type"
)

@property
def default_value(self) -> Optional[Any]:
Expand Down Expand Up @@ -357,24 +378,6 @@ def full_serialized_name(self) -> str:
origin_name = f"self._config.{self.serialized_name}"
return origin_name

@property
def is_keyword_only(self) -> bool:
# this means in async mode, I am documented like def hello(positional_1, *, me!)
return self._keyword_only or False

@is_keyword_only.setter
def is_keyword_only(self, val: bool) -> None:
self._keyword_only = val
self.is_kwarg = False

@property
def is_hidden(self) -> bool:
return (
self.serialized_name in _HIDDEN_KWARGS
and self.is_kwarg
or (self.yaml_data["implementation"] == "Client" and self.constant)
)

@property
def is_content_type(self) -> bool:
return (
Expand All @@ -383,8 +386,20 @@ def is_content_type(self) -> bool:
)

@property
def is_positional(self) -> bool:
return self.in_method_signature and not (self.is_keyword_only or self.is_kwarg)
def method_location(self) -> ParameterMethodLocation:
if self._method_location:
return self._method_location
if self.serialized_name in _HIDDEN_KWARGS or (
self._implementation == "Client" and self.constant
):
return ParameterMethodLocation.HIDDEN_KWARG
if self.constant and self.inputtable_by_user:
return ParameterMethodLocation.KWARG
return ParameterMethodLocation.POSITIONAL

@method_location.setter
def method_location(self, val: ParameterMethodLocation) -> None:
self._method_location = val

@classmethod
def from_yaml(
Expand Down Expand Up @@ -453,21 +468,26 @@ def imports(self) -> FileImport:

class ParameterOnlyPathAndBodyPositional(Parameter):
@property
def is_keyword_only(self) -> bool:
if self._keyword_only is not None:
return self._keyword_only
return self.in_method_signature and not (
self.is_hidden
or self.location == ParameterLocation.Path
or self.location == ParameterLocation.Uri
or self.location == ParameterLocation.Body
or self.is_kwarg
)

@is_keyword_only.setter
def is_keyword_only(self, val: bool) -> None:
self._keyword_only = val
self.is_kwarg = False
def method_location(self) -> ParameterMethodLocation:
super_method_location = super().method_location
if super_method_location in (
ParameterMethodLocation.KWARG,
ParameterMethodLocation.HIDDEN_KWARG,
):
return super_method_location
if self._method_location:
return self._method_location
if self.location not in (
ParameterLocation.Path,
ParameterLocation.Uri,
ParameterLocation.Body,
):
return ParameterMethodLocation.KEYWORD_ONLY
return super_method_location

@method_location.setter
def method_location(self, val: ParameterMethodLocation) -> None:
self._method_location = val


def get_parameter(code_model):
Expand Down
101 changes: 76 additions & 25 deletions autorest/codegen/models/parameter_list.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import logging
from typing import cast, List, Callable, Optional, TypeVar, Dict, TYPE_CHECKING

from .parameter import Parameter, ParameterLocation
from .parameter import Parameter, ParameterLocation, ParameterMethodLocation
from .base_schema import BaseSchema
from .primitive_schemas import StringSchema
from .utils import JSON_REGEXP
Expand Down Expand Up @@ -199,14 +199,22 @@ def _filter_out_multiple_content_type(self, kwarg_params):
if len(content_type_params) > 1:
# we don't want multiple content type params in the method, just one
# we'll pick the one with the default content type
kwarg_params = [
k
for k in kwarg_params
if not (
k.rest_api_name == "Content-Type"
and k.default_value_declaration != f'"{self.default_content_type}"'
)
]
seen_content_type = False
new_kwarg_params = []
for k in kwarg_params:
if k.rest_api_name == "Content-Type":
if (
not seen_content_type
and k.default_value_declaration
== f'"{self.default_content_type}"'
):
new_kwarg_params.append(k)
seen_content_type = True
else:
continue
else:
new_kwarg_params.append(k)
kwarg_params = new_kwarg_params
return kwarg_params

@property
Expand All @@ -215,13 +223,27 @@ def method(self) -> List[Parameter]:
# Client level should not be on Method, etc.
parameters_of_this_implementation = self.get_from_predicate(
lambda parameter: parameter.implementation == self.implementation
and parameter.in_method_signature
)
positional = [p for p in parameters_of_this_implementation if p.is_positional]
positional = [
p
for p in parameters_of_this_implementation
if p.method_location == ParameterMethodLocation.POSITIONAL
]
keyword_only = self._filter_out_multiple_content_type(
[p for p in parameters_of_this_implementation if p.is_keyword_only]
[
p
for p in parameters_of_this_implementation
if p.method_location == ParameterMethodLocation.KEYWORD_ONLY
]
)
kwargs = self._filter_out_multiple_content_type(
[p for p in parameters_of_this_implementation if p.is_kwarg]
[
p
for p in parameters_of_this_implementation
if p.method_location
in (ParameterMethodLocation.KWARG, ParameterMethodLocation.HIDDEN_KWARG)
]
)

def _sort(params):
Expand Down Expand Up @@ -258,28 +280,41 @@ def method_signature_kwargs(is_python3_file: bool) -> List[str]:

@property
def positional(self) -> List[Parameter]:
return [p for p in self.method if p.is_positional]
return [
p
for p in self.method
if p.method_location == ParameterMethodLocation.POSITIONAL
]

@property
def keyword_only(self) -> List[Parameter]:
return [p for p in self.method if p.is_keyword_only]
return [
p
for p in self.method
if p.method_location == ParameterMethodLocation.KEYWORD_ONLY
]

@property
def kwargs(self) -> List[Parameter]:
return [p for p in self.method if p.is_kwarg]
return [
p
for p in self.method
if p.method_location
in (ParameterMethodLocation.KWARG, ParameterMethodLocation.HIDDEN_KWARG)
]

def kwargs_to_pop(self, is_python3_file: bool) -> List[Parameter]:
kwargs_to_pop = self.kwargs
if not is_python3_file:
kwargs_to_pop += self.keyword_only
return kwargs_to_pop

@property
def call(self) -> List[str]:
def call(self, is_python3_file: bool) -> List[str]:
retval = [p.serialized_name for p in self.positional]
retval.extend(
[f"{p.serialized_name}={p.serialized_name}" for p in self.keyword_only]
)
if is_python3_file:
retval.extend(
[f"{p.serialized_name}={p.serialized_name}" for p in self.keyword_only]
)
retval.append("**kwargs")
return retval

Expand All @@ -299,12 +334,25 @@ def implementation(self) -> str:
def method(self) -> List[Parameter]:
"""The list of parameter used in method signature."""
# Client level should not be on Method, etc.
positional = [p for p in self.parameters if p.is_positional]
positional = [
p
for p in self.parameters
if p.method_location == ParameterMethodLocation.POSITIONAL
]
keyword_only = self._filter_out_multiple_content_type(
[p for p in self.parameters if p.is_keyword_only]
[
p
for p in self.parameters
if p.method_location == ParameterMethodLocation.KEYWORD_ONLY
]
)
kwargs = self._filter_out_multiple_content_type(
[p for p in self.parameters if p.is_kwarg]
[
p
for p in self.parameters
if p.method_location
in (ParameterMethodLocation.KWARG, ParameterMethodLocation.HIDDEN_KWARG)
]
)

def _sort(params):
Expand Down Expand Up @@ -347,9 +395,12 @@ def add_host(self, host_value: Optional[str]) -> None:
skip_url_encoding=False,
constraints=[],
client_default_value=host_value,
keyword_only=self.code_model.options["version_tolerant"]
or self.code_model.options["low_level_client"],
)
if (
self.code_model.options["version_tolerant"]
or self.code_model.options["low_level_client"]
):
host_param.method_location = ParameterMethodLocation.KEYWORD_ONLY
self.parameters.append(host_param)

def add_credential_global_parameter(self) -> None:
Expand Down
Loading

0 comments on commit ae129f5

Please sign in to comment.