Skip to content

Commit 9e2f865

Browse files
authored
BREAKING CHANGE: Change GraphQLID to only use strings (ExpediaGroup#345)
* Change GraphQLID to only use strings Fixes ExpediaGroup#317 Due to deserliazation issues that can happen with UUID, we are only going to support Strings as the explicit type for GraphQLID. You can still use UUID in your data fetchers but you will have to do the parsing yourself and handle the exceptions if it is not a valid UUID. * Drop extra id fields * Fix unit testS
1 parent 1ec833e commit 9e2f865

File tree

6 files changed

+32
-34
lines changed

6 files changed

+32
-34
lines changed

examples/spring/src/main/kotlin/com/expediagroup/graphql/sample/query/ScalarQuery.kt

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ class ScalarQuery: Query {
3131
@GraphQLDescription("generates random UUID")
3232
fun generateRandomUUID() = UUID.randomUUID()
3333

34-
fun findPersonById(id: Int) = Person(id, "Nelson")
34+
fun findPersonById(@GraphQLID id: String) = Person(id, "Nelson")
3535
}
3636

3737
@Component
@@ -41,7 +41,6 @@ class ScalarMutation : Mutation {
4141

4242
data class Person(
4343
@GraphQLID
44-
val id: Int,
45-
44+
val id: String,
4645
val name: String
4746
)

graphql-kotlin-schema-generator/src/main/kotlin/com/expediagroup/graphql/exceptions/InvalidIdTypeException.kt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,5 +21,5 @@ import kotlin.reflect.KClass
2121
/**
2222
* Throws when the KClass is not one of the supported types for a GraphQLID
2323
*/
24-
class InvalidIdTypeException(kClass: KClass<*>, types: String)
25-
: GraphQLKotlinException("${kClass.simpleName} is not a valid ID type, only $types are accepted")
24+
class InvalidIdTypeException(kClass: KClass<*>)
25+
: GraphQLKotlinException("${kClass.simpleName} is not a valid ID type, only Strings are accepted")

graphql-kotlin-schema-generator/src/main/kotlin/com/expediagroup/graphql/generator/types/ScalarBuilder.kt

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,14 @@ import com.expediagroup.graphql.exceptions.InvalidIdTypeException
2020
import com.expediagroup.graphql.generator.SchemaGenerator
2121
import com.expediagroup.graphql.generator.TypeBuilder
2222
import com.expediagroup.graphql.generator.extensions.getKClass
23-
import com.expediagroup.graphql.generator.extensions.getQualifiedName
2423
import com.expediagroup.graphql.generator.extensions.safeCast
2524
import graphql.Scalars
2625
import graphql.schema.GraphQLScalarType
2726
import java.math.BigDecimal
2827
import java.math.BigInteger
29-
import java.util.UUID
3028
import kotlin.reflect.KClass
3129
import kotlin.reflect.KType
30+
import kotlin.reflect.full.isSubclassOf
3231

3332
internal class ScalarBuilder(generator: SchemaGenerator) : TypeBuilder(generator) {
3433

@@ -46,18 +45,14 @@ internal class ScalarBuilder(generator: SchemaGenerator) : TypeBuilder(generator
4645

4746
@Throws(InvalidIdTypeException::class)
4847
private fun getId(kClass: KClass<*>): GraphQLScalarType? {
49-
return if (validIdTypes.contains(kClass)) {
48+
return if (kClass.isSubclassOf(String::class)) {
5049
Scalars.GraphQLID
5150
} else {
52-
val types = validIdTypes.joinToString(prefix = "[", postfix = "]", separator = ", ") {
53-
it.getQualifiedName()
54-
}
55-
throw InvalidIdTypeException(kClass, types)
51+
throw InvalidIdTypeException(kClass)
5652
}
5753
}
5854

5955
private companion object {
60-
private val validIdTypes = listOf(Int::class, String::class, Long::class, UUID::class)
6156
private val defaultScalarsMap = mapOf(
6257
Int::class to Scalars.GraphQLInt,
6358
Long::class to Scalars.GraphQLLong,

graphql-kotlin-schema-generator/src/test/kotlin/com/expediagroup/graphql/generator/SchemaGeneratorTest.kt

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -323,7 +323,7 @@ class SchemaGeneratorTest {
323323
val bobChildren = res?.get("children") as? List<Map<String, Any>>
324324
assertNotNull(bobChildren)
325325

326-
val firstChild = bobChildren.get(0)
326+
val firstChild = bobChildren.first()
327327
assertEquals("Alice", firstChild["name"])
328328
assertNull(firstChild["children"])
329329
}
@@ -333,19 +333,14 @@ class SchemaGeneratorTest {
333333
val schema = toSchema(queries = listOf(TopLevelObject(QueryWithId())), config = testSchemaConfig)
334334

335335
val placeType = schema.getObjectType("PlaceOfIds")
336-
assertEquals(Scalars.GraphQLID, (placeType.getFieldDefinition("intId").type as? GraphQLNonNull)?.wrappedType)
337-
assertEquals(Scalars.GraphQLID, (placeType.getFieldDefinition("longId").type as? GraphQLNonNull)?.wrappedType)
338-
assertEquals(Scalars.GraphQLID, (placeType.getFieldDefinition("stringId").type as? GraphQLNonNull)?.wrappedType)
339-
assertEquals(Scalars.GraphQLID, (placeType.getFieldDefinition("uuid").type as? GraphQLNonNull)?.wrappedType)
336+
assertEquals(Scalars.GraphQLID, (placeType.getFieldDefinition("id").type as? GraphQLNonNull)?.wrappedType)
340337
}
341338

342339
@Test
343340
fun `SchemaGenerator throws an exception for invalid GraphQLID`() {
344-
val exception = assertFailsWith(InvalidIdTypeException::class) {
341+
assertFailsWith(InvalidIdTypeException::class) {
345342
toSchema(queries = listOf(TopLevelObject(QueryWithInvalidId())), config = testSchemaConfig)
346343
}
347-
348-
assertEquals("Person is not a valid ID type, only [kotlin.Int, kotlin.String, kotlin.Long, java.util.UUID] are accepted", exception.message)
349344
}
350345

351346
@Test
@@ -506,17 +501,12 @@ class SchemaGeneratorTest {
506501

507502
data class Person(val name: String, val children: List<Person>? = null)
508503

509-
data class PlaceOfIds(
510-
@GraphQLID val intId: Int,
511-
@GraphQLID val longId: Long,
512-
@GraphQLID val stringId: String,
513-
@GraphQLID val uuid: UUID
514-
)
504+
data class PlaceOfIds(@GraphQLID val id: String)
515505

516506
data class InvalidIds(@GraphQLID val person: Person)
517507

518508
class QueryWithId {
519-
fun query(): PlaceOfIds = PlaceOfIds(42, 24, "42", UUID.randomUUID())
509+
fun query(): PlaceOfIds = PlaceOfIds(UUID.randomUUID().toString())
520510
}
521511

522512
class QueryWithInvalidId {
@@ -568,7 +558,7 @@ class SchemaGeneratorTest {
568558
}
569559

570560
data class Furniture(
571-
@GraphQLID val serial: UUID,
561+
@GraphQLID val serial: String,
572562
val type: String
573563
)
574564

graphql-kotlin-schema-generator/src/test/kotlin/com/expediagroup/graphql/generator/types/ArgumentBuilderTest.kt

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import com.expediagroup.graphql.annotations.GraphQLID
2121
import com.expediagroup.graphql.annotations.GraphQLName
2222
import com.expediagroup.graphql.exceptions.InvalidInputFieldTypeException
2323
import com.expediagroup.graphql.test.utils.SimpleDirective
24+
import graphql.Scalars
2425
import graphql.schema.GraphQLNonNull
2526
import org.junit.jupiter.api.Test
2627
import kotlin.reflect.full.findParameterByName
@@ -47,7 +48,7 @@ internal class ArgumentBuilderTest : TypeTestHelper() {
4748

4849
fun changeName(@GraphQLName("newName") input: String) = input
4950

50-
fun id(@GraphQLID idArg: Int) = "Your id is $idArg"
51+
fun id(@GraphQLID idArg: String) = "Your id is $idArg"
5152

5253
fun interfaceArg(input: MyInterface) = input.id
5354
}
@@ -90,7 +91,7 @@ internal class ArgumentBuilderTest : TypeTestHelper() {
9091
val result = builder.argument(kParameter)
9192

9293
assertEquals(expected = "idArg", actual = result.name)
93-
assertEquals("ID", (result.type as? GraphQLNonNull)?.wrappedType?.name)
94+
assertEquals(Scalars.GraphQLID, (result.type as? GraphQLNonNull)?.wrappedType)
9495
}
9596

9697
@Test

graphql-kotlin-schema-generator/src/test/kotlin/com/expediagroup/graphql/generator/types/ScalarBuilderTest.kt

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,22 @@ internal class ScalarBuilderTest : TypeTestHelper() {
6262
@Test
6363
fun id() {
6464
verify(Ids::stringID.returnType, Scalars.GraphQLID, true)
65-
verify(Ids::intID.returnType, Scalars.GraphQLID, true)
66-
verify(Ids::longID.returnType, Scalars.GraphQLID, true)
67-
verify(Ids::uuid.returnType, Scalars.GraphQLID, true)
65+
66+
assertFailsWith(InvalidIdTypeException::class) {
67+
verify(Ids::intID.returnType, Scalars.GraphQLID, true)
68+
}
69+
70+
assertFailsWith(InvalidIdTypeException::class) {
71+
verify(Ids::longID.returnType, Scalars.GraphQLID, true)
72+
}
73+
74+
assertFailsWith(InvalidIdTypeException::class) {
75+
verify(Ids::uuid.returnType, Scalars.GraphQLID, true)
76+
}
77+
78+
assertFailsWith(InvalidIdTypeException::class) {
79+
verify(Ids::invalidID.returnType, Scalars.GraphQLID, true)
80+
}
6881

6982
assertFailsWith(InvalidIdTypeException::class) {
7083
verify(Ids::invalidID.returnType, Scalars.GraphQLID, true)

0 commit comments

Comments
 (0)