Ensure dataloader is called only once

Author: DmitryTsepelev

lib/graphql/fragment_cache/fragment.rb

@@ -16,12 +16,10 @@ def read_multi(fragments)
             return fragments.map { |f| [f, f.read] }.to_h
           end
 
-          fragments_to_cache_keys = fragments
-            .map { |f| [f, f.cache_key] }.to_h
+          fragments_to_cache_keys = fragments.map { |f| [f, f.cache_key] }.to_h
 
           # Filter out all the cache_keys for fragments with renew_cache: true in their context
-          cache_keys = fragments_to_cache_keys
-            .reject { |k, _v| k.context[:renew_cache] == true }.values
+          cache_keys = fragments_to_cache_keys.reject { |k, _v| k.context[:renew_cache] == true }.values
 
           # If there are cache_keys look up values with read_multi otherwise return an empty hash
           cache_keys_to_values = if cache_keys.empty?
@@ -46,8 +44,7 @@ def read_multi(fragments)
           end
 
           # Fragmenst without values or with renew_cache: true in their context will have nil values like the read method
-          fragments_to_cache_keys
-            .map { |fragment, cache_key| [fragment, cache_keys_to_values[cache_key]] }.to_h
+          fragments_to_cache_keys.map { |fragment, cache_key| [fragment, cache_keys_to_values[cache_key]] }.to_h
         end
       end
 

lib/graphql/fragment_cache/object_helpers.rb

@@ -48,13 +48,6 @@ def cache_fragment(object_to_cache = NO_OBJECT, **options, &block)
 
         fragment = Fragment.new(context_to_use, **options)
 
-        if options.delete(:dataloader)
-          # the following line will block the current fiber until Dataloader is resolved, and we will
-          # use the resolved value as the `object_to_cache`
-          object_to_cache = block.call
-          block = nil
-        end
-
         GraphQL::FragmentCache::Schema::LazyCacheResolver.new(fragment, context_to_use, object_to_cache, &block)
       end
     end

lib/graphql/fragment_cache/schema/lazy_cache_resolver.rb

@@ -16,6 +16,8 @@ def initialize(fragment, query_ctx, object_to_cache, &block)
           @block = block
 
           @lazy_state[:pending_fragments] << @fragment
+
+          ensure_dataloader_resulution! if @fragment.options[:dataloader]
         end
 
         def resolve
@@ -35,6 +37,15 @@ def resolve
             @query_ctx.fragments << @fragment
           end
         end
+
+        private
+
+        def ensure_dataloader_resulution!
+          return if FragmentCache.cache_store.exist?(@fragment.cache_key)
+
+          @object_to_cache = @block.call
+          @block = nil
+        end
       end
     end
   end

spec/graphql/fragment_cache/object_helpers_spec.rb

@@ -804,12 +804,13 @@ def post(id:, expires_in: nil)
     let!(:post1) { Post.create(id: 1, title: "object test 1", author: user1) }
     let!(:post2) { Post.create(id: 2, title: "object test 2", author: user2) }
 
+    let(:memory_store) { GraphQL::FragmentCache::MemoryStore.new }
+
     before do
       allow(User).to receive(:find_by_post_ids).and_call_original
 
       # warmup cache
       execute_query
-      expect(User).to have_received(:find_by_post_ids).with([post1.id, post2.id])
 
       # make objects dirty
       user1.name = "User #1 new"
@@ -827,6 +828,8 @@ def post(id:, expires_in: nil)
           "dataloaderCachedAuthor" => {"name" => "User #2"}
         }
       ])
+
+      expect(User).to have_received(:find_by_post_ids).with([post1.id, post2.id]).once
     end
   end
 

spec/graphql/fragment_cache/schema/lazy_cache_resolver_spec.rb

@@ -6,21 +6,27 @@
   describe "#initialize" do
     context "lazy cache resolver state management" do
       let(:state_key) { :lazy_cache_resolver_statez }
+      let(:gql_context) { instance_double "Context" }
+      let(:fragment) { GraphQL::FragmentCache::Fragment.new(gql_context) }
+
+      before do
+        allow(gql_context).to receive(:namespace).and_return({})
+      end
 
       it "adds lazy state property to the query context" do
         context = {}
 
         expect(context).not_to have_key(state_key)
 
-        GraphQL::FragmentCache::Schema::LazyCacheResolver.new(nil, context, {})
+        GraphQL::FragmentCache::Schema::LazyCacheResolver.new(fragment, context, {})
 
         expect(context).to have_key(state_key)
       end
 
       it "has :pending_fragments Set in state" do
         context = {}
 
-        GraphQL::FragmentCache::Schema::LazyCacheResolver.new({}, context, {})
+        GraphQL::FragmentCache::Schema::LazyCacheResolver.new(fragment, context, {})
 
         expect(context[state_key]).to have_key(:pending_fragments)
         expect(context[state_key][:pending_fragments]).to be_instance_of(Set)
@@ -29,7 +35,7 @@
       it "has :resolved_fragments Hash in state" do
         context = {}
 
-        GraphQL::FragmentCache::Schema::LazyCacheResolver.new({}, context, {})
+        GraphQL::FragmentCache::Schema::LazyCacheResolver.new(fragment, context, {})
 
         expect(context[state_key]).to have_key(:resolved_fragments)
         expect(context[state_key][:resolved_fragments]).to be_instance_of(Hash)
@@ -39,7 +45,7 @@
         context = {}
         fragments = []
 
-        3.times { fragments.push(Object.new) }
+        3.times { fragments.push(GraphQL::FragmentCache::Fragment.new(gql_context)) }
 
         fragments.each do |f|
           GraphQL::FragmentCache::Schema::LazyCacheResolver.new(f, context, {})
@@ -51,10 +57,4 @@
       end
     end
   end
-
-  it "has :resolve method" do
-    lazy_cache_resolver = GraphQL::FragmentCache::Schema::LazyCacheResolver.new({}, {}, {})
-
-    expect(lazy_cache_resolver).to respond_to(:resolve)
-  end
 end