Merge pull request #1195 from maartenvanvliet/validate-unions

Validate type references for invalid wrapped types

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## Unreleased
 
+- Bug Fix: [Validate type references for invalid wrapped types](https://github.com/absinthe-graphql/absinthe/pull/1195)
 - Breaking Bugfix: [Validate repeatable directives on schemas](https://github.com/absinthe-graphql/absinthe/pull/1179)
 - Bug Fix: Adds **optional fix** for non compliant built-in scalar Int type. `use Absinthe.Schema, use_spec_compliant_int_scalar: true` in your schema to use the fixed Int type. It is also advisable to upgrade for custom types if you are leveraging the use of integers outside the GraphQl standard. [#1131](https://github.com/absinthe-graphql/absinthe/pull/1131).
 - Feature: [Support error tuples when scalar parsing fails](https://github.com/absinthe-graphql/absinthe/pull/1187)

lib/absinthe/blueprint/schema.ex

@@ -160,6 +160,11 @@ defmodule Absinthe.Blueprint.Schema do
     build_types(rest, [obj | stack], buff)
   end
 
+  defp build_types([{:type, type} | rest], [obj | stack], buff) do
+    obj = Map.update!(obj, :types, &[type | &1])
+    build_types(rest, [obj | stack], buff)
+  end
+
   defp build_types([{:__private__, private} | rest], [entity | stack], buff) do
     entity = Map.update!(entity, :__private__, &update_private(&1, private))
     build_types(rest, [entity | stack], buff)
@@ -265,6 +270,7 @@ defmodule Absinthe.Blueprint.Schema do
   end
 
   defp build_types([:close | rest], [%Schema.UnionTypeDefinition{} = union, schema | stack], buff) do
+    union = Map.update!(union, :types, &Enum.reverse/1)
     build_types(rest, [push(schema, :type_definitions, union) | stack], buff)
   end
 

lib/absinthe/blueprint/type_reference.ex

@@ -45,6 +45,10 @@ defmodule Absinthe.Blueprint.TypeReference do
     name
   end
 
+  def name(name) do
+    name |> to_string() |> String.capitalize()
+  end
+
   def to_type(%__MODULE__.NonNull{of_type: type}, schema) do
     %Absinthe.Type.NonNull{of_type: to_type(type, schema)}
   end

lib/absinthe/phase/schema/validation/type_references_exist.ex

@@ -1,6 +1,9 @@
 defmodule Absinthe.Phase.Schema.Validation.TypeReferencesExist do
   @moduledoc false
 
+  # Checks whether all types referenced in the schema exist and
+  # are of the correct kind.
+
   use Absinthe.Phase
   alias Absinthe.Blueprint
   alias Absinthe.Blueprint.Schema
@@ -24,29 +27,37 @@ defmodule Absinthe.Phase.Schema.Validation.TypeReferencesExist do
   def validate_schema(node), do: node
 
   def validate_types(%Blueprint.Schema.FieldDefinition{} = field, types) do
-    check_or_error(field, field.type, types)
+    check_or_error(field, field.type, types, inner_type: true)
   end
 
   def validate_types(%Blueprint.Schema.ObjectTypeDefinition{} = object, types) do
     object
-    |> check_types(:interfaces, &check_or_error(&2, &1, types))
-    |> check_types(:imports, fn {type, _}, obj -> check_or_error(obj, type, types) end)
+    |> check_types(:interfaces, &check_or_error(&2, &1, types, inner_type: false))
+    |> check_types(:imports, fn {type, _}, obj ->
+      check_or_error(obj, type, types, inner_type: false)
+    end)
   end
 
   def validate_types(%Blueprint.Schema.InterfaceTypeDefinition{} = interface, types) do
-    check_types(interface, :interfaces, &check_or_error(&2, &1, types))
+    interface
+    |> check_types(:interfaces, &check_or_error(&2, &1, types, inner_type: false))
+    |> check_types(:imports, fn {type, _}, obj ->
+      check_or_error(obj, type, types, inner_type: false)
+    end)
   end
 
   def validate_types(%Blueprint.Schema.InputObjectTypeDefinition{} = object, types) do
-    check_types(object, :imports, fn {type, _}, obj -> check_or_error(obj, type, types) end)
+    check_types(object, :imports, fn {type, _}, obj ->
+      check_or_error(obj, type, types, inner_type: false)
+    end)
   end
 
   def validate_types(%Blueprint.Schema.InputValueDefinition{} = input, types) do
-    check_or_error(input, input.type, types)
+    check_or_error(input, input.type, types, inner_type: true)
   end
 
   def validate_types(%Blueprint.Schema.UnionTypeDefinition{} = union, types) do
-    check_types(union, :types, &check_or_error(&2, &1, types))
+    check_types(union, :types, &check_or_error(&2, &1, types, inner_type: false))
   end
 
   def validate_types(%Blueprint.Schema.TypeExtensionDefinition{} = extension, types) do
@@ -55,22 +66,21 @@ defmodule Absinthe.Phase.Schema.Validation.TypeReferencesExist do
         declaration
 
       definition ->
-        check_or_error(extension, definition.identifier, types)
+        check_or_error(extension, definition.identifier, types, inner_type: false)
     end
   end
 
   @no_types [
     Blueprint.Schema.DirectiveDefinition,
     Blueprint.Schema.EnumTypeDefinition,
     Blueprint.Schema.EnumValueDefinition,
-    Blueprint.Schema.InterfaceTypeDefinition,
-    Blueprint.Schema.ObjectTypeDefinition,
     Blueprint.Schema.ScalarTypeDefinition,
     Blueprint.Schema.SchemaDefinition,
     Blueprint.Schema.SchemaDeclaration,
     Blueprint.TypeReference.NonNull,
     Blueprint.TypeReference.ListOf,
-    Absinthe.Blueprint.TypeReference.Name
+    Blueprint.TypeReference.Name,
+    Blueprint.TypeReference.Identifier
   ]
   def validate_types(%struct{} = type, _) when struct in @no_types do
     type
@@ -86,34 +96,46 @@ defmodule Absinthe.Phase.Schema.Validation.TypeReferencesExist do
     |> Enum.reduce(entity, fun)
   end
 
-  defp check_or_error(thing, type, types) do
-    type = unwrap(type)
+  defp check_or_error(thing, type, types, opts) when is_list(opts) do
+    check_or_error(thing, type, types, Map.new(opts))
+  end
+
+  defp check_or_error(thing, type, types, %{inner_type: true}) do
+    type = inner_type(type)
+    check_or_error(thing, type, types, inner_type: false)
+  end
+
+  defp check_or_error(thing, type, types, %{inner_type: false}) do
+    case unwrapped?(type) do
+      {:ok, type} ->
+        if type in types do
+          thing
+        else
+          put_error(thing, error(thing, type))
+        end
 
-    if type in types do
-      thing
-    else
-      put_error(thing, error(thing, type))
+      :error ->
+        put_error(thing, wrapped_error(thing, type))
     end
   end
 
-  defp unwrap(value) when is_binary(value) or is_atom(value) do
+  defp inner_type(value) when is_binary(value) or is_atom(value) do
     value
   end
 
-  defp unwrap(%Absinthe.Blueprint.TypeReference.Name{name: name}) do
-    name
+  defp inner_type(%{of_type: type}) do
+    inner_type(type)
   end
 
-  defp unwrap(type) do
-    unwrap_type = Absinthe.Blueprint.TypeReference.unwrap(type)
-
-    if unwrap_type == type do
-      type
-    else
-      unwrap(unwrap_type)
-    end
+  defp inner_type(%Absinthe.Blueprint.TypeReference.Name{name: name}) do
+    name
   end
 
+  defp unwrapped?(value) when is_binary(value) or is_atom(value), do: {:ok, value}
+  defp unwrapped?(%Absinthe.Blueprint.TypeReference.Name{name: name}), do: {:ok, name}
+  defp unwrapped?(%Absinthe.Blueprint.TypeReference.Identifier{id: id}), do: {:ok, id}
+  defp unwrapped?(_), do: :error
+
   defp error(thing, type) do
     %Absinthe.Phase.Error{
       message: message(thing, type),
@@ -141,4 +163,24 @@ defmodule Absinthe.Phase.Schema.Validation.TypeReferencesExist do
     Types must exist if referenced.
     """
   end
+
+  defp wrapped_error(thing, type) do
+    %Absinthe.Phase.Error{
+      message: wrapped_message(thing, type),
+      locations: [thing.__reference__.location],
+      phase: __MODULE__
+    }
+  end
+
+  defp wrapped_message(thing, type) do
+    kind = Absinthe.Blueprint.Schema.struct_to_kind(thing)
+    artifact_name = String.capitalize(thing.name)
+
+    """
+    In #{kind} #{artifact_name}, cannot accept a non-null or a list type.
+
+    Got: #{Blueprint.TypeReference.name(type)}
+
+    """
+  end
 end

lib/absinthe/schema/notation.ex

@@ -1710,11 +1710,9 @@ defmodule Absinthe.Schema.Notation do
 
   @doc false
   # Record an implemented interface in the current scope
-  def record_interface!(env, identifier) do
-    put_attr(env.module, {:interface, identifier})
-    # Scope.put_attribute(env.module, :interfaces, identifier, accumulate: true)
-    # Scope.recorded!(env.module, :attr, :interface)
-    # :ok
+  def record_interface!(env, type) do
+    type = expand_ast(type, env)
+    put_attr(env.module, {:interface, type})
   end
 
   @doc false
@@ -1734,7 +1732,12 @@ defmodule Absinthe.Schema.Notation do
   @doc false
   # Record a list of member types for a union in the current scope
   def record_types!(env, types) do
-    put_attr(env.module, {:types, types})
+    Enum.each(types, &record_type!(env, &1))
+  end
+
+  defp record_type!(env, type) do
+    type = expand_ast(type, env)
+    put_attr(env.module, {:type, type})
   end
 
   @doc false

test/absinthe/phase/schema/type_references_exist_test.exs

@@ -0,0 +1,147 @@
+defmodule Absinthe.Schema.Validation.TypeReferencesExistTest do
+  use Absinthe.Case, async: true
+
+  describe "fields" do
+    @schema ~S{
+  defmodule FieldSchema do
+    use Absinthe.Schema
+
+    query do
+      field :foo, :string
+      field :bar, non_null(:string)
+      field :baz, :qux
+    end
+  end
+  }
+
+    test "errors unknown type reference" do
+      error = ~r/In field Baz, :qux is not defined in your schema./
+
+      assert_raise(Absinthe.Schema.Error, error, fn ->
+        Code.eval_string(@schema, [], __ENV__)
+      end)
+    end
+  end
+
+  describe "objects" do
+    @schema ~S{
+    defmodule ObjectImportSchema do
+      use Absinthe.Schema
+
+      query do
+        field :foo, :string
+      end
+
+      object :baz do
+        field :name, :string
+      end
+
+      object :bar do
+        import_fields non_null(:baz)
+      end
+    end
+    }
+    test "errors on import_fields with wrapped type" do
+      error = ~r/In object Bar, cannot accept a non-null or a list type.\n\nGot: Baz!/
+
+      assert_raise(Absinthe.Schema.Error, error, fn ->
+        Code.eval_string(@schema, [], __ENV__)
+      end)
+    end
+
+    @schema ~S{
+    defmodule ObjectInterfaceSchema do
+      use Absinthe.Schema
+
+      query do
+        field :foo, :string
+      end
+
+      object :baz do
+        field :name, :string
+      end
+
+      object :qux do
+        interface list_of(:baz)
+      end
+    end
+    }
+    test "errors on interface with wrapped type" do
+      error = ~r/In object Qux, cannot accept a non-null or a list type./
+
+      assert_raise(Absinthe.Schema.Error, error, fn ->
+        Code.eval_string(@schema, [], __ENV__)
+      end)
+    end
+  end
+
+  describe "interface" do
+    @schema ~S{
+    defmodule InterfaceSchema do
+      use Absinthe.Schema
+
+      query do
+        field :foo, :string
+      end
+
+      interface :qux do
+        interface list_of(:baz)
+      end
+    end
+    }
+    test "errors on interface with wrapped type" do
+      error = ~r/In interface Qux, cannot accept a non-null or a list type./
+
+      assert_raise(Absinthe.Schema.Error, error, fn ->
+        Code.eval_string(@schema, [], __ENV__)
+      end)
+    end
+  end
+
+  describe "input object" do
+    @schema ~S{
+    defmodule InputObjectSchema do
+      use Absinthe.Schema
+
+      query do
+        field :foo, :string
+      end
+
+      input_object :bar do
+        import_fields non_null(:baz)
+      end
+    end
+    }
+    test "errors on import_fields with wrapped type" do
+      error = ~r/In input object Bar, cannot accept a non-null or a list type.\n\nGot: Baz!/
+
+      assert_raise(Absinthe.Schema.Error, error, fn ->
+        Code.eval_string(@schema, [], __ENV__)
+      end)
+    end
+  end
+
+  describe "union type" do
+    @schema ~S{
+    defmodule UnionSchema do
+      use Absinthe.Schema
+
+      query do
+        field :foo, :string
+      end
+
+      union :bar do
+        types [list_of(:baz)]
+      end
+
+    end
+    }
+    test "errors on types with wrapped type" do
+      error = ~r/In union Bar, cannot accept a non-null or a list type./
+
+      assert_raise(Absinthe.Schema.Error, error, fn ->
+        Code.eval_string(@schema, [], __ENV__)
+      end)
+    end
+  end
+end

test/absinthe/schema/notation/import_test.exs

@@ -106,6 +106,7 @@ defmodule Absinthe.Schema.Notation.ImportTest do
 
         object :baz do
           import_fields :bar
+
           field :age, :integer
         end
       end