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.

Fragment cache not generating the proper cache key

#4827

The cache key is not being generated properly in rails 2.3.8 compared to rails 2.3.5 view cache method.

cache(['text',model]) do
... end

Should generate a key that concatenates the 'text' string to the model.cache_key string(where model is an ActiveRecord model instance).

In rails 2.3.8 the generated cache key becomes:

text#<Model:0x12312313>

Reported by Brad Forsyth · June 11th, 2010 @ 01:22 AM

State: resolved
Milestone: 2.3.9
Assigned to: José Valim José Valim
Importance: Low

Activity

  1. Neeraj Singh
    Neeraj Singh

    I tested with rails 2.3.8. Here is my code.

    ActionController::Base.cache_store = :file_store, "#{RAILS_ROOT}/tmp/cache"
    
    <% cache(['hello', User.first]) do %>
      My name is <%= User.first.name %>
    <% end %>
    

    When I visit the page in production mode then cache is generated at

    rails_root/tmp/cache/views/hello/users

    and the file name is

    1-20100611015342.cache

    I am not sure what would be the cache key in 2.3.5 but the result I got looks ok to me.

    June 11th, 2010 @ 03:21 AM

  2. Brad Forsyth
    Brad Forsyth

    In rails 2.3.5 the cache key for this example would be:

    views/hello/users/1-20100611015342

    The log should show 'Cached fragment miss: views/hello/users/1-20100611015342'

    June 11th, 2010 @ 08:48 PM

  3. Neeraj Singh
    Neeraj Singh

    In my application this is what I get.

    Cached fragment exists?: views/hello/users/1-20100611015731 (0.1ms)
    Cached fragment hit: hello#<User:0x10365c080> (0.5ms)
    

    Now the question is why in one case I see fully expanded key and not in the other case. Answer is below.

    def read_fragment
      self.class.benchmark "Cached fragment hit: #{key}" do
        key = fragment_cache_key(key)
        result = cache_store.read(key, options)
        result.respond_to?(:html_safe) ? result.html_safe : result
      end
    end
    
    def fragment_exist?(key, options = nil)
      key = fragment_cache_key(key)
      self.class.benchmark "Cached fragment exists?: #{key}" do
        cache_store.exist?(key, options)
      end
    end
    

    calling fragment_cache_key could be potentially expensive. So it's okay to leave the code as it is.

    June 11th, 2010 @ 09:59 PM

  4. Brad Forsyth
    Brad Forsyth

    I am not following this logic. The fragment_cache_key is called anyway, it should be called before the benchmark call so that the log clearly shows what key is being used. This is the way it worked previously and I see no advantage nor usefulness to the existing benchmark message.

    June 13th, 2010 @ 03:52 PM

  5. Neeraj Singh
    Neeraj Singh

    @Brad You are right. Not sure what I was thinking.

    I will provide a patch soon.

    June 13th, 2010 @ 04:27 PM

  6. Neeraj Singh
  7. Neeraj Singh
    Neeraj Singh
    • Tag changed from rails 2.3.8 to rails 2.3.8, patch

    June 13th, 2010 @ 05:29 PM

  8. Neeraj Singh
    Neeraj Singh
    • Assigned user set to José Valim

    June 13th, 2010 @ 09:53 PM

  9. José Valim
    José Valim
    • Milestone cleared.
    • State changed from new to open

    June 15th, 2010 @ 12:07 PM

  10. José Valim
    José Valim
    • Milestone set to 2.3.9

    Neeraj, thanks for the patch once again! Could you please check if the issue exists in Rails master as well?

    June 20th, 2010 @ 11:27 AM

  11. Repository
    Repository
    • State changed from open to resolved

    June 23rd, 2010 @ 08:17 AM