Skip to content

Commit

Permalink
refactor: Make valueNode of ScalarCoercion required (#63)
Browse files Browse the repository at this point in the history 
The `valueNode` of `ScalarCoercion` was nullable, leading to explicit
`null` cases in various places. However, the code always calls it with a
non-null `ValueNode`, cf.
https://github.com/stuebingerb/KGraphQL/blob/main/kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/scalar/Coercion.kt#L25
  • Loading branch information
@stuebingerb
stuebingerb authored Nov 8, 2024
2 parents 47e05f0 + f3b9aeb commit 097d78d
Show file tree
Hide file tree
Showing 13 changed files with 51 additions and 85 deletions.
50 changes: 8 additions & 42 deletions kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/builtin/BUILT_IN_TYPE.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ package com.apurebase.kgraphql.schema.builtin

import com.apurebase.kgraphql.GraphQLError
import com.apurebase.kgraphql.defaultKQLTypeName
import com.apurebase.kgraphql.dropQuotes
import com.apurebase.kgraphql.isLiteral
import com.apurebase.kgraphql.schema.model.TypeDef
import com.apurebase.kgraphql.schema.model.ast.ValueNode
import com.apurebase.kgraphql.schema.model.ast.ValueNode.BooleanValueNode
Expand Down Expand Up @@ -43,7 +41,7 @@ object BUILT_IN_TYPE {

val INT = TypeDef.Scalar(Int::class.defaultKQLTypeName(), Int::class, INT_COERCION, INT_DESCRIPTION)

//GraphQL does not differ float and double, treat double like float
// GraphQL does not differ float and double, treat double like float
val DOUBLE = TypeDef.Scalar(Float::class.defaultKQLTypeName(), Double::class, DOUBLE_COERCION, FLOAT_DESCRIPTION)

val FLOAT = TypeDef.Scalar(Float::class.defaultKQLTypeName(), Float::class, FLOAT_COERCION, FLOAT_DESCRIPTION)
Expand All @@ -57,8 +55,7 @@ object BUILT_IN_TYPE {
object STRING_COERCION : StringScalarCoercion<String> {
override fun serialize(instance: String): String = instance

override fun deserialize(raw: String, valueNode: ValueNode?) = when (valueNode) {
null -> raw.dropQuotes()
override fun deserialize(raw: String, valueNode: ValueNode) = when (valueNode) {
is StringValueNode -> valueNode.value
else -> throw GraphQLError(
"Cannot coerce ${valueNode.valueNodeName} to string constant",
Expand All @@ -70,12 +67,7 @@ object STRING_COERCION : StringScalarCoercion<String> {
object DOUBLE_COERCION : StringScalarCoercion<Double> {
override fun serialize(instance: Double): String = instance.toString()

override fun deserialize(raw: String, valueNode: ValueNode?) = when (valueNode) {
null -> {
if (!raw.isLiteral()) raw.toDouble()
else throw GraphQLError("Cannot coerce string literal, expected numeric string constant")
}

override fun deserialize(raw: String, valueNode: ValueNode) = when (valueNode) {
is DoubleValueNode -> valueNode.value
is NumberValueNode -> valueNode.value.toDouble()
else -> throw GraphQLError(
Expand All @@ -88,8 +80,7 @@ object DOUBLE_COERCION : StringScalarCoercion<Double> {
object FLOAT_COERCION : StringScalarCoercion<Float> {
override fun serialize(instance: Float): String = instance.toDouble().toString()

override fun deserialize(raw: String, valueNode: ValueNode?) = when (valueNode) {
null -> DOUBLE_COERCION.deserialize(raw).toFloat()
override fun deserialize(raw: String, valueNode: ValueNode) = when (valueNode) {
is DoubleValueNode -> DOUBLE_COERCION.deserialize(raw, valueNode).toFloat()
is NumberValueNode -> DOUBLE_COERCION.deserialize(raw, valueNode).toFloat()
else -> throw GraphQLError(
Expand All @@ -102,12 +93,7 @@ object FLOAT_COERCION : StringScalarCoercion<Float> {
object INT_COERCION : StringScalarCoercion<Int> {
override fun serialize(instance: Int): String = instance.toString()

override fun deserialize(raw: String, valueNode: ValueNode?) = when (valueNode) {
null -> {
if (!raw.isLiteral()) raw.toInt()
else throw GraphQLError("Cannot coerce string literal, expected numeric string constant")
}

override fun deserialize(raw: String, valueNode: ValueNode) = when (valueNode) {
is NumberValueNode -> when {
valueNode.value > Integer.MAX_VALUE -> throw GraphQLError(
"Cannot coerce to type of Int as '${valueNode.value}' is greater than (2^-31)-1",
Expand All @@ -132,12 +118,7 @@ object INT_COERCION : StringScalarCoercion<Int> {
object SHORT_COERCION : StringScalarCoercion<Short> {
override fun serialize(instance: Short): String = instance.toString()

override fun deserialize(raw: String, valueNode: ValueNode?) = when (valueNode) {
null -> {
if (!raw.isLiteral()) raw.toShort()
else throw GraphQLError("Cannot coerce string literal, expected numeric string constant")
}

override fun deserialize(raw: String, valueNode: ValueNode) = when (valueNode) {
is NumberValueNode -> when {
valueNode.value > Short.MAX_VALUE -> throw GraphQLError(
"Cannot coerce to type of Int as '${valueNode.value}' is greater than (2^-15)-1",
Expand All @@ -162,12 +143,7 @@ object SHORT_COERCION : StringScalarCoercion<Short> {
object LONG_COERCION : StringScalarCoercion<Long> {
override fun serialize(instance: Long): String = instance.toString()

override fun deserialize(raw: String, valueNode: ValueNode?) = when (valueNode) {
null -> {
if (!raw.isLiteral()) raw.toLong()
else throw GraphQLError("Cannot coerce string literal, expected numeric string constant")
}

override fun deserialize(raw: String, valueNode: ValueNode) = when (valueNode) {
is NumberValueNode -> valueNode.value
else -> throw GraphQLError(
"Cannot coerce ${valueNode.valueNodeName} to expected numeric constant",
Expand All @@ -180,17 +156,7 @@ object LONG_COERCION : StringScalarCoercion<Long> {
object BOOLEAN_COERCION : StringScalarCoercion<Boolean> {
override fun serialize(instance: Boolean): String = instance.toString()

override fun deserialize(raw: String, valueNode: ValueNode?) = when (valueNode) {
null -> {
if (raw.isLiteral()) throw GraphQLError("Cannot coerce string literal, expected numeric string constant")
when {
//custom parsing, because String#toBoolean() returns false for any input != true
raw.equals("true", true) -> true
raw.equals("false", true) -> false
else -> throw IllegalArgumentException("$raw does not represent valid Boolean value")
}
}

override fun deserialize(raw: String, valueNode: ValueNode) = when (valueNode) {
is BooleanValueNode -> valueNode.value
is StringValueNode -> when {
valueNode.value.equals("true", true) -> true
Expand Down
2 changes: 1 addition & 1 deletion kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/dsl/types/BooleanScalarDSL.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class BooleanScalarDSL<T : Any>(kClass: KClass<T>) : ScalarDSL<T, Boolean>(kClas

override fun serialize(instance: T): Boolean = serializeImpl(instance)

override fun deserialize(raw: Boolean, valueNode: ValueNode?): T = deserializeImpl(raw)
override fun deserialize(raw: Boolean, valueNode: ValueNode): T = deserializeImpl(raw)
}
}
}
2 changes: 1 addition & 1 deletion kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/dsl/types/DoubleScalarDSL.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class DoubleScalarDSL<T : Any>(kClass: KClass<T>) : ScalarDSL<T, Double>(kClass)

override fun serialize(instance: T): Double = serializeImpl(instance)

override fun deserialize(raw: Double, valueNode: ValueNode?): T = deserializeImpl(raw)
override fun deserialize(raw: Double, valueNode: ValueNode): T = deserializeImpl(raw)
}
}
}
2 changes: 1 addition & 1 deletion kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/dsl/types/IntScalarDSL.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class IntScalarDSL<T : Any>(kClass: KClass<T>) : ScalarDSL<T, Int>(kClass) {

override fun serialize(instance: T): Int = serializeImpl(instance)

override fun deserialize(raw: Int, valueNode: ValueNode?): T = deserializeImpl(raw)
override fun deserialize(raw: Int, valueNode: ValueNode): T = deserializeImpl(raw)
}
}
}
2 changes: 1 addition & 1 deletion kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/dsl/types/LongScalarDSL.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class LongScalarDSL<T : Any>(kClass: KClass<T>) : ScalarDSL<T, Long>(kClass) {

override fun serialize(instance: T): Long = serializeImpl(instance)

override fun deserialize(raw: Long, valueNode: ValueNode?): T = deserializeImpl(raw)
override fun deserialize(raw: Long, valueNode: ValueNode): T = deserializeImpl(raw)
}
}
}
2 changes: 1 addition & 1 deletion kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/dsl/types/ShortScalarDSL.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class ShortScalarDSL<T : Any>(kClass: KClass<T>) : ScalarDSL<T, Short>(kClass) {

override fun serialize(instance: T): Short = serializeImpl(instance)

override fun deserialize(raw: Short, valueNode: ValueNode?): T = deserializeImpl(raw)
override fun deserialize(raw: Short, valueNode: ValueNode): T = deserializeImpl(raw)
}
}
}
2 changes: 1 addition & 1 deletion kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/dsl/types/StringScalarDSL.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class StringScalarDSL<T : Any>(kClass: KClass<T>) : ScalarDSL<T, String>(kClass)

override fun serialize(instance: T): String = serializeImpl(instance)

override fun deserialize(raw: String, valueNode: ValueNode?): T = deserializeImpl(raw)
override fun deserialize(raw: String, valueNode: ValueNode): T = deserializeImpl(raw)
}
}
}
2 changes: 1 addition & 1 deletion kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/scalar/Coercion.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ private typealias JsonValueNode = com.fasterxml.jackson.databind.node.ValueNode
fun <T : Any> deserializeScalar(scalar: Type.Scalar<T>, value: ValueNode): T {
try {
return when (scalar.coercion) {
//built in scalars
// built-in scalars
STRING_COERCION -> STRING_COERCION.deserialize(value.valueNodeName, value as StringValueNode) as T
FLOAT_COERCION -> FLOAT_COERCION.deserialize(value.valueNodeName, value) as T
DOUBLE_COERCION -> DOUBLE_COERCION.deserialize(value.valueNodeName, value) as T
Expand Down
2 changes: 1 addition & 1 deletion kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/scalar/ScalarCoercion.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,6 @@ interface ScalarCoercion<Scalar, Raw> {
/**
* strategy for scalar deserialization
*/
fun deserialize(raw: Raw, valueNode: ValueNode? = null): Scalar
fun deserialize(raw: Raw, valueNode: ValueNode): Scalar
}

2 changes: 1 addition & 1 deletion kgraphql/src/test/kotlin/com/apurebase/kgraphql/TestClasses.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class Id(val literal: String, val numeric: Int)
class IdScalarSupport : StringScalarCoercion<Id> {
override fun serialize(instance: Id): String = "${instance.literal}:${instance.numeric}"

override fun deserialize(raw: String, valueNode: ValueNode?) = Id(raw.split(':')[0], raw.split(':')[1].toInt())
override fun deserialize(raw: String, valueNode: ValueNode) = Id(raw.split(':')[0], raw.split(':')[1].toInt())
}

enum class FilmType { FULL_LENGTH, SHORT_LENGTH }
Expand Down
21 changes: 2 additions & 19 deletions kgraphql/src/test/kotlin/com/apurebase/kgraphql/schema/SchemaBuilderTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import com.apurebase.kgraphql.schema.dsl.SchemaBuilder
import com.apurebase.kgraphql.schema.dsl.types.TypeDSL
import com.apurebase.kgraphql.schema.execution.DefaultGenericTypeResolver
import com.apurebase.kgraphql.schema.introspection.TypeKind
import com.apurebase.kgraphql.schema.model.ast.ValueNode
import com.apurebase.kgraphql.schema.scalar.StringScalarCoercion
import com.apurebase.kgraphql.schema.structure.Field
import kotlinx.coroutines.Dispatchers
Expand All @@ -41,22 +42,6 @@ import kotlin.reflect.typeOf
* Tests for SchemaBuilder behaviour, not request execution
*/
class SchemaBuilderTest {
@Suppress("UNCHECKED_CAST")
@Test
fun `DSL created UUID scalar support`() {
val testedSchema = defaultSchema {
stringScalar<UUID> {
description = "unique identifier of object"
deserialize = { uuid: String -> UUID.fromString(uuid) }
serialize = UUID::toString
}
}

val uuidScalar = testedSchema.model.scalars[UUID::class]!!.coercion as StringScalarCoercion<UUID>
val testUuid = UUID.randomUUID()
assertThat(uuidScalar.serialize(testUuid), equalTo(testUuid.toString()))
assertThat(uuidScalar.deserialize(testUuid.toString()), equalTo(testUuid))
}

@Test
fun `ignored property DSL`() {
Expand Down Expand Up @@ -107,7 +92,6 @@ class SchemaBuilderTest {
resolver { -> Scenario(Id("GKalus", 234234), "Gamil Kalus", "TOO LONG") }
}
type<Scenario> {

transformation(Scenario::content) { content: String, capitalized: Boolean? ->
if (capitalized == true) content.replaceFirstChar { it.uppercase() } else content
}
Expand Down Expand Up @@ -140,7 +124,6 @@ class SchemaBuilderTest {

assertThat(scenarioType.kind, equalTo(TypeKind.OBJECT))
assertThat(scenarioType["pdf"], notNullValue())

}

@Test
Expand Down Expand Up @@ -239,7 +222,7 @@ class SchemaBuilderTest {
}

@Test
fun ` _ is allowed as receiver argument name`() {
fun ` _ should be allowed as receiver argument name`() {
val schema = defaultSchema {
query("actor") {
resolver { -> Actor("Boguś Linda", 4343) }
Expand Down
23 changes: 19 additions & 4 deletions ...test/kotlin/com/apurebase/kgraphql/specification/language/InputValuesSpecificationTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import org.hamcrest.CoreMatchers.equalTo
import org.hamcrest.MatcherAssert.assertThat
import org.junit.jupiter.api.Test
import org.junit.jupiter.params.ParameterizedTest
import org.junit.jupiter.params.provider.CsvSource
import org.junit.jupiter.params.provider.ValueSource

@Specification("2.9 Input Values")
Expand Down Expand Up @@ -76,12 +77,26 @@ class InputValuesSpecificationTest {
assertThat(response.extract<Double>("data/Double"), equalTo(input))
}

@Test
@ParameterizedTest
@CsvSource(
value = [
"true, true",
"false, false",
"\"true\", true",
"\"false\", false",
"\"TRUE\", true",
"\"FALSE\", false",
"\"tRuE\", true",
"\"faLSe\", false",
"1, true",
"0, false",
"-1, false"
]
)
@Specification("2.9.3 Boolean Value")
fun `Boolean input value`() {
val input = true
fun `Boolean input value`(input: String, expected: Boolean) {
val response = deserialize(schema.executeBlocking("{ Boolean(value: $input) }"))
assertThat(response.extract<Boolean>("data/Boolean"), equalTo(input))
assertThat(response.extract<Boolean>("data/Boolean"), equalTo(expected))
}

@ParameterizedTest
Expand Down
24 changes: 13 additions & 11 deletions ...c/test/kotlin/com/apurebase/kgraphql/specification/typesystem/ScalarsSpecificationTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class ScalarsSpecificationTest {
coercion = object : StringScalarCoercion<UUID> {
override fun serialize(instance: UUID): String = instance.toString()

override fun deserialize(raw: String, valueNode: ValueNode?): UUID = UUID.fromString(raw)
override fun deserialize(raw: String, valueNode: ValueNode): UUID = UUID.fromString(raw)
}
}
query("person") {
Expand Down Expand Up @@ -88,7 +88,7 @@ class ScalarsSpecificationTest {
stringScalar<UUID> {
name = "ID"
description = "the unique identifier of object"
deserialize = { uuid: String -> UUID.fromString(uuid) }
deserialize = UUID::fromString
serialize = UUID::toString
}
query("personById") {
Expand Down Expand Up @@ -285,8 +285,6 @@ class ScalarsSpecificationTest {
mutation("addPart") {
description = "Adds a new part in the parts inventory database"
resolver { newPart: NewPart ->
println(newPart)

newPart
}
}
Expand All @@ -295,21 +293,25 @@ class ScalarsSpecificationTest {
}

val manufacturer = """Joe Bloggs"""
val addedDate = "2001-09-01"

val response = deserialize(
schema.executeBlocking(
"mutation Mutation(\$newPart : NewPart!){ addPart(newPart: \$newPart) {manufacturer} }",
"mutation Mutation(\$newPart: NewPart!) { addPart(newPart: \$newPart) { addedDate manufacturer } }",
"""
{ "newPart" : {
"manufacturer":"$manufacturer",
"name":"Front bumper",
"oem":true,
"addedDate":"2001-09-01"
}}
{
"newPart": {
"manufacturer": "$manufacturer",
"name": "Front bumper",
"oem": true,
"addedDate": "$addedDate"
}
}
""".trimIndent()
)
)

assertThat(response.extract<String>("data/addPart/manufacturer"), equalTo(manufacturer))
assertThat(response.extract<String>("data/addPart/addedDate"), equalTo(addedDate))
}
}

0 comments on commit 097d78d

Please sign in to comment.