Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

This project is archived and is in readonly mode.

Action Caching with caches_action and :layout => false

#4140

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!

Reported by Pete Taylor · March 10th, 2010 @ 07:44 AM

State: invalid
Milestone: 3.0.2
Assigned to: Yehuda Katz (wycats) Yehuda Katz (wycats)
Importance: Medium

Activity

  1. Steve
    Steve

    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

    March 17th, 2010 @ 07:39 AM

  2. Pete Taylor
    Pete Taylor

    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!

    March 17th, 2010 @ 04:40 PM

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

    March 27th, 2010 @ 08:44 AM

  4. Yehuda Katz (wycats)
    Yehuda Katz (wycats)
    • 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).

    March 29th, 2010 @ 02:11 AM

  5. Pete Taylor
    Pete Taylor

    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!

    March 29th, 2010 @ 02:49 AM

  6. Jeremy Kemper
    Jeremy Kemper
    • Milestone set to 3.0.2
    • Importance changed from to Medium

    October 15th, 2010 @ 11:01 PM

  7. Nico
    Nico

    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).

    November 26th, 2010 @ 09:23 PM

  8. Nico
    Nico

    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.

    November 30th, 2010 @ 08:58 PM

  9. Nico
    Nico

    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).

    December 1st, 2010 @ 08:26 AM