#15144: Max age set in cache control no longer obeys timeout set with
@cache_page
decorator
-----------------------------------+----------------------------------------
Reporter: jsdalton | Owner: nobody
Status: new | Milestone: 1.3
Component: Cache system | Version: SVN
Resolution: | Keywords: blocker, regression
Stage: Unreviewed | Has_patch: 1
Needs_docs: 0 | Needs_tests: 0
Needs_better_patch: 0 |
-----------------------------------+----------------------------------------
Comment (by jsdalton):
Okay, I'm uploading a new patch that:
* Includes the regression tests I wrote under `test_view_decorator` that
demonstrating the original issue
* Fixes the issue originally raised in this ticket by incorporating
Joshua's patch (15144.patch).
* Includes a new test under `CacheMiddlewareTest`, `test_constructor`,
which highlight a few bugs in the constructor that were discussed briefly
here: http://groups.google.com/group/django-
developers/browse_thread/thread/1a019e9d30de5c9d
* Fixes the issues uncovered by the new test via a fairly minor rewrite
of the `__init__` method of `CacheMiddleware`
With regards to the first issue and fix, that's pretty much been discussed
above so no need to say more about it.
With regards to the second issue and fix, the main problem was that if the
`CacheMiddleware` class was being used as middleware, the values for some
of the instance attributes were not being set properly. (Basically, the
`except KeyError` code path was never being executed.)
The new test should be pretty self explanatory and should raise several
failures when run against the original `CacheMiddleware.__init__` code
currently in trunk
The rewrite is also hopefully self explanatory as well. In addition to
fixing the bugs I tried my best to simplify some of the code in an effort
to make it more readable and easy to maintain going forward. A few notes
of clarification:
* I changed `cache_alias` to an instance attribute (i.e.
`self.cache_alias`) in order to make it more testable (since it's not
possible to determine which alias is being used from `self.cache` itself.)
* I smoothed out a lot of places where variables were being explicitly
compared against None etc. I think the new test adequately ensures that
the appropriate initialization behavior is taking place, and some of those
constructions were (IMO) making the code a bit unnecessarily verbose and
hard to read.
* I wish there was better testing to ensure the right values are being
set in `cache_kwargs` and passed to `get_cache()` but I think the other
upstream tests should fail if any of that were going wrong so it's
probably okay.
Anyhow, please let me know if there are any issues with the patch or
anything else that needs to be addressed. (Apologies if I should have
opened a new ticket to address those other bugs, I just figured it was
easier to get it all taken care of here.)
Cheers.
Jim
--
Ticket URL: <http://code.djangoproject.com/ticket/15144#comment:4>
Django <http://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/django-updates?hl=en.
