diff --git a/executions/graphql-kotlin-dataloader-instrumentation/src/main/kotlin/com/expediagroup/graphql/dataloader/instrumentation/extensions/ExecutionContextExtensions.kt b/executions/graphql-kotlin-dataloader-instrumentation/src/main/kotlin/com/expediagroup/graphql/dataloader/instrumentation/extensions/ExecutionContextExtensions.kt index b73206ef2a..97e662de27 100644 --- a/executions/graphql-kotlin-dataloader-instrumentation/src/main/kotlin/com/expediagroup/graphql/dataloader/instrumentation/extensions/ExecutionContextExtensions.kt +++ b/executions/graphql-kotlin-dataloader-instrumentation/src/main/kotlin/com/expediagroup/graphql/dataloader/instrumentation/extensions/ExecutionContextExtensions.kt @@ -19,6 +19,7 @@ package com.expediagroup.graphql.dataloader.instrumentation.extensions import graphql.analysis.QueryTraverser import graphql.analysis.QueryVisitorFieldEnvironment import graphql.execution.ExecutionContext +import graphql.language.OperationDefinition import kotlin.math.max /** @@ -41,3 +42,10 @@ internal fun ExecutionContext.getDocumentHeight(): Int { 0 ) } + +/** + * Checks if the [ExecutionContext] is a [OperationDefinition.Operation.MUTATION] + * @return Boolean indicating if GraphQL Operation is a Mutation + */ +internal fun ExecutionContext.isMutation(): Boolean = + this.operationDefinition.operation == OperationDefinition.Operation.MUTATION diff --git a/executions/graphql-kotlin-dataloader-instrumentation/src/main/kotlin/com/expediagroup/graphql/dataloader/instrumentation/level/execution/AbstractExecutionLevelDispatchedInstrumentation.kt b/executions/graphql-kotlin-dataloader-instrumentation/src/main/kotlin/com/expediagroup/graphql/dataloader/instrumentation/level/execution/AbstractExecutionLevelDispatchedInstrumentation.kt index d94149e5f3..57290c6dac 100644 --- a/executions/graphql-kotlin-dataloader-instrumentation/src/main/kotlin/com/expediagroup/graphql/dataloader/instrumentation/level/execution/AbstractExecutionLevelDispatchedInstrumentation.kt +++ b/executions/graphql-kotlin-dataloader-instrumentation/src/main/kotlin/com/expediagroup/graphql/dataloader/instrumentation/level/execution/AbstractExecutionLevelDispatchedInstrumentation.kt @@ -17,6 +17,7 @@ package com.expediagroup.graphql.dataloader.instrumentation.level.execution import com.expediagroup.graphql.dataloader.instrumentation.NoOpExecutionStrategyInstrumentationContext +import com.expediagroup.graphql.dataloader.instrumentation.extensions.isMutation import com.expediagroup.graphql.dataloader.instrumentation.level.state.ExecutionLevelDispatchedState import com.expediagroup.graphql.dataloader.instrumentation.level.state.Level import graphql.ExecutionInput @@ -54,16 +55,16 @@ abstract class AbstractExecutionLevelDispatchedInstrumentation : SimpleInstrumen override fun beginExecuteOperation( parameters: InstrumentationExecuteOperationParameters ): InstrumentationContext = - parameters.executionContext - .graphQLContext.get(ExecutionLevelDispatchedState::class) + parameters.executionContext.takeIf { !it.isMutation() } + ?.graphQLContext?.get(ExecutionLevelDispatchedState::class) ?.beginExecuteOperation(parameters) ?: SimpleInstrumentationContext.noOp() override fun beginExecutionStrategy( parameters: InstrumentationExecutionStrategyParameters ): ExecutionStrategyInstrumentationContext = - parameters.executionContext - .graphQLContext.get(ExecutionLevelDispatchedState::class) + parameters.executionContext.takeIf { !it.isMutation() } + ?.graphQLContext?.get(ExecutionLevelDispatchedState::class) ?.beginExecutionStrategy( parameters, this.getOnLevelDispatchedCallback( @@ -78,8 +79,8 @@ abstract class AbstractExecutionLevelDispatchedInstrumentation : SimpleInstrumen override fun beginFieldFetch( parameters: InstrumentationFieldFetchParameters ): InstrumentationContext = - parameters.executionContext - .graphQLContext.get(ExecutionLevelDispatchedState::class) + parameters.executionContext.takeIf { !it.isMutation() } + ?.graphQLContext?.get(ExecutionLevelDispatchedState::class) ?.beginFieldFetch( parameters, this.getOnLevelDispatchedCallback( @@ -95,8 +96,8 @@ abstract class AbstractExecutionLevelDispatchedInstrumentation : SimpleInstrumen dataFetcher: DataFetcher<*>, parameters: InstrumentationFieldFetchParameters ): DataFetcher<*> = - parameters.executionContext - .graphQLContext.get(ExecutionLevelDispatchedState::class) + parameters.executionContext.takeIf { !it.isMutation() } + ?.graphQLContext?.get(ExecutionLevelDispatchedState::class) ?.instrumentDataFetcher(dataFetcher, parameters) ?: dataFetcher } diff --git a/executions/graphql-kotlin-dataloader-instrumentation/src/main/kotlin/com/expediagroup/graphql/dataloader/instrumentation/syncexhaustion/execution/AbstractSyncExecutionExhaustedInstrumentation.kt b/executions/graphql-kotlin-dataloader-instrumentation/src/main/kotlin/com/expediagroup/graphql/dataloader/instrumentation/syncexhaustion/execution/AbstractSyncExecutionExhaustedInstrumentation.kt index 295079bce7..ac3e67c933 100644 --- a/executions/graphql-kotlin-dataloader-instrumentation/src/main/kotlin/com/expediagroup/graphql/dataloader/instrumentation/syncexhaustion/execution/AbstractSyncExecutionExhaustedInstrumentation.kt +++ b/executions/graphql-kotlin-dataloader-instrumentation/src/main/kotlin/com/expediagroup/graphql/dataloader/instrumentation/syncexhaustion/execution/AbstractSyncExecutionExhaustedInstrumentation.kt @@ -17,6 +17,7 @@ package com.expediagroup.graphql.dataloader.instrumentation.syncexhaustion.execution import com.expediagroup.graphql.dataloader.instrumentation.NoOpExecutionStrategyInstrumentationContext +import com.expediagroup.graphql.dataloader.instrumentation.extensions.isMutation import com.expediagroup.graphql.dataloader.instrumentation.syncexhaustion.state.SyncExecutionExhaustedState import graphql.ExecutionInput import graphql.ExecutionResult @@ -54,24 +55,24 @@ abstract class AbstractSyncExecutionExhaustedInstrumentation : SimpleInstrumenta override fun beginExecuteOperation( parameters: InstrumentationExecuteOperationParameters ): InstrumentationContext = - parameters.executionContext - .graphQLContext.get(SyncExecutionExhaustedState::class) + parameters.executionContext.takeIf { !it.isMutation() } + ?.graphQLContext?.get(SyncExecutionExhaustedState::class) ?.beginExecuteOperation(parameters) ?: SimpleInstrumentationContext.noOp() override fun beginExecutionStrategy( parameters: InstrumentationExecutionStrategyParameters ): ExecutionStrategyInstrumentationContext = - parameters.executionContext - .graphQLContext.get(SyncExecutionExhaustedState::class) + parameters.executionContext.takeIf { !it.isMutation() } + ?.graphQLContext?.get(SyncExecutionExhaustedState::class) ?.beginExecutionStrategy(parameters) ?: NoOpExecutionStrategyInstrumentationContext override fun beginFieldFetch( parameters: InstrumentationFieldFetchParameters ): InstrumentationContext = - parameters.executionContext - .graphQLContext.get(SyncExecutionExhaustedState::class) + parameters.executionContext.takeIf { !it.isMutation() } + ?.graphQLContext?.get(SyncExecutionExhaustedState::class) ?.beginFieldFetch( parameters, this.getOnSyncExecutionExhaustedCallback( diff --git a/executions/graphql-kotlin-dataloader-instrumentation/src/test/kotlin/com/expediagroup/graphql/dataloader/instrumentation/fixture/AstronautGraphQL.kt b/executions/graphql-kotlin-dataloader-instrumentation/src/test/kotlin/com/expediagroup/graphql/dataloader/instrumentation/fixture/AstronautGraphQL.kt index 52a3da5b71..250f080c07 100644 --- a/executions/graphql-kotlin-dataloader-instrumentation/src/test/kotlin/com/expediagroup/graphql/dataloader/instrumentation/fixture/AstronautGraphQL.kt +++ b/executions/graphql-kotlin-dataloader-instrumentation/src/test/kotlin/com/expediagroup/graphql/dataloader/instrumentation/fixture/AstronautGraphQL.kt @@ -22,6 +22,7 @@ import com.expediagroup.graphql.dataloader.instrumentation.fixture.datafetcher.A import com.expediagroup.graphql.dataloader.instrumentation.fixture.domain.Astronaut import com.expediagroup.graphql.dataloader.instrumentation.fixture.datafetcher.AstronautService import com.expediagroup.graphql.dataloader.instrumentation.fixture.datafetcher.AstronautServiceRequest +import com.expediagroup.graphql.dataloader.instrumentation.fixture.datafetcher.CreateAstronautServiceRequest import com.expediagroup.graphql.dataloader.instrumentation.fixture.datafetcher.MissionDataLoader import com.expediagroup.graphql.dataloader.instrumentation.fixture.datafetcher.MissionService import com.expediagroup.graphql.dataloader.instrumentation.fixture.datafetcher.MissionServiceRequest @@ -56,6 +57,9 @@ object AstronautGraphQL { missions(ids: [ID!]): [Mission]! nasa: Nasa! } + type Mutation { + createAstronaut(name: String!): Astronaut! + } type Nasa { astronaut(id: ID!): Astronaut mission(id: ID!): Mission @@ -95,6 +99,14 @@ object AstronautGraphQL { environment ) } + private val createAstronautDataFetcher = DataFetcher { environment -> + astronautService.createAstronaut( + CreateAstronautServiceRequest( + environment.getArgument("name") + ), + environment + ) + } private val astronautsDataFetcher = DataFetcher { environment -> astronautService.getAstronauts( environment.getArgument>("ids") @@ -155,6 +167,10 @@ object AstronautGraphQL { .dataFetcher("missions", missionsDataFetcher) .dataFetcher("nasa") { Nasa() } ) + type( + TypeRuntimeWiring.newTypeWiring("Mutation") + .dataFetcher("createAstronaut", createAstronautDataFetcher) + ) type( TypeRuntimeWiring.newTypeWiring("Astronaut") .dataFetcher("missions", missionsByAstronautDataFetcher) diff --git a/executions/graphql-kotlin-dataloader-instrumentation/src/test/kotlin/com/expediagroup/graphql/dataloader/instrumentation/fixture/datafetcher/AstronautService.kt b/executions/graphql-kotlin-dataloader-instrumentation/src/test/kotlin/com/expediagroup/graphql/dataloader/instrumentation/fixture/datafetcher/AstronautService.kt index f06a154cea..19cb36020c 100644 --- a/executions/graphql-kotlin-dataloader-instrumentation/src/test/kotlin/com/expediagroup/graphql/dataloader/instrumentation/fixture/datafetcher/AstronautService.kt +++ b/executions/graphql-kotlin-dataloader-instrumentation/src/test/kotlin/com/expediagroup/graphql/dataloader/instrumentation/fixture/datafetcher/AstronautService.kt @@ -28,10 +28,13 @@ import org.dataloader.DataLoader import org.dataloader.DataLoaderFactory import org.dataloader.DataLoaderOptions import org.dataloader.stats.SimpleStatisticsCollector +import reactor.kotlin.core.publisher.toMono +import java.time.Duration import java.util.Optional import java.util.concurrent.CompletableFuture data class AstronautServiceRequest(val id: Int) +data class CreateAstronautServiceRequest(val name: String) class AstronautDataLoader : KotlinDataLoader { override val dataLoaderName: String = "AstronautDataLoader" @@ -57,6 +60,12 @@ class AstronautService { .getDataLoader("AstronautDataLoader") .load(request) + fun createAstronaut( + request: CreateAstronautServiceRequest, + environment: DataFetchingEnvironment + ): CompletableFuture = + Astronaut(100, request.name).toMono().delayElement(Duration.ofMillis(100)).toFuture() + fun getAstronauts( requests: List, environment: DataFetchingEnvironment diff --git a/executions/graphql-kotlin-dataloader-instrumentation/src/test/kotlin/com/expediagroup/graphql/dataloader/instrumentation/level/DataLoaderLevelDispatchedInstrumentationTest.kt b/executions/graphql-kotlin-dataloader-instrumentation/src/test/kotlin/com/expediagroup/graphql/dataloader/instrumentation/level/DataLoaderLevelDispatchedInstrumentationTest.kt index 2f9f0adba9..68cdad68a9 100644 --- a/executions/graphql-kotlin-dataloader-instrumentation/src/test/kotlin/com/expediagroup/graphql/dataloader/instrumentation/level/DataLoaderLevelDispatchedInstrumentationTest.kt +++ b/executions/graphql-kotlin-dataloader-instrumentation/src/test/kotlin/com/expediagroup/graphql/dataloader/instrumentation/level/DataLoaderLevelDispatchedInstrumentationTest.kt @@ -16,19 +16,29 @@ package com.expediagroup.graphql.dataloader.instrumentation.level -import com.expediagroup.graphql.dataloader.instrumentation.fixture.DataLoaderInstrumentationStrategy import com.expediagroup.graphql.dataloader.instrumentation.fixture.AstronautGraphQL +import com.expediagroup.graphql.dataloader.instrumentation.fixture.DataLoaderInstrumentationStrategy +import io.mockk.Called +import io.mockk.clearAllMocks +import io.mockk.spyk import io.mockk.verify +import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test import kotlin.test.assertEquals class DataLoaderLevelDispatchedInstrumentationTest { + private val dataLoaderLevelDispatchedInstrumentation = spyk(DataLoaderLevelDispatchedInstrumentation()) private val graphQL = AstronautGraphQL.builder - .instrumentation(DataLoaderLevelDispatchedInstrumentation()) + .instrumentation(dataLoaderLevelDispatchedInstrumentation) // graphql java adds DataLoaderDispatcherInstrumentation by default .doNotAddDefaultInstrumentations() .build() + @BeforeEach + fun clear() { + clearAllMocks() + } + @Test fun `Instrumentation should batch transactions on async top level fields`() { val queries = listOf( @@ -132,4 +142,22 @@ class DataLoaderLevelDispatchedInstrumentationTest { kotlinDataLoaderRegistry.dispatchAll() } } + + @Test + fun `Instrumentation should not apply to mutations`() { + val queries = listOf( + """mutation { createAstronaut(name: "spaceMan") { id name } }""" + ) + + val (results, _) = AstronautGraphQL.execute( + graphQL, + queries, + DataLoaderInstrumentationStrategy.LEVEL_DISPATCHED + ) + + assertEquals(1, results.size) + verify { + dataLoaderLevelDispatchedInstrumentation.getOnLevelDispatchedCallback(ofType()) wasNot Called + } + } } diff --git a/executions/graphql-kotlin-dataloader-instrumentation/src/test/kotlin/com/expediagroup/graphql/dataloader/instrumentation/syncexhaustion/DataLoaderSyncExecutionExhaustedInstrumentationTest.kt b/executions/graphql-kotlin-dataloader-instrumentation/src/test/kotlin/com/expediagroup/graphql/dataloader/instrumentation/syncexhaustion/DataLoaderSyncExecutionExhaustedInstrumentationTest.kt index 3d3de8c00e..d21abb774a 100644 --- a/executions/graphql-kotlin-dataloader-instrumentation/src/test/kotlin/com/expediagroup/graphql/dataloader/instrumentation/syncexhaustion/DataLoaderSyncExecutionExhaustedInstrumentationTest.kt +++ b/executions/graphql-kotlin-dataloader-instrumentation/src/test/kotlin/com/expediagroup/graphql/dataloader/instrumentation/syncexhaustion/DataLoaderSyncExecutionExhaustedInstrumentationTest.kt @@ -19,13 +19,18 @@ package com.expediagroup.graphql.dataloader.instrumentation.syncexhaustion 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 io.mockk.Called +import io.mockk.clearAllMocks +import io.mockk.spyk import io.mockk.verify +import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test import kotlin.test.assertEquals class DataLoaderSyncExecutionExhaustedInstrumentationTest { + private val dataLoaderSyncExecutionExhaustedInstrumentation = spyk(DataLoaderSyncExecutionExhaustedInstrumentation()) private val graphQL = AstronautGraphQL.builder - .instrumentation(DataLoaderSyncExecutionExhaustedInstrumentation()) + .instrumentation(dataLoaderSyncExecutionExhaustedInstrumentation) // graphql java adds DataLoaderDispatcherInstrumentation by default .doNotAddDefaultInstrumentations() .build() @@ -36,6 +41,11 @@ class DataLoaderSyncExecutionExhaustedInstrumentationTest { .doNotAddDefaultInstrumentations() .build() + @BeforeEach + fun clear() { + clearAllMocks() + } + @Test fun `Instrumentation should batch transactions on async top level fields`() { val queries = listOf( @@ -498,4 +508,22 @@ class DataLoaderSyncExecutionExhaustedInstrumentationTest { assertEquals(1, productStatistics?.batchInvokeCount) assertEquals(2, productStatistics?.batchLoadCount) } + + @Test + fun `Instrumentation should not apply to mutations`() { + val queries = listOf( + """mutation { createAstronaut(name: "spaceMan") { id name } }""" + ) + + val (results, _) = AstronautGraphQL.execute( + graphQL, + queries, + DataLoaderInstrumentationStrategy.LEVEL_DISPATCHED + ) + + assertEquals(1, results.size) + verify { + dataLoaderSyncExecutionExhaustedInstrumentation.getOnSyncExecutionExhaustedCallback(ofType()) wasNot Called + } + } }