trailblazer / cells

View components for Ruby and Rails.
https://trailblazer.to/2.1/docs/cells.html
3.06k stars 236 forks source link

Default cache key produces clashes #491

Open samstickland opened 3 years ago

samstickland commented 3 years ago

If there are two cells in the same namespace the same cache will be produced by default, which of course causes all sorts of problems in production.

In caching.rb we have:

def state_cache_key(state, key_parts={})
  expand_cache_key([controller_path, state, key_parts])
end

https://github.com/trailblazer/cells/blob/master/lib/cell/caching.rb#L30

And in cell.rb

def controller_path
  @controller_path ||= File.join(util.underscore(name.sub(/(::Cell.+)/, '')), views_dir)
end

https://github.com/trailblazer/trailblazer-cells/blob/master/lib/trailblazer/cell.rb#L36

This means that a cells called SomeNamespace::Cell::One and SomeNamespace::Cell::Two will produce the same controller_path and hence the same cache key.

I can't see why we simply wouldn't the class name for cache key?

Our workaround for this is currently:

cache :show do
  self.class.name
end

etc.

I think the first code should be:

def state_cache_key(state, key_parts={})
  expand_cache_key([self.class.name, state, key_parts])
end
samstickland commented 3 years ago

As example, this was the result of a cache key clash in production!

image

yogeshjain999 commented 3 years ago

@samstickland Thanks for reporting this! Created #494 for same but with the difference of using snake case style for naming instead of direct class names.

apotonick commented 3 years ago

@yogeshjain999 also found https://github.com/trailblazer/trailblazer-cells/issues/13 which is a the same bug.

apotonick commented 3 years ago

@yogeshjain999 Can't we fix this in Trailblazer::Cell? I think it's a problem there and not in the cells gem?