fix: abstract classes as input are invalid (#223)

* fix: abstract classes as input are invalid

* update unit tests for detekt

Author: Shane Myrick

src/main/kotlin/com/expedia/graphql/generator/TypeBuilder.kt

@@ -51,7 +51,7 @@ internal open class TypeBuilder constructor(protected val generator: SchemaGener
         kClass.isEnum() -> @Suppress("UNCHECKED_CAST") (generator.enumType(kClass as KClass<Enum<*>>))
         kClass.isListType() -> generator.listType(type, inputType)
         kClass.isUnion() -> generator.unionType(kClass)
-        kClass.isInterface() || kClass.isAbstract -> generator.interfaceType(kClass)
+        kClass.isInterface() -> generator.interfaceType(kClass)
         inputType -> generator.inputObjectType(kClass)
         else -> generator.objectType(kClass)
     }

src/main/kotlin/com/expedia/graphql/generator/extensions/kClassExtensions.kt

@@ -39,7 +39,7 @@ internal fun KClass<*>.getValidSuperclasses(hooks: SchemaGeneratorHooks): List<K
 internal fun KClass<*>.findConstructorParamter(name: String): KParameter? =
     this.primaryConstructor?.findParameterByName(name)
 
-internal fun KClass<*>.isInterface(): Boolean = this.java.isInterface
+internal fun KClass<*>.isInterface(): Boolean = this.java.isInterface || this.isAbstract
 
 internal fun KClass<*>.isUnion(): Boolean =
     this.isInterface() && this.declaredMemberProperties.isEmpty() && this.declaredMemberFunctions.isEmpty()
@@ -67,3 +67,5 @@ internal fun KClass<*>.getSimpleName(isInputClass: Boolean = false): String {
 internal fun KClass<*>.getQualifiedName(): String = this.qualifiedName.orEmpty()
 
 internal fun KClass<*>.isPublic(): Boolean = this.visibility == KVisibility.PUBLIC
+
+internal fun KClass<*>.isNotPublic(): Boolean = this.isPublic().not()

src/main/kotlin/com/expedia/graphql/generator/extensions/kPropertyExtensions.kt

@@ -5,22 +5,20 @@ import kotlin.reflect.KProperty
 
 internal fun KProperty<*>.isPropertyGraphQLID(parentClass: KClass<*>): Boolean = when {
     this.isGraphQLID() -> true
-    parentClass.findConstructorParamter(this.name)
-        ?.isGraphQLID()
-        .isTrue() -> true
+    getConstructorParameter(parentClass)?.isGraphQLID().isTrue() -> true
     else -> false
 }
 
 internal fun KProperty<*>.isPropertyGraphQLIgnored(parentClass: KClass<*>): Boolean = when {
     this.isGraphQLIgnored() -> true
-    parentClass.findConstructorParamter(this.name)
-        ?.isGraphQLIgnored()
-        .isTrue() -> true
+    getConstructorParameter(parentClass)?.isGraphQLIgnored().isTrue() -> true
     else -> false
 }
 
 internal fun KProperty<*>.getPropertyDeprecationReason(parentClass: KClass<*>): String? =
-    this.getDeprecationReason() ?: parentClass.findConstructorParamter(this.name)?.getDeprecationReason()
+    this.getDeprecationReason() ?: getConstructorParameter(parentClass)?.getDeprecationReason()
 
 internal fun KProperty<*>.getPropertyDescription(parentClass: KClass<*>): String? =
-    this.getGraphQLDescription() ?: parentClass.findConstructorParamter(this.name)?.getGraphQLDescription()
+    this.getGraphQLDescription() ?: getConstructorParameter(parentClass)?.getGraphQLDescription()
+
+private fun KProperty<*>.getConstructorParameter(parentClass: KClass<*>) = parentClass.findConstructorParamter(this.name)

src/main/kotlin/com/expedia/graphql/generator/filters/superclassFilters.kt

@@ -8,7 +8,7 @@ import kotlin.reflect.KClass
 private typealias SuperclassFilter = (KClass<*>) -> Boolean
 
 private val isPublic: SuperclassFilter = { it.isPublic() }
-private val isInterface: SuperclassFilter = { it.isInterface() || it.isAbstract }
+private val isInterface: SuperclassFilter = { it.isInterface() }
 private val isNotUnion: SuperclassFilter = { it.isUnion().not() }
 
 internal val superclassFilters: List<SuperclassFilter> = listOf(isPublic, isInterface, isNotUnion)

src/main/kotlin/com/expedia/graphql/generator/types/MutationBuilder.kt

@@ -5,7 +5,7 @@ import com.expedia.graphql.exceptions.InvalidMutationTypeException
 import com.expedia.graphql.generator.SchemaGenerator
 import com.expedia.graphql.generator.TypeBuilder
 import com.expedia.graphql.generator.extensions.getValidFunctions
-import com.expedia.graphql.generator.extensions.isPublic
+import com.expedia.graphql.generator.extensions.isNotPublic
 import graphql.schema.GraphQLObjectType
 
 internal class MutationBuilder(generator: SchemaGenerator) : TypeBuilder(generator) {
@@ -20,7 +20,7 @@ internal class MutationBuilder(generator: SchemaGenerator) : TypeBuilder(generat
         mutationBuilder.name(config.topLevelNames.mutation)
 
         for (mutation in mutations) {
-            if (mutation.kClass.isPublic().not()) {
+            if (mutation.kClass.isNotPublic()) {
                 throw InvalidMutationTypeException(mutation.kClass)
             }
 

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

@@ -6,7 +6,7 @@ import com.expedia.graphql.exceptions.InvalidSchemaException
 import com.expedia.graphql.generator.SchemaGenerator
 import com.expedia.graphql.generator.TypeBuilder
 import com.expedia.graphql.generator.extensions.getValidFunctions
-import com.expedia.graphql.generator.extensions.isPublic
+import com.expedia.graphql.generator.extensions.isNotPublic
 import graphql.schema.GraphQLObjectType
 
 internal class QueryBuilder(generator: SchemaGenerator) : TypeBuilder(generator) {
@@ -22,7 +22,7 @@ internal class QueryBuilder(generator: SchemaGenerator) : TypeBuilder(generator)
         queryBuilder.name(config.topLevelNames.query)
 
         for (query in queries) {
-            if (query.kClass.isPublic().not()) {
+            if (query.kClass.isNotPublic()) {
                 throw InvalidQueryTypeException(query.kClass)
             }
 

src/main/kotlin/com/expedia/graphql/generator/types/SubscriptionBuilder.kt

@@ -5,7 +5,7 @@ import com.expedia.graphql.exceptions.InvalidSubscriptionTypeException
 import com.expedia.graphql.generator.SchemaGenerator
 import com.expedia.graphql.generator.TypeBuilder
 import com.expedia.graphql.generator.extensions.getValidFunctions
-import com.expedia.graphql.generator.extensions.isPublic
+import com.expedia.graphql.generator.extensions.isNotPublic
 import graphql.schema.GraphQLObjectType
 
 internal class SubscriptionBuilder(generator: SchemaGenerator) : TypeBuilder(generator) {
@@ -20,7 +20,7 @@ internal class SubscriptionBuilder(generator: SchemaGenerator) : TypeBuilder(gen
         subscriptionBuilder.name(config.topLevelNames.subscription)
 
         for (subscription in subscriptions) {
-            if (subscription.kClass.isPublic().not()) {
+            if (subscription.kClass.isNotPublic()) {
                 throw InvalidSubscriptionTypeException(subscription.kClass)
             }
 

src/test/kotlin/com/expedia/graphql/generator/extensions/KClassExtensionsTest.kt

@@ -198,6 +198,7 @@ open class KClassExtensionsTest {
     @Test
     fun `test graphql interface extension`() {
         assertTrue(TestInterface::class.isInterface())
+        assertTrue(SomeAbstractClass::class.isInterface())
         assertFalse(MyTestClass::class.isInterface())
     }
 
@@ -246,4 +247,12 @@ open class KClassExtensionsTest {
         assertFalse(MyProtectedClass::class.isPublic())
         assertFalse(MyTestClass::class.isPublic())
     }
+
+    @Test
+    fun isNotPublic() {
+        assertFalse(MyPublicClass::class.isNotPublic())
+        assertTrue(MyInternalClass::class.isNotPublic())
+        assertTrue(MyProtectedClass::class.isNotPublic())
+        assertTrue(MyTestClass::class.isNotPublic())
+    }
 }

src/test/kotlin/com/expedia/graphql/generator/extensions/KParameterExtensionsKtTest.kt

@@ -23,10 +23,19 @@ internal class KParameterExtensionsKtTest {
         val value: String
     }
 
+    internal abstract class MyAbstractClass {
+
+        abstract val implementMe: String
+
+        val value: String = "test"
+    }
+
     internal class Container {
 
         internal fun interfaceInput(myInterface: MyInterface) = myInterface
 
+        internal fun absctractInput(myAbstractClass: MyAbstractClass) = myAbstractClass
+
         internal fun noDescription(myClass: MyClass) = myClass
 
         internal fun paramDescription(@GraphQLDescription("param description") myClass: MyClass) = myClass
@@ -77,6 +86,12 @@ internal class KParameterExtensionsKtTest {
         assertTrue(param?.isInterface().isTrue())
     }
 
+    @Test
+    fun `abstract class input is invalid`() {
+        val param = Container::absctractInput.findParameterByName("myAbstractClass")
+        assertTrue(param?.isInterface().isTrue())
+    }
+
     @Test
     fun `valid DataFetchingEnvironment passes`() {
         val param = Container::dataFetchingEnvironment.findParameterByName("environment")