Skip to content

Commit

Permalink
Ensure dataloader is called only once
Browse files Browse the repository at this point in the history
  • Loading branch information
DmitryTsepelev committed Jan 7, 2025
1 parent 74bc9c4 commit 56d59b3
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 24 deletions.
9 changes: 3 additions & 6 deletions lib/graphql/fragment_cache/fragment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand All @@ -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

Expand Down
7 changes: 0 additions & 7 deletions lib/graphql/fragment_cache/object_helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 11 additions & 0 deletions lib/graphql/fragment_cache/schema/lazy_cache_resolver.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
5 changes: 4 additions & 1 deletion spec/graphql/fragment_cache/object_helpers_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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

Expand Down
20 changes: 10 additions & 10 deletions spec/graphql/fragment_cache/schema/lazy_cache_resolver_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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, {})
Expand All @@ -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

0 comments on commit 56d59b3

Please sign in to comment.