Merge pull request #345 from simonsaffer/avoid-case-class-clashes

Don't allow ObjectTypes derived from case classes to overwrite each other

src/main/scala/sangria/schema/Schema.scala

@@ -743,12 +743,29 @@ case class Schema[Ctx, Val](
   def renderCompact(filter: SchemaFilter): String = toAst(filter).renderCompact
 
   lazy val types: Map[String, (Int, Type with Named)] = {
-    def sameType(t1: Type, t2: Type) =
-      t1.getClass.getSimpleName == t2.getClass.getSimpleName
 
-    def typeConflict(name: String, t1: Type, t2: Type, parentInfo: String) =
-      throw SchemaValidationException(Vector(ConflictingTypeDefinitionViolation(
-        name, t1.getClass.getSimpleName :: t2.getClass.getSimpleName :: Nil, parentInfo)))
+    def sameType(t1: Type, t2: Type): Boolean = {
+      val sameSangriaType = t1.getClass.getName == t2.getClass.getName
+
+      (t1, t2) match {
+        case (ot1: ObjectType[_, _], ot2: ObjectType[_, _]) ⇒ sameSangriaType && (ot1.valClass == ot2.valClass)
+        case _ ⇒ sameSangriaType
+      }
+    }
+
+    def typeConflict(name: String, t1: Type, t2: Type, parentInfo: String) = {
+
+      (t1, t2) match {
+        case (ot1: ObjectType[_, _], ot2: ObjectType[_, _]) ⇒ {
+          throw SchemaValidationException(Vector(ConflictingObjectTypeCaseClassViolation(name, parentInfo)))
+        }
+        case _ => {
+          val conflictingTypes = List(t1, t2).map(_.getClass.getSimpleName)
+          throw SchemaValidationException(Vector(ConflictingTypeDefinitionViolation(
+            name, conflictingTypes, parentInfo)))
+        }
+      }
+    }
 
     def updated(priority: Int, name: String, tpe: Type with Named, result: Map[String, (Int, Type with Named)], parentInfo: String) =
       result get name match {

src/main/scala/sangria/validation/Violation.scala

@@ -444,6 +444,11 @@ case class ConflictingTypeDefinitionViolation(typeName: String, conflictingTypes
   lazy val errorMessage = s"Type name '$typeName' is used for several conflicting GraphQL type kinds: ${conflictingTypes mkString ", "}. Conflict found in $parentInfo."
 }
 
+case class ConflictingObjectTypeCaseClassViolation(typeName: String, parentInfo: String) extends Violation {
+  // Ideally this error message should include the conflicting classes canonical names but due to https://issues.scala-lang.org/browse/SI-2034 that's not possible
+  lazy val errorMessage = s"""Type name '$typeName' is used for several conflicting GraphQL ObjectTypes based on different classes. Conflict found in $parentInfo. One possible fix is to use ObjectTypeName like this: deriveObjectType[Foo, Bar](ObjectTypeName("OtherBar")) to avoid that two ObjectTypes have the same name."""
+}
+
 case class ReservedTypeNameViolation(typeName: String, sourceMapper: Option[SourceMapper], locations: List[AstLocation]) extends AstNodeViolation {
   lazy val simpleErrorMessage = s"Type name '$typeName' must not begin with '__', which is reserved by GraphQL introspection."
 }

src/test/scala/sangria/schema/SchemaConstraintsSpec.scala

@@ -5,6 +5,7 @@ import sangria.ast
 import sangria.execution.{MaterializedSchemaValidationError, WithViolations}
 import sangria.validation._
 import sangria.macros._
+import sangria.macros.derive.{ObjectTypeName, deriveObjectType}
 import sangria.parser.QueryParser
 import sangria.util.Pos
 
@@ -142,6 +143,35 @@ class SchemaConstraintsSpec extends WordSpec with Matchers {
         "Interface type 'Interface2' must define one or more fields.",
         "Object type 'Output' must define one or more fields."))
     }
+
+    "Not allow ObjectTypes with same name to be based on different case classes" in {
+
+      implicit val fooBazType = deriveObjectType[Unit, foo.Baz]()
+      implicit val barBazType = deriveObjectType[Unit, bar.Baz]()
+
+      val queryType = ObjectType("Query", fields[Unit, Unit](
+        Field("fooBaz", OptionType(fooBazType), resolve = _ => Some(foo.Baz(1))),
+        Field("barBaz", barBazType, resolve = _ => bar.Baz("2", 3.0))
+      ))
+
+      val error = intercept [SchemaValidationException] (Schema(queryType))
+
+      error.getMessage should include (
+        """Type name 'Baz' is used for several conflicting GraphQL ObjectTypes based on different classes. Conflict found in a field 'barBaz' of 'Query' type. One possible fix is to use ObjectTypeName like this: deriveObjectType[Foo, Bar](ObjectTypeName("OtherBar")) to avoid that two ObjectTypes have the same name.""")
+    }
+
+    "Allow ObjectTypes based on different case classes but with different names" in {
+
+      implicit val fooBazType = deriveObjectType[Unit, foo.Baz]()
+      implicit val barBazType = deriveObjectType[Unit, bar.Baz](ObjectTypeName("BazWithNewName"))
+
+      val queryType = ObjectType("Query", fields[Unit, Unit](
+        Field("fooBaz", OptionType(fooBazType), resolve = _ => Some(foo.Baz(1))),
+        Field("barBaz", barBazType, resolve = _ => bar.Baz("2", 3.0))
+      ))
+
+      Schema(queryType) // Should not throw any SchemaValidationExceptions
+    }
   }
 
   "Type System: Union types must be valid" should {

src/test/scala/sangria/schema/bar/Baz.scala

@@ -0,0 +1,3 @@
+package sangria.schema.bar
+
+case class Baz(y: String, z: Double)

src/test/scala/sangria/schema/foo/Baz.scala

@@ -0,0 +1,3 @@
+package sangria.schema.foo
+
+case class Baz(x: Int)