diff --git a/examples/server/ktor-server/src/main/kotlin/com/expediagroup/graphql/examples/server/ktor/schema/ExampleSubscriptionService.kt b/examples/server/ktor-server/src/main/kotlin/com/expediagroup/graphql/examples/server/ktor/schema/ExampleSubscriptionService.kt index 1d12922f64..9ab7551f1a 100644 --- a/examples/server/ktor-server/src/main/kotlin/com/expediagroup/graphql/examples/server/ktor/schema/ExampleSubscriptionService.kt +++ b/examples/server/ktor-server/src/main/kotlin/com/expediagroup/graphql/examples/server/ktor/schema/ExampleSubscriptionService.kt @@ -67,7 +67,7 @@ class ExampleSubscriptionService : Subscription { @GraphQLDescription("Returns stream of errors") fun flowOfErrors(): Publisher> { - val dfr: DataFetcherResult = DataFetcherResult.newResult() + val dfr: DataFetcherResult = DataFetcherResult.Builder() .data(null) .error(GraphqlErrorException.newErrorException().cause(Exception("error thrown")).build()) .build() diff --git a/examples/server/spring-server/src/main/kotlin/com/expediagroup/graphql/examples/server/spring/query/DataAndErrorsQuery.kt b/examples/server/spring-server/src/main/kotlin/com/expediagroup/graphql/examples/server/spring/query/DataAndErrorsQuery.kt index 894019bf1e..b9f101fb3e 100644 --- a/examples/server/spring-server/src/main/kotlin/com/expediagroup/graphql/examples/server/spring/query/DataAndErrorsQuery.kt +++ b/examples/server/spring-server/src/main/kotlin/com/expediagroup/graphql/examples/server/spring/query/DataAndErrorsQuery.kt @@ -27,14 +27,14 @@ import java.util.concurrent.CompletableFuture class DataAndErrorsQuery : Query { fun returnDataAndErrors(): DataFetcherResult { - return DataFetcherResult.newResult() + return DataFetcherResult.Builder() .data("Hello from data fetcher") .error(getError()) .build() } fun completableFutureDataAndErrors(): CompletableFuture> { - val dataFetcherResult = DataFetcherResult.newResult() + val dataFetcherResult = DataFetcherResult.Builder() .data("Hello from data fetcher") .error(getError()) .build() diff --git a/examples/server/spring-server/src/main/kotlin/com/expediagroup/graphql/examples/server/spring/subscriptions/SimpleSubscription.kt b/examples/server/spring-server/src/main/kotlin/com/expediagroup/graphql/examples/server/spring/subscriptions/SimpleSubscription.kt index 83093fc7ba..2d0efd9fc3 100644 --- a/examples/server/spring-server/src/main/kotlin/com/expediagroup/graphql/examples/server/spring/subscriptions/SimpleSubscription.kt +++ b/examples/server/spring-server/src/main/kotlin/com/expediagroup/graphql/examples/server/spring/subscriptions/SimpleSubscription.kt @@ -71,7 +71,7 @@ class SimpleSubscription : Subscription { @GraphQLDescription("Returns stream of errors") fun flowOfErrors(): Publisher> { - val dfr: DataFetcherResult = DataFetcherResult.newResult() + val dfr: DataFetcherResult = DataFetcherResult.Builder() .data(null) .error(GraphqlErrorException.newErrorException().cause(Exception("error thrown")).build()) .build() diff --git a/executions/graphql-kotlin-automatic-persisted-queries/src/main/kotlin/com/expediagroup/graphql/apq/cache/AutomaticPersistedQueriesCache.kt b/executions/graphql-kotlin-automatic-persisted-queries/src/main/kotlin/com/expediagroup/graphql/apq/cache/AutomaticPersistedQueriesCache.kt index ca8aa1fb84..add6c5b69b 100644 --- a/executions/graphql-kotlin-automatic-persisted-queries/src/main/kotlin/com/expediagroup/graphql/apq/cache/AutomaticPersistedQueriesCache.kt +++ b/executions/graphql-kotlin-automatic-persisted-queries/src/main/kotlin/com/expediagroup/graphql/apq/cache/AutomaticPersistedQueriesCache.kt @@ -20,6 +20,7 @@ import graphql.ExecutionInput import graphql.execution.preparsed.PreparsedDocumentEntry import graphql.execution.preparsed.persisted.PersistedQueryCache import graphql.execution.preparsed.persisted.PersistedQueryCacheMiss +import graphql.execution.preparsed.persisted.PersistedQuerySupport import java.util.concurrent.CompletableFuture interface AutomaticPersistedQueriesCache : PersistedQueryCache { @@ -27,10 +28,18 @@ interface AutomaticPersistedQueriesCache : PersistedQueryCache { persistedQueryId: Any, executionInput: ExecutionInput, onCacheMiss: PersistedQueryCacheMiss - ): CompletableFuture = - getOrElse(persistedQueryId.toString(), executionInput) { - onCacheMiss.apply(executionInput.query) + ): CompletableFuture { + // In graphql-java 25+, ExecutionInput substitutes PERSISTED_QUERY_MARKER for a null/empty + // query string when an APQ extension is present, to satisfy the invariant that getQuery() + // is never null. We must treat it the same as a blank query — both signal that no query + // text was sent with this request and a cache miss (PersistedQueryNotFound) should follow. + val queryText = executionInput.query.takeUnless { + it.isBlank() || it == PersistedQuerySupport.PERSISTED_QUERY_MARKER } + return getOrElse(persistedQueryId.toString(), executionInput) { + onCacheMiss.apply(queryText ?: "") + } + } /** * Get the [PreparsedDocumentEntry] associated with the [key] from the cache. diff --git a/executions/graphql-kotlin-dataloader-instrumentation/src/main/kotlin/com/expediagroup/graphql/dataloader/instrumentation/syncexhaustion/state/SyncExecutionExhaustedState.kt b/executions/graphql-kotlin-dataloader-instrumentation/src/main/kotlin/com/expediagroup/graphql/dataloader/instrumentation/syncexhaustion/state/SyncExecutionExhaustedState.kt index 40654a06e9..0184d089e4 100644 --- a/executions/graphql-kotlin-dataloader-instrumentation/src/main/kotlin/com/expediagroup/graphql/dataloader/instrumentation/syncexhaustion/state/SyncExecutionExhaustedState.kt +++ b/executions/graphql-kotlin-dataloader-instrumentation/src/main/kotlin/com/expediagroup/graphql/dataloader/instrumentation/syncexhaustion/state/SyncExecutionExhaustedState.kt @@ -56,7 +56,7 @@ class SyncExecutionExhaustedState( fun beginExecution( parameters: InstrumentationExecutionParameters ): InstrumentationContext { - executions.computeIfAbsent(parameters.executionInput.executionId) { + executions.computeIfAbsent(parameters.executionInput.executionIdNonNull) { ExecutionInputState(parameters.executionInput) } return object : SimpleInstrumentationContext() { @@ -93,7 +93,7 @@ class SyncExecutionExhaustedState( fun beginRecursiveExecution( parameters: InstrumentationExecutionStrategyParameters ) { - val executionId = parameters.executionContext.executionInput.executionId + val executionId = parameters.executionContext.executionInput.executionIdNonNull executions.computeIfPresent(executionId) { _, executionState -> val executionStrategyParameters = parameters.executionStrategyParameters @@ -117,7 +117,7 @@ class SyncExecutionExhaustedState( fun beginFieldFetching( parameters: InstrumentationFieldFetchParameters ): FieldFetchingInstrumentationContext { - val executionId = parameters.executionContext.executionInput.executionId + val executionId = parameters.executionContext.executionInput.executionIdNonNull val field = parameters.executionStepInfo.field.singleField val fieldExecutionStrategyPath = parameters.executionStepInfo.path.parent val fieldGraphQLType = parameters.executionStepInfo.unwrappedNonNullType diff --git a/executions/graphql-kotlin-dataloader-instrumentation/src/test/kotlin/com/expediagroup/graphql/dataloader/instrumentation/syncexhaustion/GraphQLSyncExecutionExhaustedDataLoaderDispatcherTest.kt b/executions/graphql-kotlin-dataloader-instrumentation/src/test/kotlin/com/expediagroup/graphql/dataloader/instrumentation/syncexhaustion/GraphQLSyncExecutionExhaustedDataLoaderDispatcherTest.kt index c72c1eebbf..7a18e7fbc5 100644 --- a/executions/graphql-kotlin-dataloader-instrumentation/src/test/kotlin/com/expediagroup/graphql/dataloader/instrumentation/syncexhaustion/GraphQLSyncExecutionExhaustedDataLoaderDispatcherTest.kt +++ b/executions/graphql-kotlin-dataloader-instrumentation/src/test/kotlin/com/expediagroup/graphql/dataloader/instrumentation/syncexhaustion/GraphQLSyncExecutionExhaustedDataLoaderDispatcherTest.kt @@ -16,16 +16,38 @@ package com.expediagroup.graphql.dataloader.instrumentation.syncexhaustion +import com.expediagroup.graphql.dataloader.instrumentation.exceptions.MissingInstrumentationStateException +import com.expediagroup.graphql.dataloader.instrumentation.extensions.dispatchIfNeeded import com.expediagroup.graphql.dataloader.instrumentation.fixture.DataLoaderInstrumentationStrategy import com.expediagroup.graphql.dataloader.instrumentation.fixture.AstronautGraphQL import com.expediagroup.graphql.dataloader.instrumentation.fixture.ProductGraphQL +import com.expediagroup.graphql.dataloader.instrumentation.syncexhaustion.state.SyncExecutionExhaustedState import graphql.ExecutionInput +import graphql.ExecutionResult +import graphql.GraphQLContext +import graphql.execution.ExecutionContext +import graphql.execution.instrumentation.ExecutionStrategyInstrumentationContext +import graphql.execution.instrumentation.FieldFetchingInstrumentationContext +import graphql.execution.instrumentation.InstrumentationContext +import graphql.execution.instrumentation.parameters.InstrumentationExecutionParameters +import graphql.execution.instrumentation.parameters.InstrumentationExecutionStrategyParameters +import graphql.execution.instrumentation.parameters.InstrumentationFieldFetchParameters +import graphql.language.OperationDefinition +import graphql.schema.DataFetchingEnvironment import io.mockk.clearAllMocks +import io.mockk.every +import io.mockk.just +import io.mockk.mockk +import io.mockk.runs import io.mockk.verify import org.dataloader.DataLoaderRegistry import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test +import java.util.concurrent.CompletableFuture import kotlin.test.assertEquals +import kotlin.test.assertFailsWith +import kotlin.test.assertNull +import kotlin.test.assertSame import kotlin.test.assertTrue class GraphQLSyncExecutionExhaustedDataLoaderDispatcherTest { @@ -47,6 +69,171 @@ class GraphQLSyncExecutionExhaustedDataLoaderDispatcherTest { clearAllMocks() } + @Test + fun `beginExecution returns null when sync state is not present in context`() { + val parameters = mockk() + every { parameters.graphQLContext } returns GraphQLContext.newContext().build() + + val result = graphQLSyncExecutionExhaustedDataLoaderDispatcher.beginExecution(parameters, null) + + assertNull(result) + } + + @Test + fun `beginExecution delegates to sync state when present`() { + val syncState = mockk() + val delegatedContext = mockk>() + val parameters = mockk() + + every { parameters.graphQLContext } returns GraphQLContext.newContext() + .of(SyncExecutionExhaustedState::class, syncState) + .build() + every { syncState.beginExecution(parameters) } returns delegatedContext + + val result = graphQLSyncExecutionExhaustedDataLoaderDispatcher.beginExecution(parameters, null) + + assertSame(delegatedContext, result) + } + + @Test + fun `beginExecutionStrategy returns null and skips delegation when sync state is missing`() { + val executionContext = mockExecutionContext(GraphQLContext.newContext().build()) + val parameters = mockk() + every { parameters.executionContext } returns executionContext + + val result = graphQLSyncExecutionExhaustedDataLoaderDispatcher.beginExecutionStrategy(parameters, null) + + assertNull(result) + } + + @Test + fun `beginExecutionStrategy delegates recursive execution when sync state is present`() { + val syncState = mockk() + every { syncState.beginRecursiveExecution(any()) } just runs + + val executionContext = mockExecutionContext( + GraphQLContext.newContext().of(SyncExecutionExhaustedState::class, syncState).build() + ) + val parameters = mockk() + every { parameters.executionContext } returns executionContext + + val result: ExecutionStrategyInstrumentationContext? = + graphQLSyncExecutionExhaustedDataLoaderDispatcher.beginExecutionStrategy(parameters, null) + + assertNull(result) + verify(exactly = 1) { syncState.beginRecursiveExecution(parameters) } + } + + @Test + fun `beginExecuteObject returns null and skips delegation when sync state is missing`() { + val executionContext = mockExecutionContext(GraphQLContext.newContext().build()) + val parameters = mockk() + every { parameters.executionContext } returns executionContext + + val result = graphQLSyncExecutionExhaustedDataLoaderDispatcher.beginExecuteObject(parameters, null) + + assertNull(result) + } + + @Test + fun `beginExecuteObject delegates recursive execution when sync state is present`() { + val syncState = mockk() + every { syncState.beginRecursiveExecution(any()) } just runs + + val executionContext = mockExecutionContext( + GraphQLContext.newContext().of(SyncExecutionExhaustedState::class, syncState).build() + ) + val parameters = mockk() + every { parameters.executionContext } returns executionContext + + val result = graphQLSyncExecutionExhaustedDataLoaderDispatcher.beginExecuteObject(parameters, null) + + assertNull(result) + verify(exactly = 1) { syncState.beginRecursiveExecution(parameters) } + } + + @Test + fun `beginFieldFetching returns null when sync state is missing`() { + val executionContext = mockExecutionContext(GraphQLContext.newContext().build()) + val parameters = mockk() + every { parameters.executionContext } returns executionContext + + val result = graphQLSyncExecutionExhaustedDataLoaderDispatcher.beginFieldFetching(parameters, null) + + assertNull(result) + } + + @Test + fun `beginFieldFetching delegates when sync state is present`() { + val syncState = mockk() + val delegatedContext = mockk() + every { syncState.beginFieldFetching(any()) } returns delegatedContext + + val executionContext = mockExecutionContext( + GraphQLContext.newContext().of(SyncExecutionExhaustedState::class, syncState).build() + ) + val parameters = mockk() + every { parameters.executionContext } returns executionContext + + val result = graphQLSyncExecutionExhaustedDataLoaderDispatcher.beginFieldFetching(parameters, null) + + assertSame(delegatedContext, result) + verify(exactly = 1) { syncState.beginFieldFetching(parameters) } + } + + @Test + fun `dispatchIfNeeded throws when sync execution state is missing`() { + val dataLoaderRegistry = mockk(relaxed = true) + val environment = mockk() + every { environment.dataLoaderRegistry } returns dataLoaderRegistry + every { environment.graphQlContext } returns GraphQLContext.newContext().build() + + assertFailsWith { + CompletableFuture.completedFuture("value").dispatchIfNeeded(environment) + } + + verify(exactly = 0) { dataLoaderRegistry.dispatchAll() } + } + + @Test + fun `dispatchIfNeeded dispatches all when chained loads happened and executions are exhausted`() { + val dataLoaderRegistry = mockk(relaxed = true) + val syncState = mockk() + every { syncState.dataLoadersLoadInvokedAfterDispatchAll() } returns true + every { syncState.allSyncExecutionsExhausted() } returns true + + val environment = mockk() + every { environment.dataLoaderRegistry } returns dataLoaderRegistry + every { environment.graphQlContext } returns GraphQLContext.newContext() + .of(SyncExecutionExhaustedState::class, syncState) + .build() + + val future = CompletableFuture.completedFuture("value") + + val result = future.dispatchIfNeeded(environment) + + assertSame(future, result) + verify(exactly = 1) { dataLoaderRegistry.dispatchAll() } + } + + @Test + fun `dispatchIfNeeded does not dispatch when executions are not exhausted`() { + val dataLoaderRegistry = mockk(relaxed = true) + val syncState = mockk() + every { syncState.dataLoadersLoadInvokedAfterDispatchAll() } returns true + every { syncState.allSyncExecutionsExhausted() } returns false + + val environment = mockk() + every { environment.dataLoaderRegistry } returns dataLoaderRegistry + every { environment.graphQlContext } returns GraphQLContext.newContext() + .of(SyncExecutionExhaustedState::class, syncState) + .build() + + CompletableFuture.completedFuture("value").dispatchIfNeeded(environment) + + verify(exactly = 0) { dataLoaderRegistry.dispatchAll() } + } + @Test fun `Instrumentation should batch transactions on async top level fields`() { val queries = listOf( @@ -561,7 +748,7 @@ class GraphQLSyncExecutionExhaustedDataLoaderDispatcherTest { """mutation { createAstronaut(name: "spaceMan") { id name } }""" ) - val (results, dataLoaderRegistry, graphQLContext) = AstronautGraphQL.executeOperations( + val (results, _, graphQLContext) = AstronautGraphQL.executeOperations( astronautGraphQL, queries, DataLoaderInstrumentationStrategy.SYNC_EXHAUSTION @@ -634,4 +821,14 @@ class GraphQLSyncExecutionExhaustedDataLoaderDispatcherTest { graphQLContext.get(DataLoaderRegistry::class) } } + + private fun mockExecutionContext(context: GraphQLContext): ExecutionContext { + val operationDefinition = mockk() + every { operationDefinition.operation } returns OperationDefinition.Operation.QUERY + + val executionContext = mockk() + every { executionContext.operationDefinition } returns operationDefinition + every { executionContext.graphQLContext } returns context + return executionContext + } } diff --git a/generator/graphql-kotlin-federation/src/main/kotlin/com/expediagroup/graphql/generator/federation/types/LinkImport.kt b/generator/graphql-kotlin-federation/src/main/kotlin/com/expediagroup/graphql/generator/federation/types/LinkImport.kt index f24c7c170a..3c4a5324be 100644 --- a/generator/graphql-kotlin-federation/src/main/kotlin/com/expediagroup/graphql/generator/federation/types/LinkImport.kt +++ b/generator/graphql-kotlin-federation/src/main/kotlin/com/expediagroup/graphql/generator/federation/types/LinkImport.kt @@ -59,12 +59,8 @@ private class LinkImportCoercing : Coercing { 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 ObjectValue -> { - val nameValue = input.objectFields.firstOrNull { it.name == "name" }?.value as? StringValue ?: throw CoercingParseValueException("Cannot parse $input to LinkImport") - val namespacedValue = input.objectFields.firstOrNull { it.name == "as" }?.value as? StringValue - LinkImport(name = nameValue.value, `as` = namespacedValue?.value ?: nameValue.value) - } + is StringValue -> parseStringValue(input) { msg -> CoercingParseValueException(msg) } + is ObjectValue -> parseObjectValue(input) { msg -> CoercingParseValueException(msg) } else -> throw CoercingParseValueException( "Cannot parse $input to LinkImport. Expected AST type of `StringValue` or `ObjectValue` but was ${input.javaClass.simpleName} " ) @@ -72,17 +68,26 @@ private class LinkImportCoercing : Coercing { override fun parseLiteral(input: Value<*>, variables: CoercedVariables, graphQLContext: GraphQLContext, locale: Locale): LinkImport = when (input) { - is StringValue -> LinkImport(name = input.value, `as` = input.value) - is ObjectValue -> { - val nameValue = input.objectFields.firstOrNull { it.name == "name" }?.value as? StringValue ?: throw CoercingParseLiteralException("Cannot parse $input to LinkImport") - val namespacedValue = input.objectFields.firstOrNull { it.name == "as" }?.value as? StringValue - LinkImport(name = nameValue.value, `as` = namespacedValue?.value ?: nameValue.value) - } + is StringValue -> parseStringValue(input) { msg -> CoercingParseLiteralException(msg) } + is ObjectValue -> parseObjectValue(input) { msg -> CoercingParseLiteralException(msg) } else -> throw CoercingParseLiteralException( "Cannot parse $input to LinkImport. Expected AST type of `StringValue` or `ObjectValue` but was ${input.javaClass.simpleName} " ) } + private fun parseStringValue(input: StringValue, exception: (String) -> RuntimeException): LinkImport { + val value = input.value ?: throw exception("Cannot parse StringValue to LinkImport: string value was null") + return LinkImport(name = value, `as` = value) + } + + private fun parseObjectValue(input: ObjectValue, exception: (String) -> RuntimeException): LinkImport { + val nameValue = input.objectFields.firstOrNull { it.name == "name" }?.value as? StringValue + ?: throw exception("Cannot parse $input to LinkImport: missing or non-string 'name' field") + val name = nameValue.value ?: throw exception("Cannot parse $input to LinkImport: 'name' field StringValue was null") + val alias = (input.objectFields.firstOrNull { it.name == "as" }?.value as? StringValue)?.value ?: name + return LinkImport(name = name, `as` = alias) + } + override fun valueToLiteral(input: Any, graphQLContext: GraphQLContext, locale: Locale): Value<*> { return when (input) { is String -> StringValue.newStringValue(input).build() diff --git a/generator/graphql-kotlin-federation/src/main/kotlin/com/expediagroup/graphql/generator/federation/types/_Any.kt b/generator/graphql-kotlin-federation/src/main/kotlin/com/expediagroup/graphql/generator/federation/types/_Any.kt index f6617f9217..4104d6d386 100644 --- a/generator/graphql-kotlin-federation/src/main/kotlin/com/expediagroup/graphql/generator/federation/types/_Any.kt +++ b/generator/graphql-kotlin-federation/src/main/kotlin/com/expediagroup/graphql/generator/federation/types/_Any.kt @@ -53,7 +53,7 @@ private object AnyCoercing : Coercing { override fun parseLiteral(input: Value<*>, variables: CoercedVariables, graphQLContext: GraphQLContext, locale: Locale): Any = when (input) { is FloatValue -> input.value - is StringValue -> input.value + is StringValue -> input.value ?: throw CoercingParseLiteralException("Cannot parse null StringValue value to Any scalar") is IntValue -> input.value is BooleanValue -> input.isValue is EnumValue -> input.name diff --git a/generator/graphql-kotlin-federation/src/test/kotlin/com/expediagroup/graphql/generator/federation/execution/EntitiesDataFetcherTest.kt b/generator/graphql-kotlin-federation/src/test/kotlin/com/expediagroup/graphql/generator/federation/execution/EntitiesDataFetcherTest.kt index e1d60ecf19..6f1bd698de 100644 --- a/generator/graphql-kotlin-federation/src/test/kotlin/com/expediagroup/graphql/generator/federation/execution/EntitiesDataFetcherTest.kt +++ b/generator/graphql-kotlin-federation/src/test/kotlin/com/expediagroup/graphql/generator/federation/execution/EntitiesDataFetcherTest.kt @@ -46,7 +46,7 @@ class EntitiesDataFetcherTest { } val result = resolver.get(env).get() - verifyData(result.data, User(123, "testName")) + verifyData(result.data!!, User(123, "testName")) verifyErrors(result.errors) } @@ -60,7 +60,7 @@ class EntitiesDataFetcherTest { every { getArgumentOrDefault(any(), any()) } returns representations } val result = resolver.get(env).get() - verifyData(result.data, Author(1, "Author 1")) + verifyData(result.data!!, Author(1, "Author 1")) verifyErrors(result.errors) } @@ -76,7 +76,7 @@ class EntitiesDataFetcherTest { } val result = resolver.get(env).get() - verifyData(result.data, null) + verifyData(result.data!!, null) verifyErrors(result.errors, "Unable to resolve federated type, representation={__typename=User, userId=123, name=testName}") } @@ -91,7 +91,7 @@ class EntitiesDataFetcherTest { } val result = resolver.get(env).get() - verifyData(result.data, null) + verifyData(result.data!!, null) verifyErrors(result.errors, "Unable to resolve federated type, representation={}") } @@ -105,7 +105,7 @@ class EntitiesDataFetcherTest { } val result = resolver.get(env).get() - verifyData(result.data, null) + verifyData(result.data!!, null) verifyErrors(result.errors, "Unable to resolve federated type, representation={__typename=User, userId=123, name=testName}") } @@ -123,7 +123,7 @@ class EntitiesDataFetcherTest { } val result = resolver.get(env).get() - verifyData(result.data, null) + verifyData(result.data!!, null) verifyErrors(result.errors, "Exception was thrown while trying to resolve federated type, representation={__typename=User, userId=123, name=testName}") } @@ -145,7 +145,7 @@ class EntitiesDataFetcherTest { val resolver = EntitiesDataFetcher(listOf(spyUserResolver, spyBookResolver)) val result = resolver.get(env).get() - verifyData(result.data, user1, book, user2) + verifyData(result.data!!, user1, book, user2) verifyErrors(result.errors) coVerify(exactly = 1) { @@ -175,7 +175,7 @@ class EntitiesDataFetcherTest { val resolver = EntitiesDataFetcher(listOf(spyUserResolver, mockBookResolver)) val result = resolver.get(env).get() - verifyData(result.data, user, null) + verifyData(result.data!!, user, null) verifyErrors(result.errors, "Exception was thrown while trying to resolve federated type, representation={__typename=Book, id=988, weight=1.0}") coVerify { @@ -201,7 +201,7 @@ class EntitiesDataFetcherTest { val result = resolver.get(env).get() verifyData( - result.data, + result.data!!, User(123, "testName"), User(456, "testName 2"), Author(1, "Author 1"), diff --git a/generator/graphql-kotlin-federation/src/test/kotlin/com/expediagroup/graphql/generator/federation/types/AnyTest.kt b/generator/graphql-kotlin-federation/src/test/kotlin/com/expediagroup/graphql/generator/federation/types/AnyTest.kt index c0c7043d73..63087e674b 100644 --- a/generator/graphql-kotlin-federation/src/test/kotlin/com/expediagroup/graphql/generator/federation/types/AnyTest.kt +++ b/generator/graphql-kotlin-federation/src/test/kotlin/com/expediagroup/graphql/generator/federation/types/AnyTest.kt @@ -117,4 +117,16 @@ class AnyTest { actual = coercing.parseValue(1, GraphQLContext.getDefault(), Locale.ENGLISH) ) } + + @Test + fun `_Any scalar parseLiteral should throw exception when StringValue has null value`() { + assertThrows { + ANY_SCALAR_TYPE.coercing.parseLiteral( + StringValue.newStringValue().build(), + CoercedVariables.emptyVariables(), + GraphQLContext.getDefault(), + Locale.ENGLISH + ) + } + } } diff --git a/generator/graphql-kotlin-federation/src/test/kotlin/com/expediagroup/graphql/generator/federation/types/LinkImportTest.kt b/generator/graphql-kotlin-federation/src/test/kotlin/com/expediagroup/graphql/generator/federation/types/LinkImportTest.kt index d817ee7968..da0771cb5d 100644 --- a/generator/graphql-kotlin-federation/src/test/kotlin/com/expediagroup/graphql/generator/federation/types/LinkImportTest.kt +++ b/generator/graphql-kotlin-federation/src/test/kotlin/com/expediagroup/graphql/generator/federation/types/LinkImportTest.kt @@ -129,6 +129,36 @@ class LinkImportTest { } } + @Test + fun `parseValue should throw exception when StringValue has null value`() { + assertFailsWith { + coercing.parseValue(StringValue.newStringValue().build(), GraphQLContext.getDefault(), Locale.ENGLISH) + } + } + + @Test + fun `parseLiteral should throw exception when StringValue has null value`() { + assertFailsWith { + coercing.parseLiteral(StringValue.newStringValue().build(), CoercedVariables.emptyVariables(), GraphQLContext.getDefault(), Locale.ENGLISH) + } + } + + @Test + fun `parseValue should throw exception when ObjectValue name field has null StringValue`() { + val objectValue = ObjectValue(listOf(ObjectField("name", StringValue.newStringValue().build()))) + assertFailsWith { + coercing.parseValue(objectValue, GraphQLContext.getDefault(), Locale.ENGLISH) + } + } + + @Test + fun `parseLiteral should throw exception when ObjectValue name field has null StringValue`() { + val objectValue = ObjectValue(listOf(ObjectField("name", StringValue.newStringValue().build()))) + assertFailsWith { + coercing.parseLiteral(objectValue, CoercedVariables.emptyVariables(), GraphQLContext.getDefault(), Locale.ENGLISH) + } + } + @Test fun `valueToLiteral should map simple string import to StringValue`() { val result = coercing.valueToLiteral("@foo", GraphQLContext.getDefault(), Locale.ENGLISH) diff --git a/generator/graphql-kotlin-schema-generator/src/main/kotlin/com/expediagroup/graphql/generator/execution/FlowSubscriptionExecutionStrategy.kt b/generator/graphql-kotlin-schema-generator/src/main/kotlin/com/expediagroup/graphql/generator/execution/FlowSubscriptionExecutionStrategy.kt index 33a0607d67..a2ee7b80c0 100644 --- a/generator/graphql-kotlin-schema-generator/src/main/kotlin/com/expediagroup/graphql/generator/execution/FlowSubscriptionExecutionStrategy.kt +++ b/generator/graphql-kotlin-schema-generator/src/main/kotlin/com/expediagroup/graphql/generator/execution/FlowSubscriptionExecutionStrategy.kt @@ -20,208 +20,69 @@ import graphql.ExecutionResult import graphql.ExecutionResultImpl import graphql.execution.DataFetcherExceptionHandler import graphql.execution.ExecutionContext -import graphql.execution.ExecutionStepInfo -import graphql.execution.ExecutionStrategy import graphql.execution.ExecutionStrategyParameters import graphql.execution.FetchedValue import graphql.execution.SimpleDataFetcherExceptionHandler import graphql.execution.SubscriptionExecutionStrategy -import graphql.execution.instrumentation.ExecutionStrategyInstrumentationContext -import graphql.execution.instrumentation.SimpleInstrumentationContext -import graphql.execution.instrumentation.parameters.InstrumentationExecutionParameters -import graphql.execution.instrumentation.parameters.InstrumentationExecutionStrategyParameters -import graphql.execution.instrumentation.parameters.InstrumentationFieldParameters -import graphql.schema.GraphQLObjectType import kotlinx.coroutines.flow.Flow -import kotlinx.coroutines.flow.map -import kotlinx.coroutines.future.await import kotlinx.coroutines.reactive.asFlow +import kotlinx.coroutines.reactive.asPublisher import org.reactivestreams.Publisher -import java.util.Collections import java.util.concurrent.CompletableFuture /** - * [SubscriptionExecutionStrategy] replacement that and allows schema subscription functions - * to return either a [Flow] or a [Publisher]. + * [SubscriptionExecutionStrategy] subclass that additionally allows schema subscription functions + * to return a kotlinx [Flow]. * - * Note this implementation is mostly a java->kotlin copy of [SubscriptionExecutionStrategy], - * with updated [createSourceEventStream] that supports [Flow] and [Publisher]. Any returned - * [Flow]s will be automatically converted to corresponding [Publisher]. + * This delegates all execution logic — instrumentation, data loader dispatch, error handling, + * event processing — to the parent. It diverges in two places only: + * + * 1. [execute] — converts the [Publisher]<[ExecutionResult]> produced by the parent back into + * a kotlinx [Flow]<[ExecutionResult]> as the execution result data, to preserve the + * coroutine-friendly API for callers. + * 2. [fetchField] — converts any kotlinx [Flow] returned by a data fetcher into a + * reactive-streams [Publisher] before the parent's [createSourceEventStream] sees it, + * since the parent natively handles [Publisher] but not [Flow]. */ -class FlowSubscriptionExecutionStrategy(dfe: DataFetcherExceptionHandler) : ExecutionStrategy(dfe) { +class FlowSubscriptionExecutionStrategy(dfe: DataFetcherExceptionHandler) : SubscriptionExecutionStrategy(dfe) { constructor() : this(SimpleDataFetcherExceptionHandler()) override fun execute( executionContext: ExecutionContext, parameters: ExecutionStrategyParameters - ): CompletableFuture { - - val instrumentation = executionContext.instrumentation - val instrumentationParameters = InstrumentationExecutionStrategyParameters(executionContext, parameters) - val executionStrategyCtx = ExecutionStrategyInstrumentationContext.nonNullCtx( - instrumentation.beginExecutionStrategy( - instrumentationParameters, - executionContext.instrumentationState - ) - ) - - val sourceEventStream = createSourceEventStream(executionContext, parameters) - - // - // when the upstream source event stream completes, subscribe to it and wire in our adapter - val overallResult: CompletableFuture = sourceEventStream.thenApply { flow -> - if (flow == null) { - ExecutionResultImpl(null, executionContext.errors) + ): CompletableFuture = + super.execute(executionContext, parameters).thenApply { executionResult -> + val publisher = executionResult.getData?>() + if (publisher != null) { + ExecutionResultImpl(publisher.asFlow(), executionResult.errors) } else { - val returnFlow = flow.map { eventPayload: Any? -> - executeSubscriptionEvent( - executionContext, - parameters, - eventPayload - ).await() - } - ExecutionResultImpl(returnFlow, executionContext.errors) + executionResult } } - // dispatched the subscription query - executionStrategyCtx.onDispatched() - overallResult.whenComplete(executionStrategyCtx::onCompleted) - - return overallResult - } - - /* - https://github.com/facebook/graphql/blob/master/spec/Section%206%20--%20Execution.md - - CreateSourceEventStream(subscription, schema, variableValues, initialValue): - - Let {subscriptionType} be the root Subscription type in {schema}. - Assert: {subscriptionType} is an Object type. - Let {selectionSet} be the top level Selection Set in {subscription}. - Let {rootField} be the first top level field in {selectionSet}. - Let {argumentValues} be the result of {CoerceArgumentValues(subscriptionType, rootField, variableValues)}. - Let {fieldStream} be the result of running {ResolveFieldEventStream(subscriptionType, initialValue, rootField, argumentValues)}. - Return {fieldStream}. - */ - @Suppress("UNCHECKED_CAST") - private fun createSourceEventStream( + override fun fetchField( executionContext: ExecutionContext, parameters: ExecutionStrategyParameters - ): CompletableFuture?> { - val newParameters = firstFieldOfSubscriptionSelection(parameters) - - val fieldFetched: CompletableFuture = fetchField(executionContext, newParameters).let { fetchedValue -> - if (fetchedValue is CompletableFuture<*>) { - fetchedValue as CompletableFuture - } else { - CompletableFuture.completedFuture(fetchedValue as FetchedValue) - } - } - return fieldFetched.thenApply { fetchedValue -> - val flow = when (val publisherOrFlow: Any? = fetchedValue.fetchedValue) { - is Publisher<*> -> publisherOrFlow.asFlow() - // below explicit cast is required due to the type erasure and Kotlin declaration-site variance vs Java use-site variance - is Flow<*> -> publisherOrFlow - else -> null - } - flow - } + ): Any { + val result = super.fetchField(executionContext, parameters) + @Suppress("UNCHECKED_CAST") + return when { + result is CompletableFuture<*> -> (result as CompletableFuture).thenApply { value -> convertFlowToPublisher(value) } + else -> convertFlowToPublisher(result) + } as Any } - /* - ExecuteSubscriptionEvent(subscription, schema, variableValues, initialValue): - - Let {subscriptionType} be the root Subscription type in {schema}. - Assert: {subscriptionType} is an Object type. - Let {selectionSet} be the top level Selection Set in {subscription}. - Let {data} be the result of running {ExecuteSelectionSet(selectionSet, subscriptionType, initialValue, variableValues)} normally (allowing parallelization). - Let {errors} be any field errors produced while executing the selection set. - Return an unordered map containing {data} and {errors}. - - Note: The {ExecuteSubscriptionEvent()} algorithm is intentionally similar to {ExecuteQuery()} since this is how each event result is produced. + /** + * Converts any kotlinx [Flow] in the fetch result to a reactive-streams [Publisher] so that + * the parent [SubscriptionExecutionStrategy] can handle it natively. Handles both a bare [Flow] + * and a [Flow] wrapped inside a [FetchedValue]; all other values are passed through unchanged. */ - private fun executeSubscriptionEvent( - executionContext: ExecutionContext, - parameters: ExecutionStrategyParameters, - eventPayload: Any? - ): CompletableFuture { - val instrumentation = executionContext.instrumentation - - val newExecutionContext = executionContext.transform { builder -> - builder - .root(eventPayload) - .resetErrors() - } - val newParameters = firstFieldOfSubscriptionSelection(parameters) - val subscribedFieldStepInfo = createSubscribedFieldStepInfo(executionContext, newParameters) - - val i13nFieldParameters = InstrumentationFieldParameters(executionContext) { subscribedFieldStepInfo } - val subscribedFieldCtx = SimpleInstrumentationContext.nonNullCtx( - instrumentation.beginSubscribedFieldEvent( - i13nFieldParameters, executionContext.instrumentationState - ) - ) - - val fetchedValue = unboxPossibleDataFetcherResult(newExecutionContext, parameters, eventPayload) - - val fieldValueInfo = completeField(newExecutionContext, newParameters, fetchedValue) - val overallResult = fieldValueInfo - .fieldValueFuture - .thenApply { fv -> - wrapWithRootFieldName( - newParameters, - ExecutionResultImpl.newExecutionResult().data(fv).build() - ) - } - - // dispatch instrumentation so they can know about each subscription event - subscribedFieldCtx.onDispatched() - overallResult.whenComplete(subscribedFieldCtx::onCompleted) - - // allow them to instrument each ER should they want to - val i13nExecutionParameters = InstrumentationExecutionParameters( - executionContext.executionInput, executionContext.graphQLSchema - ) - - return overallResult.thenCompose { executionResult -> - instrumentation.instrumentExecutionResult(executionResult, i13nExecutionParameters, executionContext.instrumentationState) - } - } - - private fun wrapWithRootFieldName( - parameters: ExecutionStrategyParameters, - executionResult: ExecutionResult - ): ExecutionResult { - val rootFieldName = getRootFieldName(parameters) - return ExecutionResultImpl( - Collections.singletonMap(rootFieldName, executionResult.getData()), - executionResult.errors - ) - } - - private fun getRootFieldName(parameters: ExecutionStrategyParameters): String { - val rootField = parameters.field.singleField - return if (rootField.alias != null) rootField.alias else rootField.name - } - - private fun firstFieldOfSubscriptionSelection( - parameters: ExecutionStrategyParameters - ): ExecutionStrategyParameters { - val fields = parameters.fields - val firstField = fields.getSubField(fields.keys[0]) - - val fieldPath = parameters.path.segment(mkNameForPath(firstField.singleField)) - return parameters.transform { builder -> builder.field(firstField).path(fieldPath) } - } + @Suppress("UNCHECKED_CAST") + private fun convertFlowToPublisher(value: Any?): Any? = when (value) { + is Flow<*> -> (value as Flow).asPublisher() + is FetchedValue if value.fetchedValue is Flow<*> -> + FetchedValue((value.fetchedValue as Flow).asPublisher(), value.errors, value.localContext) - private fun createSubscribedFieldStepInfo( - executionContext: ExecutionContext, - parameters: ExecutionStrategyParameters - ): ExecutionStepInfo { - val field = parameters.field.singleField - val parentType = parameters.executionStepInfo.unwrappedNonNullType as GraphQLObjectType - val fieldDef = getFieldDef(executionContext.graphQLSchema, parentType, field) - return createExecutionStepInfo(executionContext, parameters, fieldDef, parentType) + else -> value } } diff --git a/generator/graphql-kotlin-schema-generator/src/main/kotlin/com/expediagroup/graphql/generator/execution/FunctionDataFetcher.kt b/generator/graphql-kotlin-schema-generator/src/main/kotlin/com/expediagroup/graphql/generator/execution/FunctionDataFetcher.kt index 645ea39718..3af86e827a 100644 --- a/generator/graphql-kotlin-schema-generator/src/main/kotlin/com/expediagroup/graphql/generator/execution/FunctionDataFetcher.kt +++ b/generator/graphql-kotlin-schema-generator/src/main/kotlin/com/expediagroup/graphql/generator/execution/FunctionDataFetcher.kt @@ -50,7 +50,7 @@ open class FunctionDataFetcher( * Invoke a suspend function or blocking function, passing in the [target] if not null or default to using the source from the environment. */ override fun get(environment: DataFetchingEnvironment): Any? { - val instance: Any? = target ?: environment.getSource() + val instance: Any? = target ?: environment.getSource() val instanceParameter = fn.instanceParameter return if (instance != null && instanceParameter != null) { diff --git a/generator/graphql-kotlin-schema-generator/src/main/kotlin/com/expediagroup/graphql/generator/execution/PropertyDataFetcher.kt b/generator/graphql-kotlin-schema-generator/src/main/kotlin/com/expediagroup/graphql/generator/execution/PropertyDataFetcher.kt index b83a0b61cc..6effb4ae1f 100644 --- a/generator/graphql-kotlin-schema-generator/src/main/kotlin/com/expediagroup/graphql/generator/execution/PropertyDataFetcher.kt +++ b/generator/graphql-kotlin-schema-generator/src/main/kotlin/com/expediagroup/graphql/generator/execution/PropertyDataFetcher.kt @@ -45,7 +45,7 @@ class PropertyDataFetcher(private val propertyGetter: KProperty.Getter<*>) : Lig * Invokes target getter function. */ override fun get(environment: DataFetchingEnvironment): Any? = - environment.getSource()?.let { instance -> + environment.getSource()?.let { instance -> propertyGetter.call(instance) } } diff --git a/generator/graphql-kotlin-schema-generator/src/test/kotlin/com/expediagroup/graphql/generator/execution/FlowSubscriptionExecutionStrategyTest.kt b/generator/graphql-kotlin-schema-generator/src/test/kotlin/com/expediagroup/graphql/generator/execution/FlowSubscriptionExecutionStrategyTest.kt index e084d0662f..7921702199 100644 --- a/generator/graphql-kotlin-schema-generator/src/test/kotlin/com/expediagroup/graphql/generator/execution/FlowSubscriptionExecutionStrategyTest.kt +++ b/generator/graphql-kotlin-schema-generator/src/test/kotlin/com/expediagroup/graphql/generator/execution/FlowSubscriptionExecutionStrategyTest.kt @@ -184,6 +184,36 @@ class FlowSubscriptionExecutionStrategyTest { } } + @Test + fun `verify subscription when fetchField returns raw Flow without CompletableFuture wrapping`() = runBlocking { + val request = ExecutionInput.newExecutionInput().query("subscription { syncTicker }").build() + val response = testGraphQL.execute(request) + val flow = response.getData>() + val list = mutableListOf() + flow.collect { + list.add(it.getData>().getValue("syncTicker")) + } + assertEquals(3, list.size) + for (i in list.indices) { + assertEquals(i + 1, list[i]) + } + } + + @Test + fun `verify subscription when fetchField returns FetchedValue wrapping a Flow`() = runBlocking { + val request = ExecutionInput.newExecutionInput().query("subscription { syncDataFetcher }").build() + val response = testGraphQL.execute(request) + val flow = response.getData>() + val list = mutableListOf() + flow.collect { + list.add(it.getData>().getValue("syncDataFetcher")) + } + assertEquals(3, list.size) + for (i in list.indices) { + assertEquals(i + 1, list[i]) + } + } + // GraphQL spec requires at least single query to be present as Query type is needed to run introspection queries // see: https://github.com/graphql/graphql-spec/issues/490 and https://github.com/graphql/graphql-spec/issues/568 class BasicQuery { @@ -258,5 +288,15 @@ class FlowSubscriptionExecutionStrategyTest { } } } + + // Exercises the non-CompletableFuture branch in createSourceEventStream (raw Flow return) + fun syncTicker(): Flow = flow { + for (i in 1..3) emit(i) + } + + // Exercises FetchedValue.getFetchedValue() unwrapping when fetchField returns a FetchedValue directly + fun syncDataFetcher(): Flow> = flow { + for (i in 1..3) emit(DataFetcherResult.newResult().data(i).build()) + } } } diff --git a/generator/graphql-kotlin-schema-generator/src/test/kotlin/com/expediagroup/graphql/generator/execution/FunctionDataFetcherTest.kt b/generator/graphql-kotlin-schema-generator/src/test/kotlin/com/expediagroup/graphql/generator/execution/FunctionDataFetcherTest.kt index 1907788742..3e6fe62ff6 100644 --- a/generator/graphql-kotlin-schema-generator/src/test/kotlin/com/expediagroup/graphql/generator/execution/FunctionDataFetcherTest.kt +++ b/generator/graphql-kotlin-schema-generator/src/test/kotlin/com/expediagroup/graphql/generator/execution/FunctionDataFetcherTest.kt @@ -20,6 +20,7 @@ import com.expediagroup.graphql.generator.annotations.GraphQLName import graphql.GraphQLContext import graphql.GraphQLException import graphql.schema.DataFetchingEnvironment +import graphql.schema.DataFetchingEnvironmentImpl import io.mockk.every import io.mockk.mockk import kotlinx.coroutines.coroutineScope @@ -166,12 +167,13 @@ class FunctionDataFetcherTest { @Test fun `default values are overridden when argument is passed as null`() { val dataFetcher = FunctionDataFetcher(target = null, fn = MyClass::printDefault) - val mockEnvironment: DataFetchingEnvironment = mockk { - every { getSource() } returns MyClass() - every { arguments } returns mapOf("string" to null) - every { containsArgument("string") } returns true - } - assertNull(dataFetcher.get(mockEnvironment)) + // DataFetchingEnvironment uses an internal ImmutableMapWithNullValues class to store arguments, which allows for null values + // Mockk does not support mocking this class, so we need to create a real instance of the environment with the arguments we want to test + val environment = DataFetchingEnvironmentImpl.newDataFetchingEnvironment() + .source(MyClass()) + .arguments(mapOf("string" to null)) + .build() + assertNull(dataFetcher.get(environment)) } @Test @@ -273,11 +275,10 @@ class FunctionDataFetcherTest { @Test fun `optional inputs return the value when null arguments are passed`() { val dataFetcher = FunctionDataFetcher(target = MyClass(), fn = MyClass::optionalWrapper) - val mockEnvironment: DataFetchingEnvironment = mockk { - every { arguments } returns mapOf("input" to null) - every { containsArgument("input") } returns true - } - assertEquals(expected = "input was null", actual = dataFetcher.get(mockEnvironment)) + val environment = DataFetchingEnvironmentImpl.newDataFetchingEnvironment() + .arguments(mapOf("input" to null)) + .build() + assertEquals(expected = "input was null", actual = dataFetcher.get(environment)) } @Test diff --git a/generator/graphql-kotlin-schema-generator/src/test/kotlin/com/expediagroup/graphql/generator/internal/types/GenerateFunctionTest.kt b/generator/graphql-kotlin-schema-generator/src/test/kotlin/com/expediagroup/graphql/generator/internal/types/GenerateFunctionTest.kt index ad3aba1549..126e4c895b 100644 --- a/generator/graphql-kotlin-schema-generator/src/test/kotlin/com/expediagroup/graphql/generator/internal/types/GenerateFunctionTest.kt +++ b/generator/graphql-kotlin-schema-generator/src/test/kotlin/com/expediagroup/graphql/generator/internal/types/GenerateFunctionTest.kt @@ -91,8 +91,6 @@ class GenerateFunctionTest : TypeTestHelper() { fun listDataFetcherResult(): DataFetcherResult> = DataFetcherResult.newResult>().data(listOf("Hello")).build() - fun nullableListDataFetcherResult(): DataFetcherResult?> = DataFetcherResult.newResult?>().data(listOf("Hello")).build() - fun dataFetcherCompletableFutureResult(): DataFetcherResult> { val completedFuture = CompletableFuture.completedFuture("Hello") return DataFetcherResult.newResult>().data(completedFuture).build() @@ -244,17 +242,6 @@ class GenerateFunctionTest : TypeTestHelper() { assertEquals(GraphQLString, stringType) } - @Test - fun `DataFetcherResult of a nullable List is valid and unwrapped in the schema`() { - val kFunction = Happy::nullableListDataFetcherResult - val result = generateFunction(generator, kClass = Happy::class, fn = kFunction, parentName = "Query", target = null, abstract = false) - - val listType = result.type - assertTrue(listType is GraphQLList) - val stringType = listType.wrappedType - assertEquals(GraphQLString, stringType) - } - @Test fun `DataFetcherResult of a CompletableFuture is invalid`() { val kFunction = Happy::dataFetcherCompletableFutureResult diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index a7e5b497c2..d8b10e764c 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -1,9 +1,9 @@ [versions] android-plugin = "8.5.0" classgraph = "4.8.184" -dataloader = "4.0.0" +dataloader = "6.0.0" federation = "5.5.0" -graphql-java = "24.3" +graphql-java = "25.0" graalvm = "0.11.5" jackson = "2.17.1" # kotlin version has to match the compile-testing compiler version diff --git a/plugins/client/graphql-kotlin-client-generator/src/main/kotlin/com/expediagroup/graphql/plugin/client/generator/GraphQLClientGenerator.kt b/plugins/client/graphql-kotlin-client-generator/src/main/kotlin/com/expediagroup/graphql/plugin/client/generator/GraphQLClientGenerator.kt index 74bd35edd7..241d66ec98 100644 --- a/plugins/client/graphql-kotlin-client-generator/src/main/kotlin/com/expediagroup/graphql/plugin/client/generator/GraphQLClientGenerator.kt +++ b/plugins/client/graphql-kotlin-client-generator/src/main/kotlin/com/expediagroup/graphql/plugin/client/generator/GraphQLClientGenerator.kt @@ -246,17 +246,28 @@ class GraphQLClientGenerator( } private fun findRootType(operationDefinition: OperationDefinition): ObjectTypeDefinition { - val operationNames = if (graphQLSchema.schemaDefinition().isPresent) { - graphQLSchema.schemaDefinition().get().operationTypeDefinitions.associateBy({ it.name.uppercase() }, { it.typeName.name }) - } else { - mapOf( - OperationDefinition.Operation.QUERY.name to "Query", - OperationDefinition.Operation.MUTATION.name to "Mutation", - OperationDefinition.Operation.SUBSCRIPTION.name to "Subscription" - ) - } - val rootType = operationNames[operationDefinition.operation.name] - return graphQLSchema.getType(rootType).get() as ObjectTypeDefinition + val operationName = operationDefinition.operation.name + val rootTypesByOperationName = graphQLSchema.schemaDefinition() + .map { schemaDef -> + // Get custom root type mappings from the schema definition, if available + schemaDef.operationTypeDefinitions.associateBy({ it.name.uppercase() }, { it.typeName.name }) + } + .orElseGet { + // If no schema definition is provided, use the default root type names + mapOf( + OperationDefinition.Operation.QUERY.name to "Query", + OperationDefinition.Operation.MUTATION.name to "Mutation", + OperationDefinition.Operation.SUBSCRIPTION.name to "Subscription" + ) + } + + val rootTypeName = rootTypesByOperationName[operationName] + ?: throw IllegalStateException("No root type mapping found for operation '$operationName'") + + val rootType = graphQLSchema.getTypeOrNull(rootTypeName) + ?: throw IllegalStateException("Root type '$rootTypeName' for operation '$operationName' not found in schema") + return rootType as? ObjectTypeDefinition + ?: throw IllegalStateException("Root type '$rootTypeName' is not an ObjectTypeDefinition (found ${rootType::class.simpleName})") } private fun parseSchema(path: String): TypeDefinitionRegistry { diff --git a/plugins/client/graphql-kotlin-client-generator/src/test/kotlin/com/expediagroup/graphql/plugin/client/generator/GenerateGraphQLClientIT.kt b/plugins/client/graphql-kotlin-client-generator/src/test/kotlin/com/expediagroup/graphql/plugin/client/generator/GenerateGraphQLClientIT.kt index f84df1faed..09eb54afbc 100755 --- a/plugins/client/graphql-kotlin-client-generator/src/test/kotlin/com/expediagroup/graphql/plugin/client/generator/GenerateGraphQLClientIT.kt +++ b/plugins/client/graphql-kotlin-client-generator/src/test/kotlin/com/expediagroup/graphql/plugin/client/generator/GenerateGraphQLClientIT.kt @@ -16,10 +16,13 @@ package com.expediagroup.graphql.plugin.client.generator +import org.junit.jupiter.api.Test import org.junit.jupiter.params.ParameterizedTest import org.junit.jupiter.params.provider.Arguments import org.junit.jupiter.params.provider.MethodSource import java.io.File +import java.nio.file.Files +import kotlin.test.assertTrue class GenerateGraphQLClientIT { @@ -34,6 +37,36 @@ class GenerateGraphQLClientIT { verifyClientGeneration(config, testDirectory) } + @Test + fun `verify generation falls back to default root mapping when schema definition is missing`() { + val testDir = Files.createTempDirectory("graphql-client-generator-root-types").toFile() + try { + val schemaFile = writeFile( + testDir, + "schema.graphql", + """ + type Query { + hello: String + } + """ + ) + val queryFile = writeFile( + testDir, + "defaultRootQuery.graphql", + """ + query DefaultRootQuery { + hello + } + """ + ) + + val generatedFiles = GraphQLClientGenerator(schemaFile.absolutePath, defaultConfig).generate(listOf(queryFile)) + assertTrue(generatedFiles.any { it.name == "DefaultRootQuery" }) + } finally { + testDir.deleteRecursively() + } + } + companion object { @JvmStatic fun generatorTests(): List = locateTestCaseArguments("src/test/data/generator") diff --git a/plugins/client/graphql-kotlin-client-generator/src/test/kotlin/com/expediagroup/graphql/plugin/client/generator/GenerateInvalidClientIT.kt b/plugins/client/graphql-kotlin-client-generator/src/test/kotlin/com/expediagroup/graphql/plugin/client/generator/GenerateInvalidClientIT.kt index 7aab2206d2..8e7a53ffb0 100755 --- a/plugins/client/graphql-kotlin-client-generator/src/test/kotlin/com/expediagroup/graphql/plugin/client/generator/GenerateInvalidClientIT.kt +++ b/plugins/client/graphql-kotlin-client-generator/src/test/kotlin/com/expediagroup/graphql/plugin/client/generator/GenerateInvalidClientIT.kt @@ -22,6 +22,7 @@ import org.junit.jupiter.params.ParameterizedTest import org.junit.jupiter.params.provider.Arguments import org.junit.jupiter.params.provider.MethodSource import java.io.File +import java.nio.file.Files import kotlin.test.assertEquals import kotlin.test.assertFails @@ -56,6 +57,115 @@ class GenerateInvalidClientIT { assertEquals(SchemaUnavailableException::class, exception::class) } + @Test + fun `verify missing operation mapping in schema definition throws meaningful exception`() { + val testDir = Files.createTempDirectory("graphql-client-generator-root-types").toFile() + try { + val schemaFile = writeFile( + testDir, + "schema.graphql", + """ + schema { + query: Query + } + + type Query { + hello: String + } + """ + ) + val queryFile = writeFile( + testDir, + "mutationQuery.graphql", + """ + mutation MutationQuery { + setHello + } + """ + ) + + val exception = assertFails { + GraphQLClientGenerator(schemaFile.absolutePath, defaultConfig).generate(listOf(queryFile)) + } + assertEquals("No root type mapping found for operation 'MUTATION'", exception.message) + } finally { + testDir.deleteRecursively() + } + } + + @Test + fun `verify missing root type throws meaningful exception`() { + val testDir = Files.createTempDirectory("graphql-client-generator-root-types").toFile() + try { + val schemaFile = writeFile( + testDir, + "schema.graphql", + """ + schema { + query: MissingRootType + } + + type Query { + hello: String + } + """ + ) + val queryFile = writeFile( + testDir, + "missingRootTypeQuery.graphql", + """ + query MissingRootTypeQuery { + hello + } + """ + ) + + val exception = assertFails { + GraphQLClientGenerator(schemaFile.absolutePath, defaultConfig).generate(listOf(queryFile)) + } + assertEquals("Root type 'MissingRootType' for operation 'QUERY' not found in schema", exception.message) + } finally { + testDir.deleteRecursively() + } + } + + @Test + fun `verify non-object root type throws meaningful exception`() { + val testDir = Files.createTempDirectory("graphql-client-generator-root-types").toFile() + try { + val schemaFile = writeFile( + testDir, + "schema.graphql", + """ + schema { + query: RootScalar + } + + scalar RootScalar + """ + ) + val queryFile = writeFile( + testDir, + "nonObjectRootQuery.graphql", + """ + query NonObjectRootQuery { + __typename + } + """ + ) + + val exception = assertFails { + GraphQLClientGenerator(schemaFile.absolutePath, defaultConfig).generate(listOf(queryFile)) + } + assertEquals( + "Root type 'RootScalar' is not an ObjectTypeDefinition (found ScalarTypeDefinition)", + exception.message + ) + } finally { + testDir.deleteRecursively() + } + } + companion object { @JvmStatic fun invalidTests(): List = locateTestCaseArguments("src/test/data/invalid") diff --git a/plugins/client/graphql-kotlin-client-generator/src/test/kotlin/com/expediagroup/graphql/plugin/client/generator/GraphQLTestUtils.kt b/plugins/client/graphql-kotlin-client-generator/src/test/kotlin/com/expediagroup/graphql/plugin/client/generator/GraphQLTestUtils.kt index 97e739ab7a..b00d5079b3 100644 --- a/plugins/client/graphql-kotlin-client-generator/src/test/kotlin/com/expediagroup/graphql/plugin/client/generator/GraphQLTestUtils.kt +++ b/plugins/client/graphql-kotlin-client-generator/src/test/kotlin/com/expediagroup/graphql/plugin/client/generator/GraphQLTestUtils.kt @@ -52,6 +52,12 @@ internal fun locateTestFiles(directory: File): Pair, Map DataFetchingEnvironment.getValueFromDataLoader(dataLoaderName: String, key: K): CompletableFuture { - val loader = getDataLoader(dataLoaderName) ?: throw MissingDataLoaderException(dataLoaderName) - return loader.load(key, this.graphQlContext) +inline fun DataFetchingEnvironment.getValueFromDataLoader(dataLoaderName: String, key: K): CompletableFuture { + val loader = getDataLoader(dataLoaderName) ?: throw MissingDataLoaderException(dataLoaderName) + return loader.load(key, this.graphQlContext).thenApply { maybeUnwrapOptional(it) } } /** -* Helper method to get values from a registered DataLoader. -*/ -fun DataFetchingEnvironment.getValuesFromDataLoader(dataLoaderName: String, keys: List): CompletableFuture> { - val loader = getDataLoader(dataLoaderName) ?: throw MissingDataLoaderException(dataLoaderName) - return loader.loadMany(keys, listOf(this.graphQlContext)) + * Helper method to get values from a registered DataLoader. + */ +inline fun DataFetchingEnvironment.getValuesFromDataLoader(dataLoaderName: String, keys: List): CompletableFuture> { + val loader = getDataLoader(dataLoaderName) ?: throw MissingDataLoaderException(dataLoaderName) + return loader.loadMany(keys, listOf(this.graphQlContext)).thenApply { values -> values.map { maybeUnwrapOptional(it) } } } +/** + * Returns the value in the shape requested by [V] while supporting DataLoaders that use [Optional] wrappers. + * + * - If the caller requested `Optional`, the Optional is returned as-is. + * - If the caller requested a non-Optional type and the runtime value is `Optional`, it is unwrapped to `T?`. + * - Otherwise, the runtime value is cast directly to [V]. + */ +@PublishedApi +@Suppress("UNCHECKED_CAST") +internal inline fun maybeUnwrapOptional(value: Any?): V = + if (V::class == Optional::class) { + // reified V preserves the caller's requested return classifier, so we can detect Optional requests here. + value as V + } else if (value is Optional<*>) { + // This check uses the runtime payload shape; unwrap only when caller did not request Optional<...>. + value.orElse(null) as V + } else { + // No Optional wrapper at runtime, so just cast to the caller's requested type. + value as V + } + /** * Returns a value from the graphQLContext by KClass key * @return a value or null diff --git a/servers/graphql-kotlin-server/src/main/kotlin/com/expediagroup/graphql/server/extensions/responseExtensions.kt b/servers/graphql-kotlin-server/src/main/kotlin/com/expediagroup/graphql/server/extensions/responseExtensions.kt index e2736899f2..f098ce2232 100644 --- a/servers/graphql-kotlin-server/src/main/kotlin/com/expediagroup/graphql/server/extensions/responseExtensions.kt +++ b/servers/graphql-kotlin-server/src/main/kotlin/com/expediagroup/graphql/server/extensions/responseExtensions.kt @@ -29,7 +29,7 @@ import graphql.language.SourceLocation */ fun ExecutionResult.toGraphQLResponse(): GraphQLResponse<*> { val data: Any? = getData() - val filteredErrors: List? = if (errors?.isNotEmpty() == true) errors?.map { it.toGraphQLKotlinType() } else null + val filteredErrors: List? = errors?.takeIf { it.isNotEmpty() }?.map { it.toGraphQLKotlinType() } val filteredExtensions: Map? = if (extensions?.isNotEmpty() == true) extensions else null return GraphQLResponse(data, filteredErrors, filteredExtensions) } diff --git a/servers/graphql-kotlin-server/src/test/kotlin/com/expediagroup/graphql/server/extensions/DataFetchingEnvironmentExtensionsKtTest.kt b/servers/graphql-kotlin-server/src/test/kotlin/com/expediagroup/graphql/server/extensions/DataFetchingEnvironmentExtensionsKtTest.kt index 5f5c02660c..7ca5a33ab3 100644 --- a/servers/graphql-kotlin-server/src/test/kotlin/com/expediagroup/graphql/server/extensions/DataFetchingEnvironmentExtensionsKtTest.kt +++ b/servers/graphql-kotlin-server/src/test/kotlin/com/expediagroup/graphql/server/extensions/DataFetchingEnvironmentExtensionsKtTest.kt @@ -21,9 +21,11 @@ import graphql.schema.DataFetchingEnvironment import io.mockk.every import io.mockk.mockk import org.junit.jupiter.api.Test +import java.util.Optional import java.util.concurrent.CompletableFuture import kotlin.test.assertEquals import kotlin.test.assertFailsWith +import kotlin.test.assertNull class DataFetchingEnvironmentExtensionsKtTest { @Test @@ -66,4 +68,49 @@ class DataFetchingEnvironmentExtensionsKtTest { dataFetchingEnvironment.getValueFromDataLoader("foo", "bar") } } + + @Test + fun `getting nullable value from dataloader unwraps optional`() { + val dataFetchingEnvironment = mockk { + every { graphQlContext } returns mockk() + every { getDataLoader>("foo") } returns mockk { + every { load("bar", any()) } returns CompletableFuture.completedFuture(Optional.of("123")) + } + } + + val result: CompletableFuture = dataFetchingEnvironment.getValueFromDataLoader("foo", "bar") + + assertEquals("123", result.get()) + } + + @Test + fun `getting optional value from dataloader does not unwrap optional`() { + val dataFetchingEnvironment = mockk { + every { graphQlContext } returns mockk() + every { getDataLoader>("foo") } returns mockk { + every { load("bar", any()) } returns CompletableFuture.completedFuture(Optional.of("123")) + } + } + + val result: CompletableFuture> = dataFetchingEnvironment.getValueFromDataLoader("foo", "bar") + + assertEquals("123", result.get().orElse(null)) + } + + @Test + fun `getting nullable values from dataloader unwraps optionals`() { + val dataFetchingEnvironment = mockk { + every { graphQlContext } returns mockk() + every { getDataLoader>("foo") } returns mockk { + every { loadMany(listOf("bar", "baz"), any()) } returns + CompletableFuture.completedFuture(listOf(Optional.of("123"), Optional.empty())) + } + } + + val result: CompletableFuture> = dataFetchingEnvironment.getValuesFromDataLoader("foo", listOf("bar", "baz")) + + assertEquals(2, result.get().size) + assertEquals("123", result.get().first()) + assertNull(result.get().last()) + } } diff --git a/servers/graphql-kotlin-spring-server/src/test/kotlin/com/expediagroup/graphql/server/spring/execution/SpringGraphQLSubscriptionHandlerTest.kt b/servers/graphql-kotlin-spring-server/src/test/kotlin/com/expediagroup/graphql/server/spring/execution/SpringGraphQLSubscriptionHandlerTest.kt index 93b0c1fe5e..81fe39c09d 100644 --- a/servers/graphql-kotlin-spring-server/src/test/kotlin/com/expediagroup/graphql/server/spring/execution/SpringGraphQLSubscriptionHandlerTest.kt +++ b/servers/graphql-kotlin-spring-server/src/test/kotlin/com/expediagroup/graphql/server/spring/execution/SpringGraphQLSubscriptionHandlerTest.kt @@ -21,8 +21,8 @@ import com.expediagroup.graphql.generator.TopLevelObject import com.expediagroup.graphql.generator.exceptions.GraphQLKotlinException import com.expediagroup.graphql.generator.execution.FlowSubscriptionExecutionStrategy import com.expediagroup.graphql.generator.toSchema -import com.expediagroup.graphql.dataloader.KotlinDataLoaderRegistryFactory import com.expediagroup.graphql.dataloader.KotlinDataLoader +import com.expediagroup.graphql.dataloader.KotlinDataLoaderRegistryFactory import com.expediagroup.graphql.generator.extensions.toGraphQLContext import com.expediagroup.graphql.server.execution.GraphQLRequestHandler import com.expediagroup.graphql.server.extensions.getValueFromDataLoader @@ -34,6 +34,7 @@ import graphql.schema.GraphQLSchema import kotlinx.coroutines.reactor.asFlux import org.dataloader.DataLoader import org.dataloader.DataLoaderFactory +import org.dataloader.DataLoaderOptions import org.junit.jupiter.api.Test import reactor.core.publisher.Flux import reactor.kotlin.core.publisher.toFlux @@ -43,6 +44,7 @@ import java.time.Duration import java.util.concurrent.CompletableFuture import kotlin.random.Random import kotlin.test.assertEquals +import kotlin.test.assertFalse import kotlin.test.assertNotNull import kotlin.test.assertNull import kotlin.test.assertTrue @@ -60,11 +62,12 @@ class SpringGraphQLSubscriptionHandlerTest { private val mockLoader: KotlinDataLoader = object : KotlinDataLoader { override val dataLoaderName: String = "MockDataLoader" override fun getDataLoader(graphQLContext: GraphQLContext): DataLoader = - DataLoaderFactory.newDataLoader { ids -> - CompletableFuture.supplyAsync { - ids.map { "$it:value" } - } - } + DataLoaderFactory.newDataLoader( + { ids: MutableList -> + CompletableFuture.completedFuture(ids.map { "$it:value" }) + }, + DataLoaderOptions.newOptions().setBatchingEnabled(false).build() + ) } private val dataLoaderRegistryFactory = KotlinDataLoaderRegistryFactory(listOf(mockLoader)) private val subscriptionHandler = GraphQLRequestHandler(testGraphQL, dataLoaderRegistryFactory) @@ -87,7 +90,7 @@ class SpringGraphQLSubscriptionHandlerTest { true } .expectComplete() - .verify() + .verify(Duration.ofSeconds(10)) } @Test @@ -99,7 +102,82 @@ class SpringGraphQLSubscriptionHandlerTest { ).asFlux() StepVerifier.create(responseFlux) - .thenConsumeWhile { response -> + .assertNext { response -> + assertNotNull(response.data as? Map<*, *>) { data -> + assertNotNull(data["dataLoaderValue"] as? String) { value -> + assertEquals("foo:value", value) + } + } + assertNull(response.errors) + assertNull(response.extensions) + } + .thenCancel() + .verify(Duration.ofSeconds(10)) + } + + @Test + fun `verify subscription with data loader and batching enabled`() { + // Use a batching-enabled loader here so we still cover the normal DataLoader dispatch path for subscriptions. + val batchingLoader = object : KotlinDataLoader { + override val dataLoaderName: String = "MockDataLoader" + override fun getDataLoader(graphQLContext: GraphQLContext): DataLoader = + DataLoaderFactory.newDataLoader( + { ids: MutableList -> + CompletableFuture.completedFuture(ids.map { "$it:value" }) + }, + DataLoaderOptions.newOptions().setBatchingEnabled(true).build() + ) + } + val batchingHandler = GraphQLRequestHandler(testGraphQL, KotlinDataLoaderRegistryFactory(listOf(batchingLoader))) + val graphQLContext = emptyMap().toGraphQLContext() + val request = GraphQLRequest(query = "subscription { dataLoaderValue }") + val responseFlux = batchingHandler.executeSubscription(request, graphQLContext).asFlux() + + StepVerifier.create(responseFlux) + .then { + // Subscription execution doesn't automatically flush this test loader, so dispatch explicitly to release the value. + val registry = graphQLContext.get(org.dataloader.DataLoaderRegistry::class) + assertNotNull(registry) + registry.dispatchAll() + } + .assertNext { response -> + assertNotNull(response.data as? Map<*, *>) { data -> + assertNotNull(data["dataLoaderValue"] as? String) { value -> + assertEquals("foo:value", value) + } + } + assertNull(response.errors) + assertNull(response.extensions) + } + .expectComplete() + .verify(Duration.ofSeconds(10)) + } + + @Test + fun `verify subscription with data loader async completion`() { + val pendingLoad = CompletableFuture>() + val asyncLoader = object : KotlinDataLoader { + override val dataLoaderName: String = "MockDataLoader" + override fun getDataLoader(graphQLContext: GraphQLContext): DataLoader = + DataLoaderFactory.newDataLoader( + { _: MutableList -> pendingLoad }, + DataLoaderOptions.newOptions().setBatchingEnabled(false).build() + ) + } + val asyncHandler = GraphQLRequestHandler(testGraphQL, KotlinDataLoaderRegistryFactory(listOf(asyncLoader))) + val request = GraphQLRequest(query = "subscription { dataLoaderValue }") + val responseFlux = asyncHandler.executeSubscription(request, emptyMap().toGraphQLContext()).asFlux() + + StepVerifier.create(responseFlux) + // Keep the future pending first so we can prove the subscription waits for the async DataLoader result. + .expectSubscription() + .expectNoEvent(Duration.ofMillis(100)) + .then { + assertFalse(pendingLoad.isDone) + // Complete the future under test control to make the async handoff deterministic instead of scheduler-driven. + pendingLoad.complete(listOf("foo:value")) + } + .assertNext { response -> assertNotNull(response.data as? Map<*, *>) { data -> assertNotNull(data["dataLoaderValue"] as? String) { value -> assertEquals("foo:value", value) @@ -107,10 +185,9 @@ class SpringGraphQLSubscriptionHandlerTest { } assertNull(response.errors) assertNull(response.extensions) - true } .expectComplete() - .verify() + .verify(Duration.ofSeconds(10)) } @Test @@ -132,7 +209,7 @@ class SpringGraphQLSubscriptionHandlerTest { true } .expectComplete() - .verify() + .verify(Duration.ofSeconds(10)) } @Test @@ -154,7 +231,7 @@ class SpringGraphQLSubscriptionHandlerTest { assertNull(response.extensions) } .expectComplete() - .verify() + .verify(Duration.ofSeconds(10)) } // GraphQL spec requires at least single query to be present as Query type is needed to run introspection queries