Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement "All Variable Usages are Allowed" validation #1092

Merged
merged 2 commits into from
Oct 16, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions adapters/play/src/test/scala/caliban/PlayAdapterSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
1 change: 0 additions & 1 deletion benchmarks/src/main/scala/caliban/GraphQLBenchmarks.scala
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,6 @@ class GraphQLBenchmarks {

object SangriaNewValidator {
import sangria.validation.RuleBasedQueryValidator
import sangria.validation.ValidationRule
import sangria.validation.rules._

val allRules =
Expand Down
4 changes: 2 additions & 2 deletions core/src/main/scala/caliban/introspection/Introspector.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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))
)
)

Expand Down
6 changes: 6 additions & 0 deletions core/src/main/scala/caliban/parsing/adt/Type.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 ()
}
Expand Down
6 changes: 3 additions & 3 deletions core/src/main/scala/caliban/validation/FieldMap.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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 =>
Expand Down
34 changes: 12 additions & 22 deletions core/src/main/scala/caliban/validation/FragmentValidator.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 =>
Expand All @@ -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)
Expand Down Expand Up @@ -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 =>
Expand Down Expand Up @@ -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, ""))
Expand Down
23 changes: 5 additions & 18 deletions core/src/main/scala/caliban/validation/Utils.scala
Original file line number Diff line number Diff line change
@@ -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 =
Expand Down Expand Up @@ -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)] =
Expand All @@ -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)))
}
Expand Down
Loading