[generator] filter lambda properties (ExpediaGroup#1366)

Currently we don't support lambda parameters. Attempting to use a lambda property blows up with somewhat ambiguous messages (lambda return type is outside of supported packages/cannot calculate erasure type for suspendable lambda). Currently it was also not possible to filter out lambda properties using `@GraphQLIgnore` annotations as it was applied AFTER attempting to read lambda erasure type (which blows up as it cannot be calculated).

This PR adds new property filter that checks whether property is a lambda and throws `InvalidPropertyReturnTypeException` if it is. Also moved `@GraphQLIgnore` filter to be applied before we attempt to calculate JVM erasure (used to verify property type is not blacklisted).

Throwing exception from a filter might not be ideal but IMHO is a better alternative than throwing current ambiguous exception (about JVM erasure) or by just filtering lambda parameters (which leads to questions of why my public property is not available in the graph).

Author: Dariusz Kuc

generator/graphql-kotlin-schema-generator/src/main/kotlin/com/expediagroup/graphql/generator/exceptions/InvalidPropertyReturnTypeException.kt

@@ -0,0 +1,26 @@
+/*
+ * Copyright 2022 Expedia, Inc
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.expediagroup.graphql.generator.exceptions
+
+import kotlin.reflect.KClass
+import kotlin.reflect.KProperty
+
+/**
+ * Thrown when the schema defines an object property which is a lambda.
+ */
+class InvalidPropertyReturnTypeException(kClass: KClass<*>, property: KProperty<*>) :
+    GraphQLKotlinException("The class $kClass defines $property as a lambda which is currently unsupported")

generator/graphql-kotlin-schema-generator/src/main/kotlin/com/expediagroup/graphql/generator/internal/filters/propertyFilters.kt

@@ -1,5 +1,5 @@
 /*
- * Copyright 2021 Expedia, Inc
+ * Copyright 2022 Expedia, Inc
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -16,15 +16,18 @@
 
 package com.expediagroup.graphql.generator.internal.filters
 
+import com.expediagroup.graphql.generator.exceptions.InvalidPropertyReturnTypeException
 import com.expediagroup.graphql.generator.internal.extensions.isGraphQLIgnored
 import com.expediagroup.graphql.generator.internal.extensions.isPropertyGraphQLIgnored
 import com.expediagroup.graphql.generator.internal.extensions.isPublic
 import com.expediagroup.graphql.generator.internal.extensions.qualifiedName
 import com.expediagroup.graphql.generator.internal.types.utils.validGraphQLNameRegex
 import kotlin.reflect.KClass
 import kotlin.reflect.KProperty
+import kotlin.reflect.full.isSubtypeOf
 import kotlin.reflect.full.memberProperties
 import kotlin.reflect.jvm.jvmErasure
+import kotlin.reflect.typeOf
 
 private typealias PropertyFilter = (KProperty<*>, KClass<*>) -> Boolean
 
@@ -34,13 +37,21 @@ private val isPropertyPublic: PropertyFilter = { prop, _ -> prop.isPublic() }
 private val isPropertyNotGraphQLIgnored: PropertyFilter = { prop, parentClass -> prop.isPropertyGraphQLIgnored(parentClass).not() }
 private val isNotBlacklistedType: PropertyFilter = { prop, _ -> blacklistTypes.contains(prop.returnType.qualifiedName).not() }
 private val isValidPropertyName: PropertyFilter = { prop, _ -> prop.name.matches(validGraphQLNameRegex) }
+@OptIn(ExperimentalStdlibApi::class)
+private val isNotLambda: PropertyFilter = { prop, parentClass ->
+    if (prop.returnType.isSubtypeOf(typeOf<Function<*>>())) {
+        throw InvalidPropertyReturnTypeException(parentClass, prop)
+    } else {
+        true
+    }
+}
 
 private val isNotIgnoredFromSuperClass: PropertyFilter = { prop, parentClass ->
     val superPropsIgnored = parentClass.supertypes
         .flatMap { superType ->
             superType.jvmErasure.memberProperties
-                .filter { superProp -> basicPropertyFilters.all { it.invoke(superProp, superType::class) } }
                 .filter { it.isGraphQLIgnored() }
+                .filter { superProp -> basicPropertyFilters.all { it.invoke(superProp, superType::class) } }
         }
 
     superPropsIgnored.none { superProp ->
@@ -49,5 +60,5 @@ private val isNotIgnoredFromSuperClass: PropertyFilter = { prop, parentClass ->
     }
 }
 
-private val basicPropertyFilters = listOf(isPropertyPublic, isNotBlacklistedType)
-internal val propertyFilters: List<PropertyFilter> = basicPropertyFilters + isPropertyNotGraphQLIgnored + isNotIgnoredFromSuperClass + isValidPropertyName
+private val basicPropertyFilters = listOf(isPropertyPublic, isNotLambda, isNotBlacklistedType)
+internal val propertyFilters: List<PropertyFilter> = listOf(isPropertyNotGraphQLIgnored) + basicPropertyFilters + isNotIgnoredFromSuperClass + isValidPropertyName

generator/graphql-kotlin-schema-generator/src/test/kotlin/com/expediagroup/graphql/generator/internal/filters/PropertyFiltersTest.kt

@@ -1,5 +1,5 @@
 /*
- * Copyright 2019 Expedia, Inc
+ * Copyright 2022 Expedia, Inc
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -17,7 +17,9 @@
 package com.expediagroup.graphql.generator.internal.filters
 
 import com.expediagroup.graphql.generator.annotations.GraphQLIgnore
+import com.expediagroup.graphql.generator.exceptions.InvalidPropertyReturnTypeException
 import org.junit.jupiter.api.Test
+import org.junit.jupiter.api.assertThrows
 import kotlin.reflect.KClass
 import kotlin.reflect.KProperty
 import kotlin.test.assertFalse
@@ -35,6 +37,13 @@ internal class PropertyFiltersTest {
         assertFalse(isValidProperty(MyDataClass::ignoredProperty, MyDataClass::class))
         assertFalse(isValidProperty(MyClass::foo, MyClass::class))
         assertFalse(isValidProperty(MyClass::`$invalidPropertyName`, MyClass::class))
+
+        assertThrows<InvalidPropertyReturnTypeException> {
+            isValidProperty(MyClass::lambda, MyClass::class)
+        }
+        assertThrows<InvalidPropertyReturnTypeException> {
+            isValidProperty(MyClass::suspendableLambda, MyClass::class)
+        }
     }
 
     internal data class MyDataClass(
@@ -49,7 +58,10 @@ internal class PropertyFiltersTest {
         val foo: Int
     }
 
-    internal class MyClass : MyInterface {
+    internal class MyClass(
+        val lambda: () -> String,
+        val suspendableLambda: suspend () -> String
+    ) : MyInterface {
 
         val publicProperty: Int = 0
 

generator/graphql-kotlin-schema-generator/src/test/kotlin/com/expediagroup/graphql/generator/internal/types/GenerateObjectTest.kt

@@ -1,5 +1,5 @@
 /*
- * Copyright 2021 Expedia, Inc
+ * Copyright 2022 Expedia, Inc
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -18,16 +18,19 @@ package com.expediagroup.graphql.generator.internal.types
 
 import com.expediagroup.graphql.generator.annotations.GraphQLDescription
 import com.expediagroup.graphql.generator.annotations.GraphQLDirective
+import com.expediagroup.graphql.generator.annotations.GraphQLIgnore
 import com.expediagroup.graphql.generator.annotations.GraphQLName
 import com.expediagroup.graphql.generator.annotations.GraphQLValidObjectLocations
 import com.expediagroup.graphql.generator.exceptions.InvalidGraphQLNameException
 import com.expediagroup.graphql.generator.exceptions.InvalidObjectLocationException
+import com.expediagroup.graphql.generator.exceptions.InvalidPropertyReturnTypeException
 import graphql.Scalars
 import graphql.introspection.Introspection
 import graphql.schema.GraphQLNonNull
 import graphql.schema.GraphQLObjectType
 import org.junit.jupiter.api.Test
 import org.junit.jupiter.api.assertDoesNotThrow
+import org.junit.jupiter.api.assertThrows
 import kotlin.test.assertEquals
 import kotlin.test.assertFailsWith
 import kotlin.test.assertNotNull
@@ -66,6 +69,20 @@ class GenerateObjectTest : TypeTestHelper() {
     @GraphQLName("Invalid\$Name")
     class InvalidOutputTypeNameOverride
 
+    data class FooLambda(
+        val foo: () -> String
+    )
+
+    data class SuspendableFooLambda(
+        val foo: suspend () -> String?
+    )
+
+    data class Foo(
+        val foo: String,
+        @GraphQLIgnore val fooLambda: () -> String?,
+        private val fooSuspendable: suspend () -> String?
+    )
+
     @Test
     fun `Test naming`() {
         val result = generateObject(generator, BeHappy::class) as? GraphQLObjectType
@@ -139,4 +156,27 @@ class GenerateObjectTest : TypeTestHelper() {
             generateInputObject(generator, InvalidOutputTypeNameOverride::class)
         }
     }
+
+    @Test
+    fun `Generation of output object will fail if one of the properties is lambda`() {
+        assertThrows<InvalidPropertyReturnTypeException> {
+            generateObject(generator, FooLambda::class)
+        }
+    }
+
+    @Test
+    fun `Generation of output object will fail if one of the properties is suspendable lambda`() {
+        assertThrows<InvalidPropertyReturnTypeException> {
+            generateObject(generator, SuspendableFooLambda::class)
+        }
+    }
+
+    @Test
+    fun `Generation of output objects ignores private or @GraphQLIgnored lambdas `() {
+        val result = generateObject(generator, Foo::class)
+        assertNotNull(result)
+        assertEquals("Foo", result.name)
+        assertEquals(1, result.fieldDefinitions.size)
+        assertEquals("foo", result.fieldDefinitions[0].name)
+    }
 }