Implement variable validation spec (ghostdogpr#1092)

seoethereal · Oct 16, 2021 · e3bb38b · e3bb38b
1 parent 481983b
commit e3bb38b
Show file tree

Hide file tree

Showing 15 changed files with 244 additions and 101 deletions.
diff --git a/adapters/http4s/src/test/scala/caliban/Http4sAdapterSpec.scala b/adapters/http4s/src/test/scala/caliban/Http4sAdapterSpec.scala
@@ -130,7 +130,7 @@ object Http4sAdapterSpec extends DefaultRunnableSpec {
         val fileURL: URL     = getClass.getResource(s"/$fileName")
 
         val query: String =
-          """{ "query": "mutation ($file: Upload!) { uploadFile(file: $file) { hash, path, filename, mimetype } }",   "variables": {  "file": null }}"""
+          """{ "query": "mutation ($file: UploadInput!) { uploadFile(file: $file) { hash, path, filename, mimetype } }",   "variables": {  "file": null }}"""
 
         val request = basicRequest
           .post(uri)
@@ -169,7 +169,7 @@ object Http4sAdapterSpec extends DefaultRunnableSpec {
         val file2URL: URL     = getClass.getResource(s"/$file2Name")
 
         val query: String =
-          """{ "query": "mutation ($files: [Upload!]!) { uploadFiles(files: $files) { hash, path, filename, mimetype } }",   "variables": {  "files": [null, null] }}"""
+          """{ "query": "mutation ($files: [UploadInput!]!) { uploadFiles(files: $files) { hash, path, filename, mimetype } }",   "variables": {  "files": [null, null] }}"""
 
         val request = basicRequest
           .post(uri)

diff --git a/adapters/play/src/test/scala/caliban/PlayAdapterSpec.scala b/adapters/play/src/test/scala/caliban/PlayAdapterSpec.scala
@@ -133,7 +133,7 @@ object PlayAdapterSpec extends DefaultRunnableSpec {
         val fileURL: URL     = getClass.getResource(s"/$fileName")
 
         val query: String =
-          """{ "query": "mutation ($file: Upload!) { uploadFile(file: $file) { hash, path, filename, mimetype } }",   "variables": {  "file": null }}"""
+          """{ "query": "mutation ($file: UploadInput!) { uploadFile(file: $file) { hash, path, filename, mimetype } }",   "variables": {  "file": null }}"""
 
         val request = basicRequest
           .post(uri)
@@ -172,7 +172,7 @@ object PlayAdapterSpec extends DefaultRunnableSpec {
         val file2URL: URL     = getClass.getResource(s"/$file2Name")
 
         val query: String =
-          """{ "query": "mutation ($files: [Upload!]!) { uploadFiles(files: $files) { hash, path, filename, mimetype } }",   "variables": {  "files": [null, null] }}"""
+          """{ "query": "mutation ($files: [UploadInput!]!) { uploadFiles(files: $files) { hash, path, filename, mimetype } }",   "variables": {  "files": [null, null] }}"""
 
         val request = basicRequest
           .post(uri)

diff --git a/benchmarks/src/main/scala/caliban/GraphQLBenchmarks.scala b/benchmarks/src/main/scala/caliban/GraphQLBenchmarks.scala
@@ -343,7 +343,6 @@ class GraphQLBenchmarks {
 
   object SangriaNewValidator {
     import sangria.validation.RuleBasedQueryValidator
-    import sangria.validation.ValidationRule
     import sangria.validation.rules._
 
     val allRules =

diff --git a/core/src/main/scala/caliban/introspection/Introspector.scala b/core/src/main/scala/caliban/introspection/Introspector.scala
@@ -21,15 +21,15 @@ object Introspector extends IntrospectionDerivation {
         "The @skip directive may be provided for fields, fragment spreads, and inline fragments, and allows for conditional exclusion during execution as described by the if argument."
       ),
       Set(__DirectiveLocation.FIELD, __DirectiveLocation.FRAGMENT_SPREAD, __DirectiveLocation.INLINE_FRAGMENT),
-      List(__InputValue("if", None, () => Types.boolean, None))
+      List(__InputValue("if", None, () => Types.makeNonNull(Types.boolean), None))
     ),
     __Directive(
       "include",
       Some(
         "The @include directive may be provided for fields, fragment spreads, and inline fragments, and allows for conditional inclusion during execution as described by the if argument."
       ),
       Set(__DirectiveLocation.FIELD, __DirectiveLocation.FRAGMENT_SPREAD, __DirectiveLocation.INLINE_FRAGMENT),
-      List(__InputValue("if", None, () => Types.boolean, None))
+      List(__InputValue("if", None, () => Types.makeNonNull(Types.boolean), None))
     )
   )
 

diff --git a/core/src/main/scala/caliban/parsing/adt/Type.scala b/core/src/main/scala/caliban/parsing/adt/Type.scala
@@ -10,6 +10,12 @@ sealed trait Type { self =>
     case Type.NamedType(name, nonNull)  => if (nonNull) s"$name!" else name
     case Type.ListType(ofType, nonNull) => if (nonNull) s"[$ofType]!" else s"[$ofType]"
   }
+
+  def toNullable: Type =
+    self match {
+      case Type.NamedType(name, _)  => Type.NamedType(name, nonNull = false)
+      case Type.ListType(ofType, _) => Type.ListType(ofType, nonNull = false)
+    }
 }
 
 object Type {

diff --git a/core/src/main/scala/caliban/validation/DefaultValueValidator.scala b/core/src/main/scala/caliban/validation/DefaultValueValidator.scala
@@ -2,11 +2,11 @@ package caliban.validation
 
 import caliban.CalibanError.ValidationError
 import caliban.InputValue._
-import caliban.{ InputValue, Value }
 import caliban.Value._
 import caliban.introspection.adt._
 import caliban.introspection.adt.__TypeKind._
 import caliban.parsing.Parser
+import caliban.{ InputValue, Value }
 import zio.IO
 
 object DefaultValueValidator {
@@ -21,7 +21,7 @@ object DefaultValueValidator {
                 "The default value for a field must be written using GraphQL input syntax."
               )
             )
-        _     <- Validator.validateInputValues(field, value)
+        _     <- Validator.validateInputValues(field, value, Context.empty)
         _     <- validateInputTypes(field, value, errorContext)
       } yield ()
     }

diff --git a/core/src/main/scala/caliban/validation/FieldMap.scala b/core/src/main/scala/caliban/validation/FieldMap.scala
@@ -11,10 +11,10 @@ object FieldMap {
   implicit class FieldMapOps(val self: FieldMap) extends AnyVal {
     def |+|(that: FieldMap): FieldMap =
       (self.keySet ++ that.keySet).map { k =>
-        k -> (self.get(k).getOrElse(Set.empty) ++ that.get(k).getOrElse(Set.empty))
+        k -> (self.getOrElse(k, Set.empty) ++ that.getOrElse(k, Set.empty))
       }.toMap
 
-    def show =
+    def show: String =
       self.map { case (k, fields) =>
         s"$k -> ${fields.map(_.fieldDef.name).mkString(", ")}"
       }.mkString("\n")
@@ -40,7 +40,7 @@ object FieldMap {
   def apply(context: Context, parentType: __Type, selectionSet: Iterable[Selection]): FieldMap =
     selectionSet.foldLeft(FieldMap.empty)({ case (fields, selection) =>
       selection match {
-        case FragmentSpread(name, directives)               =>
+        case FragmentSpread(name, _)                        =>
           context.fragments
             .get(name)
             .map { definition =>

diff --git a/core/src/main/scala/caliban/validation/FragmentValidator.scala b/core/src/main/scala/caliban/validation/FragmentValidator.scala
@@ -3,9 +3,10 @@ package caliban.validation
 import caliban.CalibanError.ValidationError
 import caliban.introspection.adt._
 import caliban.parsing.adt.Selection
-import zio.{ Chunk, IO, UIO }
-import Utils._
-import Utils.syntax._
+import caliban.validation.Utils._
+import caliban.validation.Utils.syntax._
+import zio.{ Chunk, IO }
+
 import scala.collection.mutable
 
 object FragmentValidator {
@@ -18,7 +19,7 @@ object FragmentValidator {
     val parentsCache = scala.collection.mutable.Map.empty[Iterable[Selection], Chunk[String]]
     val groupsCache  = scala.collection.mutable.Map.empty[Set[SelectedField], Chunk[Set[SelectedField]]]
 
-    def sameResponseShapeByName(context: Context, parentType: __Type, set: Iterable[Selection]): Chunk[String] =
+    def sameResponseShapeByName(set: Iterable[Selection]): Chunk[String] =
       shapeCache.get(set) match {
         case Some(value) => value
         case None        =>
@@ -31,22 +32,22 @@ object FragmentValidator {
                     .getOrElse("")}.${f2.fieldDef.name}. Try using an alias."
                 )
               } else
-                sameResponseShapeByName(context, parentType, f1.selection.selectionSet ++ f2.selection.selectionSet)
+                sameResponseShapeByName(f1.selection.selectionSet ++ f2.selection.selectionSet)
             }
           })
           shapeCache.update(set, res)
           res
       }
 
-    def sameForCommonParentsByName(context: Context, parentType: __Type, set: Iterable[Selection]): Chunk[String] =
+    def sameForCommonParentsByName(set: Iterable[Selection]): Chunk[String] =
       parentsCache.get(set) match {
         case Some(value) => value
         case None        =>
           val fields = FieldMap(context, parentType, set)
-          val res    = Chunk.fromIterable(fields.flatMap({ case (name, fields) =>
-            groupByCommonParents(context, parentType, fields).flatMap { group =>
+          val res    = Chunk.fromIterable(fields.flatMap({ case (_, fields) =>
+            groupByCommonParents(fields).flatMap { group =>
               val merged = group.flatMap(_.selection.selectionSet)
-              requireSameNameAndArguments(group) ++ sameForCommonParentsByName(context, parentType, merged)
+              requireSameNameAndArguments(group) ++ sameForCommonParentsByName(merged)
             }
           }))
           parentsCache.update(set, res)
@@ -82,11 +83,7 @@ object FragmentValidator {
         else List()
       }
 
-    def groupByCommonParents(
-      context: Context,
-      parentType: __Type,
-      fields: Set[SelectedField]
-    ): Chunk[Set[SelectedField]] =
+    def groupByCommonParents(fields: Set[SelectedField]): Chunk[Set[SelectedField]] =
       groupsCache.get(fields) match {
         case Some(value) => value
         case None        =>
@@ -116,14 +113,7 @@ object FragmentValidator {
           res
       }
 
-    val fields = FieldMap(
-      context,
-      parentType,
-      selectionSet
-    )
-
-    val conflicts = sameResponseShapeByName(context, parentType, selectionSet) ++
-      sameForCommonParentsByName(context, parentType, selectionSet)
+    val conflicts = sameResponseShapeByName(selectionSet) ++ sameForCommonParentsByName(selectionSet)
 
     IO.whenCase(conflicts) { case Chunk(head, _*) =>
       IO.fail(ValidationError(head, ""))

diff --git a/core/src/main/scala/caliban/validation/Utils.scala b/core/src/main/scala/caliban/validation/Utils.scala
@@ -1,22 +1,9 @@
 package caliban.validation
 
-import caliban.CalibanError.ValidationError
-import caliban.InputValue.VariableValue
-import caliban.Value.NullValue
-import caliban.execution.{ ExecutionRequest, Field => F }
-import caliban.introspection.Introspector
 import caliban.introspection.adt._
 import caliban.introspection.adt.__TypeKind._
-import caliban.parsing.SourceMapper
-import caliban.parsing.adt.Definition.ExecutableDefinition.{ FragmentDefinition, OperationDefinition }
-import caliban.parsing.adt.Definition.{ TypeSystemDefinition, TypeSystemExtension }
-import caliban.parsing.adt.OperationType._
-import caliban.parsing.adt.Selection.{ Field, FragmentSpread, InlineFragment }
 import caliban.parsing.adt.Type.NamedType
-import caliban.parsing.adt._
-import caliban.schema.{ RootSchema, RootSchemaBuilder, RootType, Types }
-import caliban.{ InputValue, Rendering, Value }
-import zio.{ Chunk, IO }
+import zio.Chunk
 
 object Utils {
   def isObjectType(t: __Type): Boolean =
@@ -63,11 +50,11 @@ object Utils {
 
   def isListType(t: __Type): Boolean = t.kind == __TypeKind.LIST
 
-  def getFields(t: __Type)                                                = t.fields(__DeprecatedArgs(Some(true)))
-  def getType(t: Option[NamedType], parentType: __Type, context: Context) =
+  def getFields(t: __Type): Option[List[__Field]]                                 = t.fields(__DeprecatedArgs(Some(true)))
+  def getType(t: Option[NamedType], parentType: __Type, context: Context): __Type =
     t.fold(Option(parentType))(t => context.rootType.types.get(t.name)).getOrElse(parentType)
 
-  def getType(t: NamedType, context: Context) =
+  def getType(t: NamedType, context: Context): Option[__Type] =
     context.rootType.types.get(t.name)
 
   def cross[A](a: Iterable[A]): Chunk[(A, A)]                 =
@@ -82,7 +69,7 @@ object Utils {
         self.flatMap(a => that.map(b => (a, b)))
     }
 
-    implicit class Tuple2Syntax[+A, +B](val self: Tuple2[Option[A], Option[B]]) extends AnyVal {
+    implicit class Tuple2Syntax[+A, +B](val self: (Option[A], Option[B])) extends AnyVal {
       def mapN[C](f: (A, B) => C): Option[C] =
         self._1.flatMap(a => self._2.map(b => f(a, b)))
     }