From 6a3d0c085333a80e371387b25a378dd903ad977b Mon Sep 17 00:00:00 2001 From: Alexander Shorin Date: Thu, 3 Jul 2014 15:19:59 +0400 Subject: [PATCH 1/5] Implicitly release connection after full read of response payload Deprecate ClientResponse.read_and_close method --- README.rst | 4 ++-- aiohttp/client.py | 21 ++++++++++----------- tests/test_client.py | 24 ++++++++++++++---------- tests/test_client_functional.py | 4 ++-- 4 files changed, 28 insertions(+), 25 deletions(-) diff --git a/README.rst b/README.rst index 0dce5de7d09..25153c75984 100644 --- a/README.rst +++ b/README.rst @@ -29,13 +29,13 @@ To retrieve something from the web:: def get_body(url): response = yield from aiohttp.request('GET', url) - return (yield from response.read_and_close()) + return (yield from response.read()) You can use the get command like this anywhere in your ``asyncio`` powered program:: response = yield from aiohttp.request('GET', 'http://python.org') - body = yield from response.read_and_close() + body = yield from response.read() print(body) The signature of request is the following:: diff --git a/aiohttp/client.py b/aiohttp/client.py index ee238ef328a..5645d256b38 100644 --- a/aiohttp/client.py +++ b/aiohttp/client.py @@ -86,7 +86,7 @@ def request(method, url, *, >>> resp = yield from aiohttp.request('GET', 'http://python.org/') >>> resp - >>> data = yield from resp.read_and_close() + >>> data = yield from resp.read() """ redirects = 0 @@ -697,7 +697,10 @@ def read(self, decode=False): buf.append((chunk, size)) total += size except aiohttp.EofStream: - pass + self.close() + except: + self.close(True) + raise self._content = bytearray(total) @@ -719,15 +722,11 @@ def read(self, decode=False): @asyncio.coroutine def read_and_close(self, decode=False): """Read response payload and then close response.""" - try: - payload = yield from self.read(decode) - except: - self.close(True) - raise - else: - self.close() - - return payload + warnings.warn( + 'read_and_close is deprecated, use .read() instead', + DeprecationWarning + ) + return (yield from self.read(decode)) def str_to_bytes(s, encoding='utf-8'): diff --git a/tests/test_client.py b/tests/test_client.py index f14b59501b7..d21a888fd17 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -81,26 +81,30 @@ def test_repr(self): repr(self.response)) def test_read_and_close(self): - self.response.read = unittest.mock.Mock() - self.response.read.return_value = asyncio.Future(loop=self.loop) - self.response.read.return_value.set_result(b'payload') + def side_effect(*args, **kwargs): + def second_call(*args, **kwargs): + raise aiohttp.EofStream + fut = asyncio.Future(loop=self.loop) + fut.set_result(b'payload') + content.read.side_effect = second_call + return fut + content = self.response.content = unittest.mock.Mock() + content.read.side_effect = side_effect self.response.close = unittest.mock.Mock() - res = self.loop.run_until_complete(self.response.read_and_close()) + res = self.loop.run_until_complete(self.response.read()) self.assertEqual(res, b'payload') - self.assertTrue(self.response.read.called) self.assertTrue(self.response.close.called) def test_read_and_close_with_error(self): - self.response.read = unittest.mock.Mock() - self.response.read.return_value = asyncio.Future(loop=self.loop) - self.response.read.return_value.set_exception(ValueError) + content = self.response.content = unittest.mock.Mock() + content.read.return_value = asyncio.Future(loop=self.loop) + content.read.return_value.set_exception(ValueError) self.response.close = unittest.mock.Mock() self.assertRaises( ValueError, - self.loop.run_until_complete, self.response.read_and_close()) - self.assertTrue(self.response.read.called) + self.loop.run_until_complete, self.response.read()) self.response.close.assert_called_with(True) diff --git a/tests/test_client_functional.py b/tests/test_client_functional.py index e8ef4d10173..6157488846f 100644 --- a/tests/test_client_functional.py +++ b/tests/test_client_functional.py @@ -679,7 +679,7 @@ def go(url): self.assertIsNotNone(connection) connector = connection._connector self.assertIsNotNone(connector) - yield from r.read_and_close() + yield from r.read() self.assertEqual(0, len(connector._conns)) with test_utils.run_server(self.loop, router=Functional) as httpd: @@ -695,7 +695,7 @@ def go(url): r = yield from client.request('GET', url, connector=connector, loop=self.loop) - yield from r.read_and_close() + yield from r.read() self.assertEqual(1, len(connector._conns)) connector.close() From f325a7174c39d0176c33b5ddb17dec6c8e7d3d38 Mon Sep 17 00:00:00 2001 From: Alexander Shorin Date: Thu, 3 Jul 2014 19:28:50 +0400 Subject: [PATCH 2/5] Add ClientResponse.release method Useful, when response body isn't available for read (HEAD requests) or if you're not interested in it, but wants release connection correctly. --- aiohttp/client.py | 4 ++++ tests/test_client.py | 10 ++++++++++ 2 files changed, 14 insertions(+) diff --git a/aiohttp/client.py b/aiohttp/client.py index 5645d256b38..b56cc8ca5ad 100644 --- a/aiohttp/client.py +++ b/aiohttp/client.py @@ -674,6 +674,10 @@ def close(self, force=False): self._writer = None self._writer_wr = None + @asyncio.coroutine + def release(self): + yield from self.read() + @asyncio.coroutine def wait_for_close(self): if self._writer is not None: diff --git a/tests/test_client.py b/tests/test_client.py index d21a888fd17..a1b0e9d2be5 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -107,6 +107,16 @@ def test_read_and_close_with_error(self): self.loop.run_until_complete, self.response.read()) self.response.close.assert_called_with(True) + def test_release(self): + fut = asyncio.Future(loop=self.loop) + fut.set_exception(aiohttp.EofStream) + content = self.response.content = unittest.mock.Mock() + content.read.return_value = fut + self.response.close = unittest.mock.Mock() + + self.loop.run_until_complete(self.response.release()) + self.assertTrue(self.response.close.called) + class ClientRequestTests(unittest.TestCase): From ec14049a8a86738f990487aa6ff1e8050b8daa9c Mon Sep 17 00:00:00 2001 From: Alexander Shorin Date: Thu, 3 Jul 2014 15:13:06 +0400 Subject: [PATCH 3/5] Deprecate .read(True) in favour of .json() There are several issues with current decoding implementation inside read method: - issue #18 - no possibility to use different json lib - doesn't scale for different mime types - it crushes for empty body - on call read(True) with decoding support for multiple types you won't be sure which kind of response you'll get back TL;DR there are too many reasons to patch .read() method because of decoding logic inside. Moving it to standalone method simplifies life a lot. --- aiohttp/client.py | 24 +++++++++++++++++--- tests/test_client_functional.py | 40 ++++++++++++++++----------------- 2 files changed, 41 insertions(+), 23 deletions(-) diff --git a/aiohttp/client.py b/aiohttp/client.py index b56cc8ca5ad..b30f1af3644 100644 --- a/aiohttp/client.py +++ b/aiohttp/client.py @@ -717,9 +717,11 @@ def read(self, decode=False): data = self._content if decode: - ct = self.headers.get('CONTENT-TYPE', '').lower() - if ct == 'application/json': - data = json.loads(data.decode('utf-8')) + warnings.warn( + '.read(True) is deprecated. use .json() instead', + DeprecationWarning + ) + return (yield from self.json()) return data @@ -732,6 +734,22 @@ def read_and_close(self, decode=False): ) return (yield from self.read(decode)) + @asyncio.coroutine + def json(self): + """Reads and decodes JSON response.""" + if self._content is None: + yield from self.read() + + ctype = self.headers.get('CONTENT-TYPE', '').lower() + if not ctype.startswith('application/json'): + client_log.warning( + 'Attempt to decode JSON with unexpected mimetype: %s', ctype) + + if not self._content.strip(): + return None + + return json.loads(self._content.decode('utf-8')) + def str_to_bytes(s, encoding='utf-8'): if isinstance(s, str): diff --git a/tests/test_client_functional.py b/tests/test_client_functional.py index 6157488846f..309193ce1f6 100644 --- a/tests/test_client_functional.py +++ b/tests/test_client_functional.py @@ -251,7 +251,7 @@ def test_POST_DATA(self): loop=self.loop)) self.assertEqual(r.status, 200) - content = self.loop.run_until_complete(r.read(True)) + content = self.loop.run_until_complete(r.json()) self.assertEqual({'some': ['data']}, content['form']) self.assertEqual(r.status, 200) r.close() @@ -265,7 +265,7 @@ def test_POST_DATA_DEFLATE(self): loop=self.loop)) self.assertEqual(r.status, 200) - content = self.loop.run_until_complete(r.read(True)) + content = self.loop.run_until_complete(r.json()) self.assertEqual('deflate', content['compression']) self.assertEqual({'some': ['data']}, content['form']) self.assertEqual(r.status, 200) @@ -281,7 +281,7 @@ def test_POST_FILES(self): 'post', url, files={'some': f}, chunked=1024, headers={'Transfer-Encoding': 'chunked'}, loop=self.loop)) - content = self.loop.run_until_complete(r.read(True)) + content = self.loop.run_until_complete(r.json()) f.seek(0) filename = os.path.split(f.name)[-1] @@ -306,7 +306,7 @@ def test_POST_FILES_DEFLATE(self): chunked=1024, compress='deflate', loop=self.loop)) - content = self.loop.run_until_complete(r.read(True)) + content = self.loop.run_until_complete(r.json()) f.seek(0) filename = os.path.split(f.name)[-1] @@ -331,7 +331,7 @@ def test_POST_FILES_STR(self): client.request('post', url, files=[('some', f.read())], loop=self.loop)) - content = self.loop.run_until_complete(r.read(True)) + content = self.loop.run_until_complete(r.json()) f.seek(0) self.assertEqual(1, len(content['multipart-data'])) @@ -353,7 +353,7 @@ def test_POST_FILES_LIST(self): client.request('post', url, files=[('some', f)], loop=self.loop)) - content = self.loop.run_until_complete(r.read(True)) + content = self.loop.run_until_complete(r.json()) f.seek(0) filename = os.path.split(f.name)[-1] @@ -377,7 +377,7 @@ def test_POST_FILES_LIST_CT(self): client.request('post', url, loop=self.loop, files=[('some', f, 'text/plain')])) - content = self.loop.run_until_complete(r.read(True)) + content = self.loop.run_until_complete(r.json()) f.seek(0) filename = os.path.split(f.name)[-1] @@ -402,7 +402,7 @@ def test_POST_FILES_SINGLE(self): r = self.loop.run_until_complete( client.request('post', url, files=[f], loop=self.loop)) - content = self.loop.run_until_complete(r.read(True)) + content = self.loop.run_until_complete(r.json()) f.seek(0) filename = os.path.split(f.name)[-1] @@ -426,7 +426,7 @@ def test_POST_FILES_IO(self): r = self.loop.run_until_complete( client.request('post', url, files=[data], loop=self.loop)) - content = self.loop.run_until_complete(r.read(True)) + content = self.loop.run_until_complete(r.json()) self.assertEqual(1, len(content['multipart-data'])) self.assertEqual( @@ -446,7 +446,7 @@ def test_POST_FILES_WITH_DATA(self): client.request('post', url, loop=self.loop, data={'test': 'true'}, files={'some': f})) - content = self.loop.run_until_complete(r.read(True)) + content = self.loop.run_until_complete(r.json()) self.assertEqual(2, len(content['multipart-data'])) self.assertEqual( @@ -486,7 +486,7 @@ def stream(): 'post', url, data=stream(), headers={'Content-Length': str(len(data))}, loop=self.loop)) - content = self.loop.run_until_complete(r.read(True)) + content = self.loop.run_until_complete(r.json()) r.close() self.assertEqual(str(len(data)), @@ -500,7 +500,7 @@ def test_expect_continue(self): expect100=True, loop=self.loop)) self.assertEqual(r.status, 200) - content = self.loop.run_until_complete(r.read(True)) + content = self.loop.run_until_complete(r.json()) self.assertEqual('100-continue', content['headers']['Expect']) self.assertEqual(r.status, 200) r.close() @@ -554,7 +554,7 @@ def test_chunked(self): client.request('get', httpd.url('chunked'), loop=self.loop)) self.assertEqual(r.status, 200) self.assertEqual(r.headers.getone('TRANSFER-ENCODING'), 'chunked') - content = self.loop.run_until_complete(r.read(True)) + content = self.loop.run_until_complete(r.json()) self.assertEqual(content['path'], '/chunked') r.close() @@ -565,7 +565,7 @@ def test_broken_connection(self): self.assertEqual(r.status, 200) self.assertRaises( aiohttp.IncompleteRead, - self.loop.run_until_complete, r.read(True)) + self.loop.run_until_complete, r.json()) r.close() def test_request_conn_error(self): @@ -593,7 +593,7 @@ def test_keepalive(self): client.request('get', httpd.url('keepalive',), connector=c, loop=self.loop)) self.assertEqual(r.status, 200) - content = self.loop.run_until_complete(r.read(True)) + content = self.loop.run_until_complete(r.json()) self.assertEqual(content['content'], 'requests=1') r.close() @@ -601,7 +601,7 @@ def test_keepalive(self): client.request('get', httpd.url('keepalive'), connector=c, loop=self.loop)) self.assertEqual(r.status, 200) - content = self.loop.run_until_complete(r.read(True)) + content = self.loop.run_until_complete(r.json()) self.assertEqual(content['content'], 'requests=2') r.close() @@ -616,7 +616,7 @@ def test_session_close(self): 'get', httpd.url('keepalive') + '?close=1', connector=conn, loop=self.loop)) self.assertEqual(r.status, 200) - content = self.loop.run_until_complete(r.read(True)) + content = self.loop.run_until_complete(r.json()) self.assertEqual(content['content'], 'requests=1') r.close() @@ -624,7 +624,7 @@ def test_session_close(self): client.request('get', httpd.url('keepalive'), connector=conn, loop=self.loop)) self.assertEqual(r.status, 200) - content = self.loop.run_until_complete(r.read(True)) + content = self.loop.run_until_complete(r.json()) self.assertEqual(content['content'], 'requests=1') r.close() @@ -641,7 +641,7 @@ def test_session_cookies(self, m_log): client.request('get', httpd.url('cookies'), connector=conn, loop=self.loop)) self.assertEqual(r.status, 200) - content = self.loop.run_until_complete(r.read(True)) + content = self.loop.run_until_complete(r.json()) self.assertEqual(content['headers']['Cookie'], 'test=1') r.close() @@ -663,7 +663,7 @@ def test_multidict_headers(self): 'post', url, data=data, headers=MultiDict({'Content-Length': str(len(data))}), loop=self.loop)) - content = self.loop.run_until_complete(r.read(True)) + content = self.loop.run_until_complete(r.json()) r.close() self.assertEqual(str(len(data)), From ce2c34f56dc25985e423213a712ed5d2177e039d Mon Sep 17 00:00:00 2001 From: Alexander Shorin Date: Thu, 3 Jul 2014 19:15:53 +0400 Subject: [PATCH 4/5] Respect charset definition in Content-Type for JSON decoding --- aiohttp/client.py | 6 +++-- aiohttp/helpers.py | 53 ++++++++++++++++++++++++++++++++++++++++++++ tests/test_client.py | 18 +++++++++++++++ 3 files changed, 75 insertions(+), 2 deletions(-) create mode 100644 aiohttp/helpers.py diff --git a/aiohttp/client.py b/aiohttp/client.py index b30f1af3644..4c057830110 100644 --- a/aiohttp/client.py +++ b/aiohttp/client.py @@ -20,6 +20,7 @@ import warnings import aiohttp +from .helpers import parse_mimetype from .log import client_log from .multidict import CaseInsensitiveMultiDict, MultiDict, MutableMultiDict @@ -741,14 +742,15 @@ def json(self): yield from self.read() ctype = self.headers.get('CONTENT-TYPE', '').lower() - if not ctype.startswith('application/json'): + mtype, stype, _, params = parse_mimetype(ctype) + if not (mtype == 'application' or stype == 'json'): client_log.warning( 'Attempt to decode JSON with unexpected mimetype: %s', ctype) if not self._content.strip(): return None - return json.loads(self._content.decode('utf-8')) + return json.loads(self._content.decode(params.get('charset', 'utf-8'))) def str_to_bytes(s, encoding='utf-8'): diff --git a/aiohttp/helpers.py b/aiohttp/helpers.py new file mode 100644 index 00000000000..d5c2099f133 --- /dev/null +++ b/aiohttp/helpers.py @@ -0,0 +1,53 @@ +"""Various helper functions""" + + +def parse_mimetype(mimetype): + """Parses a MIME type into it components. + + :param str mimetype: MIME type + + :returns: 4 element tuple for MIME type, subtype, suffix and parameters + :rtype: tuple + + >>> parse_mimetype('*') + ('*', '*', '', {}) + + >>> parse_mimetype('application/json') + ('application', 'json', '', {}) + + >>> parse_mimetype('application/json; charset=utf-8') + ('application', 'json', '', {'charset': 'utf-8'}) + + >>> parse_mimetype('''application/json; + ... charset=utf-8;''') + ('application', 'json', '', {'charset': 'utf-8'}) + + >>> parse_mimetype('ApPlIcAtIoN/JSON;ChaRseT="UTF-8"') + ('application', 'json', '', {'charset': 'UTF-8'}) + + >>> parse_mimetype('application/rss+xml') + ('application', 'rss', 'xml', {}) + + >>> parse_mimetype('text/plain;base64') + ('text', 'plain', '', {'base64': ''}) + + """ + if not mimetype: + return '', '', '', {} + + parts = mimetype.split(';') + params = [] + for item in parts[1:]: + if not item: + continue + key, value = item.split('=', 2) if '=' in item else (item, '') + params.append((key.lower().strip(), value.strip(' "'))) + params = dict(params) + + fulltype = parts[0].strip().lower() + if fulltype == '*': + fulltype = '*/*' + mtype, stype = fulltype.split('/', 2) if '/' in fulltype else (fulltype, '') + stype, suffix = stype.split('+') if '+' in stype else (stype, '') + + return mtype, stype, suffix, params diff --git a/tests/test_client.py b/tests/test_client.py index a1b0e9d2be5..85f821827f2 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -117,6 +117,24 @@ def test_release(self): self.loop.run_until_complete(self.response.release()) self.assertTrue(self.response.close.called) + def test_json(self): + def side_effect(*args, **kwargs): + def second_call(*args, **kwargs): + raise aiohttp.EofStream + fut = asyncio.Future(loop=self.loop) + fut.set_result('{"тест": "пройден"}'.encode('cp1251')) + content.read.side_effect = second_call + return fut + self.response.headers = { + 'CONTENT-TYPE': 'application/json;charset=cp1251'} + content = self.response.content = unittest.mock.Mock() + content.read.side_effect = side_effect + self.response.close = unittest.mock.Mock() + + res = self.loop.run_until_complete(self.response.json()) + self.assertEqual(res, {'тест': 'пройден'}) + self.assertTrue(self.response.close.called) + class ClientRequestTests(unittest.TestCase): From bacc6355e2485441a89a57c706133dfadebfe40d Mon Sep 17 00:00:00 2001 From: Alexander Shorin Date: Thu, 3 Jul 2014 19:19:24 +0400 Subject: [PATCH 5/5] Allow to pass custom JSON encoding to override Content-Type charset --- aiohttp/client.py | 5 +++-- tests/test_client.py | 19 +++++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/aiohttp/client.py b/aiohttp/client.py index 4c057830110..dae2e092396 100644 --- a/aiohttp/client.py +++ b/aiohttp/client.py @@ -736,7 +736,7 @@ def read_and_close(self, decode=False): return (yield from self.read(decode)) @asyncio.coroutine - def json(self): + def json(self, *, encoding=None): """Reads and decodes JSON response.""" if self._content is None: yield from self.read() @@ -750,7 +750,8 @@ def json(self): if not self._content.strip(): return None - return json.loads(self._content.decode(params.get('charset', 'utf-8'))) + encoding = encoding or params.get('charset', 'utf-8') + return json.loads(self._content.decode(encoding)) def str_to_bytes(s, encoding='utf-8'): diff --git a/tests/test_client.py b/tests/test_client.py index 85f821827f2..39e4658f42e 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -135,6 +135,25 @@ def second_call(*args, **kwargs): self.assertEqual(res, {'тест': 'пройден'}) self.assertTrue(self.response.close.called) + def test_json_override_encoding(self): + def side_effect(*args, **kwargs): + def second_call(*args, **kwargs): + raise aiohttp.EofStream + fut = asyncio.Future(loop=self.loop) + fut.set_result('{"тест": "пройден"}'.encode('cp1251')) + content.read.side_effect = second_call + return fut + self.response.headers = { + 'CONTENT-TYPE': 'application/json;charset=utf8'} + content = self.response.content = unittest.mock.Mock() + content.read.side_effect = side_effect + self.response.close = unittest.mock.Mock() + + res = self.loop.run_until_complete( + self.response.json(encoding='cp1251')) + self.assertEqual(res, {'тест': 'пройден'}) + self.assertTrue(self.response.close.called) + class ClientRequestTests(unittest.TestCase):