CACHE_MIDDLEWARE_ANONYMOUS_ONLY is ugly, misleading, and unnecessary, and should be deprecated
|Reported by:||carljm||Owned by:||aaugustin|
|Component:||Core (Cache system)||Version:||master|
|Has patch:||yes||Needs documentation:||no|
|Needs tests:||no||Patch needs improvement:||no|
There are three supposed benefits of using the cache middleware with CACHE_MIDDLEWARE_ANONYMOUS_ONLY set to True: 1) Don't show user-specific data to the wrong users. 2) Avoid wasting memory on caching user-specific pages. 3) Improve performance by caching most pages.
(1) is unnecessary: Django already sets Vary: Cookie headers when the session is accessed, which will prevent users from getting another user's cached pages. In fact one danger of the current documentation of CACHE_MIDDLEWARE_ANONYMOUS_ONLY is that it does not mention Vary: Cookie and may mislead users into thinking that CACHE_MIDDLEWARE_ANONYMOUS_ONLY is necessary to avoid users being shown wrong cached pages.
CACHE_MIDDLEWARE_ANONYMOUS_ONLY is only partially effective at (2), because it will still cache session-specific pages with Vary: Cookie if a user is not logged in, which can blow up caching memory just as much or more as authenticated requests, if sessions are used with anonymous users.
And on the downside, CACHE_MIDDLEWARE_ANONYMOUS_ONLY is far from simple: it requires contrib.auth to be installed before it can be used, and is tricky to implement correctly (until the fix for #13283 was committed, the incorrect implementation in place for years meant that turning CACHE_MIDDLEWARE_ANONYMOUS_ONLY on immediately destroyed caching efficiency for your entire site, including anonymous pages).
An alternative hypothetical setting, CACHE_MIDDLEWARE_SKIP_VARY_COOKIE, would give users a more accurate model of how caching actually works, would remove both the complexity of implementation and the needless dependence on contrib.auth, and would be more effective at improving caching efficiency (because it would also avoid caching session-dependent-but-anonymous requests).
I think this behavior change should be made in any case; it's a clear improvement with no downside that I can see. It would be possible to make the behavior change and leave the current (somewhat misleading) setting name, to avoid deprecation churn. I think I lean towards going ahead and changing the name of the setting too; based on the lack of outcry around #13283 I get the impression few users are using CACHE_MIDDLEWARE_ANONYMOUS_ONLY anyway, and it's a painless setting name switch.
Thanks to natrius in the above pull request and the #13283 comments for these observations.
Change History (17)
comment:1 Changed 3 years ago by russellm
- Needs documentation unset
- Needs tests unset
- Patch needs improvement unset
- Triage Stage changed from Unreviewed to Accepted
comment:6 Changed 15 months ago by aaugustin
comment:10 Changed 11 months ago by anonymous
- Owner changed from nobody to anonymous
- Status changed from new to assigned
comment:15 Changed 11 months ago by aaugustin
- Owner changed from ambv to aaugustin
- Patch needs improvement unset
comment:16 Changed 11 months ago by ambv
- Resolution set to fixed
- Status changed from assigned to closed