[rest] change text from a property to a method (Azure#20290)

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
 

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:

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)

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

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

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)

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"<html><body>Hello, world!</html></body>"
-    assert response.text == "<html><body>Hello, world!</html></body>"
+    assert response.text() == "<html><body>Hello, world!</html></body>"
 
 @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,23 +205,23 @@ 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):
     response = await 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):
     response = await 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):
 #     """

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):

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)