Make the schema comparison to consider the addition of a mandatory ar…

…gument breaking (#1147)
ghostdogpr · Nov 17, 2021 · 6b992dc · 6b992dc
1 parent e94a07b
commit 6b992dc
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 4 deletions.
diff --git a/tools/src/main/scala/caliban/tools/SchemaComparison.scala b/tools/src/main/scala/caliban/tools/SchemaComparison.scala
@@ -137,7 +137,9 @@ object SchemaComparison {
   ): List[SchemaComparisonChange] = {
     val leftKeys     = left.keySet
     val rightKeys    = right.keySet
-    val added        = (rightKeys -- leftKeys).map(ArgumentAdded(_, target)).toList
+    val added        = (right -- leftKeys).map { case (name, arg) =>
+      ArgumentAdded(name, target, arg.ofType.nullable || arg.defaultValue.isDefined)
+    }.toList
     val deleted      = (leftKeys -- rightKeys).map(ArgumentDeleted(_, target)).toList
     val commonFields = leftKeys intersect rightKeys
     val changes      = commonFields.toList.flatMap(key => compareArgumentDefinition(left(key), right(key), target))

diff --git a/tools/src/main/scala/caliban/tools/SchemaComparisonChange.scala b/tools/src/main/scala/caliban/tools/SchemaComparisonChange.scala
@@ -123,10 +123,11 @@ object SchemaComparisonChange {
     override def breaking: Boolean = true
   }
 
-  case class ArgumentAdded(argName: String, target: Target)   extends SchemaComparisonChange {
-    override def toString: String = s"Argument '$argName' was added on $target."
+  case class ArgumentAdded(argName: String, target: Target, optional: Boolean) extends SchemaComparisonChange {
+    override def toString: String  = s"Argument '$argName' was added on $target."
+    override def breaking: Boolean = !optional
   }
-  case class ArgumentDeleted(argName: String, target: Target) extends SchemaComparisonChange {
+  case class ArgumentDeleted(argName: String, target: Target)                  extends SchemaComparisonChange {
     override def toString: String  = s"Argument '$argName' was deleted from $target."
     override def breaking: Boolean = true
   }

diff --git a/tools/src/test/scala/caliban/tools/SchemaComparisonSpec.scala b/tools/src/test/scala/caliban/tools/SchemaComparisonSpec.scala
@@ -142,6 +142,50 @@ object SchemaComparisonSpec extends DefaultRunnableSpec {
 
         compare(schema1, schema2, expected)
       },
+      testM("optional argument added") {
+        val schema1: String =
+          """
+          input HeroInput {
+            test: String
+          }
+            |""".stripMargin
+
+        val schema2: String =
+          """
+          input HeroInput {
+            test: String
+            test2: String
+          }
+            |""".stripMargin
+
+        assertM(for {
+          s1  <- Parser.parseQuery(schema1)
+          s2  <- Parser.parseQuery(schema2)
+          diff = compareDocuments(s1, s2)
+        } yield diff)(hasFirst(hasField("breaking", _.breaking, equalTo(false))))
+      },
+      testM("non-optional argument added") {
+        val schema1: String =
+          """
+          input HeroInput {
+            test: String
+          }
+            |""".stripMargin
+
+        val schema2: String =
+          """
+          input HeroInput {
+            test: String
+            test2: String!
+          }
+            |""".stripMargin
+
+        assertM(for {
+          s1  <- Parser.parseQuery(schema1)
+          s2  <- Parser.parseQuery(schema2)
+          diff = compareDocuments(s1, s2)
+        } yield diff)(hasFirst(hasField("breaking", _.breaking, equalTo(true))))
+      },
       testM("deprecated") {
         val schema1: String =
           """