From dd22319b1b8df9b1772451035fca582bd666e218 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Fri, 6 Feb 2015 01:56:27 -0600 Subject: update render_to_response to prevent renderers from mutating request.response fixes #1536 --- pyramid/renderers.py | 43 +++++++++++++++++++++++++++-------------- pyramid/tests/test_renderers.py | 25 ++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 15 deletions(-) diff --git a/pyramid/renderers.py b/pyramid/renderers.py index 3c35551ea..c2be8c2eb 100644 --- a/pyramid/renderers.py +++ b/pyramid/renderers.py @@ -1,3 +1,4 @@ +import contextlib import json import os @@ -73,20 +74,8 @@ def render(renderer_name, value, request=None, package=None): helper = RendererHelper(name=renderer_name, package=package, registry=registry) - saved_response = None - # save the current response, preventing the renderer from affecting it - attrs = request.__dict__ if request is not None else {} - if 'response' in attrs: - saved_response = attrs['response'] - del attrs['response'] - - result = helper.render(value, None, request=request) - - # restore the original response, overwriting any changes - if saved_response is not None: - attrs['response'] = saved_response - elif 'response' in attrs: - del attrs['response'] + with temporary_response(request): + result = helper.render(value, None, request=request) return result @@ -134,7 +123,31 @@ def render_to_response(renderer_name, value, request=None, package=None): package = caller_package() helper = RendererHelper(name=renderer_name, package=package, registry=registry) - return helper.render_to_response(value, None, request=request) + + with temporary_response(request): + result = helper.render_to_response(value, None, request=request) + + return result + +@contextlib.contextmanager +def temporary_response(request): + """ + Temporarily delete request.response and restore it afterward. + """ + saved_response = None + # save the current response, preventing the renderer from affecting it + attrs = request.__dict__ if request is not None else {} + if 'response' in attrs: + saved_response = attrs['response'] + del attrs['response'] + + yield + + # restore the original response, overwriting any changes + if saved_response is not None: + attrs['response'] = saved_response + elif 'response' in attrs: + del attrs['response'] def get_renderer(renderer_name, package=None): """ Return the renderer object for the renderer ``renderer_name``. diff --git a/pyramid/tests/test_renderers.py b/pyramid/tests/test_renderers.py index 6d79cc291..31e9d14f8 100644 --- a/pyramid/tests/test_renderers.py +++ b/pyramid/tests/test_renderers.py @@ -554,6 +554,31 @@ class Test_render_to_response(unittest.TestCase): renderer.assert_(a=1) renderer.assert_(request=request) + def test_response_preserved(self): + request = testing.DummyRequest() + response = object() # should error if mutated + request.response = response + # use a json renderer, which will mutate the response + result = self._callFUT('json', dict(a=1), request=request) + self.assertEqual(result.body, b'{"a": 1}') + self.assertNotEqual(request.response, result) + self.assertEqual(request.response, response) + + def test_no_response_to_preserve(self): + from pyramid.decorator import reify + class DummyRequestWithClassResponse(object): + _response = DummyResponse() + _response.content_type = None + _response.default_content_type = None + @reify + def response(self): + return self._response + request = DummyRequestWithClassResponse() + # use a json renderer, which will mutate the response + result = self._callFUT('json', dict(a=1), request=request) + self.assertEqual(result.body, b'{"a": 1}') + self.assertFalse('response' in request.__dict__) + class Test_get_renderer(unittest.TestCase): def setUp(self): self.config = testing.setUp() -- cgit v1.2.3 From d23e6986b1122e8d7344f2c882ddd3e3f423e30f Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Fri, 6 Feb 2015 02:06:07 -0600 Subject: update changelog and docs --- CHANGES.txt | 10 ++++++++++ pyramid/renderers.py | 6 +++--- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index b334f5258..30f30cec7 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -80,6 +80,16 @@ Features - Support keyword-only arguments and function annotations in views in Python 3. See https://github.com/Pylons/pyramid/pull/1556 +- ``request.response`` will no longer be mutated when using the + ``pyramid.renderers.render_to_response()`` API. Almost all renderers + mutate the ``request.response`` response object (for example, the JSON + renderer sets ``request.response.content_type`` to ``application/json``). + However, when invoking ``render_to_response`` it is not expected that the + response object being returned would be the same one used later in the + request. The response object returned from ``render_to_response`` is now + explicitly different from ``request.response``. This does not change the + API of a renderers. See https://github.com/Pylons/pyramid/pull/1563 + Bug Fixes --------- diff --git a/pyramid/renderers.py b/pyramid/renderers.py index c2be8c2eb..42a4c98dc 100644 --- a/pyramid/renderers.py +++ b/pyramid/renderers.py @@ -110,9 +110,9 @@ def render_to_response(renderer_name, value, request=None, package=None): Supply a ``request`` parameter in order to provide the renderer with the most correct 'system' values (``request`` and ``context`` - in particular). Keep in mind that if the ``request`` parameter is - not passed in, any changes to ``request.response`` attributes made - before calling this function will be ignored. + in particular). Keep in mind that any changes made to ``request.response`` + prior to calling this function will not be reflected in the resulting + response object. A new response object will be created for each call. """ try: -- cgit v1.2.3 From e382164aa71731390f97db9734ce0b0bb014c78a Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Fri, 6 Feb 2015 02:09:20 -0600 Subject: moar docs --- pyramid/renderers.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pyramid/renderers.py b/pyramid/renderers.py index 42a4c98dc..c4ea22429 100644 --- a/pyramid/renderers.py +++ b/pyramid/renderers.py @@ -114,6 +114,11 @@ def render_to_response(renderer_name, value, request=None, package=None): prior to calling this function will not be reflected in the resulting response object. A new response object will be created for each call. + .. versionchanged:: 1.6 + In previous versions, any changes made to ``request.response`` outside + of this function call would affect the returned response. This is no + longer the case. + """ try: registry = request.registry -- cgit v1.2.3 From 72bf6bb1b942a56a39d5ae33634e7aa8fac7080a Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Fri, 6 Feb 2015 13:00:59 -0600 Subject: add a respones arg to render_to_response --- pyramid/renderers.py | 14 +++++++++++--- pyramid/tests/test_renderers.py | 26 +++++++++++++++++++++++--- 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/pyramid/renderers.py b/pyramid/renderers.py index c4ea22429..088d451bb 100644 --- a/pyramid/renderers.py +++ b/pyramid/renderers.py @@ -79,7 +79,11 @@ def render(renderer_name, value, request=None, package=None): return result -def render_to_response(renderer_name, value, request=None, package=None): +def render_to_response(renderer_name, + value, + request=None, + package=None, + response=None): """ Using the renderer ``renderer_name`` (a template or a static renderer), render the value (or set of values) using the result of the renderer's ``__call__`` method (usually a string @@ -112,12 +116,14 @@ def render_to_response(renderer_name, value, request=None, package=None): with the most correct 'system' values (``request`` and ``context`` in particular). Keep in mind that any changes made to ``request.response`` prior to calling this function will not be reflected in the resulting - response object. A new response object will be created for each call. + response object. A new response object will be created for each call + unless one is passed as the ``response`` argument. .. versionchanged:: 1.6 In previous versions, any changes made to ``request.response`` outside of this function call would affect the returned response. This is no - longer the case. + longer the case. If you wish to send in a pre-initialized response + then you may pass one in the ``response`` argument. """ try: @@ -130,6 +136,8 @@ def render_to_response(renderer_name, value, request=None, package=None): registry=registry) with temporary_response(request): + if response is not None: + request.response = response result = helper.render_to_response(value, None, request=request) return result diff --git a/pyramid/tests/test_renderers.py b/pyramid/tests/test_renderers.py index 31e9d14f8..ed6344a40 100644 --- a/pyramid/tests/test_renderers.py +++ b/pyramid/tests/test_renderers.py @@ -517,10 +517,11 @@ class Test_render_to_response(unittest.TestCase): def tearDown(self): testing.tearDown() - def _callFUT(self, renderer_name, value, request=None, package=None): + def _callFUT(self, renderer_name, value, request=None, package=None, + response=None): from pyramid.renderers import render_to_response return render_to_response(renderer_name, value, request=request, - package=package) + package=package, response=response) def test_it_no_request(self): renderer = self.config.testing_add_renderer( @@ -579,6 +580,18 @@ class Test_render_to_response(unittest.TestCase): self.assertEqual(result.body, b'{"a": 1}') self.assertFalse('response' in request.__dict__) + def test_custom_response_object(self): + class DummyRequestWithClassResponse(object): + pass + request = DummyRequestWithClassResponse() + response = DummyResponse() + # use a json renderer, which will mutate the response + result = self._callFUT('json', dict(a=1), request=request, + response=response) + self.assertTrue(result is response) + self.assertEqual(result.body, b'{"a": 1}') + self.assertFalse('response' in request.__dict__) + class Test_get_renderer(unittest.TestCase): def setUp(self): self.config = testing.setUp() @@ -639,7 +652,14 @@ class Dummy: class DummyResponse: status = '200 OK' + default_content_type = 'text/html' + content_type = default_content_type headerlist = () app_iter = () - body = '' + body = b'' + + # compat for renderer that will set unicode on py3 + def _set_text(self, val): # pragma: no cover + self.body = val.encode('utf8') + text = property(fset=_set_text) -- cgit v1.2.3 From 04206845591cb5af1038a29f6e943bfb169c2d5c Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Fri, 6 Feb 2015 13:03:20 -0600 Subject: update changelog --- CHANGES.txt | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 30f30cec7..a02ae504b 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -81,14 +81,18 @@ Features Python 3. See https://github.com/Pylons/pyramid/pull/1556 - ``request.response`` will no longer be mutated when using the - ``pyramid.renderers.render_to_response()`` API. Almost all renderers + ``pyramid.renderers.render_to_response()`` API. It is now necessary to + pass in a ``response=`` argument to ``render_to_response`` if you wish to + supply the renderer with a custom response object for it to use. If you + do not pass one then a response object will be created using the + application's ``IResponseFactory``. Almost all renderers mutate the ``request.response`` response object (for example, the JSON renderer sets ``request.response.content_type`` to ``application/json``). However, when invoking ``render_to_response`` it is not expected that the response object being returned would be the same one used later in the request. The response object returned from ``render_to_response`` is now explicitly different from ``request.response``. This does not change the - API of a renderers. See https://github.com/Pylons/pyramid/pull/1563 + API of a renderer. See https://github.com/Pylons/pyramid/pull/1563 Bug Fixes --------- -- cgit v1.2.3