From 6b83166fcc66c1f30009c4ddfe6da4a45e01bc68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20W=C3=A4rnsberg?= Date: Mon, 4 Oct 2021 13:35:57 +0200 Subject: [PATCH 1/3] fix: Add failing test --- .../caliban/execution/FieldArgsSpec.scala | 88 +++++++++++++++++++ 1 file changed, 88 insertions(+) create mode 100644 core/src/test/scala/caliban/execution/FieldArgsSpec.scala diff --git a/core/src/test/scala/caliban/execution/FieldArgsSpec.scala b/core/src/test/scala/caliban/execution/FieldArgsSpec.scala new file mode 100644 index 000000000..b2ac80f5b --- /dev/null +++ b/core/src/test/scala/caliban/execution/FieldArgsSpec.scala @@ -0,0 +1,88 @@ +package caliban.execution + +import zio._ + +import caliban.CalibanError +import caliban.GraphQL._ +import caliban.Macros.gqldoc +import caliban.InputValue +import caliban.Value.EnumValue +import caliban.RootResolver +import caliban.schema.Annotations.GQLDefault +import zio.test.Assertion._ +import zio.test._ +import zio.test.environment.TestEnvironment +import caliban.Value +import caliban.GraphQLRequest + +object FieldArgsSpec extends DefaultRunnableSpec { + sealed trait COLOR + object COLOR { + case object GREEN extends COLOR + case object BLUE extends COLOR + } + + override def spec = suite("FieldArgsSpec")( + testM("it forward args of correct type") { + case class QueryInput(color: COLOR, string: String) + case class Query(query: Field => QueryInput => UIO[String]) + val query = + """query{ + | query(string: "test", color: BLUE) + |}""".stripMargin + + for { + ref <- Ref.make[Option[Field]](None) + api = graphQL( + RootResolver( + Query( + query = info => + i => + ref.set(Option(info)) *> + ZIO.succeed(i.string) + ) + ) + ) + interpreter <- api.interpreter + _ <- interpreter.execute(query) + + res <- ref.get + } yield assertTrue( + res.get.arguments.get("color").get == EnumValue("BLUE") + ) + } + ) + + testM("it forward args as correct type from variables") { + case class QueryInput(color: COLOR, string: String) + case class Query(query: Field => QueryInput => UIO[String]) + val query = + """query MyQuery($color: Color) { + | query(string: "test", color: $color) + |}""".stripMargin + + for { + ref <- Ref.make[Option[Field]](None) + api = graphQL( + RootResolver( + Query( + query = info => + i => + ref.set(Option(info)) *> + ZIO.succeed(i.string) + ) + ) + ) + interpreter <- api.interpreter + _ <- interpreter.executeRequest( + request = + GraphQLRequest(query = Some(query), variables = Some(Map("color" -> Value.StringValue("BLUE")))), + skipValidation = false, + enableIntrospection = true, + queryExecution = QueryExecution.Parallel + ) + res <- ref.get + } yield assertTrue( + res.get.arguments.get("color").get == EnumValue("BLUE") + ) + } +} From ab612894e85d126c274bc8a85ab7b9dcb30592b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20W=C3=A4rnsberg?= Date: Mon, 4 Oct 2021 15:22:49 +0200 Subject: [PATCH 2/3] fix: Properly resolve Enums --- .../main/scala/caliban/execution/Field.scala | 44 +++++++++++++++---- .../caliban/execution/FieldArgsSpec.scala | 15 ++++--- 2 files changed, 45 insertions(+), 14 deletions(-) diff --git a/core/src/main/scala/caliban/execution/Field.scala b/core/src/main/scala/caliban/execution/Field.scala index 7195b205b..fe0881351 100644 --- a/core/src/main/scala/caliban/execution/Field.scala +++ b/core/src/main/scala/caliban/execution/Field.scala @@ -3,11 +3,11 @@ package caliban.execution import caliban.InputValue import caliban.Value import caliban.Value.{ BooleanValue, NullValue } -import caliban.introspection.adt.{ __DeprecatedArgs, __Type } +import caliban.introspection.adt.{ __DeprecatedArgs, __Type, __TypeKind } import caliban.parsing.SourceMapper import caliban.parsing.adt.Definition.ExecutableDefinition.FragmentDefinition import caliban.parsing.adt.Selection.{ FragmentSpread, InlineFragment, Field => F } -import caliban.parsing.adt.{ Directive, LocationInfo, Selection, VariableDefinition } +import caliban.parsing.adt.{ Directive, LocationInfo, Selection, Type, VariableDefinition } import caliban.schema.{ RootType, Types } case class Field( @@ -58,7 +58,7 @@ object Field { alias, field.fields, None, - resolveVariables(arguments, variableDefinitions, variableValues), + resolveVariables(innerType, arguments, variableDefinitions, variableValues, rootType), () => sourceMapper.getLocation(index), directives ++ schemaDirectives ) @@ -94,9 +94,11 @@ object Field { } private def resolveVariables( + typ: __Type, arguments: Map[String, InputValue], variableDefinitions: List[VariableDefinition], - variableValues: Map[String, InputValue] + variableValues: Map[String, InputValue], + rootType: RootType ): Map[String, InputValue] = { def resolveVariable(value: InputValue): InputValue = value match { @@ -104,16 +106,40 @@ object Field { case InputValue.ObjectValue(fields) => InputValue.ObjectValue(fields.map({ case (k, v) => k -> resolveVariable(v) })) case InputValue.VariableValue(name) => - lazy val defaultInputValue = (for { - definition <- variableDefinitions.find(_.name == name) - inputValue <- definition.defaultValue - } yield inputValue) getOrElse NullValue - variableValues.getOrElse(name, defaultInputValue) + (for { + definition <- variableDefinitions.find(_.name == name) + defaultValue = definition.defaultValue getOrElse NullValue + value = variableValues.getOrElse(name, defaultValue) + } yield resolveEnumValues(value, definition, rootType)) getOrElse NullValue case value: Value => value } arguments.map({ case (k, v) => k -> resolveVariable(v) }) } + // Since we cannot separate a String from an Enum when variables + // are parsed, we need to translate from strings to enums here + // if we have a valid enum field. + private def resolveEnumValues( + value: InputValue, + definition: VariableDefinition, + rootType: RootType + ): InputValue = { + val t = Type + .innerType(definition.variableType) + + rootType.types + .get(t) + .map(_.kind) + .flatMap { kind => + (kind, value) match { + case (__TypeKind.ENUM, Value.StringValue(v)) => + Some(Value.EnumValue(v)) + case _ => None + } + } + .getOrElse(value) + } + private def subtypeNames(typeName: String, rootType: RootType): Option[List[String]] = rootType.types .get(typeName) diff --git a/core/src/test/scala/caliban/execution/FieldArgsSpec.scala b/core/src/test/scala/caliban/execution/FieldArgsSpec.scala index b2ac80f5b..df6f6acec 100644 --- a/core/src/test/scala/caliban/execution/FieldArgsSpec.scala +++ b/core/src/test/scala/caliban/execution/FieldArgsSpec.scala @@ -56,7 +56,7 @@ object FieldArgsSpec extends DefaultRunnableSpec { case class QueryInput(color: COLOR, string: String) case class Query(query: Field => QueryInput => UIO[String]) val query = - """query MyQuery($color: Color) { + """query MyQuery($color: COLOR!) { | query(string: "test", color: $color) |}""".stripMargin @@ -65,17 +65,22 @@ object FieldArgsSpec extends DefaultRunnableSpec { api = graphQL( RootResolver( Query( - query = info => + query = info => { i => ref.set(Option(info)) *> ZIO.succeed(i.string) + } ) ) ) interpreter <- api.interpreter - _ <- interpreter.executeRequest( - request = - GraphQLRequest(query = Some(query), variables = Some(Map("color" -> Value.StringValue("BLUE")))), + qres <- interpreter.executeRequest( + request = GraphQLRequest( + query = Some(query), + // "color" is a string here since it will come directly from + // parsed JSON which is unaware that it should be an Enum + variables = Some(Map("color" -> Value.StringValue("BLUE"))) + ), skipValidation = false, enableIntrospection = true, queryExecution = QueryExecution.Parallel From d5a9b554256ef4155bf6d242132cd3790e160e08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20W=C3=A4rnsberg?= Date: Mon, 4 Oct 2021 15:34:08 +0200 Subject: [PATCH 3/3] remove unused --- core/src/main/scala/caliban/execution/Field.scala | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/core/src/main/scala/caliban/execution/Field.scala b/core/src/main/scala/caliban/execution/Field.scala index fe0881351..1d92e005a 100644 --- a/core/src/main/scala/caliban/execution/Field.scala +++ b/core/src/main/scala/caliban/execution/Field.scala @@ -58,7 +58,7 @@ object Field { alias, field.fields, None, - resolveVariables(innerType, arguments, variableDefinitions, variableValues, rootType), + resolveVariables(arguments, variableDefinitions, variableValues, rootType), () => sourceMapper.getLocation(index), directives ++ schemaDirectives ) @@ -94,7 +94,6 @@ object Field { } private def resolveVariables( - typ: __Type, arguments: Map[String, InputValue], variableDefinitions: List[VariableDefinition], variableValues: Map[String, InputValue],