#5783 Action Caching ignore response content type and request header - Ruby on Rails

Type	To find
responsible:me	tickets assigned to you
tagged:"@high"	tickets tagged @high
milestone:next	tickets in the upcoming milestone
state:invalid	tickets with the state invalid
created:"last week"	tickets created last week
sort:number, importance, updated	tickets sorted by #, importance or updated
Combine keywords for powerful searching.
Use advanced searching »

This project is archived and is in readonly mode.

#5783 ✓stale

Action Caching ignore response content type and request header

Reported by Chris Hapgood | October 11th, 2010 @ 03:46 PM

I propose a reset of how cached content is matched up to requests. The biggest problem with the current implementation, in my hastily formed opinion, is that the cached content may or may not be tagged with a content type. This leads to futile attempts to match

The requested content type -which is intentionally vague ('image/*', ACCEPT header with 'q' values, IE's infamous '*/*', etc.).
The available content type(s) -which can be UNKNOWN in the common case of a cache key without an extension

There are lots of compromises that involve trying to make educated guesses (request.cache_format, AC::Base.page_cache_extension, params[:format], request.path). Here are some observations that jump out when looking at the "surprising" behavior of the current implementation:

Rails has a convention for tagging cached content with a Mime::Type-compatible extension.
At the moment the cache is created, Rails knows the content type of the cache -but ignores it.
Rails has a strong and well documented mechanism for matching requests with available content type -but doesn't use it in cache responses.

Observations 1 & 2 lead me to the conclusion that the content type (and thus extension) of the cached content should be determined after the action has processed and explicitly tagged the content (usually via #respond_to or #render). It was a pretty simple change that resulted in a noticeable simplification of the actions.rb file, particularly the ActionCachePath class. The whole "infer_extension" mess is gone, replaced by specific information from the user-generated response. This leads to the immediate benefit that a request to an extension-less path still gets a content type applied to the cache key.

Observation 3 lead me to graft the existing Mime::Responder#respond_to onto the action cache filter code. It was surprisingly easy. Now responses from cache work follow the principle of least surprise: just like regular responses, the content type is determined by respond_to, which considers the available content types (specified as a parameter to the caches_action method) and the normal cast of characters (:format parameter, path extension, HTTP_ACCEPT header, xhr logic, etc.).

Here is an example that should illustrate the impact of the changes.

Request to: /widgets/23, which uses #render :xml in the action

BEFORE

content_type of first response: application/xml

cache key: /widgets/23

content_type of subsequent response served from cache: variable depending on request and static configuration values. Possibly a surprising HTML.

Result: surprise!

AFTER

content_type of first response: application/xml

cache key: /widgets/23.xml

content_type of subsequent response served from cache: xml

Result: yawn

ch_action_cache.diff 9.7 KB

Comments and changes to this ticket

You flagged this item as spam.
Chris Hapgood October 11th, 2010 @ 03:56 PM
There are some warts. Some that come to mind: determination of available content_types, cache expiration and backwards compatibility.

content_type availability

To determine the content types available for a cache response, I see two possible solutions:
1. query the cache story for the keys at a given prefix (the base key, without a content_type suffix) and extract all the existing suffixes.
2. have the user specify the available content types in the macro (caches_action :show, :content_types => [:xml, :pdf]
Option 1 is DRY, but required a separate cache query -and I wasn't sure all cache stores could support a wildcard key lookup. Option 2 was pragmatic and I selected it.

cache expiration

Cache expiration appears to have always been a bit clunky when dealing with multiple content types. It appears that a wildcard approach is almost required here. I chose, for pragmatic reasons, to stick with specific expiration by content type. It's not pretty, but I don't think it was before either.

backwards compatibility

Backwards compatibility is important because I decided to let the user specify the content types (potentially) available from cache. Existing code won't have such a specification. In that case, I assume that the :all generic content type applies for matching. This makes the respond_to code work as expected, but may lead to unhelpful cache key extensions. I could use some help here...

Rails 3

Oh yeah. I did not test my patch against master. Not even sure if it is still appropriate.
You flagged this item as spam.
Andrea Campi October 16th, 2010 @ 10:59 PM
- Tag changed from “actionpack responder, action_controller, caches_action, responder” to “actionpack responder, action_controller, caches_action, patch, responder”
The same issue does apply to edge; unfortunately your patch would require quite a few changes, code in master changed quite a bit.
You flagged this item as spam.
Chris Hapgood October 18th, 2010 @ 08:58 PM
Andrea,
Thanks for the update. I will work on getting this code to work with master. It is working wonderfully in 2-3-stable.

-Chris
Neeraj Singh December 5th, 2010 @ 01:03 AM
- Importance changed from “” to “Low”
This is a tough one. I spent some time looking at the issue and here are my findings.

If you sent a request for .xml or .js extension at the end then everything works fine.

However if say HTTP_ACCEPT attribute is "text/xml" then there are a number of issues. Remember action caching puts a before_filter which reads from the cache to find out if a cache already exists or not. However in this case in order to find out if a cache exists for mime type html or for xml rails needs to parse HTTP_ACCEPT. Parsing actually happens only inside the process body and not in a filter.

In the before_filter controller has access to params[:format] and that is why a request with .xml or .js works.

I will discuss this issue with JV and will update here.
Neeraj Singh December 6th, 2010 @ 09:41 PM
Added to documentation that action caching does not play well with HTTP_ACCEPT attribute.

https://github.com/lifo/docrails/commit/3ac844deec6070f35634acca0ae...
rails March 7th, 2011 @ 12:00 AM
- State changed from “new” to “open”
This issue has been automatically marked as stale because it has not been commented on for at least three months.

The resources of the Rails core team are limited, and so we are asking for your help. If you can still reproduce this error on the 3-0-stable branch or on master, please reply with all of the information you have about it and add "[state:open]" to your comment. This will reopen the ticket for review. Likewise, if you feel that this is a very important feature for Rails to include, please reply with your explanation so we can consider it.

Thank you for all your contributions, and we hope you will understand this step to focus our efforts where they are most helpful.
rails March 7th, 2011 @ 12:00 AM
- State changed from “open” to “stale”