refactor: Cleanup SchemaProxy (#132)

`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.
stuebingerb · Dec 17, 2024 · 7e6fc39 · 7e6fc39
2 parents 9089567 + 3b4af49
commit 7e6fc39
Show file tree

Hide file tree

Showing 6 changed files with 50 additions and 63 deletions.
diff --git 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 <T : Any> KClass<T>.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
@@ -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
@@ -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
@@ -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<KClass<*>, TypeProxy>()
 
     private val inputTypeProxies = mutableMapOf<KClass<*>, 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<Nothing, String?>("__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<Nothing, String?>("__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<T>()")
         } 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<Nothing, String?>("__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<Nothing, String?>("__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<T, R>?
         )
     }
+
+    private fun typenameField(functionWrapper: FunctionWrapper<String>) = Field.Function(
+        PropertyDef.Function<Nothing, String>("__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
@@ -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/...tlin/com/apurebase/kgraphql/specification/introspection/IntrospectionSpecificationTest.kt b/...tlin/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<SampleEnum>()
+
+            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" }