Skip to content

Commit f1b1db5

Browse files
dariuszkuctapaderster
dariuszkuc
authored and
tapaderster
committed
fix: allow modifying GraphQLInterfaceType in the willAddGraphQLTypeToSchema hook (ExpediaGroup#293)
* fix: allow modifying GraphQLInterfaceType in the willAddGraphQLTypeToSchema hook InterfaceBuilder attempts to create all the implementing `GraphQLObjectType`s after building an interface but before executing the `willAddGraphQLTypeToSchema` hook. This means that if we attempt to update the interface in that hook there could be some objects implementing that interface and referencing the old one. Since graphql-java schema objects don't implement hashcode we end up with type conflict of two interfaces being defined in the schema. Fix is to use `GraphQLTypeReference` when referencing `GraphQLInterfaceType` from the `GraphQLObjectType` builder. Reference is just by name and they are resolved as the last step of building a schema. This allows us to modify the interface in the hook. Resolves: ExpediaGroup#281 * update test to verify hook updates interface description
1 parent f2799b3 commit f1b1db5

File tree

5 files changed

+34
-46
lines changed

5 files changed

+34
-46
lines changed

‎graphql-kotlin-schema-generator/src/main/kotlin/com/expedia/graphql/generator/types/InterfaceBuilder.kt

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,13 +35,11 @@ internal class InterfaceBuilder(generator: SchemaGenerator) : TypeBuilder(genera
3535
val implementations = subTypeMapper.getSubTypesOf(kClass)
3636
implementations.forEach { implementation ->
3737
val objectType = generator.objectType(implementation.kotlin, interfaceType)
38-
39-
// Only update the state if the object is fully constructed and not a reference
38+
// skip under construction objects
4039
if (objectType !is GraphQLTypeReference) {
4140
state.additionalTypes.add(objectType)
4241
}
4342
}
44-
4543
codeRegistry.typeResolver(interfaceType) { env: TypeResolutionEnvironment -> env.schema.getObjectType(env.getObject<Any>().javaClass.kotlin.getSimpleName()) }
4644
config.hooks.onRewireGraphQLType(interfaceType).safeCast()
4745
}

‎graphql-kotlin-schema-generator/src/main/kotlin/com/expedia/graphql/generator/types/ObjectBuilder.kt

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import com.expedia.graphql.generator.extensions.safeCast
1111
import graphql.schema.GraphQLInterfaceType
1212
import graphql.schema.GraphQLObjectType
1313
import graphql.schema.GraphQLType
14+
import graphql.schema.GraphQLTypeReference
1415
import kotlin.reflect.KClass
1516
import kotlin.reflect.full.createType
1617

@@ -29,7 +30,8 @@ internal class ObjectBuilder(generator: SchemaGenerator) : TypeBuilder(generator
2930
}
3031

3132
if (interfaceType != null) {
32-
builder.withInterface(interfaceType)
33+
// invoked from the interface builder which can still be modified by the hooks
34+
builder.withInterface(GraphQLTypeReference(interfaceType.name))
3335
} else {
3436
kClass.getValidSuperclasses(config.hooks)
3537
.map { objectFromReflection(it.createType(), false) }

‎graphql-kotlin-schema-generator/src/main/kotlin/com/expedia/graphql/generator/types/QueryBuilder.kt

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ internal class QueryBuilder(generator: SchemaGenerator) : TypeBuilder(generator)
1313

1414
@Throws(InvalidSchemaException::class)
1515
fun getQueryObject(queries: List<TopLevelObject>): GraphQLObjectType {
16-
1716
if (queries.isEmpty()) {
1817
throw InvalidSchemaException()
1918
}
@@ -32,7 +31,6 @@ internal class QueryBuilder(generator: SchemaGenerator) : TypeBuilder(generator)
3231

3332
query.kClass.getValidFunctions(config.hooks)
3433
.forEach {
35-
// NEED BUILT QUERY
3634
val function = generator.function(it, config.topLevelNames.query, query.obj)
3735
val functionFromHook = config.hooks.didGenerateQueryType(it, function)
3836
queryBuilder.field(functionFromHook)

‎graphql-kotlin-schema-generator/src/main/kotlin/com/expedia/graphql/generator/types/UnionBuilder.kt

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import com.expedia.graphql.generator.SchemaGenerator
44
import com.expedia.graphql.generator.TypeBuilder
55
import com.expedia.graphql.generator.extensions.getGraphQLDescription
66
import com.expedia.graphql.generator.extensions.getSimpleName
7-
import com.expedia.graphql.generator.state.KGraphQLType
87
import com.expedia.graphql.generator.state.TypesCacheKey
98
import graphql.TypeResolutionEnvironment
109
import graphql.schema.GraphQLObjectType
@@ -30,16 +29,10 @@ internal class UnionBuilder(generator: SchemaGenerator) : TypeBuilder(generator)
3029
val objectType = state.cache.get(TypesCacheKey(it.kotlin.createType(), false))
3130
?: generator.objectType(it.kotlin)
3231

33-
val key = TypesCacheKey(it.kotlin.createType(), false)
34-
3532
when (objectType) {
3633
is GraphQLTypeReference -> builder.possibleType(objectType)
3734
is GraphQLObjectType -> builder.possibleType(objectType)
3835
}
39-
40-
if (state.cache.doesNotContain(it.kotlin)) {
41-
state.cache.put(key, KGraphQLType(it.kotlin, objectType))
42-
}
4336
}
4437
val unionType = builder.build()
4538
codeRegistry.typeResolver(unionType) { env: TypeResolutionEnvironment -> env.schema.getObjectType(env.getObject<Any>().javaClass.kotlin.getSimpleName()) }

‎graphql-kotlin-schema-generator/src/test/kotlin/com/expedia/graphql/hooks/SchemaGeneratorHooksTest.kt

Lines changed: 30 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,16 @@
11
package com.expedia.graphql.hooks
22

33
import com.expedia.graphql.TopLevelObject
4-
import com.expedia.graphql.extensions.deepName
54
import com.expedia.graphql.generator.extensions.getSimpleName
65
import com.expedia.graphql.getTestSchemaConfigWithHooks
76
import com.expedia.graphql.toSchema
87
import graphql.schema.GraphQLFieldDefinition
8+
import graphql.schema.GraphQLInterfaceType
99
import graphql.schema.GraphQLObjectType
1010
import graphql.schema.GraphQLSchema
1111
import graphql.schema.GraphQLType
1212
import org.junit.jupiter.api.Test
13+
import kotlin.random.Random
1314
import kotlin.reflect.KFunction
1415
import kotlin.reflect.KProperty
1516
import kotlin.reflect.KType
@@ -41,27 +42,6 @@ class SchemaGeneratorHooksTest {
4142
assertNotNull(schema.getType("InjectedFromHook"))
4243
}
4344

44-
@Test
45-
fun `calls hook before generating object type`() {
46-
class MockSchemaGeneratorHooks : SchemaGeneratorHooks {
47-
var willGenerateGraphQLTypeCalled = false
48-
override fun willGenerateGraphQLType(type: KType): GraphQLType? {
49-
willGenerateGraphQLTypeCalled = true
50-
return GraphQLObjectType.newObject().name("InterceptedFromHook").build()
51-
}
52-
}
53-
54-
val hooks = MockSchemaGeneratorHooks()
55-
val schema = toSchema(
56-
queries = listOf(TopLevelObject(TestQuery())),
57-
config = getTestSchemaConfigWithHooks(hooks)
58-
)
59-
val topLevelQuery = schema.getObjectType("Query")
60-
val query = topLevelQuery.getFieldDefinition("query")
61-
assertTrue(hooks.willGenerateGraphQLTypeCalled)
62-
assertEquals("InterceptedFromHook!", query.type.deepName)
63-
}
64-
6545
@Test
6646
fun `calls hook to filter property`() {
6747
class MockSchemaGeneratorHooks : SchemaGeneratorHooks {
@@ -106,11 +86,9 @@ class SchemaGeneratorHooksTest {
10686
@Test
10787
fun `calls hook after generating object type`() {
10888
class MockSchemaGeneratorHooks : SchemaGeneratorHooks {
109-
var lastSeenType: KType? = null
110-
var lastSeenGeneratedType: GraphQLType? = null
89+
val seenTypes = mutableSetOf<KType>()
11190
override fun didGenerateGraphQLType(type: KType, generatedType: GraphQLType): GraphQLType {
112-
lastSeenType = type
113-
lastSeenGeneratedType = generatedType
91+
seenTypes.add(type)
11492
return generatedType
11593
}
11694
}
@@ -120,8 +98,10 @@ class SchemaGeneratorHooksTest {
12098
queries = listOf(TopLevelObject(TestQuery())),
12199
config = getTestSchemaConfigWithHooks(hooks)
122100
)
123-
assertEquals(SomeData::class.createType(), hooks.lastSeenType)
124-
assertEquals("SomeData!", hooks.lastSeenGeneratedType?.deepName)
101+
assertTrue(hooks.seenTypes.contains(RandomData::class.createType()))
102+
assertTrue(hooks.seenTypes.contains(SomeData::class.createType()))
103+
// TODO bug: didGenerateGraphQLType hook is not applied on the object types build from the interface
104+
// assertTrue(hooks.seenTypes.contains(OtherData::class.createType()))
125105
}
126106

127107
@Test
@@ -132,7 +112,9 @@ class SchemaGeneratorHooksTest {
132112
override fun willAddGraphQLTypeToSchema(type: KType, generatedType: GraphQLType): GraphQLType {
133113
hookCalled = true
134114
return when {
135-
generatedType is GraphQLObjectType && generatedType.name == "SomeData" -> GraphQLObjectType.Builder(generatedType).description("My custom description").build()
115+
generatedType is GraphQLObjectType && generatedType.name == "SomeData" -> GraphQLObjectType.newObject(generatedType).description("My custom description").build()
116+
generatedType is GraphQLInterfaceType && generatedType.name == "RandomData" ->
117+
GraphQLInterfaceType.newInterface(generatedType).description("My custom interface description").build()
136118
else -> generatedType
137119
}
138120
}
@@ -148,6 +130,10 @@ class SchemaGeneratorHooksTest {
148130
val type = schema.getObjectType("SomeData")
149131
assertNotNull(type)
150132
assertEquals(expected = "My custom description", actual = type.description)
133+
134+
val interfaceType = schema.getType("RandomData") as? GraphQLInterfaceType
135+
assertNotNull(interfaceType)
136+
assertEquals(expected = "My custom interface description", actual = interfaceType.description)
151137
}
152138

153139
@Test
@@ -161,7 +147,7 @@ class SchemaGeneratorHooksTest {
161147
newField.description("Hijacked Description")
162148
newField.name(fieldDefinition.name)
163149
newField.type(fieldDefinition.type)
164-
newField.argument(fieldDefinition.arguments)
150+
newField.arguments(fieldDefinition.arguments)
165151
return newField.build()
166152
}
167153
}
@@ -187,7 +173,7 @@ class SchemaGeneratorHooksTest {
187173
newField.description("Hijacked Description")
188174
newField.name(fieldDefinition.name)
189175
newField.type(fieldDefinition.type)
190-
newField.argument(fieldDefinition.arguments)
176+
newField.arguments(fieldDefinition.arguments)
191177
return newField.build()
192178
}
193179
}
@@ -212,8 +198,19 @@ class SchemaGeneratorHooksTest {
212198
}
213199

214200
class TestQuery {
215-
fun query(): SomeData = SomeData(0)
201+
fun query(): SomeData = SomeData("someData", 0)
202+
fun randomQuery(): RandomData = if (Random.nextBoolean()) {
203+
SomeData("random", 1)
204+
} else {
205+
OtherData("random", 1)
206+
}
207+
}
208+
209+
interface RandomData {
210+
val id: String
216211
}
217212

218-
data class SomeData(val someNumber: Int)
213+
data class SomeData(override val id: String, val someNumber: Int) : RandomData
214+
215+
data class OtherData(override val id: String, val otherNumber: Int) : RandomData
219216
}

0 commit comments

Comments
 (0)