Any comments on this ActionCaching patch?

1 view
Skip to first unread message

Jonathan del Strother

unread,
Feb 23, 2008, 6:43:00 AM2/23/08
to Ruby on Rails: Core
I've been struggling a bit with ActionCaching's handling of different
formats - it doesn't seem very well behaved when trying to present a
single resource in different formats. In particular, it only uses the
path to determine the cache extension to use, so trying to do
something like
curl -H "Accept: application/xml" http://foo.com/stuff
is going to read/write from stuff.cache rather than stuff.xml.cache

Also, ActionCachePath uses the controller to figure out the path to
use, even when expiring actions. As such, if you do an xml POST,
expiring the cache with
expire_action :controller => :stuff
is going to affect stuff.xml.cache.
(IIRC, expire_action :controller => :stuff, :format => :xml would
expire the stuff.xml.xml.cache...)


http://dev.rubyonrails.org/ticket/11184 tries to address both these
issues. Any comments would be appreciated.

Cheers,
Jon

Chad Woolley

unread,
Feb 24, 2008, 2:33:47 PM2/24/08
to rubyonra...@googlegroups.com
On Sat, Feb 23, 2008 at 4:43 AM, Jonathan del Strother
<jdelSt...@gmail.com> wrote:
>
> I've been struggling a bit with ActionCaching's handling of different
> formats - it doesn't seem very well behaved when trying to present a
> single resource in different formats. In particular, it only uses the
> path to determine the cache extension to use, so trying to do
> something like
> curl -H "Accept: application/xml" http://foo.com/stuff
> is going to read/write from stuff.cache rather than stuff.xml.cache

This sounds a lot like some mysterious caching bugs we are having.
We'll take a look at your patch to see if it solves our problems...

Thanks,
-- Chad

Jonathan del Strother

unread,
Feb 27, 2008, 11:48:04 AM2/27/08
to Ruby on Rails: Core
On Feb 24, 7:33 pm, "Chad Woolley" <thewoolley...@gmail.com> wrote:
> On Sat, Feb 23, 2008 at 4:43 AM, Jonathan del Strother
>
> <jdelStrot...@gmail.com> wrote:
>
> >  I've been struggling a bit with ActionCaching's handling of different
> >  formats - it doesn't seem very well behaved when trying to present a
> >  single resource in different formats.  In particular, it only uses the
> >  path to determine the cache extension to use, so trying to do
> >  something like
> >  curl -H "Accept: application/xml"http://foo.com/stuff
> >  is going to read/write from stuff.cache rather than stuff.xml.cache
>
> This sounds a lot like some mysterious caching bugs we are having.
> We'll take a look at your patch to see if it solves our problems...
>


Any luck with that patch, Chad, or was it an unrelated problem?

Chad Woolley

unread,
Feb 27, 2008, 3:57:58 PM2/27/08
to rubyonra...@googlegroups.com
On Wed, Feb 27, 2008 at 9:48 AM, Jonathan del Strother
<jdelSt...@gmail.com> wrote:
> Any luck with that patch, Chad, or was it an unrelated problem?

It won't be until we look into our bug, which might be a while for
this project. I've put links for this thread/ticket in our bug report
though, so I'll let you know what we eventually find.

Alex Chaffee

unread,
Mar 5, 2008, 8:52:15 PM3/5/08
to Ruby on Rails: Core, Shifra Pride Raffel, Chad Woolley, Alex Chaffee

On Feb 27, 12:57 pm, "Chad Woolley" <thewoolley...@gmail.com> wrote:
> On Wed, Feb 27, 2008 at 9:48 AM, Jonathan del Strother
>
> <jdelStrot...@gmail.com> wrote:
> > Any luck with that patch, Chad, or was it an unrelated problem?
>
> It won't be until we look into our bug, which might be a while for
> this project. I've put links for this thread/ticket in our bug report
> though, so I'll let you know what we eventually find.

Hi, we work with Chad and we just implemented the fix he was
promising.

The good news is that we did confirm that it's a problem with Rails
core, and we can probably provide a very simple test case to reproduce
it on any Rails app using caching. (Funny thing: we tracked down the
last time it happened in our logs and it turned out the client making
the fatal request was FeedTools, a Ruby library for fetching and
creating RSS type feeds. So in our test, we used FeedTools itself to
reproduce the bug and solve the problem.)

The bad news is that your patch didn't fix the problem. We had to
resort to overriding the cache_page method in our application
controller. Our code is not ready to be rolled back into Rails Core
because it hardcodes only the three mime types we care about instead
of using Mime::Type's lookup feature. A proper patch would take into
account all the possible mime types that are suitable for caching and
it would take someone much more knowledgeable than us about Rails
innards to do it right.

FWIW, here's our monkey patch:

class ApplicationController < ActionController::Base
...
def cache_page(content = nil, options = {})
return unless perform_caching
self.class.cache_page(content || response.body, cache_path)
end

def cache_path
# Note: this is a monkey patch for a bug in Rails caching, see
# http://groups.google.com/group/rubyonrails-core/browse_thread/thread/22430d01c6c4010
cache_path = request.path
extension = {
"application/atom+xml" => ".atom",
"application/rss+xml" => ".rss",
"text/html" => ".html"
}
request.accepts.each do |mime_type|
s = mime_type.to_s
if extension[s]
cache_path += extension[s]
break
end
end
cache_path
end

To reproduce the issue, here's what you do:

1. gem install feedtools
2. clear your cache
3. use FeedTools to hit your page (in our case, /blabs) -- note that
it's not specifying .atom in the URL, but in the Accept header
require 'feed_tools'
feed = FeedTools::Feed.open("http://example.com/blabs")
p feed.title
4. Hit your page in a web browser and see that instead of HTML, your
page is XML

Thanks for blazing the trail!

- Shifra and Alex

Jonathan del Strother

unread,
Mar 6, 2008, 6:03:23 AM3/6/08
to Ruby on Rails: Core
>     #http://groups.google.com/group/rubyonrails-core/browse_thread/thread/...
>     cache_path = request.path
>     extension = {
>       "application/atom+xml" => ".atom",
>       "application/rss+xml" => ".rss",
>       "text/html" => ".html"
>     }
>     request.accepts.each do |mime_type|
>       s = mime_type.to_s
>       if extension[s]
>         cache_path += extension[s]
>         break
>       end
>     end
>     cache_path
>   end
>

That's presumably page-caching rather than action-caching, right?

Alex Chaffee

unread,
Mar 6, 2008, 9:47:51 PM3/6/08
to Ruby on Rails: Core
>
> That's presumably page-caching rather than action-caching, right?

Yes, that's right, on this app we're doing all page caching, no action
or fragment caching.

- A

Jonathan del Strother

unread,
Apr 9, 2008, 12:26:06 PM4/9/08
to Ruby on Rails: Core
On Feb 23, 12:43 pm, Jonathan del Strother <jdelStrot...@gmail.com>
wrote:
> http://dev.rubyonrails.org/ticket/11184tries to address both these
> issues.  Any comments would be appreciated.


prod for further comments...
Reply all
Reply to author
Forward
0 new messages