Opened 5 years ago
Last modified 5 years ago
#30941 closed Bug
hasattr(request, '_cached_user') check no longer works — at Version 8
Reported by: | Collin Anderson | Owned by: | nobody |
---|---|---|---|
Component: | contrib.auth | Version: | dev |
Severity: | Release blocker | Keywords: | |
Cc: | cmawebsite@…, Sergey Fedoseev, Simon Charette | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | yes | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
Before https://github.com/django/django/commit/2f01079, it was possible to check to see whether the lazy user has been evaluated or not using hasattr(request, '_cached_user')
.
This was undocumented, but I think we should try to maintain backwards compatibility for that check, and document and test it for the future. (Or if nothing else, document and test how to do this the new way and add a release note about the change.)
This is really helpful in middleware to be able to check whether the user has been evaluated, so you can access the user if it has already been accessed, but can avoid fetching user if it hasn't already been fetched.
Change History (8)
comment:1 by , 5 years ago
Description: | modified (diff) |
---|
comment:2 by , 5 years ago
Description: | modified (diff) |
---|
comment:3 by , 5 years ago
Description: | modified (diff) |
---|
comment:4 by , 5 years ago
Cc: | added |
---|
follow-up: 6 comment:5 by , 5 years ago
This is really helpful in middleware to be able to check whether the user has been evaluated, so you can access the user if it has already been accessed, but can avoid fetching user if it hasn't already been fetched.
I can't understand the use case described here. Surely you either need to access it in the middleware, or you don't. Maybe you could explain why you'd need to only get it if it has already been accessed...
FWIW I'd much prefer we define a blessed way of looking this up then maintaining compatibility with this implementation specific
_cached_user
hack.
Agreed. This always looked like a hack and it is an undocumented and private variable after all. That said, a way to determine whether LazyObject
has been resolved would be good. The main challenge is conflicting attributes with the wrapped object, so I think .loaded
is too problematic. I would suggest creating and documenting _resolved
to go alongside _wrapped
:
class LazyObject: ... @property def _resolved(self): return self._wrapped is not empty ...
I also note that LazyObject
and SimpleLazyObject
are vaguely referenced in the documentation, but mainly in release notes where it occasionally says things like "if you use ... in your own code". There is no proper public documentation so these are sort of internals that are sometimes used in projects. Do we really want to make this officially public API?
comment:6 by , 5 years ago
Replying to Nick Pope:
This is really helpful in middleware to be able to check whether the user has been evaluated, so you can access the user if it has already been accessed, but can avoid fetching user if it hasn't already been fetched.
I can't understand the use case described here. Surely you either need to access it in the middleware, or you don't. Maybe you could explain why you'd need to only get it if it has already been accessed...
+1
I also note that
LazyObject
andSimpleLazyObject
are vaguely referenced in the documentation, but mainly in release notes where it occasionally says things like "if you use ... in your own code". There is no proper public documentation so these are sort of internals that are sometimes used in projects. Do we really want to make this officially public API?
Also do we really want to guarantee that request.user
is actually LazyObject
?
comment:7 by , 5 years ago
I have some urls (like checkout pages) that access the user and some urls that don't (like product pages). I might want to use middleware to log what the user is doing, but I don't want to cause a user+session lookup on product pages where user wasn't accessed.
Similarly, there's a request.session.accessed
, but I'm seeing now that even that is undocumented. But I'm realizing I could probably get away with just checking request.session.accessed
because most pages that look at the session are also going to pull up the user.
comment:8 by , 5 years ago
Description: | modified (diff) |
---|
If we wanted to deprecate this behavior we could add a
_cached_user
descriptor torequest
that raises a deprecation warning on access and raises anAttributeError
wheninstance.user._wrapped is empty
.FWIW I'd much prefer we define a blessed way of looking this up then maintaining compatibility with this implementation specific
_cached_user
hack.Something like
request.user.loaded
by subclassingSimpleLazyObject
or adding this feature directlyLazyObject
sounds more reasonable to me.