Merge branch 'master' into 601-schema-max-tokens-limit

Author: oryan-block

pom.xml

@@ -127,7 +127,7 @@
         <dependency>
             <groupId>ch.qos.logback</groupId>
             <artifactId>logback-classic</artifactId>
-            <version>1.2.9</version>
+            <version>1.2.10</version>
             <scope>test</scope>
             <exclusions>
                 <exclusion>
@@ -180,7 +180,7 @@
             <plugin>
                 <groupId>org.codehaus.mojo</groupId>
                 <artifactId>build-helper-maven-plugin</artifactId>
-                <version>3.2.0</version>
+                <version>3.3.0</version>
                 <executions>
                     <execution>
                         <id>add-test-source</id>
@@ -250,7 +250,7 @@
             <plugin>
                 <groupId>org.apache.maven.plugins</groupId>
                 <artifactId>maven-jar-plugin</artifactId>
-                <version>3.2.0</version>
+                <version>3.2.1</version>
                 <executions>
                     <execution>
                         <id>test-jar</id>

src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt

@@ -10,7 +10,6 @@ import graphql.language.*
 import graphql.schema.*
 import graphql.schema.idl.RuntimeWiring
 import graphql.schema.idl.ScalarInfo
-import graphql.schema.idl.SchemaGeneratorHelperExt
 import graphql.schema.visibility.NoIntrospectionGraphqlFieldVisibility
 import org.slf4j.LoggerFactory
 import kotlin.reflect.KClass
@@ -58,7 +57,6 @@ class SchemaParser internal constructor(
 
     private val codeRegistryBuilder = GraphQLCodeRegistry.newCodeRegistry()
 
-    private val schemaGeneratorHelper = SchemaGeneratorHelperExt()
     private val schemaGeneratorDirectiveHelper = SchemaGeneratorDirectiveHelper()
     private val schemaDirectiveParameters = SchemaGeneratorDirectiveHelper.Parameters(null, runtimeWiring, null, codeRegistryBuilder)
 
@@ -307,18 +305,21 @@ class SchemaParser internal constructor(
                 names.add(directive.name)
                 val graphQLDirective = GraphQLDirective.newDirective()
                     .name(directive.name)
+                    .description(getDocumentation(directive, options))
+                    .comparatorRegistry(runtimeWiring.comparatorRegistry)
+                    .validLocation(directiveLocation)
                     .apply {
                         directive.arguments.forEach { arg ->
                             argument(GraphQLArgument.newArgument()
                                 .name(arg.name)
                                 .type(buildDirectiveInputType(arg.value))
+                                .valueLiteral(arg.value)
                                 .build())
                         }
                     }
                     .build()
 
-
-                output.add(schemaGeneratorHelper.buildAppliedDirective(directive, graphQLDirective, directiveLocation, runtimeWiring.comparatorRegistry))
+                output.add(graphQLDirective)
             }
         }
 
@@ -392,12 +393,12 @@ class SchemaParser internal constructor(
         }
 
     private fun determineInputType(typeDefinition: Type<*>, inputObjects: List<GraphQLInputObjectType>, referencingInputObjects: Set<String>) =
-        determineInputType(GraphQLInputType::class, typeDefinition, permittedTypesForInputObject, inputObjects, referencingInputObjects) as GraphQLInputType
+        determineInputType(GraphQLInputType::class, typeDefinition, permittedTypesForInputObject, inputObjects, referencingInputObjects)
 
     private fun <T : Any> determineInputType(expectedType: KClass<T>,
                                              typeDefinition: Type<*>, allowedTypeReferences: Set<String>,
                                              inputObjects: List<GraphQLInputObjectType>,
-                                             referencingInputObjects: Set<String>): GraphQLType =
+                                             referencingInputObjects: Set<String>): GraphQLInputType =
         when (typeDefinition) {
             is ListType -> GraphQLList(determineType(expectedType, typeDefinition.type, allowedTypeReferences, inputObjects))
             is NonNullType -> GraphQLNonNull(determineType(expectedType, typeDefinition.type, allowedTypeReferences, inputObjects))

src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt

@@ -12,6 +12,7 @@ import graphql.schema.DataFetchingEnvironment
 import org.apache.commons.lang3.ClassUtils
 import org.apache.commons.lang3.reflect.FieldUtils
 import org.slf4j.LoggerFactory
+import java.lang.reflect.AccessibleObject
 import java.lang.reflect.Method
 import java.lang.reflect.Modifier
 import java.lang.reflect.Type
@@ -44,13 +45,13 @@ internal class FieldResolverScanner(val options: SchemaParserOptions) {
     private fun findFieldResolver(field: FieldDefinition, search: Search, scanProperties: Boolean): FieldResolver? {
         val method = findResolverMethod(field, search)
         if (method != null) {
-            return MethodFieldResolver(field, search, options, method.apply { isAccessible = true })
+            return MethodFieldResolver(field, search, options, method.apply(trySetAccessible(field, search.type)))
         }
 
         if (scanProperties) {
             val property = findResolverProperty(field, search)
             if (property != null) {
-                return PropertyFieldResolver(field, search, options, property.apply { isAccessible = true })
+                return PropertyFieldResolver(field, search, options, property.apply(trySetAccessible(field, search.type)))
             }
         }
 
@@ -61,6 +62,15 @@ internal class FieldResolverScanner(val options: SchemaParserOptions) {
         return null
     }
 
+    private fun trySetAccessible(field: FieldDefinition, type: JavaType): AccessibleObject.() -> Unit = {
+        try {
+            isAccessible = true
+        } catch (e: RuntimeException) {
+            log.warn("Unable to make field ${type.unwrap().name}#${field.name} accessible. " +
+                "Be sure to provide a resolver or open the enclosing module if possible.")
+        }
+    }
+
     private fun missingFieldResolver(field: FieldDefinition, searches: List<Search>, scanProperties: Boolean): FieldResolver {
         return if (options.allowUnimplementedResolvers || options.missingResolverDataFetcher != null) {
             if (options.allowUnimplementedResolvers) {

src/main/kotlin/graphql/kickstart/tools/util/Utils.kt

@@ -50,9 +50,9 @@ internal val Class<*>.declaredNonProxyMethods: List<JavaMethod>
         }
     }
 
-internal fun getDocumentation(node: AbstractDescribedNode<*>, options: SchemaParserOptions): String? =
+internal fun getDocumentation(node: AbstractNode<*>, options: SchemaParserOptions): String? =
     when {
-        node.description != null -> node.description.content
+        node is AbstractDescribedNode<*> && node.description != null -> node.description.content
         !options.useCommentsForDescriptions -> null
         node.comments.isNullOrEmpty() -> null
         else -> node.comments.asSequence()

src/main/kotlin/graphql/schema/idl/SchemaGeneratorHelperExt.kt

@@ -0,0 +1,98 @@
+package graphql.kickstart.tools
+
+import graphql.ExceptionWhileDataFetching
+import graphql.GraphQL
+import graphql.execution.AsyncExecutionStrategy
+import graphql.schema.GraphQLSchema
+import org.junit.Ignore
+import org.junit.Test
+import java.util.*
+
+/**
+ * Reflective access to private fields in closed modules is not possible since Java 17.
+ * When using objects from closed modules in the schema the field resolver scanner will try to access their fields but fail.
+ * If no other resolver is provided that will result in an [IllegalAccessException]
+ */
+class InaccessibleFieldResolverTest {
+
+    @Test
+    @Ignore // TODO enable test after upgrading to 17
+    fun `private field from closed module is not accessible`() {
+        val schema: GraphQLSchema = SchemaParser.newParser()
+            .schemaString(
+                """
+                type Query {
+                    locale: Locale
+                }
+                
+                type Locale {
+                  country: String!
+                  languageTag: String!
+                }
+                """)
+            .resolvers(Query())
+            .build()
+            .makeExecutableSchema()
+        val gql: GraphQL = GraphQL.newGraphQL(schema)
+            .queryExecutionStrategy(AsyncExecutionStrategy())
+            .build()
+
+        val result = gql.execute(
+            """
+            query {
+                locale {
+                    country
+                    languageTag
+                }
+            }
+            """
+        )
+
+        assertEquals(result.errors.size, 1)
+        val exceptionWhileDataFetching = result.errors[0] as ExceptionWhileDataFetching
+        assert(exceptionWhileDataFetching.exception is IllegalAccessException)
+    }
+
+    @Test
+    fun `private field from closed module is accessible through resolver`() {
+        val schema: GraphQLSchema = SchemaParser.newParser()
+            .schemaString(
+                """
+                type Query {
+                    locale: Locale
+                }
+                
+                type Locale {
+                  country: String!
+                  languageTag: String!
+                }
+                """)
+            .resolvers(Query(), LocaleResolver())
+            .build()
+            .makeExecutableSchema()
+        val gql: GraphQL = GraphQL.newGraphQL(schema)
+            .queryExecutionStrategy(AsyncExecutionStrategy())
+            .build()
+
+        val data = assertNoGraphQlErrors(gql) {
+            """
+            query {
+                locale {
+                    country
+                    languageTag
+                }
+            }
+            """
+        }
+
+        assertEquals(data["locale"], mapOf("country" to "US", "languageTag" to "en-US"))
+    }
+
+    class Query : GraphQLQueryResolver {
+        fun locale(): Locale = Locale.US
+    }
+
+    class LocaleResolver : GraphQLResolver<Locale> {
+        fun languageTag(locale: Locale): String = locale.toLanguageTag()
+    }
+}