DmitryTsepelev/graphql-ruby-fragment_cache

Adding an additional cache key

bbugh opened this issue · 4 comments

bbugh commented

We have a use case in Rails where we need to specify a cache key (from a Rails ActiveRecord::Relation), but it's not the return value - the relation is processed, which takes quite a long time. Unfortunately, specifying query_cache_key removes the path and the selections keys, which we also need. There does not seem to be a way to specify "extra key". Something like this:

cache_fragment(object_cache_key: where_class.cache_key_with_version) { where_class.do_something }

And the resulting cache key would be:

52355f50328bf0e765dd0cfcc89f11a5f503a082/special_models/query-f99c1b73939dae707295bf67deadbeef-1-20210212224532037173

(or it would be hashed)

I'm thinking it would be like this:

    class CacheKeyBuilder
      def object_key(obj)
        @options.fetch(:object_cache_key) { obj._graphql_cache_key }
      end
    end

Do you think this is a reasonable feature?

Hi @bbugh! The feature sounds helpful, I wonder if it makes sense to use more general approach and pass custom key via options:

class CacheKeyBuilder
  key = "#{schema_cache_key}/#{query_cache_key}"
  key = "#{key}/#{options[:object_cache_key]}" if options[:object_cache_key]
  Digest::SHA1.hexdigest ...
end

cache_fragment(object_cache_key: where_class.whatever_we_want) { where_class.do_something }
bbugh commented

That seems great! That also prevents having to change object_key in three places.

I think you'd have to also check for the options[:object_cache_key]:

key = "#{schema_cache_key}/#{query_cache_key}"
key = "#{key}/#{options[:object_cache_key]}" if options[:object_cache_key]
Digest::SHA1.hexdigest("#{schema_cache_key}/#{query_cache_key}").then do |base_key|
  next base_key unless object || options[:object_cache_key]
  "#{base_key}/#{object_key(object)}"
end

Maybe a little more readable:

def build
  Digest::SHA1.hexdigest("#{schema_cache_key}/#{query_cache_key}").then do |base_key|
    if options[:object_cache_key]
      "#{base_key}/#{options[:object_cache_key]}"
    elsif object
      "#{base_key}/#{object_key(object)}"
    else
      base_key
    end
  end
end

Does that seem fine? I can make a PR for this and README changes.

Yeah, looks pretty good! I'll be happy to review this PR

bbugh commented

Added this feature in #50