stephenmcd / mezzanine

CMS framework for Django
http://mezzanine.jupo.org
BSD 2-Clause "Simplified" License
4.76k stars 1.65k forks source link

Middleware caching does not store Content-Type #898

Open clarkbarz opened 10 years ago

clarkbarz commented 10 years ago

When Mezzanine 1.4.16 caches pages, it doesn't store the content-type. Thus, retrieving and rendering cached pages results in a content-type of 'text/html' instead of what was expected.

Specifically, caching an RssFeed and retrieving it causes the feed to be displayed with 'text/html' content-type instead of 'application/rss+xml'.

clarkbarz commented 10 years ago

My mistake. Didn't see the Caching Strategy page where it says that the Middleware does not take Vary headers into account. Could this be a feature that you're looking to add in the future? If so, how would you want to implement it?

Thanks.

stephenmcd commented 10 years ago

Well dealing with vary headers and preserving the content type are two different things. I think you've uncovered a valid bug. Not sure how to deal with it - I expect we'd need to store the content type as part of the cache entry, but need to think about it a bit more, and look at what other implementations (specifically Django itself) do.

AlexHill commented 8 years ago

Django's UpdateCacheMiddleware handles this by simply caching the entire Response, which I guess can be serialised.

This just requires changing two lines in mezzanine/core/middleware.py. See 535f78e.

Caching isn't well tested but this seems to work for me. Can anyone think of anything else that we would need to consider here?

stephenmcd commented 8 years ago

Presumably safe if that's what Django does, but wouldn't that cache things like cookie values as well?

AlexHill commented 8 years ago

Er, yes I guess it will. I guess Django accounts for that in its cache key? More investigation required.