dont allow >1 oneOf field even if null

sangria-graphql · Jul 22, 2024 · 4758e42 · 4758e42
1 parent a8d108b
commit 4758e42
Show file tree

Hide file tree

Showing 2 changed files with 78 additions and 36 deletions.
diff --git a/modules/core/src/main/scala/sangria/validation/rules/ExactlyOneOfFieldGiven.scala b/modules/core/src/main/scala/sangria/validation/rules/ExactlyOneOfFieldGiven.scala
@@ -38,9 +38,9 @@ class ExactlyOneOfFieldGiven extends ValidationRule {
     inputType.fold(AstVisitorCommand.RightContinue) { inputType =>
       inputType.namedInputType match {
         case namedInputType: schema.InputObjectType[_] if hasOneOfDirective(namedInputType) =>
-          val nonNullFields = node match {
+          val (allFields, nonNullFields) = node match {
             case Left(ast.ObjectValue(fields, _, _)) =>
-              fields.filter { field =>
+              val nonNullFields = fields.filter { field =>
                 field.value match {
                   case ast.NullValue(_, _) => false
                   case ast.VariableValue(name, _, _) =>
@@ -54,6 +54,7 @@ class ExactlyOneOfFieldGiven extends ValidationRule {
                   case _ => true
                 }
               }
+              (fields, nonNullFields)
 
             case Right(ast.VariableValue(name, _, _)) =>
               val variableValue = getResolvedVariableValue(name, namedInputType, ctx.variables)
@@ -62,20 +63,22 @@ class ExactlyOneOfFieldGiven extends ValidationRule {
                 variableValue match {
                   case Some(resolved) =>
                     val variableObj = resolved.asInstanceOf[Map[String, Any]]
-                    namedInputType.fields.filter { field =>
-                      variableObj.get(field.name).fold(false)(_ != None)
+                    val allFields = variableObj.filter { case (key, _) =>
+                      namedInputType.fieldsByName.contains(key)
                     }
-                  case _ => Vector.empty
+                    val nonNullFields = allFields.filter { case (_, v) => v != None }
+                    (allFields, nonNullFields)
+                  case _ => (Vector.empty, Vector.empty)
                 }
               catch {
                 // could get this from asInstanceOf failing for unexpected variable type.
                 // other validation will cover this problem.
-                case _: Throwable => Vector.empty
+                case _: Throwable => (Vector.empty, Vector.empty)
               }
           }
 
-          nonNullFields.size match {
-            case 1 => AstVisitorCommand.RightContinue
+          (allFields.size, nonNullFields.size) match {
+            case (1, 1) => AstVisitorCommand.RightContinue
             case _ =>
               val pos = node.fold(_.location, _.location)
               Left(

diff --git a/modules/core/src/test/scala/sangria/validation/rules/ExactlyOneOfFieldGivenSpec.scala b/modules/core/src/test/scala/sangria/validation/rules/ExactlyOneOfFieldGivenSpec.scala
@@ -48,7 +48,7 @@ class ExactlyOneOfFieldGivenSpec extends AnyWordSpec with ValidationSupport {
       List("Exactly one key must be specified for oneOf type 'OneOfInput'." -> Some(Pos(3, 31)))
     )
 
-    "fail with more than one non-null fields given" in expectFails(
+    "fail with more than one non-null args given" in expectFails(
       """
           query OneOfQuery {
             oneOfQuery(input: {
@@ -67,7 +67,65 @@ class ExactlyOneOfFieldGivenSpec extends AnyWordSpec with ValidationSupport {
       List("Exactly one key must be specified for oneOf type 'OneOfInput'." -> Some(Pos(3, 31)))
     )
 
-    "pass with a null variable and non-null arg given" in expectPasses(
+    "fail with one non-null arg and one null arg given" in expectFails(
+      """
+          query OneOfQuery {
+            oneOfQuery(input: {
+                catName: "Gretel",
+                dogId: null
+            }) {
+                ... on Cat {
+                    name
+                }
+                ... on Dog {
+                    name
+                }
+            }
+          }
+        """,
+      List("Exactly one key must be specified for oneOf type 'OneOfInput'." -> Some(Pos(3, 31)))
+    )
+
+    "fail with more than one null args given" in expectFails(
+      """
+          query OneOfQuery {
+            oneOfQuery(input: {
+                catName: null,
+                dogId: null
+            }) {
+                ... on Cat {
+                    name
+                }
+                ... on Dog {
+                    name
+                }
+            }
+          }
+        """,
+      List("Exactly one key must be specified for oneOf type 'OneOfInput'." -> Some(Pos(3, 31)))
+    )
+
+    "fail with an null arg and non-null variable given" in expectFails(
+      """
+      query OneOfQuery($catName: String) {
+        oneOfQuery(input: {
+          catName: $catName,
+          dogId: null
+        }) {
+            ... on Cat {
+              name
+            }
+            ... on Dog {
+              name
+            }
+          }
+        }
+    """,
+      List("Exactly one key must be specified for oneOf type 'OneOfInput'." -> Some(Pos(3, 27))),
+      "$catName: String" -> """{"catName": "Gretel"}"""
+    )
+
+    "fail with an non-null arg and null variable given" in expectFails(
       """
       query OneOfQuery($catName: String) {
         oneOfQuery(input: {
@@ -83,6 +141,7 @@ class ExactlyOneOfFieldGivenSpec extends AnyWordSpec with ValidationSupport {
           }
         }
     """,
+      List("Exactly one key must be specified for oneOf type 'OneOfInput'." -> Some(Pos(3, 27))),
       "$catName: String" -> """{"catName": null}"""
     )
 
@@ -106,7 +165,7 @@ class ExactlyOneOfFieldGivenSpec extends AnyWordSpec with ValidationSupport {
       "$catName: String" -> """{"catName": "Gretel"}"""
     )
 
-    "pass with a variable object with only one non-null value" in expectPasses(
+    "pass with a variable object with only one non-null value" in expectFails(
       """
       query OneOfQuery($input: OneOfInput!) {
         oneOfQuery(input: $input) {
@@ -119,6 +178,7 @@ class ExactlyOneOfFieldGivenSpec extends AnyWordSpec with ValidationSupport {
           }
         }
     """,
+      List("Exactly one key must be specified for oneOf type 'OneOfInput'." -> Some(Pos(3, 27))),
       "$input: OneOfInput!" -> """{"input":{"catName": "Gretel", "dogId": null}}"""
     )
 
@@ -172,12 +232,11 @@ class ExactlyOneOfFieldGivenSpec extends AnyWordSpec with ValidationSupport {
       "$input: OneOfInput!" -> """{"input":{"dogId": 123}}"""
     )
 
-    "pass with a variables with default value but only one resolved to non-null" in expectPasses(
+    "passes with a variable that has a default value" in expectPasses(
       """
-      query OneOfQuery($catName: String = "Gretel", $dogId: Int) {
+      query OneOfQuery($catName: String = "Gretel") {
         oneOfQuery(input: {
-          catName: $catName,
-          dogId: $dogId
+          catName: $catName
         }) {
             ... on Cat {
               name
@@ -188,27 +247,7 @@ class ExactlyOneOfFieldGivenSpec extends AnyWordSpec with ValidationSupport {
           }
         }
     """,
-      """$catName: String = "Gretel", $dogId: Int""" -> """{}"""
-    )
-
-    "fail with a variable object with default value that causes there to be more than one non-null value" in expectFails(
-      """
-      query OneOfQuery($catName: String = "Gretel", $dogId: Int) {
-        oneOfQuery(input: {
-          catName: $catName,
-          dogId: $dogId
-        }) {
-            ... on Cat {
-              name
-            }
-            ... on Dog {
-              name
-            }
-          }
-        }
-    """,
-      List("Exactly one key must be specified for oneOf type 'OneOfInput'." -> Some(Pos(3, 27))),
-      """$catName: String = "Gretel", $dogId: Int""" -> """{"dogId":123}"""
+      """$catName: String = "Gretel"""" -> """{}"""
     )
   }
 }