Merge pull request #4865 from gmac/gmac/analysis_fragment_skip_include

Fix fragment skip/include tracking during analysis visitation

Author: rmosolgo

lib/graphql/analysis/ast/visitor.rb

@@ -118,8 +118,12 @@ def on_fragment_definition(node, parent)
         def on_inline_fragment(node, parent)
           on_fragment_with_type(node) do
             @path.push("...#{node.type ? " on #{node.type.name}" : ""}")
+            @skipping = @skip_stack.last || skip?(node)
+            @skip_stack << @skipping
+
             call_on_enter_inline_fragment(node, parent)
             super
+            @skipping = @skip_stack.pop
             call_on_leave_inline_fragment(node, parent)
           end
         end
@@ -187,9 +191,13 @@ def on_argument(node, parent)
 
         def on_fragment_spread(node, parent)
           @path.push("... #{node.name}")
+          @skipping = @skip_stack.last || skip?(node)
+          @skip_stack << @skipping
+
           call_on_enter_fragment_spread(node, parent)
           enter_fragment_spread_inline(node)
           super
+          @skipping = @skip_stack.pop
           leave_fragment_spread_inline(node)
           call_on_leave_fragment_spread(node, parent)
           @path.pop

spec/graphql/analysis/ast/query_complexity_spec.rb

@@ -21,15 +21,15 @@
 
   describe "simple queries" do
     let(:query_string) {%|
-      query cheeses($isSkipped: Boolean = false){
+      query cheeses {
         # complexity of 3
         cheese1: cheese(id: 1) {
           id
           flavor
         }
 
         # complexity of 4
-        cheese2: cheese(id: 2) @skip(if: $isSkipped) {
+        cheese2: cheese(id: 2) {
           similarCheese(source: SHEEP) {
             ... on Cheese {
               similarCheese(source: SHEEP) {
@@ -45,12 +45,40 @@
       complexities = reduce_result.first
       assert_equal 7, complexities
     end
+  end
+
+  describe "with skip/include" do
+    let(:query_string) {%|
+      query cheeses($skip: Boolean = false, $include: Boolean = true) {
+        fields: cheese(id: 1) {
+          flavor
+          origin @skip(if: $skip)
+          source @include(if: $include)
+        }
+        inlineFragments: cheese(id: 1) {
+          ...on Cheese { flavor }
+          ...on Cheese @skip(if: $skip) { origin }
+          ...on Cheese @include(if: $include) { source }
+        }
+        fragmentSpreads: cheese(id: 1) {
+          ...Flavorful
+          ...Original @skip(if: $skip)
+          ...Sourced @include(if: $include)
+        }
+      }
+      fragment Flavorful on Cheese { flavor }
+      fragment Original on Cheese { origin }
+      fragment Sourced on Cheese { source }
+    |}
+
+    it "sums up all included complexities" do
+      assert_equal 12, reduce_result.first
+    end
 
     describe "when skipped by directives" do
-      let(:variables) { { "isSkipped" => true } }
-      it "doesn't include skipped fields" do
-        complexity = reduce_result.first
-        assert_equal 3, complexity
+      let(:variables) { { "skip" => true, "include" => false } }
+      it "doesn't include skipped fields and fragments" do
+        assert_equal 6, reduce_result.first
       end
     end
   end

spec/graphql/analysis/ast_spec.rb

@@ -105,6 +105,122 @@ def result
     end
   end
 
+  class AstSkipInclude < GraphQL::Analysis::AST::Analyzer
+    def initialize(query)
+      super
+      @included = []
+    end
+
+    def on_enter_field(node, parent, visitor)
+      @included << "enter #{node.name}" unless visitor.skipping?
+    end
+
+    def on_leave_field(node, parent, visitor)
+      @included << "leave #{node.name}" unless visitor.skipping?
+    end
+
+    def on_enter_inline_fragment(node, parent, visitor)
+      @included << "enter ...on #{node.type.name}" unless visitor.skipping?
+    end
+
+    def on_leave_inline_fragment(node, parent, visitor)
+      @included << "leave ...on #{node.type.name}" unless visitor.skipping?
+    end
+
+    def on_enter_fragment_spread(node, parent, visitor)
+      @included << "enter ...#{node.name}" unless visitor.skipping?
+    end
+
+    def on_leave_fragment_spread(node, parent, visitor)
+      @included << "leave ...#{node.name}" unless visitor.skipping?
+    end
+
+    def result
+      @included
+    end
+  end
+
+  describe "skip and include behaviors" do
+    let(:reduce_result) { GraphQL::Analysis::AST.analyze_query(query, [AstSkipInclude]) }
+    let(:query) { GraphQL::Query.new(Dummy::Schema, query_string) }
+    let(:query_string) {%|{}|}
+
+    describe "for fields" do
+      let(:query_string) {%|
+        {
+          cheese {
+            flavor
+            origin @skip(if: true)
+            source @include(if: false)
+          }
+          cheese @skip(if: true) { flavor }
+          cheese @include(if: false) { flavor }
+        }
+      |}
+
+      it "tracks inclusions" do
+        expected = [
+          "enter cheese",
+          "enter flavor",
+          "leave flavor",
+          "leave cheese",
+        ]
+        assert_equal expected, reduce_result.first
+      end
+    end
+
+    describe "for inline fragments" do
+      let(:query_string) {%|
+        {
+          cheese {
+            ...on Cheese @skip(if: true) { origin }
+            ...on Cheese { flavor }
+            ...on Cheese @include(if: false) { source }
+          }
+        }
+      |}
+
+      it "tracks inclusions" do
+        expected = [
+          "enter cheese",
+          "enter ...on Cheese",
+          "enter flavor",
+          "leave flavor",
+          "leave ...on Cheese",
+          "leave cheese",
+        ]
+        assert_equal expected, reduce_result.first
+      end
+    end
+
+    describe "for fragment spreads" do
+      let(:query_string) {%|
+        {
+          cheese {
+            ...Original @skip(if: true)
+            ...Flavorful
+            ...Sourced @include(if: false)
+          }
+        }
+        fragment Flavorful on Cheese { flavor }
+        fragment Original on Cheese { origin }
+        fragment Sourced on Cheese { source }
+      |}
+
+      it "tracks inclusions" do
+        expected = [
+          "enter cheese",
+          "enter ...Flavorful",
+          "enter flavor",
+          "leave flavor",
+          "leave ...Flavorful",
+          "leave cheese",
+        ]
+        assert_equal expected, reduce_result.first
+      end
+    end
+  end
+
   describe "using the AST analysis engine" do
     let(:schema) do
       query_type = Class.new(GraphQL::Schema::Object) do