From 39ec93901d01acd374e53a84161a1843ac8a8c61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20W=C3=A4rnsberg?= Date: Thu, 11 Nov 2021 15:49:15 +0100 Subject: [PATCH 1/3] recursively translate strings to enums --- .../caliban/parsing/VariablesUpdater.scala | 84 ++++++++++++++----- .../caliban/execution/FieldArgsSpec.scala | 59 +++++++++++++ 2 files changed, 122 insertions(+), 21 deletions(-) diff --git a/core/src/main/scala/caliban/parsing/VariablesUpdater.scala b/core/src/main/scala/caliban/parsing/VariablesUpdater.scala index 851ca4270..27dceac2b 100644 --- a/core/src/main/scala/caliban/parsing/VariablesUpdater.scala +++ b/core/src/main/scala/caliban/parsing/VariablesUpdater.scala @@ -1,8 +1,13 @@ package caliban.parsing import caliban.GraphQLRequest +import caliban.InputValue.ListValue +import caliban.Value.StringValue import caliban.introspection.adt._ +import caliban.introspection.adt.__TypeKind import caliban.parsing.adt.Definition.ExecutableDefinition.OperationDefinition +import caliban.parsing.adt.Type.ListType +import caliban.parsing.adt.Type.NamedType import caliban.parsing.adt._ import caliban.schema.RootType import caliban.{ InputValue, Value } @@ -16,38 +21,75 @@ object VariablesUpdater { ): GraphQLRequest = { val variableDefinitions = doc.operationDefinitions.flatMap(_.variableDefinitions) val updated = req.variables.getOrElse(Map.empty).map { case (key, value) => - val v = variableDefinitions.find(_.name == key).map(resolveEnumValues(value, _, rootType)).getOrElse(value) + val v = + variableDefinitions + .find(_.name == key) + .map { definition: VariableDefinition => + rewriteValues(value, definition.variableType, rootType) + } + .getOrElse(value) + key -> v } req.copy(variables = Some(updated)) } + def rewriteValues(value: InputValue, `type`: Type, rootType: RootType): InputValue = + `type` match { + case ListType(ofType, nonNull) => + value match { + case ListValue(values) => + ListValue(values.map(v => rewriteValues(v, ofType, rootType))) + case _ => value + } + case NamedType(name, nonNull) => + rootType.types.get(name).map(t => resolveEnumValues(value, t, rootType)).getOrElse(value) + } + // 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, + typ: __Type, rootType: RootType - ): InputValue = { - val t = Type - .innerType(definition.variableType) - - rootType.types - .get(t) - .map(_.kind) - .flatMap { kind => - (kind, value) match { - case (__TypeKind.ENUM, InputValue.ListValue(v)) => - Some( - InputValue.ListValue(v.map(resolveEnumValues(_, definition, rootType))) - ) - case (__TypeKind.ENUM, Value.StringValue(v)) => - Some(Value.EnumValue(v)) - case _ => None + ): InputValue = + typ.kind match { + case __TypeKind.INPUT_OBJECT => + value match { + case InputValue.ObjectValue(fields) => + val defs = typ.inputFields.getOrElse(List.empty) + InputValue.ObjectValue(fields.map { case (k, v) => + val updated = + defs.find(_.name == k).map(field => resolveEnumValues(v, field.`type`(), rootType)).getOrElse(value) + + (k, updated) + }) + case _ => + value } - } - .getOrElse(value) - } + + case __TypeKind.LIST => + value match { + case ListValue(values) => + typ.ofType + .map(innerType => ListValue(values.map(value => resolveEnumValues(value, innerType, rootType)))) + .getOrElse(value) + case _ => value + } + + case __TypeKind.NON_NULL => + typ.ofType + .map(innerType => resolveEnumValues(value, innerType, rootType)) + .getOrElse(value) + + case __TypeKind.ENUM => + value match { + case StringValue(value) => Value.EnumValue(value) + case _ => value + } + case _ => + value + } } diff --git a/core/src/test/scala/caliban/execution/FieldArgsSpec.scala b/core/src/test/scala/caliban/execution/FieldArgsSpec.scala index 77b5f548b..f2ebb5f9d 100644 --- a/core/src/test/scala/caliban/execution/FieldArgsSpec.scala +++ b/core/src/test/scala/caliban/execution/FieldArgsSpec.scala @@ -173,6 +173,65 @@ object FieldArgsSpec extends DefaultRunnableSpec { ) ) + for { + interpreter <- api.interpreter + res <- interpreter.execute(query) + } yield assert(res.errors.headOption)(isSome(anything)) + }, + testM("it correctly handles lists of objects with enums") { + case class QueryInput(filter: List[Filter]) + case class Filter(color: COLOR) + case class Query(query: QueryInput => String) + val query = + """query MyQuery($filter: [FilterInput!]!) { + | query(filter: $filter) + |}""".stripMargin + + val api = graphQL( + RootResolver( + Query( + query = q => q.filter.headOption.map(_.color.toString).getOrElse("Missing") + ) + ) + ) + + for { + interpreter <- api.interpreter + res <- interpreter.executeRequest( + request = GraphQLRequest( + query = Some(query), + variables = Some( + Map( + "filter" -> + InputValue.ListValue( + List( + InputValue.ObjectValue( + Map("color" -> Value.StringValue("BLUE")) + ) + ) + ) + ) + ) + ) + ) + } yield assertTrue(res.data.toString == "{\"query\":\"BLUE\"}") + }, + testM("it doesn't allow strings as enums in GQL syntax") { + case class QueryInput(color: COLOR) + case class Query(query: QueryInput => UIO[String]) + val query = + """query { + | query(color: "BLUE") + |}""".stripMargin + + val api = graphQL( + RootResolver( + Query( + query = i => ZIO.succeed(i.toString) + ) + ) + ) + for { interpreter <- api.interpreter res <- interpreter.execute(query) From b078f5e6f27d27b3fb0e5135d5078d3a13c74886 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20W=C3=A4rnsberg?= Date: Thu, 11 Nov 2021 15:58:02 +0100 Subject: [PATCH 2/3] scala3 fix --- core/src/main/scala/caliban/parsing/VariablesUpdater.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/scala/caliban/parsing/VariablesUpdater.scala b/core/src/main/scala/caliban/parsing/VariablesUpdater.scala index 27dceac2b..c2344b5f3 100644 --- a/core/src/main/scala/caliban/parsing/VariablesUpdater.scala +++ b/core/src/main/scala/caliban/parsing/VariablesUpdater.scala @@ -24,7 +24,7 @@ object VariablesUpdater { val v = variableDefinitions .find(_.name == key) - .map { definition: VariableDefinition => + .map { (definition: VariableDefinition) => rewriteValues(value, definition.variableType, rootType) } .getOrElse(value) From 38daa5e892b803a4d370d4d565beeeac23dc4882 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20W=C3=A4rnsberg?= Date: Thu, 11 Nov 2021 16:14:24 +0100 Subject: [PATCH 3/3] Pr comments --- .../caliban/parsing/VariablesUpdater.scala | 12 +++++------ .../caliban/execution/FieldArgsSpec.scala | 21 ------------------- 2 files changed, 5 insertions(+), 28 deletions(-) diff --git a/core/src/main/scala/caliban/parsing/VariablesUpdater.scala b/core/src/main/scala/caliban/parsing/VariablesUpdater.scala index c2344b5f3..9a249d295 100644 --- a/core/src/main/scala/caliban/parsing/VariablesUpdater.scala +++ b/core/src/main/scala/caliban/parsing/VariablesUpdater.scala @@ -4,10 +4,8 @@ import caliban.GraphQLRequest import caliban.InputValue.ListValue import caliban.Value.StringValue import caliban.introspection.adt._ -import caliban.introspection.adt.__TypeKind import caliban.parsing.adt.Definition.ExecutableDefinition.OperationDefinition -import caliban.parsing.adt.Type.ListType -import caliban.parsing.adt.Type.NamedType +import caliban.parsing.adt.Type.{ ListType, NamedType } import caliban.parsing.adt._ import caliban.schema.RootType import caliban.{ InputValue, Value } @@ -24,7 +22,7 @@ object VariablesUpdater { val v = variableDefinitions .find(_.name == key) - .map { (definition: VariableDefinition) => + .map { definition => rewriteValues(value, definition.variableType, rootType) } .getOrElse(value) @@ -35,15 +33,15 @@ object VariablesUpdater { req.copy(variables = Some(updated)) } - def rewriteValues(value: InputValue, `type`: Type, rootType: RootType): InputValue = + private def rewriteValues(value: InputValue, `type`: Type, rootType: RootType): InputValue = `type` match { - case ListType(ofType, nonNull) => + case ListType(ofType, _) => value match { case ListValue(values) => ListValue(values.map(v => rewriteValues(v, ofType, rootType))) case _ => value } - case NamedType(name, nonNull) => + case NamedType(name, _) => rootType.types.get(name).map(t => resolveEnumValues(value, t, rootType)).getOrElse(value) } diff --git a/core/src/test/scala/caliban/execution/FieldArgsSpec.scala b/core/src/test/scala/caliban/execution/FieldArgsSpec.scala index f2ebb5f9d..e406269e5 100644 --- a/core/src/test/scala/caliban/execution/FieldArgsSpec.scala +++ b/core/src/test/scala/caliban/execution/FieldArgsSpec.scala @@ -215,27 +215,6 @@ object FieldArgsSpec extends DefaultRunnableSpec { ) ) } yield assertTrue(res.data.toString == "{\"query\":\"BLUE\"}") - }, - testM("it doesn't allow strings as enums in GQL syntax") { - case class QueryInput(color: COLOR) - case class Query(query: QueryInput => UIO[String]) - val query = - """query { - | query(color: "BLUE") - |}""".stripMargin - - val api = graphQL( - RootResolver( - Query( - query = i => ZIO.succeed(i.toString) - ) - ) - ) - - for { - interpreter <- api.interpreter - res <- interpreter.execute(query) - } yield assert(res.errors.headOption)(isSome(anything)) } ) }