Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 75 additions & 0 deletions PR_BODY.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
## Summary

Upgrades graphql-java from 23.1 to 26.0.beta-2, along with required transitive dependency bumps:

| Dependency | Old Version | New Version | Reason |
|-----------|-------------|-------------|--------|
| graphql-java | 23.1 | 26.0.beta-2 | Target upgrade |
| java-dataloader | 4.0.0 | 6.0.0 | Required by graphql-java 26.0.beta-2 |
| federation-graphql-java-support | 5.2.0 | 6.0.0 | Compatibility with graphql-java 25+ |

## Scope

- **Compile-only verification** -- no test execution in this beta pass
- All modules compile cleanly with `./gradlew assemble`
- Beta version: `9.0.0-beta-gj26`

## Breaking Changes Fixed

### API Migration Table

| Old Pattern | New Pattern | Reason | Modules Affected |
|---|---|---|---|
| `generateAdditionalTypes(): Set<GraphQLType>` | `generateAdditionalTypes(): Set<GraphQLNamedType>` | gj26 `additionalTypes()` accepts `Set<GraphQLNamedType>` | schema-generator |
| `KotlinDataLoader<K, V>` | `KotlinDataLoader<K : Any, V>` | java-dataloader 6.0 JSpecify non-null K bound | dataloader |
| `GraphQLTypeUtil.isList(graphQLType)` on nullable | `val type = graphQLType ?: return false; isList(type)` | JSpecify makes return types nullable | dataloader-instrumentation |
| `executionInput.executionId` (platform type) | `executionInput.executionId ?: return` (nullable) | JSpecify @Nullable on ExecutionId | dataloader-instrumentation |
| `StringValue.value` (platform type) | `StringValue.value ?: throw` (nullable) | JSpecify @Nullable on getValue() | federation |
| `TypeDefinitionRegistry.getType(name).get()` | `getTypeOrNull(name) ?: error(...)` | `getType()` deprecated since gj26 | client-generator |
| `persistedQueryError.message` (platform type) | `persistedQueryError.message ?: "fallback"` | JSpecify @Nullable on getMessage() | APQ |

### graphql-kotlin Public API Signature Changes

| Class | Change | Impact |
|---|---|---|
| `SchemaGenerator.generateAdditionalTypes()` | Return type: `Set<GraphQLType>` -> `Set<GraphQLNamedType>` | Internal method, minimal external impact |
| `KotlinDataLoader<K, V>` | Type bound: `K` -> `K : Any` | Breaking: implementations must use non-null K type (all existing usages already do) |

### JSpecify Nullability Fixes (17+)

| Category | Count | Fix Pattern |
|---|---|---|
| GraphQLType? (was platform) | 7 | Local val + null check for smart-cast |
| ExecutionId? (was platform) | 6 | Early-return with NOOP context |
| String? from StringValue.getValue() | 8 | Null check with error throw or fallback |
| K type bounds on DataLoader | 1 | Added `: Any` bound |
| Deprecated getType -> getTypeOrNull | 3 | Direct replacement |

### Files Modified (11 files across 7 modules)

- **schema-generator:** SchemaGenerator.kt
- **federation:** FederatedSchemaGeneratorHooks.kt, LinkImport.kt
- **dataloader:** KotlinDataLoader.kt
- **dataloader-instrumentation:** ExecutionInputState.kt, ExecutionStrategyState.kt, SyncExecutionExhaustedState.kt
- **APQ:** AutomaticPersistedQueriesProvider.kt
- **client-generator:** GraphQLClientGenerator.kt, DocumentExtensions.kt, generateTypeName.kt

## Federation Compatibility

- federation-graphql-java-support 6.0.0 is binary-compatible with graphql-java 26.0.beta-2 at compile time
- Federation module compiled without any graphql-java-related errors
- Only JSpecify nullability fixes needed in FederatedSchemaGeneratorHooks and LinkImport

## Known Runtime Risks

These are deferred to the testing pass (v2 requirements):
- FetchedValue duck-typing in FlowSubscriptionExecutionStrategy
- JSpecify nullability enforcement at runtime
- federation-graphql-java-support 6.0.0 forward-compatibility with gj26

## Test Plan

- [x] `./gradlew assemble` passes (compile-only)
- [x] `./gradlew publishToMavenLocal` succeeds
- [ ] Downstream weaver-kotlin-sdk compiles against these beta artifacts
- [ ] Downstream stark-graphql-sdk compiles against the full chain
2 changes: 1 addition & 1 deletion examples/federation/products-subgraph/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
FROM openjdk:17
FROM eclipse-temurin:17-jre
EXPOSE 8080

ARG JAR_FILE=build/libs/products-subgraph.jar
Expand Down
2 changes: 1 addition & 1 deletion examples/federation/reviews-subgraph/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
FROM openjdk:17
FROM eclipse-temurin:17-jre
EXPOSE 8081

ARG JAR_FILE=build/libs/reviews-subgraph.jar
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import graphql.execution.preparsed.PreparsedDocumentProvider
import graphql.execution.preparsed.persisted.PersistedQueryError
import graphql.execution.preparsed.persisted.PersistedQueryIdInvalid
import graphql.execution.preparsed.persisted.PersistedQueryNotFound
import graphql.execution.preparsed.persisted.PersistedQuerySupport
import java.util.concurrent.CompletableFuture
import java.util.function.Function

Expand All @@ -41,7 +42,7 @@ class AutomaticPersistedQueriesProvider(
executionInput.getAutomaticPersistedQueriesExtension()?.let { apqExtension ->
cache.getPersistedQueryDocumentAsync(apqExtension.sha256Hash, executionInput) { query ->
when {
query.isBlank() -> {
query.isBlank() || query == PersistedQuerySupport.PERSISTED_QUERY_MARKER -> {
throw PersistedQueryNotFound(apqExtension.sha256Hash)
}
executionInput.isAutomaticPersistedQueriesExtensionInvalid(apqExtension) -> {
Expand All @@ -66,7 +67,7 @@ class AutomaticPersistedQueriesProvider(
PreparsedDocumentEntry(
GraphqlErrorBuilder.newError()
.errorType(persistedQueryError)
.message(persistedQueryError.message)
.message(persistedQueryError.message ?: "Persisted query error")
.extensions(
when (persistedQueryError) {
// persistedQueryError.getExtensions()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ class AutomaticPersistedQueriesCacheProviderTest {
assertEquals(firstResultWithQueryId.errors.size, 1)
assertTrue(firstResultWithQueryId.errors[0].errorType is PersistedQueryNotFound)
assertEquals("PersistedQueryNotFound", firstResultWithQueryId.errors[0].message)
assertEquals("2ec03b0d1d2e458ffafa173a7e965de18e1c91e7c28546f0ef093778ddeeb49c", firstResultWithQueryId.errors[0].extensions["persistedQueryId"])
assertEquals("graphql-java", firstResultWithQueryId.errors[0].extensions["generatedBy"])
assertEquals("2ec03b0d1d2e458ffafa173a7e965de18e1c91e7c28546f0ef093778ddeeb49c", firstResultWithQueryId.errors[0].extensions?.get("persistedQueryId"))
assertEquals("graphql-java", firstResultWithQueryId.errors[0].extensions?.get("generatedBy"))

// Second execution persists query string and hash

Expand Down Expand Up @@ -130,7 +130,7 @@ class AutomaticPersistedQueriesCacheProviderTest {
assertEquals(result.errors.size, 1)
assertTrue(result.errors[0].errorType is PersistedQueryIdInvalid)
assertEquals("PersistedQueryIdInvalid", result.errors[0].message)
assertEquals("0000000000000000000000000000000000000000000000000000000000000000", result.errors[0].extensions["persistedQueryId"])
assertEquals("0000000000000000000000000000000000000000000000000000000000000000", result.errors[0].extensions?.get("persistedQueryId"))
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ class ExecutionInputState(
}

val parentFieldExecutionStrategyPathString = when {
isList(parentFieldGraphQLType) -> fieldExecutionStrategyPath.parent?.parent?.toString()
parentFieldGraphQLType != null && isList(parentFieldGraphQLType) -> fieldExecutionStrategyPath.parent?.parent?.toString()
else -> fieldExecutionStrategyPath.parent?.toString()
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,31 +182,37 @@ class ExecutionStrategyState(
*
* @return Boolean indicating if above conditions met
*/
fun isCompletedListOfComplexObjects(): Boolean =
fetchState == FieldFetchState.COMPLETED && result != null &&
GraphQLTypeUtil.isList(graphQLType) && !GraphQLTypeUtil.isLeaf(graphQLType) &&
fun isCompletedListOfComplexObjects(): Boolean {
val type = graphQLType ?: return false
return fetchState == FieldFetchState.COMPLETED && result != null &&
GraphQLTypeUtil.isList(type) && !GraphQLTypeUtil.isLeaf(type) &&
(result as? List<*>)?.filterNotNull()?.size == executionStrategyPaths.size
}

/**
* field [fetchState] is completed with a non-null [result] and
* [graphQLType] is not a leaf complex type which executionStrategy was started.
*
* @return Boolean indicating if above conditions met
*/
fun isCompletedComplexObject(): Boolean =
fetchState == FieldFetchState.COMPLETED && result != null &&
!GraphQLTypeUtil.isList(graphQLType) && !GraphQLTypeUtil.isLeaf(graphQLType) &&
fun isCompletedComplexObject(): Boolean {
val type = graphQLType ?: return false
return fetchState == FieldFetchState.COMPLETED && result != null &&
!GraphQLTypeUtil.isList(type) && !GraphQLTypeUtil.isLeaf(type) &&
executionStrategyPaths.isNotEmpty()
}

/**
* field [fetchState] is [FieldFetchState.COMPLETED]
* field [graphQLType] is a Leaf or null.
*
* @return Boolean indicating if above conditions met
*/
fun isCompletedLeafOrNull(): Boolean =
fetchState == FieldFetchState.COMPLETED &&
(GraphQLTypeUtil.isLeaf(graphQLType) || result == null)
fun isCompletedLeafOrNull(): Boolean {
val type = graphQLType
return fetchState == FieldFetchState.COMPLETED &&
(type != null && GraphQLTypeUtil.isLeaf(type) || result == null)
}

/**
* field [fetchType] is [FieldFetchType.ASYNC],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ class SyncExecutionExhaustedState(
fun beginExecution(
parameters: InstrumentationExecutionParameters
): InstrumentationContext<ExecutionResult> {
executions.computeIfAbsent(parameters.executionInput.executionId) {
val executionId = parameters.executionInput.executionId ?: return SimpleInstrumentationContext.noOp()
executions.computeIfAbsent(executionId) {
ExecutionInputState(parameters.executionInput)
}
return object : SimpleInstrumentationContext<ExecutionResult>() {
Expand All @@ -69,9 +70,9 @@ class SyncExecutionExhaustedState(
*/
override fun onCompleted(result: ExecutionResult?, t: Throwable?) {
if ((result != null && result.errors.isNotEmpty()) || t != null) {
if (executions.containsKey(parameters.executionInput.executionId)) {
if (executions.containsKey(executionId)) {
synchronized(executions) {
executions.remove(parameters.executionInput.executionId)
executions.remove(executionId)
totalExecutions.set(totalExecutions.get() - 1)
}
if (allSyncExecutionsExhausted()) {
Expand All @@ -93,7 +94,7 @@ class SyncExecutionExhaustedState(
fun beginRecursiveExecution(
parameters: InstrumentationExecutionStrategyParameters
) {
val executionId = parameters.executionContext.executionInput.executionId
val executionId = parameters.executionContext.executionInput.executionId ?: return
executions.computeIfPresent(executionId) { _, executionState ->
val executionStrategyParameters = parameters.executionStrategyParameters

Expand All @@ -117,7 +118,7 @@ class SyncExecutionExhaustedState(
fun beginFieldFetching(
parameters: InstrumentationFieldFetchParameters
): FieldFetchingInstrumentationContext {
val executionId = parameters.executionContext.executionInput.executionId
val executionId = parameters.executionContext.executionInput.executionId ?: return FieldFetchingInstrumentationContext.NOOP
val field = parameters.executionStepInfo.field.singleField
val fieldExecutionStrategyPath = parameters.executionStepInfo.path.parent
val fieldGraphQLType = parameters.executionStepInfo.unwrappedNonNullType
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ class AstronautDataLoader : KotlinDataLoader<AstronautServiceRequest, Astronaut?
.map(List<Optional<Astronaut>>::toListOfNullables)
.toFuture()
},
DataLoaderOptions.newOptions().setStatisticsCollector(::SimpleStatisticsCollector)
DataLoaderOptions.newOptions().setStatisticsCollector(::SimpleStatisticsCollector).build()
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class MissionDataLoader : KotlinDataLoader<MissionServiceRequest, Mission?> {
.map(List<Optional<Mission>>::toListOfNullables)
.toFuture()
},
DataLoaderOptions.newOptions().setStatisticsCollector(::SimpleStatisticsCollector)
DataLoaderOptions.newOptions().setStatisticsCollector(::SimpleStatisticsCollector).build()
)
}

Expand All @@ -55,7 +55,7 @@ class MissionsByAstronautDataLoader : KotlinDataLoader<MissionServiceRequest, Li
.getMissionsByAstronautIds(keys.map(MissionServiceRequest::astronautId))
.collectList().toFuture()
},
DataLoaderOptions.newOptions().setStatisticsCollector(::SimpleStatisticsCollector)
DataLoaderOptions.newOptions().setStatisticsCollector(::SimpleStatisticsCollector).build()
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class PlanetsByMissionDataLoader : KotlinDataLoader<PlanetServiceRequest, List<P
.collectList()
.toFuture()
},
DataLoaderOptions.newOptions().setStatisticsCollector(::SimpleStatisticsCollector)
DataLoaderOptions.newOptions().setStatisticsCollector(::SimpleStatisticsCollector).build()
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class ProductDataLoader : KotlinDataLoader<ProductServiceRequest, Product?> {
.map(List<Optional<Product>>::toListOfNullables)
.toFuture()
},
DataLoaderOptions.newOptions().setStatisticsCollector(::SimpleStatisticsCollector)
DataLoaderOptions.newOptions().setStatisticsCollector(::SimpleStatisticsCollector).build()
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import org.dataloader.DataLoader
* Wrapper around the [DataLoader] class so we can have common logic around registering the loaders
* by return type and loading values in the data fetchers.
*/
interface KotlinDataLoader<K, V> {
interface KotlinDataLoader<K : Any, V> {
val dataLoaderName: String
fun getDataLoader(graphQLContext: GraphQLContext): DataLoader<K, V>
}
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ open class FederatedSchemaGeneratorHooks(
federatedQuery.field(entityField)

federatedCodeRegistry.dataFetcher(FieldCoordinates.coordinates(originalQuery.name, entityField.name), EntitiesDataFetcher(resolvers))
federatedCodeRegistry.typeResolver(ENTITY_UNION_NAME) { env: TypeResolutionEnvironment -> env.schema.getObjectType(env.getObjectName()) }
federatedCodeRegistry.typeResolver(ENTITY_UNION_NAME) { env: TypeResolutionEnvironment -> env.getObjectName()?.let { env.schema.getObjectType(it) } }

builder.query(federatedQuery)
.codeRegistry(federatedCodeRegistry.build())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,15 @@ private class LinkImportCoercing : Coercing<LinkImport, Any> {

override fun parseValue(input: Any, graphQLContext: GraphQLContext, locale: Locale): LinkImport = when (input) {
is LinkImport -> input
is StringValue -> LinkImport(name = input.value, `as` = input.value)
is StringValue -> {
val value = input.value ?: throw CoercingParseValueException("Cannot parse $input to LinkImport: null value")
LinkImport(name = value, `as` = value)
}
is ObjectValue -> {
val nameValue = input.objectFields.firstOrNull { it.name == "name" }?.value as? StringValue ?: throw CoercingParseValueException("Cannot parse $input to LinkImport")
val name = nameValue.value ?: throw CoercingParseValueException("Cannot parse $input to LinkImport: null name")
val namespacedValue = input.objectFields.firstOrNull { it.name == "as" }?.value as? StringValue
LinkImport(name = nameValue.value, `as` = namespacedValue?.value ?: nameValue.value)
LinkImport(name = name, `as` = namespacedValue?.value ?: name)
}
else -> throw CoercingParseValueException(
"Cannot parse $input to LinkImport. Expected AST type of `StringValue` or `ObjectValue` but was ${input.javaClass.simpleName} "
Expand All @@ -72,11 +76,15 @@ private class LinkImportCoercing : Coercing<LinkImport, Any> {

override fun parseLiteral(input: Value<*>, variables: CoercedVariables, graphQLContext: GraphQLContext, locale: Locale): LinkImport =
when (input) {
is StringValue -> LinkImport(name = input.value, `as` = input.value)
is StringValue -> {
val value = input.value ?: throw CoercingParseLiteralException("Cannot parse $input to LinkImport: null value")
LinkImport(name = value, `as` = value)
}
is ObjectValue -> {
val nameValue = input.objectFields.firstOrNull { it.name == "name" }?.value as? StringValue ?: throw CoercingParseLiteralException("Cannot parse $input to LinkImport")
val name = nameValue.value ?: throw CoercingParseLiteralException("Cannot parse $input to LinkImport: null name")
val namespacedValue = input.objectFields.firstOrNull { it.name == "as" }?.value as? StringValue
LinkImport(name = nameValue.value, `as` = namespacedValue?.value ?: nameValue.value)
LinkImport(name = name, `as` = namespacedValue?.value ?: name)
}
else -> throw CoercingParseLiteralException(
"Cannot parse $input to LinkImport. Expected AST type of `StringValue` or `ObjectValue` but was ${input.javaClass.simpleName} "
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*
* Compatibility shim for graphql-java 26.0.beta-2.
*
* graphql-java 25.x provided graphql.schema.idl.DirectiveInfo with
* GRAPHQL_SPECIFICATION_DIRECTIVE_MAP and isGraphqlSpecifiedDirective().
* These were removed in graphql-java 26 and replaced by graphql.Directives.
*
* federation-graphql-java-support 6.0.0 (built against gj25) still references
* DirectiveInfo at runtime in ServiceSDLPrinter. This shim bridges the gap
* until a gj26-compatible federation-graphql-java-support release is available.
*
* TODO: Remove this shim when federation-graphql-java-support releases a version
* built against graphql-java 26+.
*/

package graphql.schema.idl

import graphql.Directives
import graphql.schema.GraphQLDirective

/**
* Static fields and methods accessed via JVM bytecode by
* com.apollographql.federation.graphqljava.printer.ServiceSDLPrinter.
*/
object DirectiveInfo {
/**
* Map of specification directive names to their [GraphQLDirective] definitions.
* Equivalent to the removed graphql-java 25 DirectiveInfo.GRAPHQL_SPECIFICATION_DIRECTIVE_MAP.
*/
@JvmField
val GRAPHQL_SPECIFICATION_DIRECTIVE_MAP: Map<String, GraphQLDirective> = Directives.BUILT_IN_DIRECTIVES_MAP

/**
* Checks whether a given directive is a GraphQL specification directive.
* Equivalent to the removed graphql-java 25 DirectiveInfo.isGraphqlSpecifiedDirective().
*/
@JvmStatic
fun isGraphqlSpecifiedDirective(directive: GraphQLDirective): Boolean =
Directives.isBuiltInDirective(directive)
}
Loading
Loading