Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default cache key produces clashes #491

Open
samstickland opened this issue Jan 15, 2021 · 4 comments
Open

Default cache key produces clashes #491

samstickland opened this issue Jan 15, 2021 · 4 comments

Comments

@samstickland
Copy link

samstickland commented Jan 15, 2021

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
Copy link
Author

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

image

@yogeshjain999
Copy link
Member

@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
Copy link
Member

@yogeshjain999 also found trailblazer/trailblazer-cells#13 which is a the same bug.

@apotonick
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants