Skip to content

Commit

Permalink
Coerce int values into float when expected (#1150)
Browse files Browse the repository at this point in the history
* Coerce int values into float when expected

* Better precision

* Don't rely on implicit conversions

* Also handle the case of arguments

* Change test
  • Loading branch information
ghostdogpr authored Nov 17, 2021
1 parent 6b992dc commit 3086e20
Show file tree
Hide file tree
Showing 6 changed files with 72 additions and 34 deletions.
12 changes: 5 additions & 7 deletions core/src/main/scala/caliban/execution/Field.scala
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
package caliban.execution

import caliban.InputValue
import caliban.Value
import caliban.{ InputValue, Value }
import caliban.Value.{ BooleanValue, NullValue }
import caliban.introspection.adt.{ __DeprecatedArgs, __Type, __TypeKind }
import caliban.introspection.adt.{ __DeprecatedArgs, __Type }
import caliban.parsing.SourceMapper
import caliban.parsing.adt.Definition.ExecutableDefinition.FragmentDefinition
import caliban.parsing.adt.Selection.{ Field => F, FragmentSpread, InlineFragment }
import caliban.parsing.adt.{ Directive, LocationInfo, Selection, Type, VariableDefinition }
import caliban.parsing.adt.{ Directive, LocationInfo, Selection, VariableDefinition }
import caliban.schema.{ RootType, Types }

case class Field(
Expand Down Expand Up @@ -58,7 +57,7 @@ object Field {
alias,
field.fields,
None,
resolveVariables(arguments, variableDefinitions, variableValues, rootType),
resolveVariables(arguments, variableDefinitions, variableValues),
() => sourceMapper.getLocation(index),
directives ++ schemaDirectives
)
Expand Down Expand Up @@ -96,8 +95,7 @@ object Field {
private def resolveVariables(
arguments: Map[String, InputValue],
variableDefinitions: List[VariableDefinition],
variableValues: Map[String, InputValue],
rootType: RootType
variableValues: Map[String, InputValue]
): Map[String, InputValue] = {
def resolveVariable(value: InputValue): InputValue =
value match {
Expand Down
29 changes: 17 additions & 12 deletions core/src/main/scala/caliban/parsing/VariablesUpdater.scala
Original file line number Diff line number Diff line change
@@ -1,15 +1,12 @@
package caliban.parsing

import caliban.GraphQLRequest
import caliban.InputValue.ListValue
import caliban.Value.StringValue
import caliban.Value._
import caliban.introspection.adt._
import caliban.parsing.adt.Definition.ExecutableDefinition.OperationDefinition
import caliban.parsing.adt.Type.{ ListType, NamedType }
import caliban.parsing.adt._
import caliban.schema.RootType
import caliban.{ InputValue, Value }
import zio.{ IO, UIO }
import caliban.{ GraphQLRequest, InputValue, Value }

object VariablesUpdater {
def updateVariables(
Expand Down Expand Up @@ -42,13 +39,14 @@ object VariablesUpdater {
case _ => value
}
case NamedType(name, _) =>
rootType.types.get(name).map(t => resolveEnumValues(value, t, rootType)).getOrElse(value)
rootType.types.get(name).map(t => coerceValues(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(
// Same thing with Int into Float.
private def coerceValues(
value: InputValue,
typ: __Type,
rootType: RootType
Expand All @@ -60,7 +58,7 @@ object VariablesUpdater {
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)
defs.find(_.name == k).map(field => coerceValues(v, field.`type`(), rootType)).getOrElse(value)

(k, updated)
})
Expand All @@ -72,22 +70,29 @@ object VariablesUpdater {
value match {
case ListValue(values) =>
typ.ofType
.map(innerType => ListValue(values.map(value => resolveEnumValues(value, innerType, rootType))))
.map(innerType => ListValue(values.map(value => coerceValues(value, innerType, rootType))))
.getOrElse(value)
case _ => value
}

case __TypeKind.NON_NULL =>
typ.ofType
.map(innerType => resolveEnumValues(value, innerType, rootType))
.map(innerType => coerceValues(value, innerType, rootType))
.getOrElse(value)

case __TypeKind.ENUM =>
case __TypeKind.ENUM =>
value match {
case StringValue(value) => Value.EnumValue(value)
case _ => value
}
case _ =>
case __TypeKind.SCALAR if typ.name.contains("Float") =>
value match {
case IntValue.IntNumber(value) => Value.FloatValue(value.toDouble)
case IntValue.LongNumber(value) => Value.FloatValue(value.toDouble)
case IntValue.BigIntNumber(value) => Value.FloatValue(BigDecimal(value))
case _ => value
}
case _ =>
value
}
}
4 changes: 2 additions & 2 deletions core/src/main/scala/caliban/validation/ValueValidator.scala
Original file line number Diff line number Diff line change
Expand Up @@ -138,8 +138,8 @@ object ValueValidator {
}
case "Float" =>
argValue match {
case _: Value.FloatValue | NullValue => IO.unit
case t => failValidation(s"$errorContext has invalid type $t", "Expected 'Float'")
case _: Value.FloatValue | _: Value.IntValue | NullValue => IO.unit
case t => failValidation(s"$errorContext has invalid type $t", "Expected 'Float'")
}
case "Boolean" =>
argValue match {
Expand Down
10 changes: 0 additions & 10 deletions core/src/main/scala/caliban/wrappers/Wrapper.scala
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,6 @@ sealed trait Wrapper[-R] extends GraphQLAspect[Nothing, R] { self =>

object Wrapper {

/**
* `WrappingFunction[R, E, A, Info]` is an alias for a function that transforms an initial function
* from `Info` to `ZIO[R, E, A]` into a new function from `Info` to `ZIO[R, E, A]`.
*/
trait WrappingFunction[-R, E, A, Info] {
def wrap[R1 <: R](f: Info => ZIO[R1, E, A]): Info => ZIO[R1, E, A]
}

sealed trait SimpleWrapper[-R, E, A, Info] extends Wrapper[R] {
def wrap[R1 <: R](f: Info => ZIO[R1, E, A]): Info => ZIO[R1, E, A]
}
Expand Down Expand Up @@ -81,8 +73,6 @@ object Wrapper {
): ZQuery[R1, ExecutionError, ResponseValue]
}

trait EffectWrappingFunction[-R]

/**
* Wrapper for the introspection query processing.
* Takes a function from a `ZIO[R, ExecutionError, __Introspection]` and that returns a
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ object DefaultValueSpec extends DefaultRunnableSpec {
)
},
testM("invalid float validation") {
case class TestInput(@GQLDefault("1") float: Float)
case class TestInput(@GQLDefault("true") float: Float)
case class Query(test: TestInput => Float)
val gql = graphQL(RootResolver(Query(i => i.float)))
assertM(gql.interpreter.run)(
Expand Down
49 changes: 47 additions & 2 deletions core/src/test/scala/caliban/validation/InputObjectSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ object InputObjectSpec extends DefaultRunnableSpec {
int <- gql.interpreter
res <- int.execute(query)
} yield assert(res.errors.headOption)(
isSome((isSubtype[CalibanError.ValidationError](anything)))
isSome(isSubtype[CalibanError.ValidationError](anything))
)
},
testM("fails if a non-null field on an input object is null from variables") {
Expand Down Expand Up @@ -53,7 +53,7 @@ object InputObjectSpec extends DefaultRunnableSpec {
)
)
} yield assert(res.errors.headOption)(
isSome((isSubtype[CalibanError.ValidationError](anything)))
isSome(isSubtype[CalibanError.ValidationError](anything))
)
},
testM("allow null passed to optional enum") {
Expand Down Expand Up @@ -139,6 +139,51 @@ object InputObjectSpec extends DefaultRunnableSpec {
)
)
} yield assert(res.errors)(isEmpty)
},
testM("allow Int passed to Float field") {
val query =
"""query {
| query(input: { float: 42 })
|}""".stripMargin

case class TestInputObject(float: Float)
case class TestInput(input: TestInputObject)
case class TestOutput(value: String)
case class Query(query: TestInput => String)
val gql = graphQL(RootResolver(Query(_.input.float.toString)))

for {
int <- gql.interpreter
res <- int.executeRequest(GraphQLRequest(query = Some(query)))
} yield assert(res.errors)(isEmpty)
},
testM("allow Int passed to Float field in a variable") {
val query =
"""query QueryName($input: TestInputObjectInput!) {
| query(input: $input)
|}""".stripMargin

case class TestInputObject(float: Float)
case class TestInput(input: TestInputObject)
case class TestOutput(value: String)
case class Query(query: TestInput => String)
val gql = graphQL(RootResolver(Query(_.input.float.toString)))

for {
int <- gql.interpreter
res <- int.executeRequest(
GraphQLRequest(
query = Some(query),
variables = Some(
Map(
"input" -> InputValue.ObjectValue(
Map("float" -> Value.IntValue(42))
)
)
)
)
)
} yield assert(res.errors)(isEmpty)
}
)
}

0 comments on commit 3086e20

Please sign in to comment.