Fail the build on compiler warnings and check the release bundle in CI

.github/workflows/pull-request.yml

@@ -24,3 +24,81 @@ jobs:
           restore-keys: ${{ runner.os }}-m2
       - name: Build with Maven
         run: mvn --batch-mode --update-snapshots verify
+
+  # The same jar has to work with the oldest supported graphql-java, so build and test against it too
+  graphql-java-25:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v7
+      - name: Setup java
+        uses: actions/setup-java@v6
+        with:
+          java-version: '11'
+          distribution: 'temurin'
+      - name: Cache Maven packages
+        uses: actions/cache@v6
+        with:
+          path: ~/.m2
+          key: ${{ runner.os }}-m2-${{ hashFiles('**/pom.xml') }}
+          restore-keys: ${{ runner.os }}-m2
+      - name: Build with Maven
+        run: mvn --batch-mode --update-snapshots verify -Dgraphql-java.version=25.1
+
+  # Builds the Maven Central bundle like the release workflow does, without uploading it
+  release-bundle:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v7
+      - name: Setup java
+        uses: actions/setup-java@v6
+        with:
+          java-version: '11'
+          distribution: 'temurin'
+          server-id: central
+          server-username: MAVEN_USERNAME
+          server-password: MAVEN_PASSWORD
+      - name: Setup Maven 3.9
+        # Same Maven as the release workflow
+        run: |
+          curl -sSfL https://repo1.maven.org/maven2/org/apache/maven/apache-maven/3.9.16/apache-maven-3.9.16-bin.tar.gz | tar xz -C "$RUNNER_TEMP"
+          echo "$RUNNER_TEMP/apache-maven-3.9.16/bin" >> "$GITHUB_PATH"
+      - name: Cache Maven packages
+        uses: actions/cache@v6
+        with:
+          path: ~/.m2
+          key: ${{ runner.os }}-m2-${{ hashFiles('**/pom.xml') }}
+          restore-keys: ${{ runner.os }}-m2
+      - name: Build release bundle
+        # Snapshots aren't bundled, so drop -SNAPSHOT like github-build.sh does.
+        # The upload URL is unreachable, so the build fails at the upload, after the bundle is written.
+        run: |
+          sed -i 's/-SNAPSHOT//' pom.xml
+          mvn --batch-mode -Prelease deploy -DskipTests -Dgpg.skip -DcentralBaseUrl=http://127.0.0.1:9 || true
+        env:
+          MAVEN_USERNAME: dummy
+          MAVEN_PASSWORD: dummy
+      - name: Check release bundle
+        # Central rejects bundles with files that don't belong to a pom, e.g. maven-metadata-local.xml under Maven 3.10
+        run: |
+          bundle=target/central-publishing/central-bundle.zip
+          if [[ ! -f $bundle ]]; then
+            echo "::error::No release bundle was built, see the previous step"
+            exit 1
+          fi
+          version=$(grep -m1 -o "<version>.*</version>$" pom.xml | awk -F'[><]' '{print $3}')
+          dir="io/github/graphql-java-kickstart/graphql-java-tools/${version}"
+          files=$(unzip -Z1 "$bundle")
+          echo "$files"
+          unexpected=$(grep -vE "^${dir}/graphql-java-tools-${version}(-[a-z]+)?\.(pom|jar)(\.(md5|sha1|sha256|sha512|asc))?$" <<< "$files" || true)
+          if [[ -n $unexpected ]]; then
+            echo "::error::Unexpected files in the release bundle: ${unexpected//$'\n'/, }"
+            exit 1
+          fi
+          for file in graphql-java-tools-${version}.pom graphql-java-tools-${version}.jar graphql-java-tools-${version}-sources.jar graphql-java-tools-${version}-javadoc.jar; do
+            if ! grep -qxF "${dir}/${file}" <<< "$files"; then
+              echo "::error::${file} is missing from the release bundle"
+              exit 1
+            fi
+          done

pom.xml

@@ -262,6 +262,11 @@
                 <artifactId>kotlin-maven-plugin</artifactId>
                 <groupId>org.jetbrains.kotlin</groupId>
                 <version>${kotlin.version}</version>
+                <configuration>
+                    <args>
+                        <arg>-Werror</arg>
+                    </args>
+                </configuration>
                 <executions>
                     <execution>
                         <id>compile</id>
@@ -286,8 +291,8 @@
                 <artifactId>maven-compiler-plugin</artifactId>
                 <version>3.16.0</version>
                 <configuration>
-                    <source>11</source>
-                    <target>11</target>
+                    <release>11</release>
+                    <failOnWarning>true</failOnWarning>
                 </configuration>
             </plugin>
 

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

@@ -198,8 +198,8 @@ internal class SchemaClassScanner(
                 dictionary
                     .filter {
                         it.value.javaType != null
-                            && it.value.typeClass() != java.lang.Object::class.java
-                            && !java.util.Map::class.java.isAssignableFrom(it.value.typeClass())
+                            && it.value.typeClass() != Any::class.java
+                            && !Map::class.java.isAssignableFrom(it.value.typeClass())
                             && it.key !is InputObjectTypeDefinition
                             && it.key !is UnionTypeDefinition
                     }.mapValuesTo(it) { it.value.javaType }
@@ -313,7 +313,7 @@ internal class SchemaClassScanner(
      * Scan a new object for types that haven't been mapped yet.
      */
     private fun scanQueueItemForPotentialMatches(item: QueueItem) {
-        val resolverInfo = if (item.clazz == Object::class.java) {
+        val resolverInfo = if (item.clazz == Any::class.java) {
             getResolverInfoFromTypeDictionary(item.type.name)
                 ?: throw SchemaClassScannerError("The GraphQL schema type '${item.type.name}' maps to a field of type java.lang.Object however there is no matching entry for this type in the type dictionary. You may need to add this type to the dictionary before building the schema.")
         } else {
@@ -349,7 +349,7 @@ internal class SchemaClassScanner(
     }
 
     private fun isResolverForSupertype(resolverInfo: NormalResolverInfo, dataClass: Class<*>) =
-        resolverInfo.dataClassType != Object::class.java && resolverInfo.dataClassType.isAssignableFrom(dataClass)
+        resolverInfo.dataClassType != Any::class.java && resolverInfo.dataClassType.isAssignableFrom(dataClass)
 
     private fun scanResolverInfoForPotentialMatches(type: ObjectTypeDefinition, resolverInfo: ResolverInfo) {
         type.getExtendedFieldDefinitions(extensionDefinitions).forEach { field ->
@@ -386,7 +386,7 @@ internal class SchemaClassScanner(
                 if (options.preferGraphQLResolver && realEntry.hasResolverRef()) {
                     log.warn("The real entry ${realEntry.joinReferences()} is a GraphQLResolver so ignoring this one ${javaType.unwrap()} $reference")
                 } else {
-                    if (java.util.Map::class.java.isAssignableFrom(javaType.unwrap())) {
+                    if (Map::class.java.isAssignableFrom(javaType.unwrap())) {
                         throw SchemaClassScannerError("Two different property map classes used for type ${type.name}:\n${realEntry.joinReferences()}\n\n- ${javaType}:\n|   ${reference.getDescription()}")
                     }
                     throw SchemaClassScannerError("Two different classes used for type ${type.name}:\n${realEntry.joinReferences()}\n\n- ${javaType.typeName}:\n|   ${reference.getDescription()}")

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

@@ -12,7 +12,7 @@ import graphql.language.*
 import graphql.schema.*
 import graphql.schema.idl.RuntimeWiring
 import graphql.schema.idl.ScalarInfo
-import graphql.schema.visibility.NoIntrospectionGraphqlFieldVisibility
+import graphql.schema.visibility.BlockedFields
 import org.slf4j.LoggerFactory
 import kotlin.reflect.KClass
 
@@ -68,7 +68,9 @@ class SchemaParser internal constructor(
      */
     fun parseSchemaObjects(): SchemaObjects {
         if (!options.introspectionEnabled) {
-            codeRegistryBuilder.fieldVisibility(NoIntrospectionGraphqlFieldVisibility.NO_INTROSPECTION_FIELD_VISIBILITY)
+            // What the deprecated NoIntrospectionGraphqlFieldVisibility does. Its suggested replacement,
+            // Introspection.enabledJvmWide, would also turn introspection off for every other schema in the JVM.
+            codeRegistryBuilder.fieldVisibility(BlockedFields.newBlock().addPattern("__.*").build())
         }
         // this overrides the above introspection enabled setting obviously... todo: add documentation
         options.fieldVisibility?.let { codeRegistryBuilder.fieldVisibility(it) }
@@ -427,6 +429,7 @@ class SchemaParser internal constructor(
     }
 
     // TODO remove this once directives are fully replaced with applied directives
+    @Suppress("DEPRECATION") // legacy directive argument values
     private fun buildDirectives(
         directives: List<Directive>,
         directiveLocation: Introspection.DirectiveLocation
@@ -529,7 +532,7 @@ class SchemaParser internal constructor(
             is NonNullType -> GraphQLNonNull(determineType(expectedType, typeDefinition.type, allowedTypeReferences, inputObjects))
             is InputObjectTypeDefinition -> {
                 log.info("Create input object")
-                createInputObject(typeDefinition, inputObjects, referencingInputObjects as MutableSet<String>)
+                createInputObject(typeDefinition, inputObjects, referencingInputObjects)
             }
             is TypeName -> {
                 val scalarType = customScalars[typeDefinition.name]

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

@@ -10,7 +10,6 @@ import graphql.schema.DataFetcher
 import graphql.schema.DataFetchingEnvironment
 import graphql.schema.visibility.GraphqlFieldVisibility
 import kotlinx.coroutines.Dispatchers
-import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.channels.ReceiveChannel
 import kotlinx.coroutines.reactive.publish
 import org.reactivestreams.Publisher
@@ -45,7 +44,6 @@ data class SchemaParserOptions internal constructor(
         fun newOptions() = Builder()
 
         @JvmStatic
-        @ExperimentalCoroutinesApi
         fun defaultOptions() = Builder().build()
     }
 
@@ -164,7 +162,6 @@ data class SchemaParserOptions internal constructor(
             this.allowUndeclaredDirectives = allowUndeclaredDirectives
         }
 
-        @ExperimentalCoroutinesApi
         fun build(): SchemaParserOptions {
             val coroutineContextProvider = coroutineContextProvider
                 ?: DefaultCoroutineContextProvider(Dispatchers.Default)

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

@@ -68,7 +68,7 @@ internal class TypeClassMatcher(private val definitionsByName: Map<String, TypeD
                 realType is Class<*> && realType.isArray ->
                     match(potentialMatch, graphQLType.type, realType.componentType)
                 // e.g. a property map value: the element type is unknown, so match it as Object too
-                realType == Object::class.java ->
+                realType == Any::class.java ->
                     match(potentialMatch, graphQLType.type, realType)
                 else ->
                     throw error(potentialMatch, "Java class is not a List or generic type information was lost: $realType")

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

@@ -56,7 +56,7 @@ internal class FieldResolverScanner(val options: SchemaParserOptions) {
             }
         }
 
-        if (java.util.Map::class.java.isAssignableFrom(search.type.unwrap())) {
+        if (Map::class.java.isAssignableFrom(search.type.unwrap())) {
             return MapFieldResolver(field, search, options, search.type.unwrap())
         }
 
@@ -128,7 +128,7 @@ internal class FieldResolverScanner(val options: SchemaParserOptions) {
             .filter { !Modifier.isPrivate(it.modifiers) }
             // discard any methods that are coming off the root of the class hierarchy
             // to avoid issues with duplicate method declarations
-            .filter { it.declaringClass != Object::class.java }
+            .filter { it.declaringClass != Any::class.java }
             // subscription resolvers must return a publisher
             .filter { !search.isSubscription || resolverMethodReturnsPublisher(it) }
             .toList()

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

@@ -36,7 +36,7 @@ internal class MapFieldResolver(
         val resolvedType = TypeResolver().resolve(mapClass)
         val typeParameters = resolvedType.typeParametersFor(Map::class.java)
 
-        return typeParameters.elementAtOrElse(1) { Object::class.java }
+        return typeParameters.elementAtOrElse(1) { Any::class.java }
     }
 
     override fun createDataFetcher(): DataFetcher<*> {

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

@@ -152,6 +152,8 @@ internal class MethodFieldResolver(
      * because values can be coerced to multiple different Java types (eg. String, Long, UUID). All values of a non-concrete scalar
      * type must be converted to the target method parameter type.
      */
+    // graphql-java marks the schema non-null, but it's null in hand-built environments, e.g. in tests
+    @Suppress("UNNECESSARY_SAFE_CALL")
     private fun isConcreteScalarType(environment: DataFetchingEnvironment, type: Type<*>, genericParameterType: JavaType): Boolean {
         return when (type) {
             is ListType -> List::class.java.isAssignableFrom(this.typeResolver.getRawClass(genericParameterType))

src/main/kotlin/module-info.java

@@ -5,6 +5,8 @@
     requires kotlinx.coroutines.reactive;
 
     requires com.graphqljava;
+    // graphql-java's nullability annotations, only needed at compile time
+    requires static org.jspecify;
 
     requires com.fasterxml.jackson.core;
     requires com.fasterxml.jackson.kotlin;

src/test/kotlin/graphql/kickstart/tools/EndToEndTest.kt

@@ -79,7 +79,7 @@ class EndToEndTest {
             .graphQLContext(mapOf("newItem" to newItem))
             .variables(mapOf()))
 
-        val data = result.getData<Any>() as Publisher<ExecutionResult>
+        val data = result.getData<Publisher<ExecutionResult>>()!!
         val latch = CountDownLatch(1)
         data.subscribe(object : Subscriber<ExecutionResult> {
             override fun onNext(item: ExecutionResult?) {
@@ -117,7 +117,7 @@ class EndToEndTest {
             .graphQLContext(mapOf("newItem" to newItem))
             .variables(mapOf()))
 
-        val data = result.getData<Any>() as Publisher<ExecutionResult>
+        val data = result.getData<Publisher<ExecutionResult>>()!!
         val latch = CountDownLatch(1)
         data.subscribe(object : Subscriber<ExecutionResult> {
             override fun onNext(item: ExecutionResult?) {
@@ -639,7 +639,7 @@ class EndToEndTest {
             .graphQLContext(mapOf("newItem" to newItem))
             .variables(mapOf()))
 
-        val data = result.getData<Any>() as Publisher<ExecutionResult>
+        val data = result.getData<Publisher<ExecutionResult>>()!!
         val subscriber = TestEnvironment().newManualSubscriber(data)
 
         val subscriberResult = subscriber.requestNextElement() as ExecutionResultImpl
@@ -667,7 +667,7 @@ class EndToEndTest {
             .graphQLContext(mapOf("newItem" to newItem))
             .variables(mapOf()))
 
-        val data = result.getData<Any>() as Publisher<ExecutionResult>
+        val data = result.getData<Publisher<ExecutionResult>>()!!
         val subscriber = TestEnvironment().newManualSubscriber(data)
 
         val subscriberResult = subscriber.requestNextElement() as ExecutionResultImpl

src/test/kotlin/graphql/kickstart/tools/IntrospectionTest.kt

@@ -0,0 +1,60 @@
+package graphql.kickstart.tools
+
+import graphql.GraphQL
+import graphql.validation.ValidationError
+import graphql.validation.ValidationErrorType
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNull
+import org.junit.Test
+
+class IntrospectionTest {
+
+    private fun graphQL(introspectionEnabled: Boolean): GraphQL {
+        val schema = SchemaParser.newParser()
+            .schemaString(
+                """
+                type Query {
+                    name: String!
+                }
+                """)
+            .resolvers(object : GraphQLQueryResolver {
+                fun name() = "tools"
+            })
+            .options(SchemaParserOptions.newOptions().introspectionEnabled(introspectionEnabled).build())
+            .build()
+            .makeExecutableSchema()
+        return GraphQL.newGraphQL(schema).build()
+    }
+
+    @Test
+    fun `introspection is enabled by default`() {
+        val data = assertNoGraphQlErrors(graphQL(introspectionEnabled = true)) {
+            "{ __schema { queryType { name } } }"
+        }
+
+        assertEquals(mapOf("queryType" to mapOf("name" to "Query")), data["__schema"])
+    }
+
+    @Test
+    fun `disabling introspection hides the fields of introspection types`() {
+        val gql = graphQL(introspectionEnabled = false)
+
+        listOf("{ __schema { queryType { name } } }", "{ __type(name: \"Query\") { name } }").forEach { query ->
+            val result = gql.execute(query)
+
+            assertNull(result.getData())
+            assertEquals(
+                listOf(ValidationErrorType.FieldUndefined),
+                result.errors.map { (it as ValidationError).validationErrorType })
+        }
+    }
+
+    @Test
+    fun `disabling introspection keeps __typename`() {
+        val data = assertNoGraphQlErrors(graphQL(introspectionEnabled = false)) {
+            "{ name __typename }"
+        }
+
+        assertEquals(mapOf("name" to "tools", "__typename" to "Query"), data)
+    }
+}