From 3335c177b8b66f7cb5f99e20ccc797dd79dfe4f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20Brand=C3=A3o?= Date: Fri, 8 Sep 2017 20:03:22 -0300 Subject: [PATCH 1/8] Add support for inline and named fragments --- lib/graphql/query_resolver.rb | 41 ++++++++++++++++++++++------- spec/graphql/query_resolver_spec.rb | 38 ++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 9 deletions(-) diff --git a/lib/graphql/query_resolver.rb b/lib/graphql/query_resolver.rb index 8031d20..d6b1387 100644 --- a/lib/graphql/query_resolver.rb +++ b/lib/graphql/query_resolver.rb @@ -8,7 +8,9 @@ def self.run(model_class, context, return_type) to_load = yield dependencies = {} - reflection_dependencies = map_dependencies(model_class, context.ast_node) + fragments = context.query.fragments + + reflection_dependencies = map_dependencies(model_class, context.ast_node, fragments) dependencies = reflection_dependencies.merge(dependencies) if dependencies.any? && to_load.present? @@ -30,11 +32,11 @@ def self.using_nodes_pagination?(selection) selection.name == 'nodes' end - def self.map_relay_pagination_depencies(class_name, selection, dependencies) + def self.map_relay_pagination_depencies(class_name, selection, fragment_definitions, dependencies) node_selection = selection.selections.find { |sel| sel.name == 'node' } if node_selection.present? - map_dependencies(class_name, node_selection, dependencies) + map_dependencies(class_name, node_selection, fragment_definitions, dependencies) else dependencies end @@ -44,28 +46,41 @@ def self.has_reflection_with_name?(class_name, selection_name) class_name.reflections.with_indifferent_access[selection_name].present? end - def self.map_dependencies(class_name, ast_node, dependencies={}) + def self.map_dependencies(class_name, ast_node, fragment_definitions, dependencies={}) ast_node.selections.each do |selection| - name = selection.name + if inline_fragment?(selection) + map_dependencies(class_name, selection, fragment_definitions, dependencies) + + next + end + + if fragment_spread?(selection) + fragment_definition = fragment_definitions[selection.name] + map_dependencies(class_name, fragment_definition, fragment_definitions, dependencies) + + next + end if using_relay_pagination?(selection) - map_relay_pagination_depencies(class_name, selection, dependencies) + map_relay_pagination_depencies(class_name, selection, fragment_definitions, dependencies) next end if using_nodes_pagination?(selection) - map_dependencies(class_name, selection, dependencies) + map_dependencies(class_name, fragment_definition, selection, dependencies) next end + name = selection.name + if has_reflection_with_name?(class_name, name) begin current_class_name = selection.name.singularize.classify.constantize - dependencies[name] = map_dependencies(current_class_name, selection) + dependencies[name] = map_dependencies(current_class_name, selection, fragment_definitions) rescue NameError selection_name = class_name.reflections.with_indifferent_access[selection.name].options[:class_name] current_class_name = selection_name.singularize.classify.constantize - dependencies[selection.name.to_sym] = map_dependencies(current_class_name, selection) + dependencies[selection.name.to_sym] = map_dependencies(current_class_name, selection, fragment_definitions) next end end @@ -73,5 +88,13 @@ def self.map_dependencies(class_name, ast_node, dependencies={}) dependencies end + + def self.inline_fragment?(selection) + GraphQL::Language::Nodes::InlineFragment === selection + end + + def self.fragment_spread?(selection) + GraphQL::Language::Nodes::FragmentSpread === selection + end end end diff --git a/spec/graphql/query_resolver_spec.rb b/spec/graphql/query_resolver_spec.rb index d91b0c1..69a7376 100644 --- a/spec/graphql/query_resolver_spec.rb +++ b/spec/graphql/query_resolver_spec.rb @@ -81,6 +81,44 @@ expect(queries[4]).to include('SELECT "vendors".* FROM "vendors" WHERE "vendors"."id" IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11)') end + it 'works with inline and spread fragments' do + data = nil + + queries = track_queries do + data = GQL.query(%{ + fragment ChefFragment on chef { + name + recipes { + title + ingredients { + name, quantity + vendor { + name + } + } + } + } + + query { + restaurant(id: 1) { + ... on restaurant { + name + owner { + ... ChefFragment + } + } + } + } + }) + end + + expect(queries[0]).to include('SELECT "restaurants".* FROM "restaurants" WHERE "restaurants"."id" = ?') + expect(queries[1]).to include('SELECT "chefs".* FROM "chefs" WHERE "chefs"."id"') # AR 4 will use id IN (1), AR 5 will use id = 1 + expect(queries[2]).to include('SELECT "recipes".* FROM "recipes" WHERE "recipes"."chef_id"') # AR 4 will use id IN (1), AR 5 will use id = 1 + expect(queries[3]).to include('SELECT "ingredients".* FROM "ingredients" WHERE "ingredients"."recipe_id" IN (1, 2, 3, 4)') + expect(queries[4]).to include('SELECT "vendors".* FROM "vendors" WHERE "vendors"."id" IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11)') + end + it 'works with alias reflections' do # Owner is an instance of Chef query = %{ From 99e4b846e49c6a133c869bcb201449ebdcfedccf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20Brand=C3=A3o?= Date: Sun, 10 Sep 2017 18:24:52 -0300 Subject: [PATCH 2/8] Fix implementation that's failing after merging conflict --- lib/graphql/query_resolver.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/graphql/query_resolver.rb b/lib/graphql/query_resolver.rb index d6b1387..a98827c 100644 --- a/lib/graphql/query_resolver.rb +++ b/lib/graphql/query_resolver.rb @@ -67,7 +67,7 @@ def self.map_dependencies(class_name, ast_node, fragment_definitions, dependenci end if using_nodes_pagination?(selection) - map_dependencies(class_name, fragment_definition, selection, dependencies) + map_dependencies(class_name, selection, fragment_definitions, dependencies) next end From 22c77d6283399effb0421cab9f857c849d522400 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20Brand=C3=A3o?= Date: Sun, 10 Sep 2017 18:32:26 -0300 Subject: [PATCH 3/8] Initial refactoring of Resolver Class --- lib/graphql/query_resolver.rb | 145 ++++++++++++++++++---------------- 1 file changed, 79 insertions(+), 66 deletions(-) diff --git a/lib/graphql/query_resolver.rb b/lib/graphql/query_resolver.rb index a98827c..7d3157f 100644 --- a/lib/graphql/query_resolver.rb +++ b/lib/graphql/query_resolver.rb @@ -4,97 +4,110 @@ module GraphQL module QueryResolver - def self.run(model_class, context, return_type) - to_load = yield - dependencies = {} - - fragments = context.query.fragments + def self.run(model_class, context, _return_type, &block) + Resolver.new(model_class, context).call(&block) + end - reflection_dependencies = map_dependencies(model_class, context.ast_node, fragments) - dependencies = reflection_dependencies.merge(dependencies) + class Resolver + attr_reader :context, :fragment_definitions - if dependencies.any? && to_load.present? - if ActiveRecord::VERSION::MAJOR < 4 - ActiveRecord::Associations::Preloader.new(to_load, dependencies).run - else - ActiveRecord::Associations::Preloader.new.preload(to_load, dependencies) - end + def initialize(model_class, context) + @model_class = model_class + @context = context end - to_load - end + def call + to_load = yield + dependencies = {} - def self.using_relay_pagination?(selection) - selection.name == 'edges' - end + @fragment_definitions = context.query.fragments - def self.using_nodes_pagination?(selection) - selection.name == 'nodes' - end + reflection_dependencies = map_dependencies(@model_class, context.ast_node) + dependencies = reflection_dependencies.merge(dependencies) - def self.map_relay_pagination_depencies(class_name, selection, fragment_definitions, dependencies) - node_selection = selection.selections.find { |sel| sel.name == 'node' } + if dependencies.any? && to_load.present? + if ActiveRecord::VERSION::MAJOR < 4 + ActiveRecord::Associations::Preloader.new(to_load, dependencies).run + else + ActiveRecord::Associations::Preloader.new.preload(to_load, dependencies) + end + end - if node_selection.present? - map_dependencies(class_name, node_selection, fragment_definitions, dependencies) - else - dependencies + to_load end - end - def self.has_reflection_with_name?(class_name, selection_name) - class_name.reflections.with_indifferent_access[selection_name].present? - end + def using_relay_pagination?(selection) + selection.name == 'edges' + end + + def using_nodes_pagination?(selection) + selection.name == 'nodes' + end - def self.map_dependencies(class_name, ast_node, fragment_definitions, dependencies={}) - ast_node.selections.each do |selection| - if inline_fragment?(selection) - map_dependencies(class_name, selection, fragment_definitions, dependencies) + def map_relay_pagination_depencies(class_name, selection, dependencies) + node_selection = selection.selections.find { |sel| sel.name == 'node' } - next + if node_selection.present? + map_dependencies(class_name, node_selection, dependencies) + else + dependencies end + end - if fragment_spread?(selection) - fragment_definition = fragment_definitions[selection.name] - map_dependencies(class_name, fragment_definition, fragment_definitions, dependencies) + def has_reflection_with_name?(class_name, selection_name) + class_name.reflections.with_indifferent_access[selection_name].present? + end - next - end + def map_dependencies(class_name, ast_node, dependencies={}) + ast_node.selections.each do |selection| + if inline_fragment?(selection) + map_dependencies(class_name, selection, dependencies) - if using_relay_pagination?(selection) - map_relay_pagination_depencies(class_name, selection, fragment_definitions, dependencies) - next - end + next + end - if using_nodes_pagination?(selection) - map_dependencies(class_name, selection, fragment_definitions, dependencies) - next - end + if fragment_spread?(selection) + fragment_definition = fragment_definitions[selection.name] + map_dependencies(class_name, fragment_definition, dependencies) - name = selection.name + next + end + + if using_relay_pagination?(selection) + map_relay_pagination_depencies(class_name, selection, dependencies) + next + end - if has_reflection_with_name?(class_name, name) - begin - current_class_name = selection.name.singularize.classify.constantize - dependencies[name] = map_dependencies(current_class_name, selection, fragment_definitions) - rescue NameError - selection_name = class_name.reflections.with_indifferent_access[selection.name].options[:class_name] - current_class_name = selection_name.singularize.classify.constantize - dependencies[selection.name.to_sym] = map_dependencies(current_class_name, selection, fragment_definitions) + if using_nodes_pagination?(selection) + map_dependencies(class_name, selection, dependencies) next end + + name = selection.name + + if has_reflection_with_name?(class_name, name) + begin + current_class_name = selection.name.singularize.classify.constantize + dependencies[name] = map_dependencies(current_class_name, selection) + rescue NameError + selection_name = class_name.reflections.with_indifferent_access[selection.name].options[:class_name] + current_class_name = selection_name.singularize.classify.constantize + dependencies[selection.name.to_sym] = map_dependencies(current_class_name, selection) + next + end + end end - end - dependencies - end + dependencies + end - def self.inline_fragment?(selection) - GraphQL::Language::Nodes::InlineFragment === selection - end + def inline_fragment?(selection) + GraphQL::Language::Nodes::InlineFragment === selection + end - def self.fragment_spread?(selection) - GraphQL::Language::Nodes::FragmentSpread === selection + def fragment_spread?(selection) + GraphQL::Language::Nodes::FragmentSpread === selection + end end end end From b7b2cf49ad7e35f402da282c9bfb73176d7ea442 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20Brand=C3=A3o?= Date: Sun, 10 Sep 2017 18:37:27 -0300 Subject: [PATCH 4/8] Cleaning up and renaming methods (Rubocop suggestions) --- lib/graphql/query_resolver.rb | 73 ++++++++++++++++------------------- 1 file changed, 34 insertions(+), 39 deletions(-) diff --git a/lib/graphql/query_resolver.rb b/lib/graphql/query_resolver.rb index 7d3157f..64933e0 100644 --- a/lib/graphql/query_resolver.rb +++ b/lib/graphql/query_resolver.rb @@ -3,7 +3,6 @@ module GraphQL module QueryResolver - def self.run(model_class, context, _return_type, &block) Resolver.new(model_class, context).call(&block) end @@ -14,53 +13,55 @@ class Resolver def initialize(model_class, context) @model_class = model_class @context = context + @fragment_definitions = context.query.fragments end def call to_load = yield - dependencies = {} - - @fragment_definitions = context.query.fragments - - reflection_dependencies = map_dependencies(@model_class, context.ast_node) - dependencies = reflection_dependencies.merge(dependencies) + dependencies = map_dependencies(@model_class, context.ast_node) if dependencies.any? && to_load.present? - if ActiveRecord::VERSION::MAJOR < 4 - ActiveRecord::Associations::Preloader.new(to_load, dependencies).run - else - ActiveRecord::Associations::Preloader.new.preload(to_load, dependencies) - end + preload_dependencies(to_load, dependencies) end to_load end - def using_relay_pagination?(selection) + private + + def preload_dependencies(to_load, dependencies) + if ActiveRecord::VERSION::MAJOR < 4 + return ActiveRecord::Associations::Preloader.new( + to_load, dependencies + ).run + end + + ActiveRecord::Associations::Preloader.new.preload(to_load, dependencies) + end + + def relay_connection_using_edges?(selection) selection.name == 'edges' end - def using_nodes_pagination?(selection) + def relay_connection_using_nodes?(selection) selection.name == 'nodes' end def map_relay_pagination_depencies(class_name, selection, dependencies) node_selection = selection.selections.find { |sel| sel.name == 'node' } - if node_selection.present? - map_dependencies(class_name, node_selection, dependencies) - else - dependencies - end + return unless node_selection.present? + + map_dependencies(class_name, node_selection, dependencies) end - def has_reflection_with_name?(class_name, selection_name) + def preloadable_reflection?(class_name, selection_name) class_name.reflections.with_indifferent_access[selection_name].present? end - def map_dependencies(class_name, ast_node, dependencies={}) + def map_dependencies(class_name, ast_node, dependencies = {}) ast_node.selections.each do |selection| - if inline_fragment?(selection) + if inline_fragment?(selection) || relay_connection_using_nodes?(selection) map_dependencies(class_name, selection, dependencies) next @@ -73,40 +74,34 @@ def map_dependencies(class_name, ast_node, dependencies={}) next end - if using_relay_pagination?(selection) + if relay_connection_using_edges?(selection) map_relay_pagination_depencies(class_name, selection, dependencies) - next - end - if using_nodes_pagination?(selection) - map_dependencies(class_name, selection, dependencies) next end name = selection.name + next unless preloadable_reflection?(class_name, name) - if has_reflection_with_name?(class_name, name) - begin - current_class_name = selection.name.singularize.classify.constantize - dependencies[name] = map_dependencies(current_class_name, selection) - rescue NameError - selection_name = class_name.reflections.with_indifferent_access[selection.name].options[:class_name] - current_class_name = selection_name.singularize.classify.constantize - dependencies[selection.name.to_sym] = map_dependencies(current_class_name, selection) - next - end + begin + current_class_name = name.singularize.classify.constantize + rescue NameError + selection_name = class_name.reflections[name].options[:class_name] + current_class_name = selection_name.singularize.classify.constantize end + + dependencies[name.to_sym] = map_dependencies(current_class_name, selection) end dependencies end def inline_fragment?(selection) - GraphQL::Language::Nodes::InlineFragment === selection + selection.is_a?(GraphQL::Language::Nodes::InlineFragment) end def fragment_spread?(selection) - GraphQL::Language::Nodes::FragmentSpread === selection + selection.is_a?(GraphQL::Language::Nodes::FragmentSpread) end end end From 3de0b27c462fc3d763c49415e9052007a9279adb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20Brand=C3=A3o?= Date: Sun, 10 Sep 2017 19:05:52 -0300 Subject: [PATCH 5/8] Further code reorganization and cleanup --- lib/graphql/query_resolver.rb | 58 +++++++++++------------------ spec/graphql/query_resolver_spec.rb | 30 +++++++++++++++ 2 files changed, 52 insertions(+), 36 deletions(-) diff --git a/lib/graphql/query_resolver.rb b/lib/graphql/query_resolver.rb index 64933e0..3d4be50 100644 --- a/lib/graphql/query_resolver.rb +++ b/lib/graphql/query_resolver.rb @@ -39,44 +39,18 @@ def preload_dependencies(to_load, dependencies) ActiveRecord::Associations::Preloader.new.preload(to_load, dependencies) end - def relay_connection_using_edges?(selection) - selection.name == 'edges' - end - - def relay_connection_using_nodes?(selection) - selection.name == 'nodes' - end - - def map_relay_pagination_depencies(class_name, selection, dependencies) - node_selection = selection.selections.find { |sel| sel.name == 'node' } - - return unless node_selection.present? - - map_dependencies(class_name, node_selection, dependencies) - end - - def preloadable_reflection?(class_name, selection_name) - class_name.reflections.with_indifferent_access[selection_name].present? - end - def map_dependencies(class_name, ast_node, dependencies = {}) ast_node.selections.each do |selection| if inline_fragment?(selection) || relay_connection_using_nodes?(selection) - map_dependencies(class_name, selection, dependencies) - - next + dig_deeper = selection + elsif fragment_spread?(selection) + dig_deeper = fragment_definitions[selection.name] + elsif relay_connection_using_edges?(selection) + dig_deeper = selection.selections.find { |sel| sel.name == 'node' } end - if fragment_spread?(selection) - fragment_definition = fragment_definitions[selection.name] - map_dependencies(class_name, fragment_definition, dependencies) - - next - end - - if relay_connection_using_edges?(selection) - map_relay_pagination_depencies(class_name, selection, dependencies) - + if dig_deeper.present? + map_dependencies(class_name, dig_deeper, dependencies) next end @@ -84,18 +58,30 @@ def map_dependencies(class_name, ast_node, dependencies = {}) next unless preloadable_reflection?(class_name, name) begin - current_class_name = name.singularize.classify.constantize + next_model_class = name.singularize.classify.constantize rescue NameError selection_name = class_name.reflections[name].options[:class_name] - current_class_name = selection_name.singularize.classify.constantize + next_model_class = selection_name.singularize.classify.constantize end - dependencies[name.to_sym] = map_dependencies(current_class_name, selection) + dependencies[name.to_sym] = map_dependencies(next_model_class, selection) end dependencies end + def preloadable_reflection?(class_name, selection_name) + class_name.reflections.with_indifferent_access[selection_name].present? + end + + def relay_connection_using_edges?(selection) + selection.name == 'edges' + end + + def relay_connection_using_nodes?(selection) + selection.name == 'nodes' + end + def inline_fragment?(selection) selection.is_a?(GraphQL::Language::Nodes::InlineFragment) end diff --git a/spec/graphql/query_resolver_spec.rb b/spec/graphql/query_resolver_spec.rb index 69a7376..b84a5b2 100644 --- a/spec/graphql/query_resolver_spec.rb +++ b/spec/graphql/query_resolver_spec.rb @@ -190,4 +190,34 @@ expect(queries.size).to eq(2) end + + it 'mixing "nodes" and fragments' do + query = %{ + query { + vendors(first: 5) { + nodes { + ... on vendor { + id + name + ingredients { + ... on ingredient { + name + quantity + } + } + } + } + pageInfo { + endCursor + hasNextPage + } + } + } + } + queries = track_queries do + GQL.query(query) + end + + expect(queries.size).to eq(2) + end end From ef802695164a36793c2fe86e2c13290ec6f1c333 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20Brand=C3=A3o?= Date: Mon, 11 Sep 2017 13:43:27 -0300 Subject: [PATCH 6/8] Rename `dig_deeper` variable to `proceed_to` I guess it reflects intention better. --- lib/graphql/query_resolver.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/graphql/query_resolver.rb b/lib/graphql/query_resolver.rb index 3d4be50..b093ba9 100644 --- a/lib/graphql/query_resolver.rb +++ b/lib/graphql/query_resolver.rb @@ -42,15 +42,15 @@ def preload_dependencies(to_load, dependencies) def map_dependencies(class_name, ast_node, dependencies = {}) ast_node.selections.each do |selection| if inline_fragment?(selection) || relay_connection_using_nodes?(selection) - dig_deeper = selection + proceed_to = selection elsif fragment_spread?(selection) - dig_deeper = fragment_definitions[selection.name] + proceed_to = fragment_definitions[selection.name] elsif relay_connection_using_edges?(selection) - dig_deeper = selection.selections.find { |sel| sel.name == 'node' } + proceed_to = selection.selections.find { |sel| sel.name == 'node' } end - if dig_deeper.present? - map_dependencies(class_name, dig_deeper, dependencies) + if proceed_to.present? + map_dependencies(class_name, proceed_to, dependencies) next end From 3ea990a83330960d4475eda5ea3d18600fe90187 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20Brand=C3=A3o?= Date: Mon, 11 Sep 2017 13:43:54 -0300 Subject: [PATCH 7/8] Use if not block form instead of guard-clause with unless --- lib/graphql/query_resolver.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/graphql/query_resolver.rb b/lib/graphql/query_resolver.rb index b093ba9..eedd88e 100644 --- a/lib/graphql/query_resolver.rb +++ b/lib/graphql/query_resolver.rb @@ -55,7 +55,9 @@ def map_dependencies(class_name, ast_node, dependencies = {}) end name = selection.name - next unless preloadable_reflection?(class_name, name) + if !preloadable_reflection?(class_name, name) + next + end begin next_model_class = name.singularize.classify.constantize From 74dc3c60271b2af8b07581b1d97e166f59481182 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20Brand=C3=A3o?= Date: Mon, 11 Sep 2017 13:51:39 -0300 Subject: [PATCH 8/8] Small note on Readme.md and adding changes to changelog --- CHANGELOG.md | 4 ++++ README.md | 3 +++ 2 files changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e8a23a..f0f3de4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +# 0.3.0 +- PR #5 - Add support to Relay style pagination using `nodes` shortcut +- PR #6 - Add support to inline and named fragments + # 0.2.0 - Add support to Relay style pagination diff --git a/README.md b/README.md index 98606f2..884e03e 100644 --- a/README.md +++ b/README.md @@ -12,6 +12,9 @@ Every matched selection will be then passed on to `ActiveRecord::Associations::Preloader.new` so your queries now only issue one `SELECT` statement for every level of the GraphQL AST. +The resolver works transparently with Relay-style pagination (either when using +`edges.node` or the `nodes` shortcut as selections) + ## Installation Add this line to your application's Gemfile: