feat: dataloader instrumentations stop using deprecated methods (#1493)

* feat: dataloader instrumentations stop using deprecated methods * feat: computeIfAbsent on instrumentations state Co-authored-by: samvazquez <samvazquez@expedia.com>
ExpediaGroup · Jul 28, 2022 · 7711942 · 7711942
1 parent 6cb906d
commit 7711942
Show file tree

Hide file tree

Showing 7 changed files with 49 additions and 81 deletions.
diff --git a/executions/graphql-kotlin-dataloader-instrumentation/README.md b/executions/graphql-kotlin-dataloader-instrumentation/README.md
@@ -1,6 +1,6 @@
 # GraphQL Kotlin Data Loader Instrumentation
-[![Maven Central](https://img.shields.io/maven-central/v/com.expediagroup/graphql-kotlin-transaction-batcher-instrumentation.svg?label=Maven%20Central)](https://search.maven.org/search?q=g:%22com.expediagroup%22%20AND%20a:%22graphql-kotlin-transaction-batcher-instrumentation%22)
-[![Javadocs](https://img.shields.io/maven-central/v/com.expediagroup/graphql-kotlin-transaction-batcher-instrumentation.svg?label=javadoc&colorB=brightgreen)](https://www.javadoc.io/doc/com.expediagroup/graphql-kotlin-transaction-batcher-instrumentation)
+[![Maven Central](https://img.shields.io/maven-central/v/com.expediagroup/graphql-kotlin-dataloader-instrumentation.svg?label=Maven%20Central)](https://search.maven.org/search?q=g:%22com.expediagroup%22%20AND%20a:%22graphql-kotlin-dataloader-instrumentation%22)
+[![Javadocs](https://img.shields.io/maven-central/v/com.expediagroup/graphql-kotlin-dataloader-instrumentation.svg?label=javadoc&colorB=brightgreen)](https://www.javadoc.io/doc/com.expediagroup/graphql-kotlin-dataloader-instrumentation)
 
 `graphql-kotlin-dataloader-instrumentation` is set of custom instrumentations that will signal when is the right moment
 to dispatch a `KotlinDataLoaderRegistry`.

diff --git a/...iagroup/graphql/dataloader/instrumentation/NoOpExecutionStrategyInstrumentationContext.kt b/...iagroup/graphql/dataloader/instrumentation/NoOpExecutionStrategyInstrumentationContext.kt
diff --git a/...loader/instrumentation/level/execution/AbstractExecutionLevelDispatchedInstrumentation.kt b/...loader/instrumentation/level/execution/AbstractExecutionLevelDispatchedInstrumentation.kt
@@ -16,16 +16,16 @@
 
 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
 import graphql.ExecutionResult
+import graphql.execution.ExecutionContext
 import graphql.execution.instrumentation.ExecutionStrategyInstrumentationContext
+import graphql.execution.instrumentation.Instrumentation
 import graphql.execution.instrumentation.InstrumentationContext
-import graphql.execution.instrumentation.SimpleInstrumentation
-import graphql.execution.instrumentation.SimpleInstrumentationContext
+import graphql.execution.instrumentation.InstrumentationState
 import graphql.execution.instrumentation.parameters.InstrumentationExecuteOperationParameters
 import graphql.execution.instrumentation.parameters.InstrumentationExecutionStrategyParameters
 import graphql.execution.instrumentation.parameters.InstrumentationFieldFetchParameters
@@ -39,7 +39,7 @@ internal typealias OnLevelDispatchedCallback = (Level, List<ExecutionInput>) ->
  * Custom GraphQL [graphql.execution.instrumentation.Instrumentation] that calculate the state of executions
  * of all queries sharing the same GraphQLContext map
  */
-abstract class AbstractExecutionLevelDispatchedInstrumentation : SimpleInstrumentation() {
+abstract class AbstractExecutionLevelDispatchedInstrumentation : Instrumentation {
     /**
      * This is invoked each time instrumentation attempts to calculate a level dispatched state, this can be called from either
      * `beginFieldField` or `beginExecutionStrategy`.
@@ -53,17 +53,18 @@ abstract class AbstractExecutionLevelDispatchedInstrumentation : SimpleInstrumen
     ): OnLevelDispatchedCallback
 
     override fun beginExecuteOperation(
-        parameters: InstrumentationExecuteOperationParameters
-    ): InstrumentationContext<ExecutionResult> =
-        parameters.executionContext.takeIf { !it.isMutation() }
+        parameters: InstrumentationExecuteOperationParameters,
+        state: InstrumentationState?
+    ): InstrumentationContext<ExecutionResult>? =
+        parameters.executionContext.takeUnless(ExecutionContext::isMutation)
             ?.graphQLContext?.get<ExecutionLevelDispatchedState>(ExecutionLevelDispatchedState::class)
             ?.beginExecuteOperation(parameters)
-            ?: SimpleInstrumentationContext.noOp()
 
     override fun beginExecutionStrategy(
-        parameters: InstrumentationExecutionStrategyParameters
-    ): ExecutionStrategyInstrumentationContext =
-        parameters.executionContext.takeIf { !it.isMutation() }
+        parameters: InstrumentationExecutionStrategyParameters,
+        state: InstrumentationState?
+    ): ExecutionStrategyInstrumentationContext? =
+        parameters.executionContext.takeUnless(ExecutionContext::isMutation)
             ?.graphQLContext?.get<ExecutionLevelDispatchedState>(ExecutionLevelDispatchedState::class)
             ?.beginExecutionStrategy(
                 parameters,
@@ -74,12 +75,12 @@ abstract class AbstractExecutionLevelDispatchedInstrumentation : SimpleInstrumen
                     )
                 )
             )
-            ?: NoOpExecutionStrategyInstrumentationContext
 
     override fun beginFieldFetch(
-        parameters: InstrumentationFieldFetchParameters
-    ): InstrumentationContext<Any> =
-        parameters.executionContext.takeIf { !it.isMutation() }
+        parameters: InstrumentationFieldFetchParameters,
+        state: InstrumentationState?
+    ): InstrumentationContext<Any>? =
+        parameters.executionContext.takeUnless(ExecutionContext::isMutation)
             ?.graphQLContext?.get<ExecutionLevelDispatchedState>(ExecutionLevelDispatchedState::class)
             ?.beginFieldFetch(
                 parameters,
@@ -90,13 +91,13 @@ abstract class AbstractExecutionLevelDispatchedInstrumentation : SimpleInstrumen
                     )
                 )
             )
-            ?: SimpleInstrumentationContext.noOp()
 
     override fun instrumentDataFetcher(
         dataFetcher: DataFetcher<*>,
-        parameters: InstrumentationFieldFetchParameters
+        parameters: InstrumentationFieldFetchParameters,
+        state: InstrumentationState?
     ): DataFetcher<*> =
-        parameters.executionContext.takeIf { !it.isMutation() }
+        parameters.executionContext.takeUnless(ExecutionContext::isMutation)
             ?.graphQLContext?.get<ExecutionLevelDispatchedState>(ExecutionLevelDispatchedState::class)
             ?.instrumentDataFetcher(dataFetcher, parameters)
             ?: dataFetcher

diff --git a/...ediagroup/graphql/dataloader/instrumentation/level/state/ExecutionLevelDispatchedState.kt b/...ediagroup/graphql/dataloader/instrumentation/level/state/ExecutionLevelDispatchedState.kt
@@ -24,7 +24,6 @@ import graphql.ExecutionResult
 import graphql.execution.FieldValueInfo
 import graphql.execution.instrumentation.ExecutionStrategyInstrumentationContext
 import graphql.execution.instrumentation.InstrumentationContext
-import graphql.execution.instrumentation.SimpleInstrumentationContext
 import graphql.execution.instrumentation.parameters.InstrumentationExecuteOperationParameters
 import graphql.execution.instrumentation.parameters.InstrumentationExecutionStrategyParameters
 import graphql.execution.instrumentation.parameters.InstrumentationFieldFetchParameters
@@ -49,12 +48,11 @@ class ExecutionLevelDispatchedState(
      */
     fun beginExecuteOperation(
         parameters: InstrumentationExecuteOperationParameters
-    ): InstrumentationContext<ExecutionResult> {
-        val executionState = ExecutionBatchState(parameters.executionContext.getDocumentHeight())
+    ): InstrumentationContext<ExecutionResult>? {
         executions.computeIfAbsent(parameters.executionContext.executionInput) {
-            executionState
+            ExecutionBatchState(parameters.executionContext.getDocumentHeight())
         }
-        return SimpleInstrumentationContext.noOp()
+        return null
     }
 
     /**

diff --git a/...instrumentation/syncexhaustion/execution/AbstractSyncExecutionExhaustedInstrumentation.kt b/...instrumentation/syncexhaustion/execution/AbstractSyncExecutionExhaustedInstrumentation.kt
@@ -16,17 +16,16 @@
 
 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
 import graphql.GraphQLContext
+import graphql.execution.ExecutionContext
 import graphql.execution.instrumentation.ExecutionStrategyInstrumentationContext
 import graphql.execution.instrumentation.Instrumentation
 import graphql.execution.instrumentation.InstrumentationContext
-import graphql.execution.instrumentation.SimpleInstrumentation
-import graphql.execution.instrumentation.SimpleInstrumentationContext
+import graphql.execution.instrumentation.InstrumentationState
 import graphql.execution.instrumentation.parameters.InstrumentationExecuteOperationParameters
 import graphql.execution.instrumentation.parameters.InstrumentationExecutionStrategyParameters
 import graphql.execution.instrumentation.parameters.InstrumentationFieldFetchParameters
@@ -40,7 +39,7 @@ internal typealias OnSyncExecutionExhaustedCallback = (List<ExecutionInput>) ->
  * Custom GraphQL [Instrumentation] that calculate the synchronous execution exhaustion
  * of all GraphQL operations sharing the same [GraphQLContext]
  */
-abstract class AbstractSyncExecutionExhaustedInstrumentation : SimpleInstrumentation() {
+abstract class AbstractSyncExecutionExhaustedInstrumentation : Instrumentation {
     /**
      * This is invoked each time instrumentation attempts to calculate exhaustion state, this can be called from either
      * `beginFieldField.dispatch` or `beginFieldFetch.complete`.
@@ -53,31 +52,31 @@ abstract class AbstractSyncExecutionExhaustedInstrumentation : SimpleInstrumenta
     ): OnSyncExecutionExhaustedCallback
 
     override fun beginExecuteOperation(
-        parameters: InstrumentationExecuteOperationParameters
-    ): InstrumentationContext<ExecutionResult> =
-        parameters.executionContext.takeIf { !it.isMutation() }
+        parameters: InstrumentationExecuteOperationParameters,
+        state: InstrumentationState?
+    ): InstrumentationContext<ExecutionResult>? =
+        parameters.executionContext.takeUnless(ExecutionContext::isMutation)
             ?.graphQLContext?.get<SyncExecutionExhaustedState>(SyncExecutionExhaustedState::class)
             ?.beginExecuteOperation(parameters)
-            ?: SimpleInstrumentationContext.noOp()
 
     override fun beginExecutionStrategy(
-        parameters: InstrumentationExecutionStrategyParameters
-    ): ExecutionStrategyInstrumentationContext =
-        parameters.executionContext.takeIf { !it.isMutation() }
+        parameters: InstrumentationExecutionStrategyParameters,
+        state: InstrumentationState?
+    ): ExecutionStrategyInstrumentationContext? =
+        parameters.executionContext.takeUnless(ExecutionContext::isMutation)
             ?.graphQLContext?.get<SyncExecutionExhaustedState>(SyncExecutionExhaustedState::class)
             ?.beginExecutionStrategy(parameters)
-            ?: NoOpExecutionStrategyInstrumentationContext
 
     override fun beginFieldFetch(
-        parameters: InstrumentationFieldFetchParameters
-    ): InstrumentationContext<Any> =
-        parameters.executionContext.takeIf { !it.isMutation() }
+        parameters: InstrumentationFieldFetchParameters,
+        state: InstrumentationState?
+    ): InstrumentationContext<Any>? =
+        parameters.executionContext.takeUnless(ExecutionContext::isMutation)
             ?.graphQLContext?.get<SyncExecutionExhaustedState>(SyncExecutionExhaustedState::class)
             ?.beginFieldFetch(
                 parameters,
                 this.getOnSyncExecutionExhaustedCallback(
                     SyncExecutionExhaustedInstrumentationParameters(parameters.executionContext)
                 )
             )
-            ?: SimpleInstrumentationContext.noOp()
 }
diff --git a/...up/graphql/dataloader/instrumentation/syncexhaustion/state/SyncExecutionExhaustedState.kt b/...up/graphql/dataloader/instrumentation/syncexhaustion/state/SyncExecutionExhaustedState.kt
@@ -17,15 +17,13 @@
 package com.expediagroup.graphql.dataloader.instrumentation.syncexhaustion.state
 
 import com.expediagroup.graphql.dataloader.KotlinDataLoaderRegistry
-import com.expediagroup.graphql.dataloader.instrumentation.NoOpExecutionStrategyInstrumentationContext
 import com.expediagroup.graphql.dataloader.instrumentation.syncexhaustion.execution.OnSyncExecutionExhaustedCallback
 import graphql.ExecutionInput
 import graphql.ExecutionResult
 import graphql.GraphQLContext
 import graphql.execution.MergedField
 import graphql.execution.instrumentation.ExecutionStrategyInstrumentationContext
 import graphql.execution.instrumentation.InstrumentationContext
-import graphql.execution.instrumentation.SimpleInstrumentationContext
 import graphql.execution.instrumentation.parameters.InstrumentationExecuteOperationParameters
 import graphql.execution.instrumentation.parameters.InstrumentationExecutionStrategyParameters
 import graphql.execution.instrumentation.parameters.InstrumentationFieldFetchParameters
@@ -51,9 +49,11 @@ class SyncExecutionExhaustedState(
      */
     fun beginExecuteOperation(
         parameters: InstrumentationExecuteOperationParameters
-    ): InstrumentationContext<ExecutionResult> {
-        executions[parameters.executionContext.executionInput] = ExecutionBatchState()
-        return SimpleInstrumentationContext.noOp()
+    ): InstrumentationContext<ExecutionResult>? {
+        executions.computeIfAbsent(parameters.executionContext.executionInput) {
+            ExecutionBatchState()
+        }
+        return null
     }
 
     /**
@@ -65,7 +65,7 @@ class SyncExecutionExhaustedState(
      */
     fun beginExecutionStrategy(
         parameters: InstrumentationExecutionStrategyParameters
-    ): ExecutionStrategyInstrumentationContext {
+    ): ExecutionStrategyInstrumentationContext? {
         val executionInput = parameters.executionContext.executionInput
 
         executions.computeIfPresent(executionInput) { _, executionState ->
@@ -79,7 +79,8 @@ class SyncExecutionExhaustedState(
             executionState.addExecutionStrategyState(field, path, selectionFields, parentGraphQLType)
             executionState
         }
-        return NoOpExecutionStrategyInstrumentationContext
+
+        return null
     }
 
     /**
@@ -129,7 +130,7 @@ class SyncExecutionExhaustedState(
     /**
      * Provide the information about when all [ExecutionInput] sharing a [GraphQLContext] exhausted their execution
      * A Synchronous Execution is considered Exhausted when all [DataFetcher]s of all paths were executed up until
-     * an scalar leaf or a [DataFetcher] that returns a [CompletableFuture]
+     * a scalar leaf or a [DataFetcher] that returns a [CompletableFuture]
      */
     fun allSyncExecutionsExhausted(): Boolean = synchronized(executions) {
         when {

diff --git a/gradle.properties b/gradle.properties
@@ -23,7 +23,7 @@ androidPluginVersion = 7.1.2
 classGraphVersion = 4.8.143
 federationGraphQLVersion = 0.9.0
 graphQLJavaVersion = 19.0
-graphQLJavaDataLoaderVersion = 3.1.3
+graphQLJavaDataLoaderVersion = 3.2.0
 jacksonVersion = 2.13.2
 kotlinPoetVersion = 1.11.0
 ktorVersion = 2.0.0