Skip to content

chore: tests for batching and deduplication by field selection #1449

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 1 commit
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
Prev Previous commit
Next Next commit
feat: renaming some fields and methods
  • Loading branch information
samvazquez committed May 4, 2022
commit 396765f99de19b70dd6c951a97269599986b639f
Original file line number Diff line number Diff line change
Expand Up @@ -74,15 +74,18 @@ class AstronautService {
fun getPlanets(
request: AstronautServiceRequest,
environment: DataFetchingEnvironment
): CompletableFuture<List<Planet>> =
environment
.getDataLoaderFromContext<MissionServiceRequest, List<Mission>>("MissionsByAstronautDataLoader")
): CompletableFuture<List<Planet>> {
val missionsByAstronautDataLoader = environment.getDataLoaderFromContext<MissionServiceRequest, List<Mission>>("MissionsByAstronautDataLoader")
val planetsByMissionDataLoader = environment.getDataLoaderFromContext<PlanetServiceRequest, List<Planet>>("PlanetsByMissionDataLoader")
return missionsByAstronautDataLoader
.load(MissionServiceRequest(0, astronautId = request.id))
.thenCompose { missions ->
environment
.getDataLoaderFromContext<PlanetServiceRequest, List<Planet>>("PlanetsByMissionDataLoader")
planetsByMissionDataLoader
.loadMany(missions.map { PlanetServiceRequest(0, it.id) })
.dispatchIfNeeded(environment)
.thenApply { planetsByMission -> planetsByMission.flatten().distinct() }
}
.thenApply { planetsByMission ->
planetsByMission.flatten().distinct()
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,7 @@ class DataLoaderSyncExecutionExhaustedInstrumentationTest {
}

@Test
fun `Instrumentation multiple dataLoaders per field 2`() {
fun `Instrumentation should batch chained dataLoaders per field dataFetcher`() {
val queries = listOf(
"""
fragment AstronautFragment on Astronaut { planets { name } }
Expand Down Expand Up @@ -378,4 +378,47 @@ class DataLoaderSyncExecutionExhaustedInstrumentationTest {
assertEquals(1, missionsByAstronautStatistics?.batchInvokeCount)
assertEquals(1, planetStatistics?.batchInvokeCount)
}

@Test
fun `Instrumentation should batch chained dataLoaders per field dataFetcher with different queries`() {
val queries = listOf(
"""
{
astronaut(id: 1) {
name
planets {
name
}
}
}
""".trimIndent(),
"""
{
astronaut(id: 3) {
name
missions {
designation
planets {
name
}
}
}
}
""".trimIndent(),
)

val (results, kotlinDataLoaderRegistry) = TestGraphQL.execute(
graphQL,
queries,
DataLoaderInstrumentationStrategy.SYNC_EXHAUSTION
)

val missionsByAstronautStatistics = kotlinDataLoaderRegistry.dataLoadersMap["MissionsByAstronautDataLoader"]?.statistics
val planetStatistics = kotlinDataLoaderRegistry.dataLoadersMap["PlanetsByMissionDataLoader"]?.statistics

assertEquals(2, results.size)

assertEquals(1, missionsByAstronautStatistics?.batchInvokeCount)
assertEquals(1, planetStatistics?.batchInvokeCount)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,15 @@ import java.util.function.Function

/**
* Custom [DataLoaderRegistry] decorator that has access to the [CacheMap] of each registered [DataLoader]
* in order to keep track of the [onDispatchAllFutures] when [dispatchAll] is invoked,
* 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()
) : DataLoaderRegistry() {

private val onDispatchAllFutures: MutableList<CompletableFuture<*>> = mutableListOf()
private val onDispatchFutures: MutableList<CompletableFuture<*>> = mutableListOf()

override fun register(key: String, dataLoader: DataLoader<*, *>): DataLoaderRegistry = registry.register(key, dataLoader)
override fun <K, V> computeIfAbsent(key: String, mappingFunction: Function<String, DataLoader<*, *>>): DataLoader<K, V> = registry.computeIfAbsent(key, mappingFunction)
Expand All @@ -50,42 +50,37 @@ class KotlinDataLoaderRegistry(
/**
* This will invoke [DataLoader.dispatch] on each of the registered [DataLoader]s,
* it will start to keep track of the [CompletableFuture]s of each [DataLoader] by adding them to
* [onDispatchAllFutures]
* [onDispatchFutures]
*/
override fun dispatchAll() {
onDispatchAllFutures.clear()
onDispatchAllFutures.addAll(getCurrentFutures())
onDispatchFutures.clear()
onDispatchFutures.addAll(getCurrentFutures())
registry.dispatchAll()
}

/**
* will return a list of futures that represents the state of the [CompletableFuture]s from each
* [DataLoader] cacheMap right before [dispatchAll] is invoked.
*
* @return list of completable futures gathered right before calling [dispatchAll].
*/
fun getOnDispatchFutures(): List<CompletableFuture<*>> = onDispatchAllFutures

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

/**
* Will signal when all dependants of all [onDispatchAllFutures] were invoked,
* [onDispatchAllFutures] is the list of all [CompletableFuture]s that will complete because the [dispatchAll]
* Will signal when all dependants of all [onDispatchFutures] were invoked,
* [onDispatchFutures] is the list of all [CompletableFuture]s that will complete because the [dispatchAll]
* method was invoked
*
* @return weather or not all futures gathered before [dispatchAll] were handled
*/
fun onDispatchFuturesHandled(): Boolean = onDispatchAllFutures.all { it.numberOfDependents == 0 }
fun onDispatchFuturesHandled(): Boolean =
onDispatchFutures.all { it.numberOfDependents == 0 }

/**
* Will signal if more dataLoaders where invoked during the [dispatchAll] invocation
* @return weather or not futures where loaded during [dispatchAll]
*/
fun dataLoadersInvokedOnDispatch(): Boolean = getCurrentFutures().size > getOnDispatchFutures().size
fun dataLoadersInvokedOnDispatch(): Boolean =
getCurrentFutures().size > onDispatchFutures.size

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