From 16dd81784373e4e4c7ba08b6329cfd396b96d366 Mon Sep 17 00:00:00 2001 From: iscai-msft Date: Mon, 28 Feb 2022 13:35:37 -0500 Subject: [PATCH 1/7] don't reformat query params for dpg next link calls --- autorest/codegen/models/__init__.py | 3 ++- autorest/codegen/serializers/builder_serializer.py | 13 +++++++++++++ .../aio/operations/_operations.py | 4 +--- .../operations/_operations.py | 4 +--- .../aio/operations/_operations.py | 4 +--- .../pagingversiontolerant/operations/_operations.py | 4 +--- .../aio/operations/_operations.py | 2 -- .../operations/_operations.py | 2 -- 8 files changed, 19 insertions(+), 17 deletions(-) diff --git a/autorest/codegen/models/__init__.py b/autorest/codegen/models/__init__.py index b1b7a409606..b2d96fa5c02 100644 --- a/autorest/codegen/models/__init__.py +++ b/autorest/codegen/models/__init__.py @@ -17,7 +17,7 @@ from .imports import FileImport, ImportType, TypingSection from .lro_operation import LROOperation from .paging_operation import PagingOperation -from .parameter import Parameter, ParameterStyle +from .parameter import Parameter, ParameterStyle, ParameterLocation from .operation import Operation from .property import Property from .operation_group import OperationGroup @@ -50,6 +50,7 @@ "PagingOperation", "Parameter", "ParameterList", + "ParameterLocation", "OperationGroup", "Property", "RequestBuilder", diff --git a/autorest/codegen/serializers/builder_serializer.py b/autorest/codegen/serializers/builder_serializer.py index db0d7c445ad..4f8bcf2dd34 100644 --- a/autorest/codegen/serializers/builder_serializer.py +++ b/autorest/codegen/serializers/builder_serializer.py @@ -27,6 +27,7 @@ SchemaResponse, IOSchema, ParameterStyle, + ParameterLocation ) from . import utils @@ -798,6 +799,7 @@ def _call_request_builder_helper( builder, request_builder: RequestBuilder, template_url: Optional[str] = None, + is_next_request: bool = False, ) -> List[str]: retval = [] if len(builder.body_kwargs_to_pass_to_request_builder) > 1: @@ -837,6 +839,15 @@ def _call_request_builder_helper( parameter.serialized_name not in builder.body_kwargs_to_pass_to_request_builder ): continue + if ( + is_next_request and + not bool(builder.next_request_builder) and + self.code_model.options["version_tolerant"] and + parameter.location == ParameterLocation.Query + ): + # for version tolerant, we don't want to reformat query parameters if + # there is just one defined paging operation in the swagger + continue high_level_name = cast(RequestBuilderParameter, parameter).name_in_high_level_operation retval.append(f" {parameter.serialized_name}={high_level_name},") if not self.code_model.options["version_tolerant"]: @@ -1083,11 +1094,13 @@ def call_next_link_request_builder(self, builder) -> List[str]: else: request_builder = builder.request_builder template_url = "next_link" + request_builder = builder.next_request_builder or builder.request_builder return self._call_request_builder_helper( builder, request_builder, template_url=template_url, + is_next_request=True ) def _prepare_request_callback(self, builder) -> List[str]: diff --git a/test/azure/version-tolerant/Expected/AcceptanceTests/CustomPollerPagerVersionTolerant/custompollerpagerversiontolerant/aio/operations/_operations.py b/test/azure/version-tolerant/Expected/AcceptanceTests/CustomPollerPagerVersionTolerant/custompollerpagerversiontolerant/aio/operations/_operations.py index 37449a9cd81..bf928a57ab8 100644 --- a/test/azure/version-tolerant/Expected/AcceptanceTests/CustomPollerPagerVersionTolerant/custompollerpagerversiontolerant/aio/operations/_operations.py +++ b/test/azure/version-tolerant/Expected/AcceptanceTests/CustomPollerPagerVersionTolerant/custompollerpagerversiontolerant/aio/operations/_operations.py @@ -538,9 +538,7 @@ def prepare_request(next_link=None): else: - request = build_paging_duplicate_params_request( - filter=filter, - ) + request = build_paging_duplicate_params_request() request.url = self._client.format_url(next_link) request.method = "GET" return request diff --git a/test/azure/version-tolerant/Expected/AcceptanceTests/CustomPollerPagerVersionTolerant/custompollerpagerversiontolerant/operations/_operations.py b/test/azure/version-tolerant/Expected/AcceptanceTests/CustomPollerPagerVersionTolerant/custompollerpagerversiontolerant/operations/_operations.py index 1cd8d52d9e0..215f3422282 100644 --- a/test/azure/version-tolerant/Expected/AcceptanceTests/CustomPollerPagerVersionTolerant/custompollerpagerversiontolerant/operations/_operations.py +++ b/test/azure/version-tolerant/Expected/AcceptanceTests/CustomPollerPagerVersionTolerant/custompollerpagerversiontolerant/operations/_operations.py @@ -892,9 +892,7 @@ def prepare_request(next_link=None): else: - request = build_paging_duplicate_params_request( - filter=filter, - ) + request = build_paging_duplicate_params_request() request.url = self._client.format_url(next_link) request.method = "GET" return request diff --git a/test/azure/version-tolerant/Expected/AcceptanceTests/PagingVersionTolerant/pagingversiontolerant/aio/operations/_operations.py b/test/azure/version-tolerant/Expected/AcceptanceTests/PagingVersionTolerant/pagingversiontolerant/aio/operations/_operations.py index 59081d1fea3..1a2a30178c9 100644 --- a/test/azure/version-tolerant/Expected/AcceptanceTests/PagingVersionTolerant/pagingversiontolerant/aio/operations/_operations.py +++ b/test/azure/version-tolerant/Expected/AcceptanceTests/PagingVersionTolerant/pagingversiontolerant/aio/operations/_operations.py @@ -536,9 +536,7 @@ def prepare_request(next_link=None): else: - request = build_paging_duplicate_params_request( - filter=filter, - ) + request = build_paging_duplicate_params_request() request.url = self._client.format_url(next_link) request.method = "GET" return request diff --git a/test/azure/version-tolerant/Expected/AcceptanceTests/PagingVersionTolerant/pagingversiontolerant/operations/_operations.py b/test/azure/version-tolerant/Expected/AcceptanceTests/PagingVersionTolerant/pagingversiontolerant/operations/_operations.py index 6073dc1e044..205a52f9081 100644 --- a/test/azure/version-tolerant/Expected/AcceptanceTests/PagingVersionTolerant/pagingversiontolerant/operations/_operations.py +++ b/test/azure/version-tolerant/Expected/AcceptanceTests/PagingVersionTolerant/pagingversiontolerant/operations/_operations.py @@ -890,9 +890,7 @@ def prepare_request(next_link=None): else: - request = build_paging_duplicate_params_request( - filter=filter, - ) + request = build_paging_duplicate_params_request() request.url = self._client.format_url(next_link) request.method = "GET" return request diff --git a/test/azure/version-tolerant/Expected/AcceptanceTests/StorageManagementClientVersionTolerant/storageversiontolerant/aio/operations/_operations.py b/test/azure/version-tolerant/Expected/AcceptanceTests/StorageManagementClientVersionTolerant/storageversiontolerant/aio/operations/_operations.py index bfbd57166d7..07304d9e4fc 100644 --- a/test/azure/version-tolerant/Expected/AcceptanceTests/StorageManagementClientVersionTolerant/storageversiontolerant/aio/operations/_operations.py +++ b/test/azure/version-tolerant/Expected/AcceptanceTests/StorageManagementClientVersionTolerant/storageversiontolerant/aio/operations/_operations.py @@ -843,7 +843,6 @@ def prepare_request(next_link=None): request = build_storage_accounts_list_request( subscription_id=self._config.subscription_id, - api_version=api_version, ) request.url = self._client.format_url(next_link) request.method = "GET" @@ -990,7 +989,6 @@ def prepare_request(next_link=None): request = build_storage_accounts_list_by_resource_group_request( resource_group_name=resource_group_name, subscription_id=self._config.subscription_id, - api_version=api_version, ) request.url = self._client.format_url(next_link) request.method = "GET" diff --git a/test/azure/version-tolerant/Expected/AcceptanceTests/StorageManagementClientVersionTolerant/storageversiontolerant/operations/_operations.py b/test/azure/version-tolerant/Expected/AcceptanceTests/StorageManagementClientVersionTolerant/storageversiontolerant/operations/_operations.py index ce22aa4923f..d43ed7ecfd7 100644 --- a/test/azure/version-tolerant/Expected/AcceptanceTests/StorageManagementClientVersionTolerant/storageversiontolerant/operations/_operations.py +++ b/test/azure/version-tolerant/Expected/AcceptanceTests/StorageManagementClientVersionTolerant/storageversiontolerant/operations/_operations.py @@ -1154,7 +1154,6 @@ def prepare_request(next_link=None): request = build_storage_accounts_list_request( subscription_id=self._config.subscription_id, - api_version=api_version, ) request.url = self._client.format_url(next_link) request.method = "GET" @@ -1301,7 +1300,6 @@ def prepare_request(next_link=None): request = build_storage_accounts_list_by_resource_group_request( resource_group_name=resource_group_name, subscription_id=self._config.subscription_id, - api_version=api_version, ) request.url = self._client.format_url(next_link) request.method = "GET" From 5d4583bda12676b3a638be6b80b9830d87349e2a Mon Sep 17 00:00:00 2001 From: iscai-msft Date: Mon, 28 Feb 2022 15:01:50 -0500 Subject: [PATCH 2/7] add acceptance tests --- .../AcceptanceTests/asynctests/test_paging.py | 7 +++++++ test/azure/version-tolerant/AcceptanceTests/test_paging.py | 6 ++++++ test/azure/version-tolerant/AcceptanceTests/test_zzz.py | 2 -- 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/test/azure/version-tolerant/AcceptanceTests/asynctests/test_paging.py b/test/azure/version-tolerant/AcceptanceTests/asynctests/test_paging.py index 91aad9b6684..f461d4e552e 100644 --- a/test/azure/version-tolerant/AcceptanceTests/asynctests/test_paging.py +++ b/test/azure/version-tolerant/AcceptanceTests/asynctests/test_paging.py @@ -228,3 +228,10 @@ async def test_item_name_with_xms_client_name(self, client): async for item in pages: items.append(item) assert len(items) == 1 + + @pytest.mark.asyncio + async def test_duplicate_params(self, client): + pages = [p async for p in client.paging.duplicate_params(filter="foo")] + assert len(pages) == 1 + assert pages[0]["properties"]["id"] == 1 + assert pages[0]["properties"]["name"] == "Product" diff --git a/test/azure/version-tolerant/AcceptanceTests/test_paging.py b/test/azure/version-tolerant/AcceptanceTests/test_paging.py index 407d8918fb0..5ffac65c6c0 100644 --- a/test/azure/version-tolerant/AcceptanceTests/test_paging.py +++ b/test/azure/version-tolerant/AcceptanceTests/test_paging.py @@ -164,3 +164,9 @@ def test_initial_response_no_items(client): pages = client.paging.first_response_empty() items = [i for i in pages] assert len(items) == 1 + +def test_duplicate_params(client): + pages = list(client.paging.duplicate_params(filter="foo")) + assert len(pages) == 1 + assert pages[0]["properties"]["id"] == 1 + assert pages[0]["properties"]["name"] == "Product" diff --git a/test/azure/version-tolerant/AcceptanceTests/test_zzz.py b/test/azure/version-tolerant/AcceptanceTests/test_zzz.py index 1075dbafde8..f50668f381c 100644 --- a/test/azure/version-tolerant/AcceptanceTests/test_zzz.py +++ b/test/azure/version-tolerant/AcceptanceTests/test_zzz.py @@ -37,8 +37,6 @@ def test_ensure_coverage(self): # Add tests that wont be supported due to the nature of Python here not_supported = { - "LROPatchInlineCompleteIgnoreHeaders": 1, - "PagingDuplicateParameters": 1 # skipping for now, going to do another PR soon changing paging behavior for version tolerant } # Please add missing features or failing tests here From 925e0025e65747a648c101196da59654590f8c88 Mon Sep 17 00:00:00 2001 From: iscai-msft Date: Mon, 28 Feb 2022 15:04:06 -0500 Subject: [PATCH 3/7] undo test class --- .../AcceptanceTests/asynctests/test_paging.py | 363 +++++++++--------- 1 file changed, 181 insertions(+), 182 deletions(-) diff --git a/test/azure/version-tolerant/AcceptanceTests/asynctests/test_paging.py b/test/azure/version-tolerant/AcceptanceTests/asynctests/test_paging.py index f461d4e552e..d7574fffc63 100644 --- a/test/azure/version-tolerant/AcceptanceTests/asynctests/test_paging.py +++ b/test/azure/version-tolerant/AcceptanceTests/asynctests/test_paging.py @@ -53,185 +53,184 @@ async def custom_url_client(): async with AutoRestParameterizedHostTestPagingClient(host="host:3000") as client: await yield_(client) -class TestPaging(object): - @pytest.mark.asyncio - async def test_get_no_item_name_pages(self, client): - pages = client.paging.get_no_item_name_pages() - items = [] - async for item in pages: - items.append(item) - assert len(items) == 1 - assert items[0]["properties"]["id"] == 1 - assert items[0]["properties"]["name"] == "Product" - - @pytest.mark.asyncio - async def test_get_null_next_link_name_pages(self, client): - pages = client.paging.get_null_next_link_name_pages() - items = [] - async for item in pages: - items.append(item) - assert len(items) == 1 - assert items[0]["properties"]["id"] == 1 - assert items[0]["properties"]["name"] == "Product" - - @pytest.mark.asyncio - async def test_get_single_pages_with_cb(self, client): - def cb(list_of_obj): - for obj in list_of_obj: - obj["marked"] = True - return list_of_obj - async for obj in client.paging.get_single_pages(cls=cb): - assert obj["marked"] - - @pytest.mark.asyncio - async def test_get_single_pages(self, client): - pages = client.paging.get_single_pages() - items = [] - async for item in pages: - items.append(item) - assert len(items) == 1 - assert items[0]["properties"]["id"] == 1 - assert items[0]["properties"]["name"] == "Product" - - @pytest.mark.asyncio - async def test_get_multiple_pages(self, client): - pages = client.paging.get_multiple_pages() - items = [] - async for item in pages: - items.append(item) - assert len(items) == 10 - - @pytest.mark.asyncio - async def test_query_params(self, client): - pages = client.paging.get_with_query_params(required_query_parameter='100') - items = [] - async for item in pages: - items.append(item) - assert len(items) == 2 - - @pytest.mark.asyncio - async def test_get_odata_multiple_pages(self, client): - pages = client.paging.get_odata_multiple_pages() - items = [] - async for item in pages: - items.append(item) - assert len(items) == 10 - - @pytest.mark.asyncio - async def test_get_multiple_pages_retry_first(self, client): - pages = client.paging.get_multiple_pages_retry_first() - items = [] - async for item in pages: - items.append(item) - assert len(items) == 10 - - @pytest.mark.asyncio - async def test_get_multiple_pages_retry_second(self, client): - pages = client.paging.get_multiple_pages_retry_second() - items = [] - async for item in pages: - items.append(item) - assert len(items) == 10 - - @pytest.mark.asyncio - async def test_get_multiple_pages_with_offset(self, client): - pages = client.paging.get_multiple_pages_with_offset(offset=100) - items = [] - async for item in pages: - items.append(item) - assert len(items) == 10 - assert items[-1]["properties"]["id"] == 110 - - @pytest.mark.asyncio - async def test_get_single_pages_failure(self, client): - pages = client.paging.get_single_pages_failure() - with pytest.raises(HttpResponseError): - async for i in pages: - ... - - @pytest.mark.asyncio - async def test_get_multiple_pages_failure(self, client): - pages = client.paging.get_multiple_pages_failure() - with pytest.raises(HttpResponseError): - async for i in pages: - ... - - @pytest.mark.asyncio - async def test_get_multiple_pages_failure_uri(self, client): - pages = client.paging.get_multiple_pages_failure_uri() - with pytest.raises(HttpResponseError): - async for i in pages: - ... - - @pytest.mark.asyncio - async def test_paging_fragment_path(self, client): - - pages = client.paging.get_multiple_pages_fragment_next_link(api_version="1.6", tenant="test_user") - items = [] - async for item in pages: - items.append(item) - assert len(items) == 10 - - with pytest.raises(AttributeError): - # Be sure this method is not generated (Transform work) - await client.paging.get_multiple_pages_fragment_next_link_next() # pylint: disable=E1101 - - @pytest.mark.asyncio - async def test_custom_url_get_pages_partial_url(self, custom_url_client): - pages = custom_url_client.paging.get_pages_partial_url("local") - items = [] - async for item in pages: - items.append(item) - - assert len(items) == 2 - assert items[0]["properties"]["id"] == 1 - assert items[1]["properties"]["id"] == 2 - - @pytest.mark.asyncio - async def test_custom_url_get_pages_partial_url_operation(self, custom_url_client): - pages = custom_url_client.paging.get_pages_partial_url_operation("local") - items = [] - async for item in pages: - items.append(item) - - assert len(items) == 2 - assert items[0]["properties"]["id"] == 1 - assert items[1]["properties"]["id"] == 2 - - @pytest.mark.asyncio - async def test_get_multiple_pages_lro(self, client): - """LRO + Paging at the same time. - """ - from azure.mgmt.core.polling.async_arm_polling import AsyncARMPolling - poller = await client.paging.begin_get_multiple_pages_lro(polling=AsyncARMPolling(timeout=0)) - pager = await poller.result() - items = [] - async for item in pager: - items.append(item) - - assert len(items) == 10 - assert items[0]["properties"]["id"] == 1 - assert items[1]["properties"]["id"] == 2 - - @pytest.mark.asyncio - async def test_initial_response_no_items(self, client): - pages = client.paging.first_response_empty() - items = [] - async for item in pages: - items.append(item) - assert len(items) == 1 - - @pytest.mark.asyncio - async def test_item_name_with_xms_client_name(self, client): - pages = client.paging.get_paging_model_with_item_name_with_xms_client_name() - items = [] - async for item in pages: - items.append(item) - assert len(items) == 1 - - @pytest.mark.asyncio - async def test_duplicate_params(self, client): - pages = [p async for p in client.paging.duplicate_params(filter="foo")] - assert len(pages) == 1 - assert pages[0]["properties"]["id"] == 1 - assert pages[0]["properties"]["name"] == "Product" +@pytest.mark.asyncio +async def test_get_no_item_name_pages(client): + pages = client.paging.get_no_item_name_pages() + items = [] + async for item in pages: + items.append(item) + assert len(items) == 1 + assert items[0]["properties"]["id"] == 1 + assert items[0]["properties"]["name"] == "Product" + +@pytest.mark.asyncio +async def test_get_null_next_link_name_pages(client): + pages = client.paging.get_null_next_link_name_pages() + items = [] + async for item in pages: + items.append(item) + assert len(items) == 1 + assert items[0]["properties"]["id"] == 1 + assert items[0]["properties"]["name"] == "Product" + +@pytest.mark.asyncio +async def test_get_single_pages_with_cb(client): + def cb(list_of_obj): + for obj in list_of_obj: + obj["marked"] = True + return list_of_obj + async for obj in client.paging.get_single_pages(cls=cb): + assert obj["marked"] + +@pytest.mark.asyncio +async def test_get_single_pages(client): + pages = client.paging.get_single_pages() + items = [] + async for item in pages: + items.append(item) + assert len(items) == 1 + assert items[0]["properties"]["id"] == 1 + assert items[0]["properties"]["name"] == "Product" + +@pytest.mark.asyncio +async def test_get_multiple_pages(client): + pages = client.paging.get_multiple_pages() + items = [] + async for item in pages: + items.append(item) + assert len(items) == 10 + +@pytest.mark.asyncio +async def test_query_params(client): + pages = client.paging.get_with_query_params(required_query_parameter='100') + items = [] + async for item in pages: + items.append(item) + assert len(items) == 2 + +@pytest.mark.asyncio +async def test_get_odata_multiple_pages(client): + pages = client.paging.get_odata_multiple_pages() + items = [] + async for item in pages: + items.append(item) + assert len(items) == 10 + +@pytest.mark.asyncio +async def test_get_multiple_pages_retry_first(client): + pages = client.paging.get_multiple_pages_retry_first() + items = [] + async for item in pages: + items.append(item) + assert len(items) == 10 + +@pytest.mark.asyncio +async def test_get_multiple_pages_retry_second(client): + pages = client.paging.get_multiple_pages_retry_second() + items = [] + async for item in pages: + items.append(item) + assert len(items) == 10 + +@pytest.mark.asyncio +async def test_get_multiple_pages_with_offset(client): + pages = client.paging.get_multiple_pages_with_offset(offset=100) + items = [] + async for item in pages: + items.append(item) + assert len(items) == 10 + assert items[-1]["properties"]["id"] == 110 + +@pytest.mark.asyncio +async def test_get_single_pages_failure(client): + pages = client.paging.get_single_pages_failure() + with pytest.raises(HttpResponseError): + async for i in pages: + ... + +@pytest.mark.asyncio +async def test_get_multiple_pages_failure(client): + pages = client.paging.get_multiple_pages_failure() + with pytest.raises(HttpResponseError): + async for i in pages: + ... + +@pytest.mark.asyncio +async def test_get_multiple_pages_failure_uri(client): + pages = client.paging.get_multiple_pages_failure_uri() + with pytest.raises(HttpResponseError): + async for i in pages: + ... + +@pytest.mark.asyncio +async def test_paging_fragment_path(client): + + pages = client.paging.get_multiple_pages_fragment_next_link(api_version="1.6", tenant="test_user") + items = [] + async for item in pages: + items.append(item) + assert len(items) == 10 + + with pytest.raises(AttributeError): + # Be sure this method is not generated (Transform work) + await client.paging.get_multiple_pages_fragment_next_link_next() # pylint: disable=E1101 + +@pytest.mark.asyncio +async def test_custom_url_get_pages_partial_url(custom_url_client): + pages = custom_url_client.paging.get_pages_partial_url("local") + items = [] + async for item in pages: + items.append(item) + + assert len(items) == 2 + assert items[0]["properties"]["id"] == 1 + assert items[1]["properties"]["id"] == 2 + +@pytest.mark.asyncio +async def test_custom_url_get_pages_partial_url_operation(custom_url_client): + pages = custom_url_client.paging.get_pages_partial_url_operation("local") + items = [] + async for item in pages: + items.append(item) + + assert len(items) == 2 + assert items[0]["properties"]["id"] == 1 + assert items[1]["properties"]["id"] == 2 + +@pytest.mark.asyncio +async def test_get_multiple_pages_lro(client): + """LRO + Paging at the same time. + """ + from azure.mgmt.core.polling.async_arm_polling import AsyncARMPolling + poller = await client.paging.begin_get_multiple_pages_lro(polling=AsyncARMPolling(timeout=0)) + pager = await poller.result() + items = [] + async for item in pager: + items.append(item) + + assert len(items) == 10 + assert items[0]["properties"]["id"] == 1 + assert items[1]["properties"]["id"] == 2 + +@pytest.mark.asyncio +async def test_initial_response_no_items(client): + pages = client.paging.first_response_empty() + items = [] + async for item in pages: + items.append(item) + assert len(items) == 1 + +@pytest.mark.asyncio +async def test_item_name_with_xms_client_name(client): + pages = client.paging.get_paging_model_with_item_name_with_xms_client_name() + items = [] + async for item in pages: + items.append(item) + assert len(items) == 1 + +@pytest.mark.asyncio +async def test_duplicate_params(client): + pages = [p async for p in client.paging.duplicate_params(filter="foo")] + assert len(pages) == 1 + assert pages[0]["properties"]["id"] == 1 + assert pages[0]["properties"]["name"] == "Product" From 395db75bd80ead63c05e33ca9da6667d275d572a Mon Sep 17 00:00:00 2001 From: iscai-msft Date: Mon, 28 Feb 2022 15:04:14 -0500 Subject: [PATCH 4/7] update changelog and version --- ChangeLog.md | 6 +++++- package.json | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/ChangeLog.md b/ChangeLog.md index 194fb67e033..62a3ddd0cfe 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -1,6 +1,6 @@ # Change Log -### 2022-xx-xx - 5.12.7 +### 2022-xx-xx - 5.13.0 | Library | Min Version | --------------- | ------- @@ -10,6 +10,10 @@ |`msrest` dep of generated code | `0.6.21` |`azure-mgmt-core` dep of generated code (If generating mgmt plane code) | `1.3.0` +**Breaking Changes in Version Tolerant Generation** + +- Version tolerant paging does not reformat initial query parameters into the next link #1168 + **Bug Fixes** - Add default value consistently for parameters #1164 diff --git a/package.json b/package.json index a2b26750eed..0c2bb486a65 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@autorest/python", - "version": "5.12.6", + "version": "5.13.0", "description": "The Python extension for generators in AutoRest.", "scripts": { "prepare": "node run-python3.js prepare.py", From d9e2f0d6e95da12aebe7bba094b0c71374f335e3 Mon Sep 17 00:00:00 2001 From: iscai-msft Date: Thu, 3 Mar 2022 11:50:50 -0500 Subject: [PATCH 5/7] add flag reformat-next-link --- autorest/codegen/__init__.py | 7 +++++++ autorest/codegen/serializers/builder_serializer.py | 7 ++++--- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/autorest/codegen/__init__.py b/autorest/codegen/__init__.py index 0e7bab00b83..c00906aec09 100644 --- a/autorest/codegen/__init__.py +++ b/autorest/codegen/__init__.py @@ -76,6 +76,12 @@ def _validate_code_model_options(options: Dict[str, Any]) -> None: "If you want operation files, pass in flag --show-operations" ) + if options["reformat_next_link"] and options["version_tolerant"]: + raise ValueError( + "--reformat-next-link can not be true for version tolerant generations. " + "Please remove --reformat-next-link from your call for version tolerant generations." + ) + _LOGGER = logging.getLogger(__name__) class CodeGenerator(Plugin): @staticmethod @@ -292,6 +298,7 @@ def _build_code_model_options(self) -> Dict[str, Any]: "default_optional_constants_to_none": self._autorestapi.get_boolean_value( "default-optional-constants-to-none", low_level_client or version_tolerant ), + "reformat_next_link": self._autorestapi.get_boolean_value("reformat-next-link", not version_tolerant) } if options["builders_visibility"] is None: diff --git a/autorest/codegen/serializers/builder_serializer.py b/autorest/codegen/serializers/builder_serializer.py index 4f8bcf2dd34..68d3a9a3768 100644 --- a/autorest/codegen/serializers/builder_serializer.py +++ b/autorest/codegen/serializers/builder_serializer.py @@ -842,11 +842,12 @@ def _call_request_builder_helper( if ( is_next_request and not bool(builder.next_request_builder) and - self.code_model.options["version_tolerant"] and + not self.code_model.options["reformat_next_link"] and parameter.location == ParameterLocation.Query ): - # for version tolerant, we don't want to reformat query parameters if - # there is just one defined paging operation in the swagger + # if we don't want to reformat query parameters for next link calls + # in paging operations with a single swagger operation defintion, + # we skip passing query params when building the next request continue high_level_name = cast(RequestBuilderParameter, parameter).name_in_high_level_operation retval.append(f" {parameter.serialized_name}={high_level_name},") From 794ead27d91f0c0f2aae2cc51667875a994f1691 Mon Sep 17 00:00:00 2001 From: iscai-msft Date: Thu, 3 Mar 2022 13:12:38 -0500 Subject: [PATCH 6/7] have legacy calls do opaque next calls --- tasks.py | 1 + .../AcceptanceTests/Paging/paging/_patch.py | 44 +------------------ .../Paging/paging/aio/_patch.py | 18 +------- .../aio/operations/_paging_operations.py | 1 - .../paging/operations/_paging_operations.py | 1 - .../_storage_accounts_operations.py | 2 - .../_storage_accounts_operations.py | 2 - .../AcceptanceTests/test_complex.py | 2 +- 8 files changed, 4 insertions(+), 67 deletions(-) diff --git a/tasks.py b/tasks.py index eb603a6ca60..33b9e6dd415 100644 --- a/tasks.py +++ b/tasks.py @@ -160,6 +160,7 @@ def _build_flags( generation_section += "/legacy" override_flags = override_flags or {} override_flags["payload-flattening-threshold"] = 1 + override_flags["reformat-next-link"] = False flags = { "use": autorest_dir, diff --git a/test/azure/legacy/Expected/AcceptanceTests/Paging/paging/_patch.py b/test/azure/legacy/Expected/AcceptanceTests/Paging/paging/_patch.py index 0644979e481..f99e77fef98 100644 --- a/test/azure/legacy/Expected/AcceptanceTests/Paging/paging/_patch.py +++ b/test/azure/legacy/Expected/AcceptanceTests/Paging/paging/_patch.py @@ -24,50 +24,8 @@ # IN THE SOFTWARE. # # -------------------------------------------------------------------------- -from typing import List -import importlib -from ._auto_rest_paging_test_service import AutoRestPagingTestService as AutoRestPagingTestServiceGenerated -from azure.core.pipeline.policies import SansIOHTTPPolicy - -try: - binary_type = str - import urlparse # type: ignore - from urllib import urlencode -except ImportError: - binary_type = bytes # type: ignore - from urllib import parse as urlparse - from urllib.parse import urlencode - - -class RemoveDuplicateParamsPolicy(SansIOHTTPPolicy): - def __init__(self, duplicate_param_names): - # type: (List[str]) -> None - self.duplicate_param_names = duplicate_param_names - - def on_request(self, request): - parsed_url = urlparse.urlparse(request.http_request.url) - query_params = urlparse.parse_qs(parsed_url.query) - # service returned will be later in the url because of how we format - filtered_query_params = {k: v[-1:] if k in self.duplicate_param_names else v for k, v in query_params.items()} - request.http_request.url = request.http_request.url.replace(parsed_url.query, "") + urlencode( - filtered_query_params, doseq=True - ) - return super(RemoveDuplicateParamsPolicy, self).on_request(request) - - -class AutoRestPagingTestService(AutoRestPagingTestServiceGenerated): - def __init__(self, *args, **kwargs): - per_call_policies = kwargs.pop("per_call_policies", []) - params_policy = RemoveDuplicateParamsPolicy(duplicate_param_names=["$filter", "$skiptoken"]) - try: - per_call_policies.append(params_policy) - except AttributeError: - per_call_policies = [per_call_policies, params_policy] - super(AutoRestPagingTestService, self).__init__(*args, per_call_policies=per_call_policies, **kwargs) - # This file is used for handwritten extensions to the generated code. Example: # https://github.com/Azure/azure-sdk-for-python/blob/main/doc/dev/customize_code/how-to-patch-sdk-code.md def patch_sdk(): - curr_package = importlib.import_module("paging") - curr_package.AutoRestPagingTestService = AutoRestPagingTestService + pass diff --git a/test/azure/legacy/Expected/AcceptanceTests/Paging/paging/aio/_patch.py b/test/azure/legacy/Expected/AcceptanceTests/Paging/paging/aio/_patch.py index 0638c4f9cbc..f99e77fef98 100644 --- a/test/azure/legacy/Expected/AcceptanceTests/Paging/paging/aio/_patch.py +++ b/test/azure/legacy/Expected/AcceptanceTests/Paging/paging/aio/_patch.py @@ -24,24 +24,8 @@ # IN THE SOFTWARE. # # -------------------------------------------------------------------------- -import importlib -from .._patch import RemoveDuplicateParamsPolicy -from ._auto_rest_paging_test_service import AutoRestPagingTestService as AutoRestPagingTestServiceGenerated - - -class AutoRestPagingTestService(AutoRestPagingTestServiceGenerated): - def __init__(self, *args, **kwargs): - per_call_policies = kwargs.pop("per_call_policies", []) - params_policy = RemoveDuplicateParamsPolicy(duplicate_param_names=["$filter", "$skiptoken"]) - try: - per_call_policies.append(params_policy) - except AttributeError: - per_call_policies = [per_call_policies, params_policy] - super().__init__(*args, per_call_policies=per_call_policies, **kwargs) - # This file is used for handwritten extensions to the generated code. Example: # https://github.com/Azure/azure-sdk-for-python/blob/main/doc/dev/customize_code/how-to-patch-sdk-code.md def patch_sdk(): - curr_package = importlib.import_module("paging.aio") - curr_package.AutoRestPagingTestService = AutoRestPagingTestService + pass diff --git a/test/azure/legacy/Expected/AcceptanceTests/Paging/paging/aio/operations/_paging_operations.py b/test/azure/legacy/Expected/AcceptanceTests/Paging/paging/aio/operations/_paging_operations.py index 1ce2f0278d5..2b4b90aef67 100644 --- a/test/azure/legacy/Expected/AcceptanceTests/Paging/paging/aio/operations/_paging_operations.py +++ b/test/azure/legacy/Expected/AcceptanceTests/Paging/paging/aio/operations/_paging_operations.py @@ -489,7 +489,6 @@ def prepare_request(next_link=None): else: request = build_duplicate_params_request( - filter=filter, template_url=next_link, ) request = _convert_request(request) diff --git a/test/azure/legacy/Expected/AcceptanceTests/Paging/paging/operations/_paging_operations.py b/test/azure/legacy/Expected/AcceptanceTests/Paging/paging/operations/_paging_operations.py index 3503b23b075..92c3e467e1d 100644 --- a/test/azure/legacy/Expected/AcceptanceTests/Paging/paging/operations/_paging_operations.py +++ b/test/azure/legacy/Expected/AcceptanceTests/Paging/paging/operations/_paging_operations.py @@ -1041,7 +1041,6 @@ def prepare_request(next_link=None): else: request = build_duplicate_params_request( - filter=filter, template_url=next_link, ) request = _convert_request(request) diff --git a/test/azure/legacy/Expected/AcceptanceTests/StorageManagementClient/storage/aio/operations/_storage_accounts_operations.py b/test/azure/legacy/Expected/AcceptanceTests/StorageManagementClient/storage/aio/operations/_storage_accounts_operations.py index c48e49635e2..efbf27fc48c 100644 --- a/test/azure/legacy/Expected/AcceptanceTests/StorageManagementClient/storage/aio/operations/_storage_accounts_operations.py +++ b/test/azure/legacy/Expected/AcceptanceTests/StorageManagementClient/storage/aio/operations/_storage_accounts_operations.py @@ -506,7 +506,6 @@ def prepare_request(next_link=None): request = build_list_request( subscription_id=self._config.subscription_id, - api_version=api_version, template_url=next_link, ) request = _convert_request(request) @@ -577,7 +576,6 @@ def prepare_request(next_link=None): request = build_list_by_resource_group_request( resource_group_name=resource_group_name, subscription_id=self._config.subscription_id, - api_version=api_version, template_url=next_link, ) request = _convert_request(request) diff --git a/test/azure/legacy/Expected/AcceptanceTests/StorageManagementClient/storage/operations/_storage_accounts_operations.py b/test/azure/legacy/Expected/AcceptanceTests/StorageManagementClient/storage/operations/_storage_accounts_operations.py index 65b38e911b8..5aa449234fa 100644 --- a/test/azure/legacy/Expected/AcceptanceTests/StorageManagementClient/storage/operations/_storage_accounts_operations.py +++ b/test/azure/legacy/Expected/AcceptanceTests/StorageManagementClient/storage/operations/_storage_accounts_operations.py @@ -847,7 +847,6 @@ def prepare_request(next_link=None): request = build_list_request( subscription_id=self._config.subscription_id, - api_version=api_version, template_url=next_link, ) request = _convert_request(request) @@ -921,7 +920,6 @@ def prepare_request(next_link=None): request = build_list_by_resource_group_request( resource_group_name=resource_group_name, subscription_id=self._config.subscription_id, - api_version=api_version, template_url=next_link, ) request = _convert_request(request) diff --git a/test/vanilla/version-tolerant/AcceptanceTests/test_complex.py b/test/vanilla/version-tolerant/AcceptanceTests/test_complex.py index 27ea8c9666b..0df996acae8 100644 --- a/test/vanilla/version-tolerant/AcceptanceTests/test_complex.py +++ b/test/vanilla/version-tolerant/AcceptanceTests/test_complex.py @@ -54,7 +54,7 @@ def min_date(): min_date = datetime.min return min_date.replace(tzinfo=UTC()) -def test_basic_get_and_put_valid(client): +def test_basic_get_and_put_valid(client: AutoRestComplexTestService): # GET basic/valid basic_result = client.basic.get_valid() assert 2 == basic_result['id'] From 56784c7808bc7cf5ecb057fed74e5a52bd387fc9 Mon Sep 17 00:00:00 2001 From: iscai-msft Date: Thu, 3 Mar 2022 13:36:33 -0500 Subject: [PATCH 7/7] make next link paging part of 5.13.0 --- ChangeLog.md | 22 ++-------------------- 1 file changed, 2 insertions(+), 20 deletions(-) diff --git a/ChangeLog.md b/ChangeLog.md index 312e8b73d67..181754ada28 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -1,25 +1,5 @@ # Change Log -### 2022-03-xx - 5.14.0 - -| Library | Min Version | -| ----------------------------------------------------------------------- | ----------- | -| `@autorest/core` | `3.6.2` | -| `@autorest/modelerfour` | `4.19.1` | -| `azure-core` dep of generated code | `1.20.1` | -| `msrest` dep of generated code | `0.6.21` | -| `azure-mgmt-core` dep of generated code (If generating mgmt plane code) | `1.3.0` | - -**Breaking Changes in Version Tolerant Generation** - -- Version tolerant paging does not reformat initial query parameters into the next link #1168 - -**New Features** - -- Add flag `--reformat-next-link` that defaults to `True`. This flag determines whether we reformat query parameters - for next link for paging defined with a single operation in the swagger. Forced to `True` for version tolerant - generations #1168 - ### 2022-03-03 - 5.13.0 | Library | Min Version | @@ -33,10 +13,12 @@ **Breaking Changes in Version Tolerant Generation** - We now generate with optional constant parameters as None by defaulting `--default-optional-constants-to-none` to True #1171 +- Version tolerant paging does not reformat initial query parameters into the next link #1168 **New Features** - Add flag `--default-optional-constants-to-none` with which optional constant parameters is default to None #1171 +- Add flag `--reformat-next-link`, determines whether we reformat initial query parameters into the next link. Defaults to `True` for the GA generator, forced to `False` for `--version-tolerant`. **Bug Fixes**