-
Notifications
You must be signed in to change notification settings - Fork 28
Fix hydration selection set on nested input objects #707
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
Changes from all commits
f835bab
31c4647
6a660a6
3360802
cb28214
32d5447
2cb6fff
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ import graphql.nadel.engine.blueprint.hydration.NadelHydrationArgument | |
| import graphql.nadel.engine.blueprint.hydration.NadelHydrationCondition | ||
| import graphql.nadel.engine.blueprint.hydration.NadelHydrationStrategy | ||
| import graphql.nadel.engine.transform.query.NadelQueryPath | ||
| import graphql.normalized.ExecutableNormalizedField | ||
| import graphql.schema.FieldCoordinates | ||
| import graphql.schema.GraphQLFieldDefinition | ||
| import graphql.schema.GraphQLFieldsContainer | ||
|
|
@@ -72,8 +73,16 @@ interface NadelGenericHydrationInstruction { | |
| * This can be the fields described in [NadelHydrationArgument.ValueSource.FieldResultValue.queryPathToField] | ||
| * or [NadelBatchHydrationMatchStrategy.MatchObjectIdentifier.sourceId]. | ||
| */ | ||
| @Deprecated("To be replaced by executableSourceFields") | ||
| val sourceFields: List<NadelQueryPath> | ||
|
|
||
| /** | ||
| * The fields required to be queried on the source object in order to complete the hydration. | ||
| * This can be the fields described in [NadelHydrationArgument.ValueSource.FieldResultValue.queryPathToField] | ||
| * or [NadelBatchHydrationMatchStrategy.MatchObjectIdentifier.sourceId]. | ||
| */ | ||
| val executableSourceFields: List<ExecutableNormalizedField> | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We now build the entire |
||
|
|
||
| /** | ||
| * The field definition in the overall schema referenced by [queryPathToBackingField]. | ||
| */ | ||
|
|
@@ -107,7 +116,9 @@ data class NadelHydrationFieldInstruction( | |
| override val queryPathToBackingField: NadelQueryPath, | ||
| override val backingFieldArguments: List<NadelHydrationArgument>, | ||
| override val timeout: Int, | ||
| @Deprecated("To be replaced by executableSourceFields") | ||
| override val sourceFields: List<NadelQueryPath>, | ||
| override val executableSourceFields: List<ExecutableNormalizedField>, | ||
| override val backingFieldDef: GraphQLFieldDefinition, | ||
| override val backingFieldContainer: GraphQLFieldsContainer, | ||
| override val backingFieldReturnsObjectTypeNames: Set<String>, | ||
|
|
@@ -131,7 +142,9 @@ data class NadelBatchHydrationFieldInstruction( | |
| override val queryPathToBackingField: NadelQueryPath, | ||
| override val backingFieldArguments: List<NadelHydrationArgument>, | ||
| override val timeout: Int, | ||
| @Deprecated("To be replaced by executableSourceFields") | ||
| override val sourceFields: List<NadelQueryPath>, | ||
| override val executableSourceFields: List<ExecutableNormalizedField>, | ||
| override val backingFieldDef: GraphQLFieldDefinition, | ||
| override val backingFieldContainer: GraphQLFieldsContainer, | ||
| override val backingFieldReturnsObjectTypeNames: Set<String>, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -148,12 +148,25 @@ internal object NadelHydrationFieldsBuilder { | |
| } | ||
|
|
||
| fun makeRequiredSourceFields( | ||
| hints: NadelExecutionHints, | ||
| service: Service, | ||
| executionBlueprint: NadelOverallExecutionBlueprint, | ||
| aliasHelper: NadelAliasHelper, | ||
| objectTypeName: GraphQLObjectTypeName, | ||
| instructions: List<NadelGenericHydrationInstruction>, | ||
| ): List<ExecutableNormalizedField> { | ||
| if (hints.hydrationExecutableSourceFields(service)) { | ||
| return instructions | ||
| .asSequence() | ||
| .flatMap { | ||
| it.executableSourceFields | ||
| } | ||
| .map { | ||
| aliasHelper.toArtificial(it) | ||
| } | ||
| .toList() | ||
| } | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. New code here, the ENFs are built during validation time, now we just insert them from the instruction. |
||
|
|
||
| val underlyingTypeName = executionBlueprint.getUnderlyingTypeName(service, overallTypeName = objectTypeName) | ||
| val underlyingObjectType = service.underlyingSchema.getObjectType(underlyingTypeName) | ||
| ?: error("No underlying object type") | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -93,6 +93,7 @@ internal class NadelBatchHydrationTransform( | |
| artificialFields = state.instructionsByObjectTypeNames | ||
| .flatMap { (objectTypeName, instructions) -> | ||
| NadelHydrationFieldsBuilder.makeRequiredSourceFields( | ||
| hints = executionContext.hints, | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For FFing |
||
| service = service, | ||
| executionBlueprint = executionBlueprint, | ||
| aliasHelper = state.aliasHelper, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| package graphql.nadel.hints | ||
|
|
||
| import graphql.nadel.Service | ||
|
|
||
| fun interface NadelHydrationExecutableSourceFields { | ||
| operator fun invoke(service: Service): Boolean | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,179 @@ | ||||||||||||
| package graphql.nadel.validation.hydration | ||||||||||||
|
|
||||||||||||
| import graphql.nadel.engine.blueprint.hydration.NadelBatchHydrationMatchStrategy | ||||||||||||
| import graphql.nadel.engine.blueprint.hydration.NadelHydrationArgument | ||||||||||||
| import graphql.nadel.engine.blueprint.hydration.NadelHydrationCondition | ||||||||||||
| import graphql.nadel.engine.transform.query.NFUtil | ||||||||||||
| import graphql.nadel.engine.transform.query.NadelQueryPath | ||||||||||||
| import graphql.nadel.engine.util.getFieldContainerFor | ||||||||||||
| import graphql.nadel.engine.util.isNonNull | ||||||||||||
| import graphql.nadel.engine.util.unwrapAll | ||||||||||||
| import graphql.nadel.validation.NadelValidationContext | ||||||||||||
| import graphql.nadel.validation.NadelValidationInterimResult | ||||||||||||
| import graphql.nadel.validation.NadelValidationInterimResult.Success.Companion.asInterimSuccess | ||||||||||||
| import graphql.nadel.validation.onError | ||||||||||||
| import graphql.nadel.validation.onErrorCast | ||||||||||||
| import graphql.normalized.ExecutableNormalizedField | ||||||||||||
| import graphql.schema.GraphQLInputObjectType | ||||||||||||
| import graphql.schema.GraphQLObjectType | ||||||||||||
|
|
||||||||||||
| /** | ||||||||||||
| * Forked version of [NadelHydrationSourceFieldValidation] that has new functionality so we can feature flag the code. | ||||||||||||
| */ | ||||||||||||
| internal class NadelHydrationSourceFieldValidation2 { | ||||||||||||
| context(NadelValidationContext, NadelHydrationValidationContext) | ||||||||||||
| fun getSourceFields( | ||||||||||||
| arguments: List<NadelHydrationArgument>, | ||||||||||||
| hydrationCondition: NadelHydrationCondition?, | ||||||||||||
| ): NadelValidationInterimResult<List<ExecutableNormalizedField>> { | ||||||||||||
| val argumentSourceFields = arguments.getSourceFields() | ||||||||||||
| .onError { return it } | ||||||||||||
| val conditionSourceFields = listOfNotNull(hydrationCondition?.fieldPath).map { makeLeafField(it) } | ||||||||||||
|
|
||||||||||||
| return (argumentSourceFields + conditionSourceFields) | ||||||||||||
| .dedupSourceFields() | ||||||||||||
| .asInterimSuccess() | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| context(NadelValidationContext, NadelHydrationValidationContext) | ||||||||||||
| fun getBatchHydrationSourceFields( | ||||||||||||
| arguments: List<NadelHydrationArgument>, | ||||||||||||
| matchStrategy: NadelBatchHydrationMatchStrategy, | ||||||||||||
| hydrationCondition: NadelHydrationCondition?, | ||||||||||||
| ): NadelValidationInterimResult<List<ExecutableNormalizedField>> { | ||||||||||||
| val argumentSourceFields = arguments.getSourceFields() | ||||||||||||
| .onError { return it } | ||||||||||||
| val conditionSourceFields = listOfNotNull(hydrationCondition?.fieldPath).map { makeLeafField(it) } | ||||||||||||
|
|
||||||||||||
| return (argumentSourceFields + conditionSourceFields) | ||||||||||||
| .dedupSourceFields() | ||||||||||||
| .asInterimSuccess() | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| context(NadelValidationContext, NadelHydrationValidationContext) | ||||||||||||
| private fun List<NadelHydrationArgument>.getSourceFields(): NadelValidationInterimResult<List<ExecutableNormalizedField>> { | ||||||||||||
| return mapNotNull { argument -> | ||||||||||||
| when (argument.valueSource) { | ||||||||||||
| is NadelHydrationArgument.ValueSource.ArgumentValue -> null | ||||||||||||
| is NadelHydrationArgument.ValueSource.FieldResultValue -> | ||||||||||||
| getSourceFieldQueryPaths(argument, argument.valueSource) | ||||||||||||
| .onErrorCast { return it } | ||||||||||||
| is NadelHydrationArgument.ValueSource.StaticValue -> null | ||||||||||||
| is NadelHydrationArgument.ValueSource.RemainingArguments -> null | ||||||||||||
| } | ||||||||||||
| }.asInterimSuccess() | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| context(NadelValidationContext, NadelHydrationValidationContext) | ||||||||||||
| private fun getSourceFieldQueryPaths( | ||||||||||||
| argument: NadelHydrationArgument, | ||||||||||||
| hydrationValueSource: NadelHydrationArgument.ValueSource.FieldResultValue, | ||||||||||||
| ): NadelValidationInterimResult<ExecutableNormalizedField> { | ||||||||||||
| val hydrationSourceType = hydrationValueSource.fieldDefinition.type.unwrapAll() | ||||||||||||
|
|
||||||||||||
| if (hydrationSourceType is GraphQLObjectType) { | ||||||||||||
| return createObjectField(argument, hydrationValueSource) | ||||||||||||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the main change, before this was nadel/lib/src/main/java/graphql/nadel/validation/hydration/NadelHydrationSourceFieldValidation.kt Lines 89 to 93 in 2cb6fff
Which only selected one level of children. Now it iterates through the required input object fields and then creates the selection set from that. It does this recursively to create the correct selection set. |
||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| return makeLeafField(hydrationValueSource.queryPathToField) | ||||||||||||
| .asInterimSuccess() | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| context(NadelValidationContext, NadelHydrationValidationContext) | ||||||||||||
| private fun createObjectField( | ||||||||||||
| argument: NadelHydrationArgument, | ||||||||||||
| hydrationValueSource: NadelHydrationArgument.ValueSource.FieldResultValue, | ||||||||||||
| ): NadelValidationInterimResult<ExecutableNormalizedField> { | ||||||||||||
| val parentObjectType = | ||||||||||||
| parent.underlying.getFieldContainerFor(hydrationValueSource.queryPathToField.segments) as GraphQLObjectType | ||||||||||||
|
|
||||||||||||
| // todo: should probably check cardinality here too | ||||||||||||
| val field = makeObjectField( | ||||||||||||
| parentObjectType = parentObjectType, | ||||||||||||
| fieldName = hydrationValueSource.fieldDefinition.name, | ||||||||||||
| inputObjectType = argument.backingArgumentDef.type.unwrapAll() as GraphQLInputObjectType, | ||||||||||||
| outputObjectType = hydrationValueSource.fieldDefinition.type.unwrapAll() as GraphQLObjectType, | ||||||||||||
| ) | ||||||||||||
|
|
||||||||||||
| return if (hydrationValueSource.queryPathToField.size > 1) { | ||||||||||||
| NFUtil.createField( | ||||||||||||
| schema = backingService.underlyingSchema, | ||||||||||||
| parentType = parent.underlying as GraphQLObjectType, | ||||||||||||
| queryPathToField = hydrationValueSource.queryPathToField.dropLast(1), | ||||||||||||
| fieldArguments = emptyMap(), | ||||||||||||
| fieldChildren = listOf(field), | ||||||||||||
| ) | ||||||||||||
| } else { | ||||||||||||
| field | ||||||||||||
| }.asInterimSuccess() | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| private fun makeObjectField( | ||||||||||||
| parentObjectType: GraphQLObjectType, | ||||||||||||
| fieldName: String, | ||||||||||||
| inputObjectType: GraphQLInputObjectType, | ||||||||||||
| outputObjectType: GraphQLObjectType, | ||||||||||||
| ): ExecutableNormalizedField { | ||||||||||||
| val children = inputObjectType.fields | ||||||||||||
| .mapNotNull { inputField -> | ||||||||||||
| val equivalentOutputField = outputObjectType.getField(inputField.name) | ||||||||||||
| if (equivalentOutputField == null) { | ||||||||||||
| if (inputField.type.isNonNull) { // i.e. required | ||||||||||||
| error("Required input field is missed") // todo: proper error here | ||||||||||||
| } else { | ||||||||||||
| null | ||||||||||||
| } | ||||||||||||
| } else { | ||||||||||||
| val parentObjectType = parentObjectType.getField(fieldName).type.unwrapAll() as GraphQLObjectType | ||||||||||||
| if ( | ||||||||||||
| inputField.type.unwrapAll() is GraphQLInputObjectType | ||||||||||||
| || equivalentOutputField.type.unwrapAll() is GraphQLObjectType | ||||||||||||
| ) { | ||||||||||||
| makeObjectField( | ||||||||||||
| parentObjectType = parentObjectType, | ||||||||||||
| fieldName = inputField.name, | ||||||||||||
| inputObjectType = inputField.type.unwrapAll() as GraphQLInputObjectType, | ||||||||||||
| outputObjectType = equivalentOutputField.type.unwrapAll() as GraphQLObjectType, | ||||||||||||
| ) | ||||||||||||
| } else { | ||||||||||||
| ExecutableNormalizedField.newNormalizedField() | ||||||||||||
| .objectTypeNames(listOf(parentObjectType.name)) | ||||||||||||
| .fieldName(inputField.name) | ||||||||||||
| .build() | ||||||||||||
| } | ||||||||||||
| } | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| return ExecutableNormalizedField.newNormalizedField() | ||||||||||||
| .objectTypeNames(listOf(parentObjectType.name)) | ||||||||||||
| .fieldName(fieldName) | ||||||||||||
| .children(children) | ||||||||||||
| .build() | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| context(NadelValidationContext, NadelHydrationValidationContext) | ||||||||||||
| private fun makeLeafField( | ||||||||||||
| path: NadelQueryPath, | ||||||||||||
| ): ExecutableNormalizedField { | ||||||||||||
| // todo: should do some validation here?? e.g. arg is a scalar value, type validation? maybe type validation is done elsewhere already | ||||||||||||
| return NFUtil.createField( | ||||||||||||
| schema = parent.service.underlyingSchema, | ||||||||||||
| parentType = parent.underlying as GraphQLObjectType, | ||||||||||||
| queryPathToField = path, | ||||||||||||
| fieldArguments = emptyMap(), | ||||||||||||
| fieldChildren = emptyList(), // This must be a leaf node | ||||||||||||
| ) | ||||||||||||
| } | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| private fun List<ExecutableNormalizedField>.dedupSourceFields(): List<ExecutableNormalizedField> { | ||||||||||||
| return groupBy { | ||||||||||||
| listOf(it.objectTypeNames, it.resultKey, it.name, it.normalizedArguments.size) | ||||||||||||
| }.flatMap { (_, fields) -> | ||||||||||||
| if (fields.all { it.normalizedArguments.isEmpty() }) { | ||||||||||||
| listOf(fields.first()) | ||||||||||||
| } else { | ||||||||||||
| fields | ||||||||||||
| } | ||||||||||||
| } | ||||||||||||
| } | ||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added missing copy.