This project is archived and is in readonly mode.

#4140 ✓ invalid
Pete Taylor

Action Caching with caches_action and :layout => false

Reported by Pete Taylor | March 10th, 2010 @ 07:44 AM | in 3.0.2

I've had trouble with action caching, and I've debugged it down to a line in actionpack/lib/action_controller/caching/actions.rb

on line 127 starts a method as follows:

    def cache_layout\?
      @options[:layout] == false
    end

My class caches_action method looks like this:

    caches_action :index, :expires_in => 6.hours, :cache_path => Proc.new {|controller| controller.send(:generate_cache_path) }, :layout => false

This, to me, would indicate that we don't want to cache the layout, only the action. But by checking @options[:layout] == false in the cache_layout? method, then when layout is set to false, the method itself will return true, saying that you should cache the layout.

That seems like a bug to me. For now, I've just changed my layout line to :layout => true, and it seems to work, but that's obviously counter-intuitive, and causes :layout => true to be passed to the render action for the controller method call.

For reference, the cache_layout? method gets called here:

    def after(controller)
      return if controller.rendered_action_cache || !caching_allowed(controller)
      action_content = cache_layout? ? content_for_layout(controller) : controller.response.body
      controller.write_fragment(controller.action_cache_path.path, action_content, @options[:store_options])
    end

I know I should include a test, but honestly I'm not sure if it's a bug, or if I'm simply misunderstanding how it should work.

Thanks!

Comments and changes to this ticket

  • Steve

    Steve March 17th, 2010 @ 07:39 AM

    Hi Pete.

    The short story is that it's not a bug, it's just named counter-intuitively. Read on if you would like the "long story". :)

    I've been looking at this file for the past week or so, working on adding support for caching content_for stuff when you do caches_action.

    I've been working in the 3.0b1 branch of rails, so I think it's a teensy bit different than what you're looking at, but I was tripped up by the exact same handling of the layout boolean (though there is no longer a "cache_layout?" method). I think the method may have been taken out because it was misleadingly named, and the implementation was so short it didn't really deserve its own method. I'm guessing the thinking that led to "options[:layout] == false" being used is that that value is used when it renders from cache (render :text => cached, :layout => options[:layout]==false).

    Anyway what I've found is that this works properly, it's simply named strangely. Keep in mind that if you want to cache your action but not your entire layout, then you can't use content_for anywhere in that action or template (yet! ;).

    Keep in mind that content_for_layout(controller) returns just that rendered template, but controller.response.body returns the entire rendered layout (everything from <html> to </html>).

    Hopefully this helps. I know it's confusing. Just believe me when I say it works, but it's named counter-intuitively.

    -Steve

  • Pete Taylor

    Pete Taylor March 17th, 2010 @ 04:40 PM

    Steve, thanks for the response! Sorry for filing a bogus ticket, the behavior I was seeing must be related to something dumb I'm doing. Thanks!

  • Yehuda Katz (wycats)

    Yehuda Katz (wycats) March 27th, 2010 @ 08:44 AM

    • Milestone cleared.
    • Tag changed from caches_action to caches_action, rails3
    • State changed from “new” to “open”
    • Assigned user set to “Yehuda Katz (wycats)”
  • Yehuda Katz (wycats)

    Yehuda Katz (wycats) March 29th, 2010 @ 02:11 AM

    • State changed from “open” to “invalid”

    This naming actually tripped ME up a bunch of times recently. We tried to rename some of the variables to make it clearer. We also changed how it works internally so that the layout is always applied in the same context, regardless of whether it's the first call to the method or a cached call.

    body = controller.render_to_string(:text => cache, :layout => true) unless @cache_layout
    

    Here, we're taking the text out of the cache, and then rendering the text in the context of the layout only if @cache_layout is false. That's because if the layout was cached, we wouldn't need to apply the layout (it would already be in the cached content).

  • Pete Taylor

    Pete Taylor March 29th, 2010 @ 02:49 AM

    Thanks for taking a look! (wycats read my ticket, OMG ;) ).

    Seriously, thanks, I think the behavior I was experiencing in my app was due to something dumb I was doing, and I incorrectly tracked it to this. Thanks for the input!

  • Jeremy Kemper

    Jeremy Kemper October 15th, 2010 @ 11:01 PM

    • Milestone set to 3.0.2
    • Importance changed from “” to “Medium”
  • Nico

    Nico November 26th, 2010 @ 09:23 PM

    I also have some trouble with this. Action caching without the layout parameter works fine.
    But as soon as I add layout => false I get problems:
    Normal get requests are cached fine, without the layout, as expected. But Ajax requests
    are not properly cached: I get cache misses all the time. Then, when I log in to my app
    neither normal, nor ajax requests are cached at all.

    I also tried to set :layout => Proc.new { |c| c.request.xhr? }

    When I do that ajax and normal requests are properly cached (no matter if logged out or logged in)
    but normal requests also cache the layout. Seems odd to me as normal requests should have :layout => false.

    I'm trying to debug this for hours now, but I haven't got a clue what's going on so far. What especially trips me up
    is that the logged in state of my app has an influence on the caching behaviour.

    Here is my full caching call (for the case of layout => false):

    caches_action :index, :cache_path => Proc.new { |c| c.params.clone.delete_if { |k,v| ['authenticity_token'].include?(k) }.merge(:xhr => c.request.xhr? ? 't' : 'f') }, :layout => false

    I'd greatly appreciate any help on this! I'm using Rails 2.3.8 on Heroku with Memcached (via Dalli Gem).

  • Nico

    Nico November 30th, 2010 @ 08:58 PM

    Okay, I debugged a bit more and found out what the problem is
    (at least when locally in development environment). Inside the action cache content_for_layout is called to get the content to be cached if layout is set to false. This causes
    my ajax requests to be not cached because content_for_layout returns nil as
    I don't render the layout with ajax calls, only a partial. So I do need to set
    layout to true for ajax calls. However I can't do that the way I tried to, as
    the layout param doesn't accept a Proc. So I overwrote part of the ActionCacheFilter class
    to test the layout for a call method and call it if present. That works great locally!
    However, in production on Heroku with Memcached it still won't work properly.
    Ajax calls are not cached.

  • Nico

    Nico December 1st, 2010 @ 08:26 AM

    Okay, works fine now! I slightly changed the way I overwrote the ActionCacheFilter.

    Possibly you can take these findings into account in future versions of action cache.
    I think my use case is rather common (Having an action that needs to be cached without layout and
    ajax calls to that action that need to be cached as well).

  • ugg classic cardy tall

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

Tickets have moved to Github

The new ticket tracker is available at https://github.com/rails/rails/issues

Shared Ticket Bins

People watching this ticket

Pages