Merge pull request #4268 from rmosolgo/fix-duplicate-second-level-fra…

…gment-name

Properly handle duplicate fragment names used indirectly

lib/graphql/static_validation/definition_dependencies.rb

@@ -127,8 +127,14 @@ def resolve_dependencies
         # same name as if they were the same name. If _any_ of the fragments
         # with that name has a dependency, we record it.
         independent_fragment_nodes = @defdep_fragment_definitions.values.flatten - @defdep_immediate_dependencies.keys
-
+        visited_fragment_names = Set.new
         while fragment_node = independent_fragment_nodes.pop
+          if visited_fragment_names.add?(fragment_node.name)
+            # this is a new fragment name
+          else
+            # this is a duplicate fragment name
+            next
+          end
           loops += 1
           if loops > max_loops
             raise("Resolution loops exceeded the number of definitions; infinite loop detected. (Max: #{max_loops}, Current: #{loops})")

spec/graphql/static_validation/rules/fragment_names_are_unique_spec.rb

@@ -50,4 +50,29 @@
       assert_includes(errors, fragment_def_error)
     end
   end
+
+  describe "when used at second level" do
+    let(:query_string) {"
+      query {
+        cheese(id: 1) {
+          ... frag1
+        }
+      }
+
+      fragment frag1 on Cheese { ...frag2 }
+      fragment frag2 on Cheese { id }
+      fragment frag2 on Cheese { id }
+    "}
+
+    it "finds the error" do
+      assert_equal(1, errors.length)
+      fragment_def_error = {
+        "message"=>"Fragment name \"frag2\" must be unique",
+        "locations"=>[{"line"=>9, "column"=>7}, {"line"=>10, "column"=>7}],
+        "path"=>[],
+        "extensions"=>{"code"=>"fragmentNotUnique", "fragmentName"=>"frag2"}
+      }
+      assert_includes(errors, fragment_def_error)
+    end
+  end
 end