Skip to content

Commit

Permalink
Unify behavior between asgi and wsgi (#1695)
Browse files Browse the repository at this point in the history
* refactor(requrest): avoid duplication of prefix, add get_media to wsgi method

* refactor(response): add render_body to wsgi response.

Also unify data and media behavior between asgi and wsgi

* test(response): additional tests to render_body

* docs: add newsfragment

* docs: fix typo in changelog

* doc(falcon.Response): Expand news fragment for render_body()

* doc(Response): Update docstrings for render_body()

* doc(Response): Update docstrings for media and get_body()

* doc(Request): Add news fragment re get_media()

Fixes #1679
Partially-Implements #1358

Co-authored-by: Vytautas Liuolia <vytautas.liuolia@gmail.com>
Co-authored-by: Kurt Griffiths <mail@kgriffs.com>
  • Loading branch information
3 people authored Apr 8, 2020
1 parent b437e5e commit 09e1220
Show file tree
Hide file tree
Showing 14 changed files with 238 additions and 129 deletions.
7 changes: 7 additions & 0 deletions docs/_newsfragments/1679.breakingchange.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
The :attr:`falcon.Response.data` property now just simply returns the same data
object that it was set to, if any, rather than also checking and serializing
the value of the :attr:`falcon.Response.media` property. Instead, a new
:meth:`~falcon.Response.render_body` method has been implemented, which can be
used to obtain the HTTP response body for the request, taking into account
the :attr:`~falcon.Response.body`, :attr:`~falcon.Response.data`, and
:attr:`~falcon.Response.media` attributes.
5 changes: 5 additions & 0 deletions docs/_newsfragments/1679.misc.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
A new method, :meth:`~falcon.Request.get_media`, was added that can now be used
instead of the :attr:`falcon.Request.media` property to make it more clear to
app maintainers that getting the media object for a request involves a
side-effect of consuming and deserializing the body stream. The original
property remains available to ensure backwards-compatibility with existing apps.
1 change: 1 addition & 0 deletions docs/api/request_and_response_asgi.rst
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ Request
.. autoclass:: falcon.asgi.Request
:members:
:inherited-members:
:exclude-members: media

.. autoclass:: falcon.asgi.BoundedStream
:members:
Expand Down
2 changes: 2 additions & 0 deletions docs/api/request_and_response_wsgi.rst
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ Request

.. autoclass:: falcon.Request
:members:
:exclude-members: media


.. autoclass:: falcon.Forwarded
:members:
Expand Down
5 changes: 0 additions & 5 deletions falcon/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,6 @@
"""Set to ``True`` when ASGI is supported for the current Python version."""


# NOTE(kgriffs): Special singleton to be used internally whenever using
# None would be ambiguous.
_UNSET = object()


# NOTE(kgriffs): Only to be used internally on the rare occasion that we
# need to log something that we can't communicate any other way.
_logger = _logging.getLogger('falcon')
Expand Down
14 changes: 3 additions & 11 deletions falcon/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -969,24 +969,16 @@ def _get_body(self, resp, wsgi_file_wrapper=None):
The length is returned as ``None`` when unknown. The
iterable is determined as follows:
* If resp.body is not ``None``, returns
([resp.body], len(resp.body)),
encoded as UTF-8 if it is a Unicode string.
Bytestrings are returned as-is.
* If resp.data is not ``None``, returns ([resp.data], len(resp.data))
* If the result of render_body() is not ``None``, returns
([data], len(data))
* If resp.stream is not ``None``, returns resp.stream
iterable using wsgi.file_wrapper, if necessary:
(closeable_iterator, None)
* Otherwise, returns ([], 0)
"""
body = resp.body
if body is not None:
if not isinstance(body, bytes):
body = body.encode('utf-8')
return [body], len(body)

data = resp.data
data = resp.render_body()
if data is not None:
return [data], len(data)

Expand Down
21 changes: 5 additions & 16 deletions falcon/asgi/request.py
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ class Request(falcon.request.Request):
See also: :class:`falcon.asgi.BoundedStream`
media (object): An awaitable property that acts as an alias for
:meth:`~.get_media`. This can be used to ease the porting of a
:meth:`~.get_media`. This can be used to ease the porting of
a WSGI app to ASGI, although the ``await`` keyword must still be
added when referencing the property::
Expand Down Expand Up @@ -566,17 +566,6 @@ def forwarded_scheme(self):

return scheme

@property
def prefix(self):
if self._cached_prefix is None:
self._cached_prefix = (
self.scheme + '://' +
self.netloc +
self.app
)

return self._cached_prefix

@property
def host(self):
try:
Expand Down Expand Up @@ -691,9 +680,9 @@ def netloc(self):
async def get_media(self):
"""Returns a deserialized form of the request stream.
When called the first time, the request stream will be deserialized
using the Content-Type header as well as the media-type handlers
configured via :class:`falcon.RequestOptions`. The result will
The first time this method is called, the request stream will be
deserialized using the Content-Type header as well as the media-type
handlers configured via :class:`falcon.RequestOptions`. The result will
be cached and returned in subsequent calls::
deserialized_media = await req.get_media()
Expand All @@ -702,7 +691,7 @@ async def get_media(self):
deserialize the request body, the exception will propagate up
to the caller.
See :ref:`media` for more information regarding media handling.
See also :ref:`media` for more information regarding media handling.
Warning:
This operation will consume the request stream the first time
Expand Down
45 changes: 12 additions & 33 deletions falcon/asgi/response.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
from asyncio.coroutines import CoroWrapper # type: ignore
from inspect import iscoroutine, iscoroutinefunction

from falcon import _UNSET
from falcon.constants import _UNSET
import falcon.response
from falcon.util.misc import is_python_func

Expand Down Expand Up @@ -68,14 +68,6 @@ class Response(falcon.response.Response):
ensure Unicode characters are properly encoded in the
HTTP response.
Note:
Unlike the WSGI Response class, the ASGI Response class
does not implement the side-effect of serializing
the media object (if one is set) when the `data`
attribute is read. Instead,
:py:meth:`falcon.asgi.Response.render_body` should
be used to get the correct content for the response.
stream: An async iterator or generator that yields a series of
byte strings that will be streamed to the ASGI server as a
series of "http.response.body" events. Falcon will assume the
Expand Down Expand Up @@ -199,7 +191,6 @@ async def emitter():
# an additional function call.
_sse = None
_registered_callbacks = None
_media_rendered = _UNSET

@property
def sse(self):
Expand All @@ -209,23 +200,6 @@ def sse(self):
def sse(self, value):
self._sse = value

@property
def media(self):
return self._media

@media.setter
def media(self, value):
self._media = value
self._media_rendered = _UNSET

@property
def data(self):
return self._data

@data.setter
def data(self, value):
self._data = value

def set_stream(self, stream, content_length):
"""Convenience method for setting both `stream` and `content_length`.
Expand Down Expand Up @@ -258,16 +232,21 @@ def set_stream(self, stream, content_length):
self._headers['content-length'] = str(content_length)

async def render_body(self):
"""Get the raw content for the response body.
"""Get the raw bytestring content for the response body.
This coroutine can be awaited to get the raw body data that should
be returned in the HTTP response.
This coroutine can be awaited to get the raw data for the
HTTP response body, taking into account the :attr:`~.body`,
:attr:`~.data`, and :attr:`~.media` attributes.
Note:
This method ignores :attr:`~.stream`; the caller must check
and handle that attribute directly.
Returns:
bytes: The UTF-8 encoded value of the `body` attribute, if
set. Otherwise, the value of the `data` attribute if set, or
finally the serialized value of the `media` attribute. If
none of these attributes are set, ``None`` is returned.
set. Otherwise, the value of the `data` attribute if set, or
finally the serialized value of the `media` attribute. If
none of these attributes are set, ``None`` is returned.
"""

body = self.body
Expand Down
4 changes: 4 additions & 0 deletions falcon/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,3 +106,7 @@
'referer',
'user-agent',
])

# NOTE(kgriffs): Special singleton to be used internally whenever using
# None would be ambiguous.
_UNSET = object()
43 changes: 30 additions & 13 deletions falcon/request.py
Original file line number Diff line number Diff line change
Expand Up @@ -332,17 +332,13 @@ class Request:
doc = json.load(req.bounded_stream)
media (object): Returns a deserialized form of the request stream.
When called, it will attempt to deserialize the request stream
using the Content-Type header as well as the media-type handlers
configured via :class:`falcon.RequestOptions`.
media (object): Property that acts as an alias for
:meth:`~.get_media`. This alias provides backwards-compatibility
for apps that were built for versions of the framework prior to
3.0::
See :ref:`media` for more information regarding media handling.
Warning:
This operation will consume the request stream the first time
it's called and cache the results. Follow-up calls will just
retrieve a cached version of the object.
# Equivalent to: deserialized_media = req.get_media()
deserialized_media = req.media
expect (str): Value of the Expect header, or ``None`` if the
header is missing.
Expand Down Expand Up @@ -793,7 +789,7 @@ def relative_uri(self):
def prefix(self):
if self._cached_prefix is None:
self._cached_prefix = (
self.env['wsgi.url_scheme'] + '://' +
self.scheme + '://' +
self.netloc +
self.app
)
Expand Down Expand Up @@ -975,8 +971,27 @@ def netloc(self):

return netloc_value

@property
def media(self):
def get_media(self):
"""Returns a deserialized form of the request stream.
The first time this method is called, the request stream will be
deserialized using the Content-Type header as well as the media-type
handlers configured via :class:`falcon.RequestOptions`. The result will
be cached and returned in subsequent calls::
deserialized_media = req.get_media()
If the matched media handler raises an error while attempting to
deserialize the request body, the exception will propagate up
to the caller.
See also :ref:`media` for more information regarding media handling.
Warning:
This operation will consume the request stream the first time
it's called and cache the results. Follow-up calls will just
retrieve a cached version of the object.
"""
if self._media is not None or self.bounded_stream.eof:
return self._media

Expand All @@ -997,6 +1012,8 @@ def media(self):

return self._media

media = property(get_media)

# ------------------------------------------------------------------------
# Methods
# ------------------------------------------------------------------------
Expand Down
Loading

0 comments on commit 09e1220

Please sign in to comment.