From 563a406038680bab25e521c392def699520d8c08 Mon Sep 17 00:00:00 2001 From: iscai-msft <43154838+iscai-msft@users.noreply.github.com> Date: Thu, 19 Aug 2021 20:04:36 -0400 Subject: [PATCH] [rest] change text from a property to a method (#20290) --- sdk/core/azure-core/CHANGELOG.md | 5 +- .../azure-core/azure/core/rest/_helpers.py | 2 + .../azure/core/rest/_requests_basic.py | 30 ---------- sdk/core/azure-core/azure/core/rest/_rest.py | 25 +++++---- .../azure-core/azure/core/rest/_rest_py3.py | 25 ++++----- .../test_rest_context_manager_async.py | 6 +- .../test_rest_http_response_async.py | 55 ++++++++++++------- .../test_rest_stream_responses_async.py | 4 +- .../test_rest_context_manager.py | 6 +- .../test_rest_http_response.py | 49 +++++++++++------ .../test_rest_stream_responses.py | 6 +- 11 files changed, 110 insertions(+), 103 deletions(-) diff --git a/sdk/core/azure-core/CHANGELOG.md b/sdk/core/azure-core/CHANGELOG.md index d9d8db2e1adab..16165e58c6adf 100644 --- a/sdk/core/azure-core/CHANGELOG.md +++ b/sdk/core/azure-core/CHANGELOG.md @@ -4,7 +4,10 @@ ### Features Added -### Breaking Changes +### Breaking Changes in the Provisional `azure.core.rest` package + +- The `text` property on `azure.core.rest.HttpResponse` and `azure.core.rest.AsyncHttpResponse` has changed to a method, which also takes +an `encoding` parameter. ### Bugs Fixed diff --git a/sdk/core/azure-core/azure/core/rest/_helpers.py b/sdk/core/azure-core/azure/core/rest/_helpers.py index 4975d09d655c4..a2f8736dcc68c 100644 --- a/sdk/core/azure-core/azure/core/rest/_helpers.py +++ b/sdk/core/azure-core/azure/core/rest/_helpers.py @@ -293,6 +293,8 @@ def get_charset_encoding(response): def decode_to_text(encoding, content): # type: (Optional[str], bytes) -> str + if not content: + return "" if encoding == "utf-8": encoding = "utf-8-sig" if encoding: diff --git a/sdk/core/azure-core/azure/core/rest/_requests_basic.py b/sdk/core/azure-core/azure/core/rest/_requests_basic.py index e8ef734e12752..bd83dc29bd39d 100644 --- a/sdk/core/azure-core/azure/core/rest/_requests_basic.py +++ b/sdk/core/azure-core/azure/core/rest/_requests_basic.py @@ -61,36 +61,6 @@ def content(self): # requests throws a RuntimeError if the content for a response is already consumed raise ResponseNotReadError(self) - @property - def encoding(self): - # type: () -> Optional[str] - retval = super(_RestRequestsTransportResponseBase, self).encoding - if not retval: - # There is a few situation where "requests" magic doesn't fit us: - # - https://github.com/psf/requests/issues/654 - # - https://github.com/psf/requests/issues/1737 - # - https://github.com/psf/requests/issues/2086 - from codecs import BOM_UTF8 - if self._internal_response.content[:3] == BOM_UTF8: - retval = "utf-8-sig" - if retval: - if retval == "utf-8": - retval = "utf-8-sig" - return retval - - @encoding.setter # type: ignore - def encoding(self, value): - # type: (str) -> None - # ignoring setter bc of known mypy issue https://github.com/python/mypy/issues/1465 - self._encoding = value - self._internal_response.encoding = value - - @property - def text(self): - # this will trigger errors if response is not read in - self.content # pylint: disable=pointless-statement - return self._internal_response.text - def _stream_download_helper(decompress, response): if response.is_stream_consumed: raise StreamConsumedError(response) diff --git a/sdk/core/azure-core/azure/core/rest/_rest.py b/sdk/core/azure-core/azure/core/rest/_rest.py index 295051a83c50e..10a8486a2c647 100644 --- a/sdk/core/azure-core/azure/core/rest/_rest.py +++ b/sdk/core/azure-core/azure/core/rest/_rest.py @@ -235,17 +235,19 @@ def encoding(self, value): # type: (str) -> None """Sets the response encoding""" self._encoding = value + self._text = None # clear text cache - @property - def text(self): - # type: (...) -> str - """Returns the response body as a string""" - if self._text is None: - content = self.content - if not content: - self._text = "" - else: - self._text = decode_to_text(self.encoding, self.content) + def text(self, encoding=None): + # type: (Optional[str]) -> str + """Returns the response body as a string + + :param optional[str] encoding: The encoding you want to decode the text with. Can + also be set independently through our encoding property + :return: The response's content decoded as a string. + """ + if self._text is None or encoding: + encoding_to_pass = encoding or self.encoding + self._text = decode_to_text(encoding_to_pass, self.content) return self._text def json(self): @@ -259,7 +261,7 @@ def json(self): # this will trigger errors if response is not read in self.content # pylint: disable=pointless-statement if not self._json: - self._json = loads(self.text) + self._json = loads(self.text()) return self._json def raise_for_status(self): @@ -314,7 +316,6 @@ class HttpResponse(_HttpResponseBase): # pylint: disable=too-many-instance-attr :ivar str text: The response body as a string. :ivar request: The request that resulted in this response. :vartype request: ~azure.core.rest.HttpRequest - :ivar internal_response: The object returned from the HTTP library. :ivar str content_type: The content type of the response :ivar bool is_closed: Whether the network connection has been closed yet :ivar bool is_stream_consumed: When getting a stream response, checks diff --git a/sdk/core/azure-core/azure/core/rest/_rest_py3.py b/sdk/core/azure-core/azure/core/rest/_rest_py3.py index af957f767a392..21e42f46b0448 100644 --- a/sdk/core/azure-core/azure/core/rest/_rest_py3.py +++ b/sdk/core/azure-core/azure/core/rest/_rest_py3.py @@ -262,16 +262,18 @@ def encoding(self) -> Optional[str]: def encoding(self, value: str) -> None: """Sets the response encoding""" self._encoding = value + self._text = None # clear text cache - @property - def text(self) -> str: - """Returns the response body as a string""" - if self._text is None: - content = self.content - if not content: - self._text = "" - else: - self._text = decode_to_text(self.encoding, self.content) + def text(self, encoding: Optional[str] = None) -> str: + """Returns the response body as a string + + :param optional[str] encoding: The encoding you want to decode the text with. Can + also be set independently through our encoding property + :return: The response's content decoded as a string. + """ + if self._text is None or encoding: + encoding_to_pass = encoding or self.encoding + self._text = decode_to_text(encoding_to_pass, self.content) return self._text def json(self) -> Any: @@ -284,7 +286,7 @@ def json(self) -> Any: # this will trigger errors if response is not read in self.content # pylint: disable=pointless-statement if not self._json: - self._json = loads(self.text) + self._json = loads(self.text()) return self._json def raise_for_status(self) -> None: @@ -328,7 +330,6 @@ class HttpResponse(_HttpResponseBase): :ivar str text: The response body as a string. :ivar request: The request that resulted in this response. :vartype request: ~azure.core.rest.HttpRequest - :ivar internal_response: The object returned from the HTTP library. :ivar str content_type: The content type of the response :ivar bool is_closed: Whether the network connection has been closed yet :ivar bool is_stream_consumed: When getting a stream response, checks @@ -421,7 +422,6 @@ class AsyncHttpResponse(_HttpResponseBase): :keyword request: The request that resulted in this response. :paramtype request: ~azure.core.rest.HttpRequest - :keyword internal_response: The object returned from the HTTP library. :ivar int status_code: The status code of this response :ivar mapping headers: The response headers :ivar str reason: The reason phrase for this response @@ -432,7 +432,6 @@ class AsyncHttpResponse(_HttpResponseBase): :ivar str text: The response body as a string. :ivar request: The request that resulted in this response. :vartype request: ~azure.core.rest.HttpRequest - :ivar internal_response: The object returned from the HTTP library. :ivar str content_type: The content type of the response :ivar bool is_closed: Whether the network connection has been closed yet :ivar bool is_stream_consumed: When getting a stream response, checks diff --git a/sdk/core/azure-core/tests/testserver_tests/async_tests/test_rest_context_manager_async.py b/sdk/core/azure-core/tests/testserver_tests/async_tests/test_rest_context_manager_async.py index 4afcac42172fd..70ca63685b84f 100644 --- a/sdk/core/azure-core/tests/testserver_tests/async_tests/test_rest_context_manager_async.py +++ b/sdk/core/azure-core/tests/testserver_tests/async_tests/test_rest_context_manager_async.py @@ -13,7 +13,7 @@ async def test_normal_call(client): async def _raise_and_get_text(response): response.raise_for_status() - assert response.text == "Hello, world!" + assert response.text() == "Hello, world!" assert response.is_closed request = HttpRequest("GET", url="/basic/string") response = await client.send_request(request) @@ -33,9 +33,9 @@ async def _raise_and_get_text(response): response.raise_for_status() assert not response.is_closed with pytest.raises(ResponseNotReadError): - response.text + response.text() await response.read() - assert response.text == "Hello, world!" + assert response.text() == "Hello, world!" assert response.is_closed request = HttpRequest("GET", url="/streams/basic") response = await client.send_request(request, stream=True) diff --git a/sdk/core/azure-core/tests/testserver_tests/async_tests/test_rest_http_response_async.py b/sdk/core/azure-core/tests/testserver_tests/async_tests/test_rest_http_response_async.py index 47286b76254b3..40587252a14f7 100644 --- a/sdk/core/azure-core/tests/testserver_tests/async_tests/test_rest_http_response_async.py +++ b/sdk/core/azure-core/tests/testserver_tests/async_tests/test_rest_http_response_async.py @@ -14,9 +14,10 @@ @pytest.fixture def send_request(client): async def _send_request(request): - response = await client.send_request(request, stream=False) - response.raise_for_status() - return response + async with client: + response = await client.send_request(request, stream=False) + response.raise_for_status() + return response return _send_request @pytest.mark.asyncio @@ -27,7 +28,7 @@ async def test_response(send_request, port): assert response.status_code == 200 assert response.reason == "OK" assert response.content == b"Hello, world!" - assert response.text == "Hello, world!" + assert response.text() == "Hello, world!" assert response.request.method == "GET" assert response.request.url == "http://localhost:{}/basic/string".format(port) @@ -40,7 +41,7 @@ async def test_response_content(send_request): assert response.reason == "OK" content = await response.read() assert content == b"Hello, world!" - assert response.text == "Hello, world!" + assert response.text() == "Hello, world!" @pytest.mark.asyncio async def test_response_text(send_request): @@ -51,7 +52,7 @@ async def test_response_text(send_request): assert response.reason == "OK" content = await response.read() assert content == b"Hello, world!" - assert response.text == "Hello, world!" + assert response.text() == "Hello, world!" assert response.headers["Content-Length"] == '13' assert response.headers['Content-Type'] == "text/plain; charset=utf-8" @@ -64,7 +65,7 @@ async def test_response_html(send_request): assert response.reason == "OK" content = await response.read() assert content == b"Hello, world!" - assert response.text == "Hello, world!" + assert response.text() == "Hello, world!" @pytest.mark.asyncio async def test_raise_for_status(client): @@ -106,7 +107,7 @@ async def test_response_content_type_encoding(send_request): await response.read() assert response.content_type == "text/plain; charset=latin-1" assert response.content == b'Latin 1: \xff' - assert response.text == "Latin 1: ÿ" + assert response.text() == "Latin 1: ÿ" assert response.encoding == "latin-1" @@ -119,7 +120,7 @@ async def test_response_autodetect_encoding(send_request): request=HttpRequest("GET", "/encoding/latin-1") ) await response.read() - assert response.text == u'Latin 1: ÿ' + assert response.text() == u'Latin 1: ÿ' assert response.encoding == "latin-1" @@ -133,7 +134,7 @@ async def test_response_fallback_to_autodetect(send_request): ) await response.read() assert response.headers["Content-Type"] == "text/plain; charset=invalid-codec-name" - assert response.text == "おはようございます。" + assert response.text() == "おはようございます。" assert response.encoding is None @@ -152,20 +153,20 @@ async def test_response_no_charset_with_ascii_content(send_request): assert response.encoding is None content = await response.read() assert content == b"Hello, world!" - assert response.text == "Hello, world!" + assert response.text() == "Hello, world!" @pytest.mark.asyncio async def test_response_no_charset_with_iso_8859_1_content(send_request): """ - A response with ISO 8859-1 encoded content should decode correctly, - even with no charset specified. + We don't support iso-8859-1 by default following conversations + about endoding flow """ response = await send_request( request=HttpRequest("GET", "/encoding/iso-8859-1"), ) await response.read() - assert response.text == "Accented: �sterreich" # aiohttp is having diff behavior than requests + assert response.text() == "Accented: �sterreich" assert response.encoding is None # NOTE: aiohttp isn't liking this @@ -177,7 +178,7 @@ async def test_response_no_charset_with_iso_8859_1_content(send_request): # assert response.headers["Content-Type"] == "text/plain; charset=utf-8" # response.encoding = "latin-1" # await response.read() -# assert response.text == "Latin 1: ÿ" +# assert response.text() == "Latin 1: ÿ" # assert response.encoding == "latin-1" @pytest.mark.asyncio @@ -204,7 +205,7 @@ async def test_emoji(send_request): request=HttpRequest("GET", "/encoding/emoji"), ) await response.read() - assert response.text == "👩" + assert response.text() == "👩" @pytest.mark.asyncio async def test_emoji_family_with_skin_tone_modifier(send_request): @@ -212,7 +213,7 @@ async def test_emoji_family_with_skin_tone_modifier(send_request): request=HttpRequest("GET", "/encoding/emoji-family-skin-tone-modifier"), ) await response.read() - assert response.text == "👩🏻‍👩🏽‍👧🏾‍👦🏿 SSN: 859-98-0987" + assert response.text() == "👩🏻‍👩🏽‍👧🏾‍👦🏿 SSN: 859-98-0987" @pytest.mark.asyncio async def test_korean_nfc(send_request): @@ -220,7 +221,7 @@ async def test_korean_nfc(send_request): request=HttpRequest("GET", "/encoding/korean"), ) await response.read() - assert response.text == "아가" + assert response.text() == "아가" @pytest.mark.asyncio async def test_urlencoded_content(send_request): @@ -249,9 +250,25 @@ async def test_send_request_return_pipeline_response(client): assert hasattr(response, "http_request") assert hasattr(response, "http_response") assert hasattr(response, "context") - assert response.http_response.text == "Hello, world!" + assert response.http_response.text() == "Hello, world!" assert hasattr(response.http_request, "content") +@pytest.mark.asyncio +async def test_text_and_encoding(send_request): + response = await send_request( + request=HttpRequest("GET", "/encoding/emoji"), + ) + assert response.content == u"👩".encode("utf-8") + assert response.text() == u"👩" + + # try setting encoding as a property + response.encoding = "utf-16" + assert response.text() == u"鿰ꦑ" == response.content.decode(response.encoding) + + # assert latin-1 changes text decoding without changing encoding property + assert response.text("latin-1") == 'ð\x9f\x91©' == response.content.decode("latin-1") + assert response.encoding == "utf-16" + # @pytest.mark.asyncio # async def test_multipart_encode_non_seekable_filelike(send_request): # """ diff --git a/sdk/core/azure-core/tests/testserver_tests/async_tests/test_rest_stream_responses_async.py b/sdk/core/azure-core/tests/testserver_tests/async_tests/test_rest_stream_responses_async.py index 6731487497193..19cd093222e17 100644 --- a/sdk/core/azure-core/tests/testserver_tests/async_tests/test_rest_stream_responses_async.py +++ b/sdk/core/azure-core/tests/testserver_tests/async_tests/test_rest_stream_responses_async.py @@ -170,11 +170,11 @@ async def test_iter_read_back_and_forth(client): async for line in response.iter_lines(): assert line with pytest.raises(ResponseNotReadError): - response.text + response.text() with pytest.raises(StreamConsumedError): await response.read() with pytest.raises(ResponseNotReadError): - response.text + response.text() @pytest.mark.asyncio async def test_stream_with_return_pipeline_response(client): diff --git a/sdk/core/azure-core/tests/testserver_tests/test_rest_context_manager.py b/sdk/core/azure-core/tests/testserver_tests/test_rest_context_manager.py index 34f8029715370..0531cfe1505c4 100644 --- a/sdk/core/azure-core/tests/testserver_tests/test_rest_context_manager.py +++ b/sdk/core/azure-core/tests/testserver_tests/test_rest_context_manager.py @@ -11,7 +11,7 @@ def test_normal_call(client, port): def _raise_and_get_text(response): response.raise_for_status() - assert response.text == "Hello, world!" + assert response.text() == "Hello, world!" assert response.is_closed request = HttpRequest("GET", url="/basic/string") response = client.send_request(request) @@ -30,9 +30,9 @@ def _raise_and_get_text(response): response.raise_for_status() assert not response.is_closed with pytest.raises(ResponseNotReadError): - response.text + response.text() response.read() - assert response.text == "Hello, world!" + assert response.text() == "Hello, world!" assert response.is_closed request = HttpRequest("GET", url="/streams/basic") response = client.send_request(request, stream=True) diff --git a/sdk/core/azure-core/tests/testserver_tests/test_rest_http_response.py b/sdk/core/azure-core/tests/testserver_tests/test_rest_http_response.py index 804a98b8890e9..f3abec23a30ae 100644 --- a/sdk/core/azure-core/tests/testserver_tests/test_rest_http_response.py +++ b/sdk/core/azure-core/tests/testserver_tests/test_rest_http_response.py @@ -29,7 +29,7 @@ def test_response(send_request, port): ) assert response.status_code == 200 assert response.reason == "OK" - assert response.text == "Hello, world!" + assert response.text() == "Hello, world!" assert response.request.method == "GET" assert response.request.url == "http://localhost:{}/basic/string".format(port) @@ -40,7 +40,7 @@ def test_response_content(send_request): ) assert response.status_code == 200 assert response.reason == "OK" - assert response.text == "Hello, world!" + assert response.text() == "Hello, world!" def test_response_text(send_request): @@ -49,7 +49,7 @@ def test_response_text(send_request): ) assert response.status_code == 200 assert response.reason == "OK" - assert response.text == "Hello, world!" + assert response.text() == "Hello, world!" assert response.headers["Content-Length"] == '13' assert response.headers['Content-Type'] == "text/plain; charset=utf-8" assert response.content_type == "text/plain; charset=utf-8" @@ -60,7 +60,7 @@ def test_response_html(send_request): ) assert response.status_code == 200 assert response.reason == "OK" - assert response.text == "Hello, world!" + assert response.text() == "Hello, world!" def test_raise_for_status(client): response = client.send_request( @@ -97,7 +97,7 @@ def test_response_content_type_encoding(send_request): request=HttpRequest("GET", "/encoding/latin-1") ) assert response.content_type == "text/plain; charset=latin-1" - assert response.text == u"Latin 1: ÿ" + assert response.text() == u"Latin 1: ÿ" assert response.encoding == "latin-1" @@ -109,7 +109,7 @@ def test_response_autodetect_encoding(send_request): request=HttpRequest("GET", "/encoding/latin-1") ) - assert response.text == u'Latin 1: ÿ' + assert response.text() == u'Latin 1: ÿ' assert response.encoding == "latin-1" @pytest.mark.skipif(sys.version_info < (3, 0), @@ -123,7 +123,7 @@ def test_response_fallback_to_autodetect(send_request): ) assert response.headers["Content-Type"] == "text/plain; charset=invalid-codec-name" - assert response.text == u"おはようございます。" + assert response.text() == u"おはようございます。" assert response.encoding is None @@ -139,18 +139,18 @@ def test_response_no_charset_with_ascii_content(send_request): assert response.headers["Content-Type"] == "text/plain" assert response.status_code == 200 assert response.encoding is None - assert response.text == "Hello, world!" + assert response.text() == "Hello, world!" def test_response_no_charset_with_iso_8859_1_content(send_request): """ - A response with ISO 8859-1 encoded content should decode correctly, - even with no charset specified. + We don't support iso-8859-1 by default following conversations + about endoding flow """ response = send_request( request=HttpRequest("GET", "/encoding/iso-8859-1"), ) - assert response.text == u"Accented: Österreich" + assert response.text() == u"Accented: �sterreich" assert response.encoding is None def test_response_set_explicit_encoding(send_request): @@ -160,7 +160,7 @@ def test_response_set_explicit_encoding(send_request): ) assert response.headers["Content-Type"] == "text/plain; charset=utf-8" response.encoding = "latin-1" - assert response.text == u"Latin 1: ÿ" + assert response.text() == u"Latin 1: ÿ" assert response.encoding == "latin-1" def test_json(send_request): @@ -181,19 +181,19 @@ def test_emoji(send_request): response = send_request( request=HttpRequest("GET", "/encoding/emoji"), ) - assert response.text == u"👩" + assert response.text() == u"👩" def test_emoji_family_with_skin_tone_modifier(send_request): response = send_request( request=HttpRequest("GET", "/encoding/emoji-family-skin-tone-modifier"), ) - assert response.text == u"👩🏻‍👩🏽‍👧🏾‍👦🏿 SSN: 859-98-0987" + assert response.text() == u"👩🏻‍👩🏽‍👧🏾‍👦🏿 SSN: 859-98-0987" def test_korean_nfc(send_request): response = send_request( request=HttpRequest("GET", "/encoding/korean"), ) - assert response.text == u"아가" + assert response.text() == u"아가" def test_urlencoded_content(send_request): send_request( @@ -255,7 +255,7 @@ def test_get_xml_basic(send_request): "/xml/basic", ) response = send_request(request) - parsed_xml = ET.fromstring(response.text) + parsed_xml = ET.fromstring(response.text()) assert parsed_xml.tag == 'slideshow' attributes = parsed_xml.attrib assert attributes['title'] == "Sample Slide Show" @@ -294,5 +294,20 @@ def test_send_request_return_pipeline_response(client): assert hasattr(response, "http_request") assert hasattr(response, "http_response") assert hasattr(response, "context") - assert response.http_response.text == "Hello, world!" + assert response.http_response.text() == "Hello, world!" assert hasattr(response.http_request, "content") + +def test_text_and_encoding(send_request): + response = send_request( + request=HttpRequest("GET", "/encoding/emoji"), + ) + assert response.content == u"👩".encode("utf-8") + assert response.text() == u"👩" + + # try setting encoding as a property + response.encoding = "utf-16" + assert response.text() == u"鿰ꦑ" == response.content.decode(response.encoding) + + # assert latin-1 changes text decoding without changing encoding property + assert response.text("latin-1") == u'ð\x9f\x91©' == response.content.decode("latin-1") + assert response.encoding == "utf-16" diff --git a/sdk/core/azure-core/tests/testserver_tests/test_rest_stream_responses.py b/sdk/core/azure-core/tests/testserver_tests/test_rest_stream_responses.py index 61053ca7abb97..cf547d8f750eb 100644 --- a/sdk/core/azure-core/tests/testserver_tests/test_rest_stream_responses.py +++ b/sdk/core/azure-core/tests/testserver_tests/test_rest_stream_responses.py @@ -187,7 +187,7 @@ def test_iter_read(client): iterator = response.iter_lines() for line in iterator: assert line - assert response.text + assert response.text() def test_iter_read_back_and_forth(client): # thanks to McCoy Patiño for this test! @@ -202,11 +202,11 @@ def test_iter_read_back_and_forth(client): for line in iterator: assert line with pytest.raises(ResponseNotReadError): - response.text + response.text() with pytest.raises(StreamConsumedError): response.read() with pytest.raises(ResponseNotReadError): - response.text + response.text() def test_stream_with_return_pipeline_response(client): request = HttpRequest("GET", "/basic/lines")