This adds the ability to track the total number of ids to be hydrated #355
Conversation
… plus per call how many we are acting on
| @@ -12,6 +12,7 @@ import graphql.nadel.enginekt.transform.hydration.batch.NadelBatchHydrationInput | |||
| import graphql.nadel.enginekt.transform.hydration.batch.NadelBatchHydrationObjectIdFieldBuilder.makeObjectIdField | |||
| import graphql.nadel.enginekt.transform.query.NFUtil | |||
| import graphql.nadel.enginekt.transform.result.json.JsonNode | |||
| import graphql.nadel.enginekt.util.CountedBox | |||
| import graphql.nadel.enginekt.util.deepClone | |||
There was a problem hiding this comment.
CountBox is a box class that has some object and associated count. Is there a Kotlin native equivalent ?
| counter += hydrationBox.count | ||
| hydrationBox.boxedObject | ||
| } | ||
| val executableField = makeActorQueries( |
There was a problem hiding this comment.
a single executableField can be N hydration ids eg usersByIds(ids : ["1","2","n"])
So we have 1 field with a count of N associated with it
| ): List<ExecutableNormalizedField> { | ||
| val argBatches = NadelBatchHydrationInputBuilder.getInputValueBatches( | ||
| ): List<CountedBox<ExecutableNormalizedField>> { | ||
| val boxedArgBatches = NadelBatchHydrationInputBuilder.getInputValueBatches( |
There was a problem hiding this comment.
Its the input values that are the key here - a set M parent nodes can lets to N ids to be hydrated
| @@ -140,7 +146,7 @@ internal class NadelHydrationInputBuilder private constructor( | |||
| ) | |||
| } | |||
|
|
|||
| private fun getResultValue( | |||
| private fun getSingleResultValue( | |||
There was a problem hiding this comment.
renamed because if you look at it - its does emptyOrSingle on the list of values
| /** | ||
| * A box class that holds some value and a count associated with it | ||
| */ | ||
| data class CountedBox<T> (val boxedObject : T, val count : Int) |
There was a problem hiding this comment.
I know - its a Pair<T,Int> but I think this is semantically more meaningful
| } | ||
|
|
||
| private fun getBatchInputValues( | ||
| instruction: NadelBatchHydrationFieldInstruction, | ||
| parentNodes: List<JsonNode>, | ||
| aliasHelper: NadelAliasHelper, | ||
| ): List<Pair<NadelHydrationActorInputDef, NormalizedInputValue>> { | ||
| ): List<Pair<NadelHydrationActorInputDef, CountedBox<NormalizedInputValue>>> { |
There was a problem hiding this comment.
this conflicts now with the changes that I made to support multi tenanted hydrations
I think it should be something like
private fun getBatchInputValues(
instruction: NadelBatchHydrationFieldInstruction,
parentNodes: List<JsonNode>,
aliasHelper: NadelAliasHelper,
hooks: ServiceExecutionHooks,
): List<Pair<NadelHydrationActorInputDef, CountedBox<NormalizedInputValue>>> {
val batchSize = instruction.batchSize
val (batchInputDef, batchInputValueSource) = getBatchInputDef(instruction) ?: return emptyList()
val actorBatchArgDef = instruction.actorFieldDef.getArgument(batchInputDef.name)
val fieldResultValues = getFieldResultValues(batchInputValueSource, parentNodes, aliasHelper)
val partitionedArgumentList = when (hooks) {
is NadelEngineExecutionHooks -> hooks.partitionBatchHydrationArgumentList(fieldResultValues, instruction)
else -> listOf(fieldResultValues)
}
return partitionedArgumentList.flatMap { it.chunked(size = batchSize) }
.map { chunkedFieldResultValues ->
val normalizedInputValue = NormalizedInputValue(
GraphQLTypeUtil.simplePrint(actorBatchArgDef.type),
javaValueToAstValue(chunkedFieldResultValues),
)
batchInputDef to CountedBox(normalizedInputValue, chunkedFieldResultValues.size)
}
}
|
looks good imo, just the merge conflicts |
|
Hmmm I'm very iffy on this. I get that we need this info, however I'd much rather track this in some sort of context object per hydration/query execution (which I think we may need for multitenanted hydrations that Artyom is working on, see my comment on that PR), rather than mess with the return type just to return an object 20 times in the hierarchy. As an aside, I really want to add something like Dagger or kotlin-inject and clean up the way we pass around arguments and clean up how some objects are scoped to reduce the number of function parameters… |
…cts-to-be-hydrated # Conflicts: # engine-nextgen/src/main/kotlin/graphql/nadel/enginekt/transform/hydration/NadelHydrationFieldsBuilder.kt # engine-nextgen/src/main/kotlin/graphql/nadel/enginekt/transform/hydration/batch/NadelBatchHydrationInputBuilder.kt
This adds the ability to track the total number of ids to be hydrated plus per call how many we are acting on
Please make sure you consider the following: