Opened 8 years ago
Closed 8 years ago
#26764 closed Cleanup/optimization (fixed)
Session.cycle_key() throws Exception if the session hasn't been accessed
Reported by: | adam-iris | Owned by: | Adam Zapletal |
---|---|---|---|
Component: | contrib.sessions | Version: | 1.8 |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
A common use case I see is that a user changes some content then tries to view their changes on the website, but they get an older, previously-cached page.
Since the page-level cache key uses the session id, my solution is to call request.session.cycle_key()
. To the user this is presented as a "hard reload" -- it clears the session key then redirects back to the current page, the page-level cache will miss and the user will get a newly-generated version.
However, this raised an exception:
'SessionStore' object has no attribute '_session_cache'
Upon inspection, it looks like SessionBase._session_cache
doesn't get initialized except by calling SessionBase._session
. But the cycle_key()
method accesses _session_cache
without ensuring that it's been initialized.
My workaround was to prepend a meaningless access call:
request.session.get('foo') request.session.cycle_key()
This fixed the exception, but I think the proper fix is for cycle_key()
to initialize (or more carefully check) _session_cache
.
Change History (7)
comment:1 by , 8 years ago
Triage Stage: | Unreviewed → Accepted |
---|---|
Type: | Uncategorized → Cleanup/optimization |
comment:2 by , 8 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:3 by , 8 years ago
comment:4 by , 8 years ago
Did you try making that change and seeing if any tests fail? Might be cleaner than the proposed try/except.
comment:5 by , 8 years ago
Yeah, with self._session_cache = {}
in __init__
, the sessions_tests are OK
, but instead of passing with two skips and an expected failure, they pass with one unexpected success.
However, the full Django test suite does not pass with this change. It had 511 failures and 55 errors...so something deeper is going on here. Unexpected! Or maybe I'm missing something.
I'd like to look into it more and understand contrib.sessions
better in general as a separate effort.
comment:6 by , 8 years ago
Has patch: | set |
---|
PR is here: https://github.com/django/django/pull/7031
This is a simple fix, but I'm wondering why
_session_cache
isn't set to{}
in__init__
. Maybe a discussion for another ticket.Thanks!