Skip to content

feat: update graphql-java dataloader version #1450

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,16 @@ import com.expediagroup.graphql.dataloader.instrumentation.fixture.extensions.to
import com.expediagroup.graphql.dataloader.instrumentation.fixture.repository.AstronautRepository
import graphql.schema.DataFetchingEnvironment
import org.dataloader.BatchLoader
import org.dataloader.DataLoaderOptions
import org.dataloader.stats.SimpleStatisticsCollector
import java.util.Optional
import java.util.concurrent.CompletableFuture

data class AstronautServiceRequest(val id: Int)

class AstronautDataLoader : KotlinDataLoader<AstronautServiceRequest, Astronaut?> {
override val dataLoaderName: String = "AstronautDataLoader"
override fun getOptions(): DataLoaderOptions = DataLoaderOptions.newOptions().setStatisticsCollector { SimpleStatisticsCollector() }
override fun getBatchLoader(): BatchLoader<AstronautServiceRequest, Astronaut?> =
BatchLoader<AstronautServiceRequest, Astronaut?> { keys ->
AstronautRepository
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,16 @@ import com.expediagroup.graphql.dataloader.instrumentation.fixture.extensions.to
import com.expediagroup.graphql.dataloader.instrumentation.fixture.repository.MissionRepository
import graphql.schema.DataFetchingEnvironment
import org.dataloader.BatchLoader
import org.dataloader.DataLoaderOptions
import org.dataloader.stats.SimpleStatisticsCollector
import java.util.Optional
import java.util.concurrent.CompletableFuture

data class MissionServiceRequest(val id: Int, val astronautId: Int = -1)

class MissionDataLoader : KotlinDataLoader<MissionServiceRequest, Mission?> {
override val dataLoaderName: String = "MissionDataLoader"
override fun getOptions(): DataLoaderOptions = DataLoaderOptions.newOptions().setStatisticsCollector { SimpleStatisticsCollector() }
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for testing purposes we need statistics but in this new release, those are now NOOP by default

override fun getBatchLoader(): BatchLoader<MissionServiceRequest, Mission?> =
BatchLoader<MissionServiceRequest, Mission?> { keys ->
MissionRepository
Expand All @@ -41,6 +44,7 @@ class MissionDataLoader : KotlinDataLoader<MissionServiceRequest, Mission?> {

class MissionsByAstronautDataLoader : KotlinDataLoader<MissionServiceRequest, List<Mission>> {
override val dataLoaderName: String = "MissionsByAstronautDataLoader"
override fun getOptions(): DataLoaderOptions = DataLoaderOptions.newOptions().setStatisticsCollector { SimpleStatisticsCollector() }
override fun getBatchLoader(): BatchLoader<MissionServiceRequest, List<Mission>> =
BatchLoader<MissionServiceRequest, List<Mission>> { keys ->
MissionRepository
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,15 @@ import com.expediagroup.graphql.dataloader.instrumentation.fixture.domain.Planet
import com.expediagroup.graphql.dataloader.instrumentation.fixture.repository.PlanetRepository
import graphql.schema.DataFetchingEnvironment
import org.dataloader.BatchLoader
import org.dataloader.DataLoaderOptions
import org.dataloader.stats.SimpleStatisticsCollector
import java.util.concurrent.CompletableFuture

data class PlanetServiceRequest(val id: Int, val missionId: Int = -1)

class PlanetsByMissionDataLoader : KotlinDataLoader<PlanetServiceRequest, List<Planet>> {
override val dataLoaderName: String = "PlanetsByMissionDataLoader"
override fun getOptions(): DataLoaderOptions = DataLoaderOptions.newOptions().setStatisticsCollector { SimpleStatisticsCollector() }
override fun getBatchLoader(): BatchLoader<PlanetServiceRequest, List<Planet>> =
BatchLoader<PlanetServiceRequest, List<Planet>> { keys ->
PlanetRepository
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,14 @@ import com.expediagroup.graphql.dataloader.instrumentation.fixture.extensions.to
import com.expediagroup.graphql.dataloader.instrumentation.fixture.repository.ProductRepository
import graphql.schema.DataFetchingEnvironment
import org.dataloader.BatchLoader
import org.dataloader.DataLoaderOptions
import org.dataloader.stats.SimpleStatisticsCollector
import java.util.Optional
import java.util.concurrent.CompletableFuture

class ProductDataLoader : KotlinDataLoader<ProductServiceRequest, Product?> {
override val dataLoaderName: String = "ProductDataLoader"
override fun getOptions(): DataLoaderOptions = DataLoaderOptions.newOptions().setStatisticsCollector { SimpleStatisticsCollector() }
override fun getBatchLoader(): BatchLoader<ProductServiceRequest, Product?> =
BatchLoader<ProductServiceRequest, Product?> { requests ->
ProductRepository
Expand Down
4 changes: 2 additions & 2 deletions executions/graphql-kotlin-dataloader/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,12 @@ tasks {
limit {
counter = "INSTRUCTION"
value = "COVEREDRATIO"
minimum = "0.60".toBigDecimal()
minimum = "0.55".toBigDecimal()
}
limit {
counter = "BRANCH"
value = "COVEREDRATIO"
minimum = "0.60".toBigDecimal()
minimum = "0.55".toBigDecimal()
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ import org.dataloader.DataLoader
import org.dataloader.DataLoaderOptions

/**
* Configuration interface that will create a [DataLoader] instance
* so we can have common logic around registering the loaders
* Configuration interface that will create a [DataLoader] instance,
* so we can have common logic around registering the data loaders
* by return type and loading values in the data fetchers.
*/
interface KotlinDataLoader<K, V> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,12 @@ import java.util.concurrent.CompletableFuture
import java.util.function.Function

/**
* Custom [DataLoaderRegistry] decorator that has access to the [CacheMap] of each registered [DataLoader]
* Custom [DataLoaderRegistry] decorator that access the [CacheMap] of each registered [DataLoader]
* in order to keep track of the [onDispatchFutures] when [dispatchAll] is invoked,
* that way we can know if all dependants of the [CompletableFuture]s were executed.
*/
class KotlinDataLoaderRegistry(
private val registry: DataLoaderRegistry = DataLoaderRegistry(),
private val futureCacheMaps: List<KotlinDefaultCacheMap<*, *>> = emptyList()
private val registry: DataLoaderRegistry = DataLoaderRegistry()
) : DataLoaderRegistry() {

private val onDispatchFutures: MutableList<CompletableFuture<*>> = mutableListOf()
Expand All @@ -57,12 +56,12 @@ class KotlinDataLoaderRegistry(

/**
* will return a list of futures that represents the **current** state of the [CompletableFuture]s from each
* [DataLoader] cacheMap.
* [DataLoader] [CacheMap].
*
* @return list of current completable futures.
*/
fun getCurrentFutures(): List<CompletableFuture<*>> =
futureCacheMaps.map(KotlinDefaultCacheMap<*, *>::values).flatten()
registry.dataLoaders.map { it.cacheMap.all }.flatten()

/**
* This will invoke [DataLoader.dispatch] on each of the registered [DataLoader]s,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,24 +32,16 @@ class KotlinDataLoaderRegistryFactory(
* Generate [KotlinDataLoaderRegistry] to be used for GraphQL request execution.
*/
fun generate(): KotlinDataLoaderRegistry {
val futureCacheMaps = mutableListOf<KotlinDefaultCacheMap<*, *>>()

val registry = DataLoaderRegistry()
dataLoaders.forEach { dataLoader ->
val options = dataLoader.getOptions()

// override DefaultCacheMap if no cache provided in options
if (options.cachingEnabled() && options.cacheMap().isEmpty) {
val futureCacheMap = KotlinDefaultCacheMap<Any?, Any?>()
options.setCacheMap(futureCacheMap)
futureCacheMaps += futureCacheMap
}

registry.register(
dataLoader.dataLoaderName,
DataLoaderFactory.newDataLoader(dataLoader.getBatchLoader(), options)
DataLoaderFactory.newDataLoader(
dataLoader.getBatchLoader(),
dataLoader.getOptions()
)
)
}
return KotlinDataLoaderRegistry(registry, futureCacheMaps)
return KotlinDataLoaderRegistry(registry)
}
}

This file was deleted.

2 changes: 1 addition & 1 deletion gradle.properties
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ androidPluginVersion = 7.1.2
classGraphVersion = 4.8.143
federationGraphQLVersion = 0.9.0
graphQLJavaVersion = 18.0
graphQLJavaDataLoaderVersion = 3.1.2
graphQLJavaDataLoaderVersion = 3.1.3
jacksonVersion = 2.13.2
kotlinPoetVersion = 1.11.0
ktorVersion = 2.0.0
Expand Down
7 changes: 4 additions & 3 deletions website/docs/server/data-loader/data-loader.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,12 @@ A `DataLoader` caches the types by some unique value, usually by the type id, an
```kotlin
class UserDataLoader : KotlinDataLoader<ID, User> {
override val dataLoaderName = "UserDataLoader"
override fun getOptions() = DataLoaderOptions.newOptions().setCachingEnabled(false)
override fun getBatchLoader() = BatchLoader<ID, User> { ids ->
CompletableFuture.supplyAsync {
ids.map { id -> userService.getUser(id) }
}
}
override fun getOptions() = DataLoaderOptions.newOptions().setCachingEnabled(false)
}

class FriendsDataLoader : KotlinDataLoader<ID, List<User>> {
Expand All @@ -78,8 +78,9 @@ val dataLoaderRegistry = dataLoaderRegistryFactory.generate()

## `KotlinDataLoaderRegistry`

`KotlinDataLoaderRegistry` is a decorator of the original `graphql-java` [DataLoaderRegistry](https://github.com/graphql-java/java-dataloader/blob/master/src/main/java/org/dataloader/DataLoaderRegistry.java)
that provides access to all underlying `DataLoader`s future states. By providing access to cache map containing returned futures,
[KotlinDataLoaderRegistry](https://github.com/ExpediaGroup/graphql-kotlin/blob/master/executions/graphql-kotlin-dataloader/src/main/kotlin/com/expediagroup/graphql/dataloader/KotlinDataLoaderRegistry.kt)
is a decorator of the original `graphql-java` [DataLoaderRegistry](https://github.com/graphql-java/java-dataloader/blob/master/src/main/java/org/dataloader/DataLoaderRegistry.java)
that keeps track of all underlying `DataLoader`s futures. By keeping track of to cache map containing returned futures,
we get more granular control when to dispatch data loader calls.

## `getValueFromDataLoader`
Expand Down