fix: *generateGraphQLType hooks are now called on interfaces #303
Conversation
smyrick
added
type: bug
| val cacheKey = getCacheKeyString(key) | ||
|
|
||
| if (cacheKey != null) { | ||
| typeUnderConstruction.remove(kGraphQLType.kClass) | ||
| return cache.put(cacheKey, kGraphQLType) | ||
| cache[cacheKey] = kGraphQLType |
There was a problem hiding this comment.
We don't need to remove from typeUnderConstruction since the only place we add is in the buildIfNotUnderConstruction method and we also remove it there.
...kotlin-schema-generator/src/main/kotlin/com/expedia/graphql/generator/types/ObjectBuilder.kt
Show resolved
Hide resolved
...ql-kotlin-schema-generator/src/main/kotlin/com/expedia/graphql/generator/state/TypesCache.kt
Outdated
Show resolved
Hide resolved
|
@dkuc84 So I found the actual issue with this #294 This code change doesn’t fix the test you commented in previously. If I simplify the unit test and have a query class that only returns field which the return type is the interface then the schema never directly contains all the implementations of the interface, they are all in the additional types. That means to fix the issue we just need to add the hooks where we call the additional types. |
|
@smyrick actual reason behind #294 are those lines -> https://github.com/ExpediaDotCom/graphql-kotlin/blob/master/graphql-kotlin-schema-generator/src/main/kotlin/com/expedia/graphql/generator/types/InterfaceBuilder.kt#L37 and https://github.com/ExpediaDotCom/graphql-kotlin/blob/master/graphql-kotlin-schema-generator/src/main/kotlin/com/expedia/graphql/generator/types/UnionBuilder.kt#L30 They are calling object builder directly instead of using the generic type builder method. |
smyrick
force-pushed
the
fix-interface-hooks
branch
from
9ca242b
to
9d7f940
Compare
Fixes ExpediaGroup#294 Interfaces now go through the internal TypeBuilder method 'graphQLTypeOf' instead of 'objectFromReflection'. This means we can make 'objectFromReflection' private. But the type may be (if not always) wrapped in a GraphQLNonNull which means we have to cast it back to the interface type before adding it to the ObjectBuilder.
smyrick
force-pushed
the
fix-interface-hooks
branch
from
9d7f940
to
e632be2
Compare
| @@ -76,8 +75,8 @@ open class SchemaGenerator(val config: SchemaGeneratorConfig) { | |||
| open fun listType(type: KType, inputType: Boolean) = | |||
| listTypeBuilder.listType(type, inputType) | |||
|
|
|||
| open fun objectType(kClass: KClass<*>, interfaceType: GraphQLInterfaceType? = null) = | |||
| objectTypeBuilder.objectType(kClass, interfaceType) | |||
| open fun objectType(kClass: KClass<*>) = | |||
There was a problem hiding this comment.
Since we no longer directly call objectType, we never call the method passing in an interface so we can clean this up
Codecov Report
@@ Coverage Diff @@
## master #303 +/- ##
============================================
+ Coverage 94.31% 94.42% +0.11%
+ Complexity 260 256 -4
============================================
Files 73 74 +1
Lines 914 915 +1
Branches 169 169
============================================
+ Hits 862 864 +2
Misses 31 31
+ Partials 21 20 -1
Continue to review full report at Codecov.
|
|
Ready for review |
…Group#303) * fix: *generateGraphQLType hooks are now called on interfaces Fixes ExpediaGroup#294 Interfaces now go through the internal TypeBuilder method 'graphQLTypeOf' instead of 'objectFromReflection'. This means we can make 'objectFromReflection' private. But the type may be (if not always) wrapped in a GraphQLNonNull which means we have to cast it back to the interface type before adding it to the ObjectBuilder.
…Group#303) * fix: *generateGraphQLType hooks are now called on interfaces Fixes ExpediaGroup#294 Interfaces now go through the internal TypeBuilder method 'graphQLTypeOf' instead of 'objectFromReflection'. This means we can make 'objectFromReflection' private. But the type may be (if not always) wrapped in a GraphQLNonNull which means we have to cast it back to the interface type before adding it to the ObjectBuilder.
Fixes #294
Interfaces now go through the internal TypeBuilder method 'graphQLTypeOf' instead of 'objectFromReflection'. This means we can make 'objectFromReflection' private. But the type may be (if not always) wrapped in a GraphQLNonNull which means we have to cast it back to the interface type before adding it to the ObjectBuilder.