vinistock/sail

Caching a setting view fragment doesn't work because pagination doesn't return setting id

asko-ohmann opened this issue · 6 comments

The caching in setting view results in multiple copies of the same settging

<% cache setting, expires_in: Sail.configuration.cache_life_span do %>

I think the reason is in Setting.paginated method

scope :paginated, lambda { |page, per_page|
select(:name, :description, :group, :value, :cast_type, :updated_at)
.offset(page.to_i * per_page)
.limit(per_page)
}

The cache must rely on the entity id to generate the cache key but the id is not present in paginated result.

Thanks for the report. My understanding was that the method cache for fragments received a name as the first argument. The key evaluation takes place in actionpack/lib/abstract_controller/caching/fragments in the method combined_fragment_cache_key.

In my local setup, the cache key is evaluated to:

[:views, "sail/settings/_setting:afa80981078be4225ff318e8172829e2", #<Sail::Setting id: nil, name: "max_survey_answers", description: "Maximum answers per user for a survey", value: "3", cast_type: "integer", updated_at: "2019-02-22 01:17:43", group: "app">]

I'd expect it to work despite the missing ID (since setting names must be unique). I'm I missing something? Were you able to observe this in an application and can you provide reproduction steps?

Thanks for the quick reply.

Yes I observed it in a real application. When more than one setting is present the view is just repeated with the same data.

In my redis cache store it only has one key for setting and looks something like

views/sail/settings/_setting:6a7d0ddeefaae5ddc7e8cb2fe1f61ac7/sail/settings/

When I changed the pagination method to return ID as well the cache keys generated are for example

views/sail/settings/_setting:6a7d0ddeefaae5ddc7e8cb2fe1f61ac7/sail/settings/275
views/sail/settings/_setting:6a7d0ddeefaae5ddc7e8cb2fe1f61ac7/sail/settings/279

Looking into Rails cache helper it also seems to confirm that the cache key is generated with id https://api.rubyonrails.org/classes/ActionView/Helpers/CacheHelper.html#method-i-cache

That sounds about right. Would you like to put a PR together to fix it? If not, I can work on it. Thanks for reporting the issue by the way.

This was a weird bug that we with @asko-ohmann have experienced, I'm pretty sure that it worked correctly at some point. Perhaps something changed in rails cache mechanism after I updated it from 5.1 to 5.2.
Any hint when the new release is going out @vinistock ?

@oleg-kiviljov Yeah, I'm surprised that it stopped working. I can release the new version later today once the fix is merged. Thanks for all the support ❤️.

Closed by #176