Repository: falconry/falcon
Pull request #1877 (2021-03-04 19:47:38)

fix(asgi.App): only inline Response.render_body() when it's safe to
In response to @kgriffs' comment on Gitter:
> BTW I just realized that inlining `render_body()` prevents people from easily overriding that logic in a subclass but I guess we can wait to see if anyone actually needs to do that in practice.

codecov[bot]: # [Codecov](https://codecov.io/gh/falconry/falcon/pull/1877?src=pr&el=h1) Report
> Merging [#1877](https://codecov.io/gh/falconry/falcon/pull/1877?src=pr&el=desc) (ac6f399) into [master](https://codecov.io/gh/falconry/falcon/commit/70d94d4ec49f4b11e7a8c9e5c7583c9c3c271125?el=desc) (70d94d4) will **decrease** coverage by `64.02%`.
> The diff coverage is `5.26%`.

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

```diff
@@             Coverage Diff              @@
##            master    #1877       +/-   ##
============================================
- Coverage   100.00%   35.97%   -64.03%     
============================================
  Files           62       62               
  Lines         6582     6585        +3     
  Branches      1064     1065        +1     
============================================
- Hits          6582     2369     -4213     
- Misses           0     4163     +4163     
- Partials         0       53       +53     
```


| [Impacted Files](https://codecov.io/gh/falconry/falcon/pull/1877?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [falcon/asgi/app.py](https://codecov.io/gh/falconry/falcon/pull/1877/diff?src=pr&el=tree#diff-ZmFsY29uL2FzZ2kvYXBwLnB5) | `23.05% <5.26%> (-76.95%)` | :arrow_down: |
| [falcon/util/reader.py](https://codecov.io/gh/falconry/falcon/pull/1877/diff?src=pr&el=tree#diff-ZmFsY29uL3V0aWwvcmVhZGVyLnB5) | `12.62% <0.00%> (-87.38%)` | :arrow_down: |
| [falcon/asgi/stream.py](https://codecov.io/gh/falconry/falcon/pull/1877/diff?src=pr&el=tree#diff-ZmFsY29uL2FzZ2kvc3RyZWFtLnB5) | `13.58% <0.00%> (-86.42%)` | :arrow_down: |
| [falcon/asgi/structures.py](https://codecov.io/gh/falconry/falcon/pull/1877/diff?src=pr&el=tree#diff-ZmFsY29uL2FzZ2kvc3RydWN0dXJlcy5weQ==) | `14.58% <0.00%> (-85.42%)` | :arrow_down: |
| [falcon/asgi/reader.py](https://codecov.io/gh/falconry/falcon/pull/1877/diff?src=pr&el=tree#diff-ZmFsY29uL2FzZ2kvcmVhZGVyLnB5) | `16.41% <0.00%> (-83.59%)` | :arrow_down: |
| [falcon/middleware.py](https://codecov.io/gh/falconry/falcon/pull/1877/diff?src=pr&el=tree#diff-ZmFsY29uL21pZGRsZXdhcmUucHk=) | `18.36% <0.00%> (-81.64%)` | :arrow_down: |
| [falcon/testing/helpers.py](https://codecov.io/gh/falconry/falcon/pull/1877/diff?src=pr&el=tree#diff-ZmFsY29uL3Rlc3RpbmcvaGVscGVycy5weQ==) | `19.63% <0.00%> (-80.37%)` | :arrow_down: |
| [falcon/request.py](https://codecov.io/gh/falconry/falcon/pull/1877/diff?src=pr&el=tree#diff-ZmFsY29uL3JlcXVlc3QucHk=) | `23.44% <0.00%> (-76.56%)` | :arrow_down: |
| [falcon/forwarded.py](https://codecov.io/gh/falconry/falcon/pull/1877/diff?src=pr&el=tree#diff-ZmFsY29uL2ZvcndhcmRlZC5weQ==) | `25.00% <0.00%> (-75.00%)` | :arrow_down: |
| [falcon/asgi/multipart.py](https://codecov.io/gh/falconry/falcon/pull/1877/diff?src=pr&el=tree#diff-ZmFsY29uL2FzZ2kvbXVsdGlwYXJ0LnB5) | `25.00% <0.00%> (-75.00%)` | :arrow_down: |
| ... and [43 more](https://codecov.io/gh/falconry/falcon/pull/1877/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/falconry/falcon/pull/1877?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/1877?src=pr&el=footer). Last update [70d94d4...ac6f399](https://codecov.io/gh/falconry/falcon/pull/1877?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/falcon/asgi/app.py b/falcon/asgi/app.py
index 2dbaf79..34f357d 100644
--- a/falcon/asgi/app.py
+++ b/falcon/asgi/app.py
@@ -271,6 +271,7 @@ class App(falcon.app.App):
     _default_responder_path_not_found = falcon.responders.path_not_found_async
 
     __slots__ = (
+        '_standard_response_type',
         'ws_options',
     )
 
@@ -278,6 +279,7 @@ class App(falcon.app.App):
         super().__init__(*args, request_type=request_type, response_type=response_type, **kwargs)
 
         self.ws_options = WebSocketOptions()
+        self._standard_response_type = (response_type is Response)
 
         self.add_error_handler(WebSocketDisconnected, self._ws_disconnected_error_handler)
 
@@ -425,40 +427,49 @@ class App(falcon.app.App):
         data = b''
 
         try:
-            # PERF(vytas): inline Response.render_body() in this critical code
-            #   path in order to shave off an await.
-            text = resp.text
-            if text is None:
-                data = resp._data
-
-                if data is None and resp._media is not None:
-                    # NOTE(kgriffs): We use a special _UNSET singleton since
-                    #   None is ambiguous (the media handler might return None).
-                    if resp._media_rendered is _UNSET:
-                        if not resp.content_type:
-                            resp.content_type = resp.options.default_media_type
-
-                        handler, serialize_sync, _ = resp.options.media_handlers._resolve(
-                            resp.content_type,
-                            resp.options.default_media_type
-                        )
-
-                        if serialize_sync:
-                            resp._media_rendered = serialize_sync(resp._media)
-                        else:
-                            resp._media_rendered = await handler.serialize_async(
-                                resp._media,
-                                resp.content_type
+            # NOTE(vytas): It is only safe to inline Response.render_body()
+            #   where we can be sure it hasn't been overridden, either directly
+            #   or by modifying the behavior of its dependencies.
+            if self._standard_response_type:
+                # PERF(vytas): inline Response.render_body() in this critical code
+                #   path in order to shave off an await.
+                text = resp.text
+                if text is None:
+                    data = resp._data
+
+                    if data is None and resp._media is not None:
+                        # NOTE(kgriffs): We use a special _UNSET singleton since
+                        #   None is ambiguous (the media handler might return None).
+                        if resp._media_rendered is _UNSET:
+                            if not resp.content_type:
+                                resp.content_type = resp.options.default_media_type
+
+                            handler, serialize_sync, _ = resp.options.media_handlers._resolve(
+                                resp.content_type,
+                                resp.options.default_media_type
                             )
 
-                    data = resp._media_rendered
+                            if serialize_sync:
+                                resp._media_rendered = serialize_sync(resp._media)
+                            else:
+                                resp._media_rendered = await handler.serialize_async(
+                                    resp._media,
+                                    resp.content_type
+                                )
+
+                        data = resp._media_rendered
+                else:
+                    try:
+                        # NOTE(kgriffs): Normally we expect text to be a string
+                        data = text.encode()
+                    except AttributeError:
+                        # NOTE(kgriffs): Assume it was a bytes object already
+                        data = text
+
             else:
-                try:
-                    # NOTE(kgriffs): Normally we expect text to be a string
-                    data = text.encode()
-                except AttributeError:
-                    # NOTE(kgriffs): Assume it was a bytes object already
-                    data = text
+                # NOTE(vytas): Custom reponse type.
+                data = await resp.render_body()
+
         except Exception as ex:
             if not await self._handle_exception(req, resp, ex, params):
                 raise


Test patch:
diff --git a/tests/asgi/test_response_media_asgi.py b/tests/asgi/test_response_media_asgi.py
index 34ecebb..911ef01 100644
--- a/tests/asgi/test_response_media_asgi.py
+++ b/tests/asgi/test_response_media_asgi.py
@@ -282,3 +282,31 @@ def test_media_rendered_cached():
         assert first is not await resp.render_body()
 
     run_test(test)
+
+
+def test_custom_render_body():
+
+    class CustomResponse(falcon.asgi.Response):
+        async def render_body(self):
+            body = await super().render_body()
+
+            if not self.content_type.startswith('text/plain'):
+                return body
+
+            if not body.endswith(b'\n'):
+                # Be a good Unix netizen
+                return body + b'\n'
+
+            return body
+
+    class HelloResource:
+        async def on_get(self, req, resp):
+            resp.content_type = falcon.MEDIA_TEXT
+            resp.text = 'Hello, World!'
+
+    app = falcon.asgi.App(response_type=CustomResponse)
+    app.add_route('/', HelloResource())
+
+    resp = testing.simulate_get(app, '/')
+    assert resp.headers['Content-Type'] == 'text/plain; charset=utf-8'
+    assert resp.text == 'Hello, World!\n'