Merge pull request #75 from freshtonic/validations/unique-operation-names

Validations/unique operation names

Author: Josh Price

lib/graphql/lang/ast/parallel_visitor.ex

@@ -0,0 +1,67 @@
+
+defmodule GraphQL.Lang.AST.ParallelVisitor do
+  @moduledoc ~S"""
+  A ParallelVisitor runs all child visitors in parallel instead of serially like the CompositeVisitor.
+
+  In this context, 'in parallel' really means that for each node in the AST, each visitor will be invoked
+  for each node in the AST, but the :break/:continue return value of enter and leave is maintained per-visitor.
+
+  This means invividual visitors can bail out of AST processing as soon as possible and not waste cycles.
+
+  This code based on the graphql-js *visitInParallel* function.
+  """
+
+  alias GraphQL.Lang.AST.Visitor
+  alias GraphQL.Lang.AST.InitialisingVisitor
+  alias GraphQL.Lang.AST.PostprocessingVisitor
+
+  defstruct visitors: []
+  
+  defimpl Visitor do
+    def enter(visitor, node, accumulator) do
+      accumulator = Enum.reduce(visitor.visitors, accumulator, fn(child_visitor, acc) ->
+        if !skipping?(accumulator, child_visitor) do
+          acc = case child_visitor |> Visitor.enter(node, accumulator) do
+            {:continue, next_accumulator} -> next_accumulator
+            {:break, next_accumulator} ->
+              put_in(next_accumulator[:skipping][child_visitor], node)
+          end
+        end
+        acc
+      end)
+      {:continue, accumulator}
+    end
+
+    def leave(visitor, node, accumulator) do
+      accumulator = Enum.reduce(visitor.visitors, accumulator, fn(child_visitor, acc) ->
+        cond do
+          !skipping?(acc, child_visitor) ->
+            {_, next_accumulator} = child_visitor |> Visitor.leave(node, acc)
+            next_accumulator
+          accumulator[:skipping][child_visitor] == node ->
+            put_in(acc[:skipping][child_visitor], false)
+          true -> accumulator
+        end
+      end)
+      {:continue, accumulator}
+    end
+
+    defp skipping?(accumulator, child_visitor) do
+      Map.has_key?(accumulator[:skipping], child_visitor)
+    end
+  end
+
+  defimpl InitialisingVisitor do
+    def init(visitor, accumulator) do
+      accumulator = put_in(accumulator[:skipping], %{})
+      Enum.reduce(visitor.visitors, accumulator, &InitialisingVisitor.init/2)
+    end
+  end
+
+  defimpl PostprocessingVisitor do
+    def finish(visitor, accumulator) do
+      Enum.reduce(visitor.visitors, accumulator, &PostprocessingVisitor.finish/2)
+    end
+  end
+
+end

lib/graphql/lang/ast/type_info.ex

@@ -58,7 +58,12 @@ defmodule GraphQL.Lang.AST.TypeInfo do
       name == Introspection.meta(:typename)[:name] ->
         Introspection.meta(:typename)
       parent_type.__struct__ == GraphQL.Type.ObjectType || parent_type.__struct__ == GraphQL.Type.Interface ->
-        parent_type.fields[name]
+        # FIXME: this "function or map" logic is repeated in the executor. DRY IT UP.
+        if is_function(parent_type.fields) do
+          parent_type.fields.()[name]
+        else
+          parent_type.fields[name]
+        end
       true ->
         nil
     end

lib/graphql/lang/ast/type_info_visitor.ex

@@ -13,6 +13,7 @@ defmodule GraphQL.Lang.AST.TypeInfoVisitor do
   alias GraphQL.Util.Stack
   alias GraphQL.Lang.AST.TypeInfo
   alias GraphQL.Lang.AST.Visitor
+  alias GraphQL.Schema
 
   defstruct name: "TypeInfoVisitor"
 
@@ -93,13 +94,13 @@ defmodule GraphQL.Lang.AST.TypeInfoVisitor do
           stack_push(:type_stack, type)
         kind when kind in [:InlineFragment, :FragmentDefinition] ->
           output_type = if node.typeCondition do
-            Schema.type_from_ast(accumulator[:type_info].schema, node.typeCondition)
+            Schema.type_from_ast(node.typeCondition, accumulator[:type_info].schema)
           else
             TypeInfo.type(accumulator[:type_info])
           end
           stack_push(:type_stack, output_type)
         :VariableDefinition ->
-          input_type =  Schema.type_from_ast(accumulator[:type_info].schema, node.type)
+          input_type = Schema.type_from_ast(node.type, accumulator[:type_info].schema)
           stack_push(:input_type_stack, input_type)
         :Argument ->
           field_or_directive = TypeInfo.directive(accumulator[:type_info]) ||

lib/graphql/type/schema.ex

@@ -14,6 +14,7 @@ defmodule GraphQL.Schema do
 
   defstruct query: nil, mutation: nil, types: []
 
+  # FIXME: I think *schema* should be the first argument in this module.
   def type_from_ast(nil, _), do: nil
   def type_from_ast(%{kind: :NonNullType,} = input_type_ast, schema) do
     %GraphQL.Type.NonNull{ofType: type_from_ast(input_type_ast.type, schema)}

lib/graphql/validation/rules/unique_operation_names.ex

@@ -0,0 +1,41 @@
+
+defmodule GraphQL.Validation.Rules.UniqueOperationNames do
+
+  alias GraphQL.Lang.AST.Visitor
+  alias GraphQL.Lang.AST.InitialisingVisitor
+
+  defstruct name: "UniqueOperationNames"
+
+  defimpl InitialisingVisitor do
+    def init(_visitor, accumulator) do
+      Map.merge(%{operation_names: %{}}, accumulator)
+    end
+  end
+
+  defimpl Visitor do
+    def enter(_visitor, node, accumulator) do
+      if node.kind == :OperationDefinition && Map.has_key?(node, :name) do
+        op_name = node.name
+        if op_name.value do
+          if accumulator[:operation_names][op_name.value] do
+            accumulator = put_in(
+              accumulator[:validation_errors],
+              [duplicate_operation_message(op_name)] ++ accumulator[:validation_errors]
+            )
+          else
+            accumulator = put_in(accumulator[:operation_names][op_name.value], true)
+          end
+        end
+      end
+      {:continue, accumulator}
+    end
+
+    def leave(_visitor, _node, accumulator) do
+      {:continue, accumulator}
+    end
+
+    defp duplicate_operation_message(op_name) do
+      "There can only be one operation named '#{op_name.value}'."
+    end
+  end
+end

test/graphql/validation/rules/unique_operation_names_test.exs

@@ -0,0 +1,108 @@
+
+Code.require_file "../../../support/validations.exs", __DIR__
+
+defmodule GraphQL.Validation.Rules.UniqueOperationNamesTest do
+  use ExUnit.Case, async: true
+
+  import ValidationsSupport
+
+  alias GraphQL.Validation.Rules.UniqueOperationNames
+
+  test "no operations" do
+    assert_passes_rule(""" 
+      fragment fragA on Type {
+        field
+      }
+    """, %UniqueOperationNames{})
+  end
+
+  test "one anon operation" do
+    assert_passes_rule("""
+      {
+        field
+      }
+    """, %UniqueOperationNames{})
+  end
+
+  test "one named operation" do
+    assert_passes_rule("""
+      query Foo {
+        field
+      }
+    """, %UniqueOperationNames{})
+  end
+
+  test "multiple operations" do
+    assert_passes_rule("""
+      query Foo {
+        field
+      }
+
+      query Bar {
+        field
+      }
+    """, %UniqueOperationNames{})
+  end
+
+  test "multiple operations of different types" do
+    assert_passes_rule("""
+      query Foo {
+        field
+      }
+
+      mutation Bar {
+        field
+      }
+
+      # TODO: add this when subscription support is added
+      #subscription Baz {
+      #  field
+      #}
+    """, %UniqueOperationNames{})
+  end
+
+  test "fragment and operation named the same" do
+    assert_passes_rule("""
+      query Foo {
+        ...Foo
+      }
+      fragment Foo on Type {
+        field
+      }
+    """, %UniqueOperationNames{})
+  end
+
+  test "multiple operations of same name" do
+    assert_fails_rule("""
+      query Foo {
+        fieldA
+      }
+      query Foo {
+        fieldB
+      }
+    """, %UniqueOperationNames{})
+  end
+
+  test "multiple ops of same name of different types (mutation)" do
+    assert_fails_rule("""
+      query Foo {
+        fieldA
+      }
+      mutation Foo {
+        fieldB
+      }
+    """, %UniqueOperationNames{})
+  end
+
+  @tag :skip
+  test "multiple ops of same name of different types (subscription)" do
+    assert_fails_rule("""
+      query Foo {
+        fieldA
+      }
+      subscription Foo {
+        fieldB
+      }
+    """, %UniqueOperationNames{})
+  end
+end