From 3b4af4946fac1d4f01fb0188115e1ba0a1ce0356 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bernd=20St=C3=BCbinger?= <41049452+stuebingerb@users.noreply.github.com> Date: Wed, 11 Dec 2024 12:23:42 +0100 Subject: [PATCH] refactor: Cleanup `SchemaProxy` `SchemaProxy` is (from my understanding) only a wrapper for schema introspection, and as such should not need to deal with Kotlin classes or offer execution options. This commit also removes some more unused code. --- .../com/apurebase/kgraphql/Extensions.kt | 13 +++---- .../schema/execution/ArgumentTransformer.kt | 2 +- .../schema/introspection/SchemaProxy.kt | 32 +++------------- .../schema/structure/SchemaCompilation.kt | 38 +++++++++---------- .../kgraphql/schema/structure/Type.kt | 6 --- .../IntrospectionSpecificationTest.kt | 22 ++++++++++- 6 files changed, 50 insertions(+), 63 deletions(-) diff --git a/kgraphql/src/main/kotlin/com/apurebase/kgraphql/Extensions.kt b/kgraphql/src/main/kotlin/com/apurebase/kgraphql/Extensions.kt index d9a6ac08..0a960a3d 100644 --- a/kgraphql/src/main/kotlin/com/apurebase/kgraphql/Extensions.kt +++ b/kgraphql/src/main/kotlin/com/apurebase/kgraphql/Extensions.kt @@ -7,23 +7,20 @@ import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.coroutineScope import kotlinx.coroutines.launch import kotlin.reflect.KClass -import kotlin.reflect.KParameter import kotlin.reflect.KType import kotlin.reflect.full.isSubclassOf import kotlin.reflect.jvm.jvmErasure internal fun KClass.defaultKQLTypeName() = this.simpleName!! -internal fun KType.defaultKQLTypeName() = this.jvmErasure.defaultKQLTypeName() - -internal fun String.dropQuotes(): String = if (isLiteral()) drop(1).dropLast(1) else this +internal fun String.dropQuotes(): String = if (isLiteral()) { + drop(1).dropLast(1) +} else { + this +} internal fun String.isLiteral(): Boolean = startsWith('\"') && endsWith('\"') -internal fun KParameter.isNullable() = type.isMarkedNullable - -internal fun KParameter.isNotNullable() = !type.isMarkedNullable - internal fun KClass<*>.isIterable() = isSubclassOf(Iterable::class) internal fun KType.isIterable() = jvmErasure.isIterable() || toString().startsWith("kotlin.Array") diff --git a/kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/execution/ArgumentTransformer.kt b/kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/execution/ArgumentTransformer.kt index fde49f39..947181fb 100644 --- a/kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/execution/ArgumentTransformer.kt +++ b/kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/execution/ArgumentTransformer.kt @@ -167,7 +167,7 @@ open class ArgumentTransformer(val schema: DefaultSchema) { fun throwInvalidEnumValue(enumType: Type.Enum<*>) { throw InvalidInputValueException( - "Invalid enum ${schema.model.enums[kClass]?.name} value. Expected one of ${enumType.values.map { it.value }}", + "Invalid enum ${enumType.name} value. Expected one of ${enumType.values.map { it.value }}", value ) } diff --git a/kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/introspection/SchemaProxy.kt b/kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/introspection/SchemaProxy.kt index 1969d9a8..bff7ed23 100644 --- a/kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/introspection/SchemaProxy.kt +++ b/kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/introspection/SchemaProxy.kt @@ -1,17 +1,11 @@ package com.apurebase.kgraphql.schema.introspection -import com.apurebase.kgraphql.Context -import com.apurebase.kgraphql.configuration.SchemaConfiguration -import com.apurebase.kgraphql.schema.execution.ExecutionOptions -import com.apurebase.kgraphql.schema.structure.LookupSchema -import com.apurebase.kgraphql.schema.structure.Type -import kotlin.reflect.KClass - +/** + * Wrapper for [proxiedSchema] to resolve introspection types + */ class SchemaProxy( - override val configuration: SchemaConfiguration, - var proxiedSchema: LookupSchema? = null -) : LookupSchema { - + var proxiedSchema: __Schema? = null +) : __Schema { companion object { const val ILLEGAL_STATE_MESSAGE = "Missing proxied __Schema instance" } @@ -34,20 +28,4 @@ class SchemaProxy( get() = getProxied().directives override fun findTypeByName(name: String): __Type? = getProxied().findTypeByName(name) - - override fun typeByKClass(kClass: KClass<*>): Type? = getProxied().typeByKClass(kClass) - - override fun inputTypeByKClass(kClass: KClass<*>): Type? = inputTypeByKClass(kClass) - - override suspend fun execute( - request: String, - variables: String?, - context: Context, - options: ExecutionOptions, - operationName: String? - ): String { - return getProxied().execute(request, variables, context, options, operationName) - } - - override fun printSchema(): String = getProxied().printSchema() } diff --git a/kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/structure/SchemaCompilation.kt b/kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/structure/SchemaCompilation.kt index b269bf98..368e1c9e 100644 --- a/kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/structure/SchemaCompilation.kt +++ b/kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/structure/SchemaCompilation.kt @@ -10,6 +10,7 @@ import com.apurebase.kgraphql.isIterable import com.apurebase.kgraphql.request.isIntrospectionType import com.apurebase.kgraphql.schema.DefaultSchema import com.apurebase.kgraphql.schema.SchemaException +import com.apurebase.kgraphql.schema.builtin.BuiltInScalars import com.apurebase.kgraphql.schema.directive.Directive import com.apurebase.kgraphql.schema.execution.Execution import com.apurebase.kgraphql.schema.introspection.NotIntrospected @@ -40,7 +41,6 @@ class SchemaCompilation( val configuration: SchemaConfiguration, val definition: SchemaDefinition ) { - private val queryTypeProxies = mutableMapOf, TypeProxy>() private val inputTypeProxies = mutableMapOf, TypeProxy>() @@ -51,7 +51,7 @@ class SchemaCompilation( private val scalars = definition.scalars.associate { scalar -> scalar.kClass to scalar.toScalarType() } - private val schemaProxy = SchemaProxy(configuration) + private val schemaProxy = SchemaProxy() private val contextType = Type._Context() @@ -136,9 +136,7 @@ class SchemaCompilation( } private suspend fun handleQueries(): Type { - val __typenameField = handleOperation( - PropertyDef.Function("__typename", FunctionWrapper.on { -> "Query" }) - ) + val __typenameField = typenameField(FunctionWrapper.on { -> "Query" }) return Type.OperationObject( name = "Query", description = "Query object", @@ -147,9 +145,7 @@ class SchemaCompilation( } private suspend fun handleMutations(): Type { - val __typenameField = handleOperation( - PropertyDef.Function("__typename", FunctionWrapper.on { -> "Mutation" }) - ) + val __typenameField = typenameField(FunctionWrapper.on { -> "Mutation" }) return Type.OperationObject( "Mutation", "Mutation object", @@ -165,9 +161,8 @@ class SchemaCompilation( definition.subscriptions.map { handleOperation(it) }) } - @Suppress("USELESS_CAST") // We are casting as __Schema so we don't get proxied types. https://github.com/aPureBase/KGraphQL/issues/45 private suspend fun introspectionSchemaQuery() = handleOperation( - QueryDef("__schema", FunctionWrapper.on<__Schema> { schemaProxy as __Schema }) + QueryDef("__schema", FunctionWrapper.on<__Schema> { schemaProxy }) ) private suspend fun introspectionTypeQuery() = handleOperation( @@ -206,6 +201,7 @@ class SchemaCompilation( else -> handleSimpleType(kType, typeCategory) } } catch (e: Throwable) { + // The `KotlinReflectionInternalError` happens during `kType.jvmErasure` and cannot be caught directly if ("KotlinReflectionInternalError" in e.toString()) { throw SchemaException("If you construct a query/mutation generically, you must specify the return type T explicitly with resolver{ ... }.returns()") } else { @@ -320,13 +316,11 @@ class SchemaCompilation( .flatMap(TypeDef.Object<*>::unionProperties) .map { property -> handleUnionProperty(property) } - val typenameResolver: suspend (Any) -> String? = { value: Any -> - schemaProxy.typeByKClass(value.javaClass.kotlin)?.name ?: typeProxy.name + val typenameResolver: suspend (Any) -> String = { value: Any -> + queryTypeProxies[value.javaClass.kotlin]?.name ?: error("No query type proxy found for $value") } - val __typenameField = handleOperation( - PropertyDef.Function("__typename", FunctionWrapper.on(typenameResolver, true)) - ) + val __typenameField = typenameField(FunctionWrapper.on(typenameResolver, true)) val declaredFields = kotlinFields + extensionFields + unionFields + dataloadExtensionFields @@ -409,11 +403,9 @@ class SchemaCompilation( throw SchemaException("Invalid union type members") } - val __typenameField = handleOperation( - PropertyDef.Function("__typename", FunctionWrapper.on({ value: Any -> - schemaProxy.typeByKClass(value.javaClass.kotlin)?.name - }, true)) - ) + val __typenameField = typenameField(FunctionWrapper.on({ value: Any -> + queryTypeProxies[value.javaClass.kotlin]?.name ?: error("No query type proxy found for $value") + }, true)) val unionType = Type.Union(union, __typenameField, possibleTypes) unions.add(unionType) @@ -461,4 +453,10 @@ class SchemaCompilation( transformation = transformation as Transformation? ) } + + private fun typenameField(functionWrapper: FunctionWrapper) = Field.Function( + PropertyDef.Function("__typename", functionWrapper), + BuiltInScalars.STRING.typeDef.toScalarType(), + emptyList() + ) } diff --git a/kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/structure/Type.kt b/kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/structure/Type.kt index 8c211fc9..206ba877 100644 --- a/kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/structure/Type.kt +++ b/kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/structure/Type.kt @@ -15,10 +15,6 @@ import kotlin.reflect.full.createType interface Type : __Type { - fun hasField(name: String): Boolean { - return fields?.any { it.name == name } ?: false - } - operator fun get(name: String): Field? = null fun unwrapped(): Type = when (kind) { @@ -69,8 +65,6 @@ interface Type : __Type { override val fields: List<__Field>? = allFields.filterNot { it.name.startsWith("__") } - override fun hasField(name: String): Boolean = fieldsByName[name] != null - override fun get(name: String): Field? = fieldsByName[name] } diff --git a/kgraphql/src/test/kotlin/com/apurebase/kgraphql/specification/introspection/IntrospectionSpecificationTest.kt b/kgraphql/src/test/kotlin/com/apurebase/kgraphql/specification/introspection/IntrospectionSpecificationTest.kt index 564b3b09..45bc43a6 100644 --- a/kgraphql/src/test/kotlin/com/apurebase/kgraphql/specification/introspection/IntrospectionSpecificationTest.kt +++ b/kgraphql/src/test/kotlin/com/apurebase/kgraphql/specification/introspection/IntrospectionSpecificationTest.kt @@ -81,6 +81,26 @@ class IntrospectionSpecificationTest { } shouldThrow GraphQLError::class withMessage "Property __typename on String does not exist" } + enum class SampleEnum { + VALUE + } + data class EnumData(val enum: SampleEnum) + + @Test + fun `__typename field cannot be used on enums`() { + val schema = defaultSchema { + enum() + + query("sample") { + resolver { -> EnumData(SampleEnum.VALUE) } + } + } + + invoking { + schema.executeBlocking("{sample{enum{__typename}}}") + } shouldThrow GraphQLError::class withMessage "Property __typename on SampleEnum does not exist" + } + data class Union1(val one: String) data class Union2(val two: String) @@ -416,7 +436,7 @@ class IntrospectionSpecificationTest { @Test fun `all available SpecLevels of the introspection query should return without errors`() { - Introspection.SpecLevel.entries.forEach { + Introspection.SpecLevel.entries.forEach { _ -> val schema = defaultSchema { query("sample") { resolver { -> "Ronaldinho" }