diff --git a/.github/workflows/pull-request.yml b/.github/workflows/pull-request.yml index 85149530..731badef 100644 --- a/.github/workflows/pull-request.yml +++ b/.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 ".*$" 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 diff --git a/pom.xml b/pom.xml index c49370d1..98921e13 100644 --- a/pom.xml +++ b/pom.xml @@ -262,6 +262,11 @@ kotlin-maven-plugin org.jetbrains.kotlin ${kotlin.version} + + + -Werror + + compile @@ -286,8 +291,8 @@ maven-compiler-plugin 3.16.0 - 11 - 11 + 11 + true diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt index 8844ba6b..24eac040 100644 --- a/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt +++ b/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()}") diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt index 7417e54c..3ce9d50a 100644 --- a/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt +++ b/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, 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) + createInputObject(typeDefinition, inputObjects, referencingInputObjects) } is TypeName -> { val scalarType = customScalars[typeDefinition.name] diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaParserOptions.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaParserOptions.kt index 8b0e0c60..d1c6a5e8 100644 --- a/src/main/kotlin/graphql/kickstart/tools/SchemaParserOptions.kt +++ b/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) diff --git a/src/main/kotlin/graphql/kickstart/tools/TypeClassMatcher.kt b/src/main/kotlin/graphql/kickstart/tools/TypeClassMatcher.kt index b6e4096e..b24b7b04 100644 --- a/src/main/kotlin/graphql/kickstart/tools/TypeClassMatcher.kt +++ b/src/main/kotlin/graphql/kickstart/tools/TypeClassMatcher.kt @@ -68,7 +68,7 @@ internal class TypeClassMatcher(private val definitionsByName: Map && 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") diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt index d787ef6d..34e5b282 100644 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt +++ b/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() diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/MapFieldResolver.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/MapFieldResolver.kt index 70c6d41b..0eea1c1d 100644 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/MapFieldResolver.kt +++ b/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<*> { diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt index 0a9e65d6..0ac8612c 100644 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt +++ b/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)) diff --git a/src/main/kotlin/module-info.java b/src/main/kotlin/module-info.java index 2ec2dd1b..90ef1592 100644 --- a/src/main/kotlin/module-info.java +++ b/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; diff --git a/src/test/kotlin/graphql/kickstart/tools/EndToEndTest.kt b/src/test/kotlin/graphql/kickstart/tools/EndToEndTest.kt index 9539a7b1..ee022f8d 100644 --- a/src/test/kotlin/graphql/kickstart/tools/EndToEndTest.kt +++ b/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() as Publisher + val data = result.getData>()!! val latch = CountDownLatch(1) data.subscribe(object : Subscriber { override fun onNext(item: ExecutionResult?) { @@ -117,7 +117,7 @@ class EndToEndTest { .graphQLContext(mapOf("newItem" to newItem)) .variables(mapOf())) - val data = result.getData() as Publisher + val data = result.getData>()!! val latch = CountDownLatch(1) data.subscribe(object : Subscriber { override fun onNext(item: ExecutionResult?) { @@ -639,7 +639,7 @@ class EndToEndTest { .graphQLContext(mapOf("newItem" to newItem)) .variables(mapOf())) - val data = result.getData() as Publisher + val data = result.getData>()!! 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() as Publisher + val data = result.getData>()!! val subscriber = TestEnvironment().newManualSubscriber(data) val subscriberResult = subscriber.requestNextElement() as ExecutionResultImpl diff --git a/src/test/kotlin/graphql/kickstart/tools/IntrospectionTest.kt b/src/test/kotlin/graphql/kickstart/tools/IntrospectionTest.kt new file mode 100644 index 00000000..fa18e90d --- /dev/null +++ b/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) + } +} diff --git a/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverDataFetcherTest.kt b/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverDataFetcherTest.kt index f163ee6b..e539b0e3 100644 --- a/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverDataFetcherTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverDataFetcherTest.kt @@ -14,6 +14,7 @@ import graphql.schema.DataFetcher import graphql.schema.DataFetchingEnvironment import graphql.schema.DataFetchingEnvironmentImpl import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.DelicateCoroutinesApi import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.Job @@ -124,6 +125,7 @@ class MethodFieldResolverDataFetcherTest { } @ExperimentalCoroutinesApi + @OptIn(DelicateCoroutinesApi::class) @Test fun `canceling subscription Publisher also cancels underlying Kotlin coroutine channel`() { // setup @@ -236,7 +238,7 @@ class MethodFieldResolverDataFetcherTest { @Test fun `data fetcher passes environment if method has extra argument`() { val resolver = createFetcher("active", object : GraphQLResolver { - fun isActive(dataClass: DataClass, env: DataFetchingEnvironment): Boolean = env is DataFetchingEnvironment + fun isActive(dataClass: DataClass, env: DataFetchingEnvironment?): Boolean = env != null }) assertEquals(resolver.get(createEnvironment(DataClass())), true) @@ -246,7 +248,7 @@ class MethodFieldResolverDataFetcherTest { fun `data fetcher passes environment if method has extra argument even if context is specified`() { val context = GraphQLContext.newContext().build() val resolver = createFetcher("active", resolver = object : GraphQLResolver { - fun isActive(dataClass: DataClass, env: DataFetchingEnvironment): Boolean = env is DataFetchingEnvironment + fun isActive(dataClass: DataClass, env: DataFetchingEnvironment?): Boolean = env != null }) assertEquals(resolver.get(createEnvironment(DataClass(), context = context)), true) @@ -320,7 +322,7 @@ class MethodFieldResolverDataFetcherTest { @Test fun `data fetcher passes environment to extension function if method has extra argument`() { val resolver = createFetcher("active", object : GraphQLResolver { - fun DataClass.isActive(env: DataFetchingEnvironment): Boolean = env is DataFetchingEnvironment + fun DataClass.isActive(env: DataFetchingEnvironment?): Boolean = env != null }) assertEquals(resolver.get(createEnvironment(DataClass())), true) @@ -411,7 +413,7 @@ class MethodFieldResolverDataFetcherTest { } private fun createEnvironment( - source: Any = Object(), + source: Any = Any(), arguments: Map = emptyMap(), context: GraphQLContext = GraphQLContext.newContext().build(), legacyContext: Any? = null diff --git a/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverTest.kt b/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverTest.kt index a59503df..a02ef092 100644 --- a/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverTest.kt @@ -51,7 +51,7 @@ class MethodFieldResolverTest { testNull(input: null) } """) - .root(Object())) + .root(Any())) assertEquals(result.getData(), mapOf( "testValue" to "Optional[test-value]", @@ -93,7 +93,7 @@ class MethodFieldResolverTest { testNull(input: null) } """) - .root(Object())) + .root(Any())) assertEquals(result.getData(), mapOf( "testValue" to "Optional[test-value]", @@ -128,7 +128,7 @@ class MethodFieldResolverTest { } """) .variables(mapOf("input" to "FooBar")) - .root(Object())) + .root(Any())) assertEquals(result.getData(), mapOf("test" to 6)) } @@ -159,7 +159,7 @@ class MethodFieldResolverTest { } """) .variables(mapOf("input" to listOf("Foo", "Bar"))) - .root(Object())) + .root(Any())) assertEquals(result.getData(), mapOf("test" to 6)) } @@ -206,7 +206,7 @@ class MethodFieldResolverTest { } """) .variables(mapOf("input" to listOf("Foo", "Bar"))) - .root(Object())) + .root(Any())) assertEquals(result.getData(), mapOf("test" to 6)) } diff --git a/src/test/kotlin/graphql/kickstart/tools/TestUtils.kt b/src/test/kotlin/graphql/kickstart/tools/TestUtils.kt index 33da8b0b..1180fc72 100644 --- a/src/test/kotlin/graphql/kickstart/tools/TestUtils.kt +++ b/src/test/kotlin/graphql/kickstart/tools/TestUtils.kt @@ -19,7 +19,7 @@ fun assertNoGraphQlErrors(gql: GraphQL, args: Map = mapOf(), contex throw AssertionError("GraphQL result contained errors!\n${result.errors.map { it.message }.joinToString("\n")}") } - return result.getData() as Map + return result.getData>()!! } fun assertEquals(actual: T, expected: T) { diff --git a/src/test/kotlin/graphql/kickstart/tools/util/PrimitivesTest.kt b/src/test/kotlin/graphql/kickstart/tools/util/PrimitivesTest.kt index 6856e838..1c227832 100644 --- a/src/test/kotlin/graphql/kickstart/tools/util/PrimitivesTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/util/PrimitivesTest.kt @@ -8,7 +8,7 @@ class PrimitivesTest { @Test fun `should wrap primitive Java class`() { val clazz = Primitives.wrap(Int::class.javaPrimitiveType) - Assert.assertEquals(Integer::class.java, clazz) + Assert.assertEquals(Int::class.javaObjectType, clazz) } @Test