codenoble / cache-crispies

Speedy Rails JSON serialization with built-in caching
MIT License
156 stars 14 forks source link

Add cache_key_method configuration option #32

Closed Flixt closed 3 years ago

Flixt commented 3 years ago

Hi there,

we have the case currently, that the hard coded call to cacheable.cache_key in the Plan is not outdating our cache when we update a model. The reason is, that the Model#cache_key value does not change after the model is updated, which is correct behavior for Rails >= 5.2 since the cache_key method should be used to for "recyclable" cache keys.

Cache versioning is disabled by default for Rails 5.2, >= 6.0 (ActiveRecord::Base.cache_versioning = false). Cache versioning is enabled by default for Rails >= 6.0 (ActiveRecord::Base.cache_versioning = true).

See https://api.rubyonrails.org/v5.2/classes/ActiveRecord/Integration.html#method-i-cache_key

I opened this PR for a few reasons instead of just overwriting cache_key on my models:

I had several other solutions in mind to solve this "problem".

I did not choose them because from my point of view they have one major draw back, that is: They are hard to reason about in an inheritance chain of serializers (which I think is even with a simple setup, at least 3 classes deep, MySerializer < MyBaseSerializer < CacheCrispies::Base). It is not really clear if such a configuration would be inherited or not.

I also did not chose to extend the addons block feature because it takes a single positional argument currently (options) and passing it the model as second positional parameter would not be consistent with the rest of the API (show_if etc..). Would be a little challenge to make this change backwards compatible and consistent with the current api (probably something to consider for a major release?).

Another reason is, I don't think it is neither necessary nor a good thing to animate developers to use different methods on different models to generate the CacheCrispies cacheable.cache_key. With the chosen solution it would be possible to enhance it to support a Proc as the cache_key_method that is called and gets passed the options and the model instance.

diazweb commented 3 years ago

Hey! This PR its important to make the cache work on rails 6.X, good job @Flixt

Have you give a look into this @adamcrown?

adamcrown commented 3 years ago

This looks great. Thanks Felix!