Repository: falconry/falcon
Pull request #1311 (2018-05-20 19:06:19)

Providing of real content-type to media handlers.
#1312 
Providing of real content-type to media handlers for deserializing and serializing media by corresponding to real content-type or specific parameters (such as "boundary" for multipart/form-data). Also provided content-length for special cases.

For avoiding reading big data to memory to media handlers now providing bounded stream instead raw data. Also consumption of stream proceed by small chunks.

codecov[bot]: # [Codecov](https://codecov.io/gh/falconry/falcon/pull/1311?src=pr&el=h1) Report
> Merging [#1311](https://codecov.io/gh/falconry/falcon/pull/1311?src=pr&el=desc) into [master](https://codecov.io/gh/falconry/falcon/commit/a560b2a54a2f748a09ea897e42fde6a5d0ffc27a?src=pr&el=desc) will **decrease** coverage by `0.03%`.
> The diff coverage is `92.3%`.

[![Impacted file tree graph](https://codecov.io/gh/falconry/falcon/pull/1311/graphs/tree.svg?height=150&width=650&src=pr&token=zOtkfNQ3h6)](https://codecov.io/gh/falconry/falcon/pull/1311?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master    #1311      +/-   ##
==========================================
- Coverage     100%   99.96%   -0.04%     
==========================================
  Files          38       38              
  Lines        2516     2519       +3     
  Branches      385      386       +1     
==========================================
+ Hits         2516     2518       +2     
- Partials        0        1       +1
```


| [Impacted Files](https://codecov.io/gh/falconry/falcon/pull/1311?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [falcon/media/base.py](https://codecov.io/gh/falconry/falcon/pull/1311/diff?src=pr&el=tree#diff-ZmFsY29uL21lZGlhL2Jhc2UucHk=) | `100% <ø> (ø)` | :arrow_up: |
| [falcon/response.py](https://codecov.io/gh/falconry/falcon/pull/1311/diff?src=pr&el=tree#diff-ZmFsY29uL3Jlc3BvbnNlLnB5) | `100% <100%> (ø)` | :arrow_up: |
| [falcon/media/json.py](https://codecov.io/gh/falconry/falcon/pull/1311/diff?src=pr&el=tree#diff-ZmFsY29uL21lZGlhL2pzb24ucHk=) | `100% <100%> (ø)` | :arrow_up: |
| [falcon/media/msgpack.py](https://codecov.io/gh/falconry/falcon/pull/1311/diff?src=pr&el=tree#diff-ZmFsY29uL21lZGlhL21zZ3BhY2sucHk=) | `100% <100%> (ø)` | :arrow_up: |
| [falcon/request.py](https://codecov.io/gh/falconry/falcon/pull/1311/diff?src=pr&el=tree#diff-ZmFsY29uL3JlcXVlc3QucHk=) | `99.81% <83.33%> (-0.19%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/falconry/falcon/pull/1311?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/falconry/falcon/pull/1311?src=pr&el=footer). Last update [a560b2a...d289167](https://codecov.io/gh/falconry/falcon/pull/1311?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

DmitriiTrofimov: @kgriffs All is done. Thanks for the review!

Patch:
diff --git a/falcon/media/base.py b/falcon/media/base.py
index 3592f44..2c18391 100644
--- a/falcon/media/base.py
+++ b/falcon/media/base.py
@@ -8,22 +8,25 @@ class BaseHandler(object):
     """Abstract Base Class for an internet media type handler"""
 
     @abc.abstractmethod  # pragma: no cover
-    def serialize(self, obj):
+    def serialize(self, media, content_type):
         """Serialize the media object on a :any:`falcon.Response`
 
         Args:
-            obj (object): A serializable object.
+            media (object): A serializable object.
+            content_type (str): Type of response content.
 
         Returns:
             bytes: The resulting serialized bytes from the input object.
         """
 
     @abc.abstractmethod  # pragma: no cover
-    def deserialize(self, raw):
+    def deserialize(self, stream, content_type, content_length):
         """Deserialize the :any:`falcon.Request` body.
 
         Args:
-            raw (bytes): Input bytes to deserialize
+            stream (object): Input data to deserialize.
+            content_type (str): Type of request content.
+            content_length (int): Length of request content.
 
         Returns:
             object: A deserialized object.
diff --git a/falcon/media/json.py b/falcon/media/json.py
index 42c9c18..7b01c73 100644
--- a/falcon/media/json.py
+++ b/falcon/media/json.py
@@ -10,16 +10,16 @@ from falcon.util import json
 class JSONHandler(BaseHandler):
     """Handler built using Python's :py:mod:`json` module."""
 
-    def deserialize(self, raw):
+    def deserialize(self, stream, content_type, content_length):
         try:
-            return json.loads(raw.decode('utf-8'))
+            return json.loads(stream.read().decode('utf-8'))
         except ValueError as err:
             raise errors.HTTPBadRequest(
                 'Invalid JSON',
                 'Could not parse JSON body - {0}'.format(err)
             )
 
-    def serialize(self, media):
+    def serialize(self, media, content_type):
         result = json.dumps(media, ensure_ascii=False)
         if six.PY3 or not isinstance(result, bytes):
             return result.encode('utf-8')
diff --git a/falcon/media/msgpack.py b/falcon/media/msgpack.py
index a30281b..ba4dd9a 100644
--- a/falcon/media/msgpack.py
+++ b/falcon/media/msgpack.py
@@ -31,16 +31,16 @@ class MessagePackHandler(BaseHandler):
             use_bin_type=True,
         )
 
-    def deserialize(self, raw):
+    def deserialize(self, stream, content_type, content_length):
         try:
             # NOTE(jmvrbanac): Using unpackb since we would need to manage
             # a buffer for Unpacker() which wouldn't gain us much.
-            return self.msgpack.unpackb(raw, encoding='utf-8')
+            return self.msgpack.unpackb(stream.read(), encoding='utf-8')
         except ValueError as err:
             raise errors.HTTPBadRequest(
                 'Invalid MessagePack',
                 'Could not parse MessagePack body - {0}'.format(err)
             )
 
-    def serialize(self, media):
+    def serialize(self, media, content_type):
         return self.packer.pack(media)
diff --git a/falcon/request.py b/falcon/request.py
index f3f7a9d..b26cb46 100644
--- a/falcon/request.py
+++ b/falcon/request.py
@@ -941,7 +941,7 @@ class Request(object):
 
     @property
     def media(self):
-        if self._media:
+        if self._media is not None or self.bounded_stream.is_exhausted:
             return self._media
 
         handler = self.options.media_handlers.find_by_media_type(
@@ -949,11 +949,15 @@ class Request(object):
             self.options.default_media_type
         )
 
-        # Consume the stream
-        raw = self.bounded_stream.read()
+        try:
+            self._media = handler.deserialize(
+                self.bounded_stream,
+                self.content_type,
+                self.content_length
+            )
+        finally:
+            self.bounded_stream.exhaust()
 
-        # Deserialize and Return
-        self._media = handler.deserialize(raw)
         return self._media
 
     # ------------------------------------------------------------------------
diff --git a/falcon/request_helpers.py b/falcon/request_helpers.py
index b2bf5e6..13b9a1e 100644
--- a/falcon/request_helpers.py
+++ b/falcon/request_helpers.py
@@ -157,6 +157,25 @@ class BoundedStream(io.IOBase):
 
         raise IOError('Stream is not writeable')
 
+    def exhaust(self, chunk_size=64 * 1024):
+        """Exhaust the stream.
+
+        This consumes all the data left until the limit is reached.
+
+        Args:
+            chunk_size (int): The size for a chunk (default: 64 KB).
+                It will read the chunk until the stream is exhausted.
+        """
+        while True:
+            chunk = self.read(chunk_size)
+            if not chunk:
+                break
+
+    @property
+    def is_exhausted(self):
+        """If the stream is exhausted this attribute is ``True``."""
+        return self._bytes_remaining <= 0
+
 
 # NOTE(kgriffs): Alias for backwards-compat
 Body = BoundedStream
diff --git a/falcon/response.py b/falcon/response.py
index 9ef1aca..fbdc4e6 100644
--- a/falcon/response.py
+++ b/falcon/response.py
@@ -192,7 +192,10 @@ class Response(object):
 
         # NOTE(kgriffs): Set _data to avoid re-serializing if the
         # data() property is called multiple times.
-        self._data = handler.serialize(self._media)
+        self._data = handler.serialize(
+            self._media,
+            self.content_type
+        )
         return self._data
 
     @data.setter


Test patch:
diff --git a/tests/test_request_media.py b/tests/test_request_media.py
index b2b4f37..0f57bc8 100644
--- a/tests/test_request_media.py
+++ b/tests/test_request_media.py
@@ -69,7 +69,7 @@ def test_msgpack(media_type):
 def test_unknown_media_type(media_type):
     client = create_client()
     headers = {'Content-Type': media_type}
-    client.simulate_post('/', body='', headers=headers)
+    client.simulate_post('/', body=b'something', headers=headers)
 
     with pytest.raises(errors.HTTPUnsupportedMediaType) as err:
         client.resource.captured_req.media
@@ -78,6 +78,19 @@ def test_unknown_media_type(media_type):
     assert err.value.description == msg
 
 
+@pytest.mark.parametrize('media_type', [
+    ('application/json'),
+])
+def test_exhausted_stream(media_type):
+    client = create_client({
+        'application/json': media.JSONHandler(),
+    })
+    headers = {'Content-Type': media_type}
+    client.simulate_post('/', body='', headers=headers)
+
+    assert client.resource.captured_req.media is None
+
+
 def test_invalid_json():
     client = create_client()
     expected_body = b'{'
@@ -111,10 +124,7 @@ def test_invalid_stream_fails_gracefully():
     req.headers['Content-Type'] = 'application/json'
     req._bounded_stream = None
 
-    with pytest.raises(errors.HTTPBadRequest) as err:
-        req.media
-
-    assert 'Could not parse JSON body' in err.value.description
+    assert req.media is None
 
 
 def test_use_cached_media():
@@ -125,3 +135,27 @@ def test_use_cached_media():
     req._media = {'something': True}
 
     assert req.media == {'something': True}
+
+
+class NopeHandler(media.BaseHandler):
+
+    def serialize(self, *args, **kwargs):
+        pass
+
+    def deserialize(self, *args, **kwargs):
+        pass
+
+
+def test_complete_consumption():
+    client = create_client({
+        'nope/nope': NopeHandler()
+    })
+    body = b'{"something": "abracadabra"}'
+    headers = {'Content-Type': 'nope/nope'}
+
+    client.simulate_post('/', body=body, headers=headers)
+
+    req_media = client.resource.captured_req.media
+    assert req_media is None
+    req_bounded_stream = client.resource.captured_req.bounded_stream
+    assert not req_bounded_stream.read()