Skip to content

Commit

Permalink
Union hydration validation (#663)
Browse files Browse the repository at this point in the history 
* Improve default hydration for unions

* Add tests and fix up default hydration definition

* Fix tests
  • Loading branch information
@gnawf
gnawf authored Jan 7, 2025
1 parent 38ac346 commit d981abb
Show file tree
Hide file tree
Showing 8 changed files with 333 additions and 59 deletions.
14 changes: 1 addition & 13 deletions lib/src/main/java/graphql/nadel/definition/hydration/NadelDefaultHydrationDefinition.kt
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package graphql.nadel.definition.hydration

import graphql.language.ArrayValue
import graphql.language.DirectiveDefinition
import graphql.language.ObjectValue
import graphql.nadel.definition.hydration.NadelDefaultHydrationDefinition.Keyword
import graphql.nadel.engine.util.parseDefinition
import graphql.schema.GraphQLAppliedDirective
Expand Down Expand Up @@ -34,11 +32,9 @@ class NadelDefaultHydrationDefinition(
"How to identify matching results"
identifiedBy: String! = "id"
"The batch size"
batchSize: Int
batchSize: Int! = 200
"The timeout to use when completing hydration"
timeout: Int! = -1
"The arguments to the hydrated field"
arguments: [NadelHydrationArgument!]! = []
) on OBJECT | INTERFACE
""".trimIndent(),
)
Expand All @@ -56,13 +52,6 @@ class NadelDefaultHydrationDefinition(
val batchSize: Int
get() = appliedDirective.getArgument(Keyword.batchSize).getValue()

val arguments: List<NadelHydrationArgumentDefinition>
get() = (appliedDirective.getArgument(Keyword.arguments).argumentValue.value as ArrayValue)
.values
.map {
NadelHydrationArgumentDefinition.from(it as ObjectValue)
}

val timeout: Int
get() = appliedDirective.getArgument(Keyword.timeout).getValue()

Expand All @@ -72,7 +61,6 @@ class NadelDefaultHydrationDefinition(
const val identifiedBy = "identifiedBy"
const val batchSize = "batchSize"
const val timeout = "timeout"
const val arguments = "arguments"
const val idArgument = "idArgument"
}
}
77 changes: 45 additions & 32 deletions lib/src/main/java/graphql/nadel/validation/NadelIdHydrationDefinitionParser.kt
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package graphql.nadel.validation

import graphql.nadel.definition.hydration.NadelBatchObjectIdentifiedByDefinition
import graphql.nadel.definition.hydration.NadelDefaultHydrationDefinition
import graphql.nadel.definition.hydration.NadelHydrationArgumentDefinition
import graphql.nadel.definition.hydration.NadelHydrationConditionDefinition
import graphql.nadel.definition.hydration.NadelHydrationDefinition
Expand Down Expand Up @@ -59,13 +60,17 @@ internal class NadelIdHydrationDefinitionParser {
): NadelValidationInterimResult<List<NadelHydrationDefinition>> {
return NadelValidationInterimResult.Success.of(
virtualFieldType.types
.map { unionMemberType ->
getHydrationDefinitionForType(
parent,
virtualField,
idHydration,
unionMemberType as GraphQLObjectType,
).onErrorCast { return it }
.mapNotNull { unionMemberType ->
if ((unionMemberType as? GraphQLNamedType)?.getDefaultHydrationOrNull() == null) {
null
} else {
getHydrationDefinitionForType(
parent,
virtualField,
idHydration,
unionMemberType as GraphQLObjectType,
).onErrorCast { return it }
}
},
)
}
Expand All @@ -79,36 +84,44 @@ internal class NadelIdHydrationDefinitionParser {
val defaultHydration = (type as? GraphQLNamedType)?.getDefaultHydrationOrNull()
?: return NadelValidationInterimResult.Error.of(NadelMissingDefaultHydrationError(parent, virtualField))

val hydration = object : NadelHydrationDefinition {
override val backingField: List<String>
get() = defaultHydration.backingField
return NadelValidationInterimResult.Success.of(
NadelIdHydratedHydrationDefinition(
idHydration = idHydration,
defaultHydration = defaultHydration,
),
)
}
}

override val identifiedBy: String?
get() = idHydration.identifiedBy ?: defaultHydration.identifiedBy
internal class NadelIdHydratedHydrationDefinition(
private val idHydration: NadelIdHydrationDefinition,
private val defaultHydration: NadelDefaultHydrationDefinition,
) : NadelHydrationDefinition {
override val backingField: List<String>
get() = defaultHydration.backingField

override val isIndexed: Boolean
get() = false
override val identifiedBy: String?
get() = idHydration.identifiedBy ?: defaultHydration.identifiedBy

override val batchSize: Int
get() = defaultHydration.batchSize
override val isIndexed: Boolean
get() = false

override val arguments: List<NadelHydrationArgumentDefinition>
get() = listOf(
NadelHydrationArgumentDefinition.ObjectField(
name = defaultHydration.idArgument,
pathToField = idHydration.idField,
),
)
override val condition: NadelHydrationConditionDefinition?
get() = null
override val batchSize: Int
get() = defaultHydration.batchSize

override val timeout: Int
get() = defaultHydration.timeout
override val arguments: List<NadelHydrationArgumentDefinition>
get() = listOf(
NadelHydrationArgumentDefinition.ObjectField(
name = defaultHydration.idArgument,
pathToField = idHydration.idField,
),
)
override val condition: NadelHydrationConditionDefinition?
get() = null

override val inputIdentifiedBy: List<NadelBatchObjectIdentifiedByDefinition>?
get() = null
}
override val timeout: Int
get() = defaultHydration.timeout

return NadelValidationInterimResult.Success.of(hydration)
}
override val inputIdentifiedBy: List<NadelBatchObjectIdentifiedByDefinition>?
get() = null
}
18 changes: 17 additions & 1 deletion lib/src/main/java/graphql/nadel/validation/NadelSchemaHydrationValidationError.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import graphql.schema.GraphQLFieldsContainer
import graphql.schema.GraphQLInputFieldsContainer
import graphql.schema.GraphQLInputObjectField
import graphql.schema.GraphQLNamedOutputType
import graphql.schema.GraphQLObjectType
import graphql.schema.GraphQLNamedType
import graphql.schema.GraphQLType
import graphql.schema.GraphQLTypeUtil

Expand Down Expand Up @@ -613,3 +613,19 @@ data class NadelPolymorphicHydrationMustOutputUnionError(

override val subject = virtualField
}

data class NadelHydrationUnionMemberNoBackingError(
val parentType: NadelServiceSchemaElement,
val virtualField: GraphQLFieldDefinition,
val membersNoBacking: List<GraphQLNamedType>,
) : NadelSchemaValidationError {
val service: Service get() = parentType.service

override val message = run {
val vf = makeFieldCoordinates(parentType.overall.name, virtualField.name)
val memberNamesNoBacking = membersNoBacking.map { it.name }
"Field $vf is missing hydration(s) for possible union type(s) $memberNamesNoBacking"
}

override val subject = virtualField
}
40 changes: 40 additions & 0 deletions lib/src/main/java/graphql/nadel/validation/hydration/NadelHydrationValidation.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import graphql.nadel.engine.util.singleOfTypeOrNull
import graphql.nadel.engine.util.startsWith
import graphql.nadel.engine.util.unwrapAll
import graphql.nadel.engine.util.unwrapNonNull
import graphql.nadel.engine.util.whenType
import graphql.nadel.validation.NadelBatchHydrationArgumentMissingSourceFieldError
import graphql.nadel.validation.NadelBatchHydrationMatchingStrategyInvalidSourceIdError
import graphql.nadel.validation.NadelBatchHydrationMatchingStrategyReferencesNonExistentSourceFieldError
Expand All @@ -35,6 +36,7 @@ import graphql.nadel.validation.NadelHydrationIncompatibleOutputTypeError
import graphql.nadel.validation.NadelHydrationMustUseIndexExclusivelyError
import graphql.nadel.validation.NadelHydrationReferencesNonExistentBackingFieldError
import graphql.nadel.validation.NadelHydrationTypeMismatchError
import graphql.nadel.validation.NadelHydrationUnionMemberNoBackingError
import graphql.nadel.validation.NadelHydrationVirtualFieldMustBeNullableError
import graphql.nadel.validation.NadelPolymorphicHydrationIncompatibleSourceFieldsError
import graphql.nadel.validation.NadelPolymorphicHydrationMustOutputUnionError
Expand Down Expand Up @@ -116,6 +118,8 @@ class NadelHydrationValidation internal constructor() {
.onError { return it }
limitSourceField(parent, virtualField, hydrations)
.onError { return it }
validateUnion(parent, virtualField, hydrations)
.onError { return it }

return hydrations
.map { hydration ->
Expand All @@ -124,6 +128,42 @@ class NadelHydrationValidation internal constructor() {
.toResult()
}

context(NadelValidationContext)
private fun validateUnion(
parent: NadelServiceSchemaElement.FieldsContainer,
virtualField: GraphQLFieldDefinition,
hydrations: List<NadelHydrationDefinition>,
): NadelSchemaValidationResult {
val unionType = virtualField.type.unwrapAll() as? GraphQLUnionType
?: return ok()

val suppliedTypes = hydrations
.flatMap { hydration ->
val backingField = engineSchema.queryType.getFieldAt(hydration.backingField)
?: return NadelHydrationReferencesNonExistentBackingFieldError(
parent,
virtualField,
hydration,
)

backingField.type.unwrapAll()
.whenType(
enumType = ::listOf,
inputObjectType = ::listOf,
interfaceType = engineSchema::getImplementations,
objectType = ::listOf,
scalarType = ::listOf,
unionType = GraphQLUnionType::getTypes,
)
}

return if (suppliedTypes.containsAll(unionType.types)) {
ok()
} else {
NadelHydrationUnionMemberNoBackingError(parent, virtualField, unionType.types - suppliedTypes.toSet())
}
}

context(NadelValidationContext)
private fun validate(
parent: NadelServiceSchemaElement.FieldsContainer,
Expand Down
26 changes: 18 additions & 8 deletions lib/src/test/kotlin/graphql/nadel/validation/NadelHydrationValidationTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ class NadelHydrationValidationTest {
}

@Test
fun `fails if hydration actor field exists only in the underlying and not in the overall`() {
fun `fails if hydration backing field exists only in the underlying and not in the overall`() {
val fixture = NadelValidationTestFixture(
overallSchema = mapOf(
"issues" to """
Expand Down Expand Up @@ -470,7 +470,7 @@ class NadelHydrationValidationTest {
}

@Test
fun `fails if hydration actor field does not exist`() {
fun `fails if hydration backing field does not exist`() {
val fixture = NadelValidationTestFixture(
overallSchema = mapOf(
"issues" to """
Expand Down Expand Up @@ -903,7 +903,7 @@ class NadelHydrationValidationTest {
}

@Test
fun `checks the output type of the actor field against the output type of the hydrated field`() {
fun `checks the output type of the backing field against the output type of the hydrated field`() {
val fixture = NadelValidationTestFixture(
overallSchema = mapOf(
"issues" to """
Expand Down Expand Up @@ -991,12 +991,17 @@ class NadelHydrationValidationTest {
id: ID!
creator: AbstractUser
@hydrated(
service: "users"
field: "externalUser"
arguments: [
{name: "id", value: "$source.creatorId"}
]
)
@hydrated(
field: "user"
arguments: [
{name: "id", value: "$source.creatorId"}
]
)
}
union AbstractUser = User
""".trimIndent(),
Expand Down Expand Up @@ -1052,7 +1057,7 @@ class NadelHydrationValidationTest {
}

@Test
fun `fails if actor output type does not implement interface`() {
fun `fails if backing output type does not implement interface`() {
val fixture = NadelValidationTestFixture(
overallSchema = mapOf(
"issues" to """
Expand Down Expand Up @@ -1128,7 +1133,7 @@ class NadelHydrationValidationTest {
}

@Test
fun `passes if actor output type implements the interface`() {
fun `passes if backing output type implements the interface`() {
val fixture = NadelValidationTestFixture(
overallSchema = mapOf(
"issues" to """
Expand Down Expand Up @@ -1200,7 +1205,7 @@ class NadelHydrationValidationTest {
}

@Test
fun `passes if actor output type belongs in union`() {
fun `passes if backing output type belongs in union`() {
val fixture = NadelValidationTestFixture(
overallSchema = mapOf(
"issues" to """
Expand All @@ -1211,12 +1216,17 @@ class NadelHydrationValidationTest {
id: ID!
creator: AbstractUser
@hydrated(
service: "users"
field: "externalUser"
arguments: [
{name: "id", value: "$source.creatorId"}
]
)
@hydrated(
field: "user"
arguments: [
{name: "id", value: "$source.creatorId"}
]
)
}
""".trimIndent(),
"users" to """
Expand Down
Loading

0 comments on commit d981abb

Please sign in to comment.