#12049 closed (fixed)
New LazyObject-wrapped User breaks queries in template tags
Reported by: | Christian Hammond | Owned by: | nobody |
---|---|---|---|
Component: | contrib.auth | Version: | dev |
Severity: | Keywords: | ||
Cc: | chipx86@… | Triage Stage: | Unreviewed |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
I spent a couple days tracking this down since the last fix (ticket #12037) for the new lazy-wrapped User. Basically, if you have a template tag that grabs the User from the context and performs a complex query with two or more Q()
's chained together, it will fail. The failure case on Python 2.4 is fatal, while on 2.5/2.6 it seems to not fail but does log internally caught exception spew.
Basically, when a deepcopy() is performed on the node list, it sees a function()
for the wrapped User and just fails.
Imagine you have a template tag that takes in the context and starts to build a query, like so:
query = Q(user=context['user']) & Q(someflag=True)
The process of using a Q() on a wrapped User and chaining with another Q() will break. On Python 2.4, we see:
File "/usr/lib/python2.4/site-packages/django/db/models/query_utils.py", line 162, in __and__ return self._combine(other, self.AND) File "/usr/lib/python2.4/site-packages/django/db/models/query_utils.py", line 154, in _combine obj = deepcopy(self) File "/usr/lib/python2.4/copy.py", line 185, in deepcopy y = copier(x, memo) File "/usr/lib/python2.4/site-packages/django/utils/tree.py", line 61, in __deepcopy__ obj.children = deepcopy(self.children, memodict) File "/usr/lib/python2.4/copy.py", line 174, in deepcopy y = copier(x, memo) File "/usr/lib/python2.4/copy.py", line 241, in _deepcopy_list y.append(deepcopy(a, memo)) File "/usr/lib/python2.4/copy.py", line 174, in deepcopy y = copier(x, memo) File "/usr/lib/python2.4/copy.py", line 248, in _deepcopy_tuple y.append(deepcopy(a, memo)) File "/usr/lib/python2.4/copy.py", line 204, in deepcopy y = _reconstruct(x, rv, 1, memo) File "/usr/lib/python2.4/copy.py", line 351, in _reconstruct state = deepcopy(state, memo) File "/usr/lib/python2.4/copy.py", line 174, in deepcopy y = copier(x, memo) File "/usr/lib/python2.4/copy.py", line 268, in _deepcopy_dict y[deepcopy(key, memo)] = deepcopy(value, memo) File "/usr/lib/python2.4/copy.py", line 204, in deepcopy y = _reconstruct(x, rv, 1, memo) File "/usr/lib/python2.4/copy.py", line 336, in _reconstruct y = callable(*args) File "/usr/lib/python2.4/copy_reg.py", line 92, in __newobj__ return cls.__new__(cls, *args) TypeError: function() takes at least 2 arguments (0 given)
On Python 2.5 and 2.6, the call succeeds (or at least doesn't throw a fatal exception, but I don't know if there are internal side effects here), but does log the following:
Exception RuntimeError: 'maximum recursion depth exceeded while calling a Python object' in <type 'exceptions.AttributeError'> ignored
I'm attaching a diff that simply amends the test_user_attrs testcase to simulate this. After the other checks, it pulls out the User and then builds a dummy query.
Attachments (2)
Change History (8)
by , 15 years ago
Attachment: | user-query-test.diff added |
---|
comment:1 by , 15 years ago
Cc: | added |
---|
comment:2 by , 15 years ago
Thanks for the test, that's really useful. Hopefully I look at fixing this on Monday. If you want to do it yourself, I imagine the fix will be to implement __deepcopy__
on SimpleLazyObject. I think the implementation is straightforward - if self._wrapped is None
, then you just assign the same _setupfunc function to the copy (no need to make a copy of the function). If self._wrapped is not None
, you copy the wrapped object itself.
In this context, I don't think we need a deep copy of the wrapped object, because it is just going to be thrown away and not modified, but to obey the __deepcopy__
contract, you might want to make it a deep copy. Alternatively, you could make a ContextLazyObject which subclasses SimpleLazyObject, and has a __deepcopy__
method that does shallow copies, which should be safe if ContextLazyObject is only used for this purpose. Perhaps. I haven't thought about it properly.
comment:3 by , 15 years ago
The attached patch implements __deepcopy__
for LazyObject
. I like the idea of providing the implementation there rather than in a subclass as it seems that this functionality should exist higher up, but if you think of a case where we really should limit this to a subclass, we can change that.
I kept the testcase in the same place even though really the fix isn't part of the auth framework. I couldn't find anything that, as far as I could tell, tested LazyObject specifically, and since the User wrapper is a real-world case of breakage (which could break again if User was wrapped in something else), I thought it'd be nice to continue testing this case. However, I'll leave this up to you to decide.
Ran the unit tests with Python 2.4, 2.5 and 2.6. All tests succeeded.
comment:4 by , 15 years ago
Great work, cheers! Sorry for the lost hours debugging, it's one of those use cases which is just really difficult to anticipate. I'll commit the patch soon.
comment:5 by , 15 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
Test case for building a query with a wrapped User.