Skip to content

Implement HttpRequest proxying with __getattr__ for optimized access. #5576

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions rest_framework/request.py
Original file line number Diff line number Diff line change
Expand Up @@ -372,19 +372,23 @@ def _not_authenticated(self):
else:
self.auth = None

def __getattribute__(self, attr):
def __getattr__(self, attr):
"""
If an attribute does not exist on this instance, then we also attempt
to proxy it to the underlying HttpRequest object.
"""
try:
return super(Request, self).__getattribute__(attr)
return getattr(self._request, attr)
except AttributeError:
info = sys.exc_info()
outer_info = sys.exc_info()

# raise the 'original' AttributeError for the proxying Request object instead of the underlying
# HttpRequest object.
try:
return getattr(self._request, attr)
self.__getattribute__(attr)
except AttributeError:
six.reraise(info[0], info[1], info[2].tb_next)
inner_info = sys.exc_info()
six.reraise(inner_info[0], inner_info[1], outer_info[2].tb_next)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is actually necessary. It was in the original implementation, since we were re-raising from the attribute access on the underlying WSGIRequest object. In this case, we're raising from Request.__getattribute__, so catching and reraising should be superfluous.


@property
def DATA(self):
Expand Down
25 changes: 25 additions & 0 deletions tests/test_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -249,3 +249,28 @@ def test_default_secure_false(self):
def test_default_secure_true(self):
request = Request(factory.get('/', secure=True))
assert request.scheme == 'https'


class TestWSGIRequestProxy(TestCase):
def test_access_inner_property(self):
wsgi_request = factory.get('/')

sentinel = object()
wsgi_request.__dict__['inner_property'] = sentinel
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It shouldn't be necessary to do reference the underlying `dict. Regular attribute setting should be sufficient.

wsgi_request.inner_property = sentinel


request = Request(wsgi_request)

assert request.inner_property is sentinel

def test_access_outer_property(self):
wsgi_request = factory.get('/')

inner_sentinel = object()
wsgi_request.__dict__['inner_property'] = inner_sentinel

request = Request(wsgi_request)

outer_sentinel = object()
request.__dict__['inner_property'] = outer_sentinel

assert request.inner_property is outer_sentinel