Skip to content

Commit

Permalink
Prevent multiple source fields even if list is not leaf field (#503)
Browse files Browse the repository at this point in the history
* Prevent multiple source fields even if list is not leaf field

* Rename var
  • Loading branch information
gnawf authored Dec 21, 2023
1 parent 895f3e8 commit 9e58b81
Show file tree
Hide file tree
Showing 2 changed files with 110 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import graphql.nadel.dsl.RemoteArgumentSource.SourceType.ObjectField
import graphql.nadel.dsl.RemoteArgumentSource.SourceType.StaticArgument
import graphql.nadel.dsl.UnderlyingServiceHydration
import graphql.nadel.engine.util.getFieldAt
import graphql.nadel.engine.util.getFieldsAlong
import graphql.nadel.engine.util.isList
import graphql.nadel.engine.util.isNonNull
import graphql.nadel.engine.util.partitionCount
Expand Down Expand Up @@ -120,30 +121,31 @@ internal class NadelHydrationValidation(
hydrations: List<UnderlyingServiceHydration>,
): List<NadelSchemaValidationError> {
if (hydrations.size > 1) {
val anyListSourceInputField = hydrations
.any { hydration ->
val parentType = parent.underlying as GraphQLFieldsContainer
val pathsToSourceFields = hydrations
.asSequence()
.flatMap { hydration ->
hydration
.arguments
.asSequence()
.mapNotNull { argument ->
argument.remoteArgumentSource.pathToField
}
.any { path ->
parentType.getFieldAt(path)?.type?.unwrapNonNull()?.isList == true
.mapNotNull { it.remoteArgumentSource.pathToField }
}
.toList()

val parentType = parent.underlying as GraphQLFieldsContainer
val anyListSourceInputField = pathsToSourceFields
.any { pathToField ->
parentType
.getFieldsAlong(pathToField)
.any { field ->
field.type.unwrapNonNull().isList
}
}

if (anyListSourceInputField) {
val sourceFields = hydrations
.flatMapTo(LinkedHashSet()) { hydration ->
hydration.arguments
.mapNotNull { argument ->
argument.remoteArgumentSource.pathToField
}
}

if (sourceFields.size > 1) {
val uniqueSourceFieldPaths = pathsToSourceFields
.toSet()

if (uniqueSourceFieldPaths.size > 1) {
return listOf(
NadelSchemaValidationError.MultipleHydrationSourceInputFields(parent, overallField),
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,97 @@ class NadelHydrationValidationTest2 {
assertTrue(errors.single().subject.name == "data")
}

@Test
fun `prohibit multiple source fields where list not the leaf`() {
val fixture = NadelValidationTestFixture(
overallSchema = mapOf(
"activity" to /* language=GraphQL*/ """
type Query {
myActivity: [Activity]
}
union ActivityContent = User | Issue
type Activity {
id: ID!
data: [ActivityContent]
@hydrated(
service: "users"
field: "usersByIds"
arguments: [
{name: "ids", value: "$source.contexts.userId"}
]
)
@hydrated(
service: "issues"
field: "issuesByIds"
arguments: [
{name: "ids", value: "$source.contexts.issueId"}
]
)
}
""".trimIndent(),
"users" to /* language=GraphQL*/ """
type Query {
usersByIds(ids: [ID]!): [User]
}
type User {
id: ID!
name: String!
}
""".trimIndent(),
"issues" to /* language=GraphQL*/ """
type Query {
issuesByIds(ids: [ID]!): [Issue]
}
type Issue {
id: ID!
key: String
}
""".trimIndent(),
),
underlyingSchema = mapOf(
"activity" to /* language=GraphQL*/ """
type Query {
myActivity: [Activity]
}
type ActivityContext {
issueId: ID
userId: ID
}
type Activity {
id: ID!
contexts: [ActivityContext]
}
""".trimIndent(),
"users" to /* language=GraphQL*/ """
type Query {
usersByIds(ids: [ID]!): [User]
}
type User {
id: ID!
name: String!
}
type Account {
id: ID!
}
""".trimIndent(),
"issues" to /* language=GraphQL*/ """
type Query {
issuesByIds(ids: [ID]!): [Issue]
}
type Issue {
id: ID!
key: String
}
""".trimIndent(),
),
)

val errors = validate(fixture)
assertTrue(errors.map { it.message }.isNotEmpty())
assertTrue(errors.single() is NadelSchemaValidationError.MultipleHydrationSourceInputFields)
assertTrue(errors.single().subject.name == "data")
}

@Test
fun `prohibit mixing list and non-list source input fields`() {
val fixture = NadelValidationTestFixture(
Expand Down

0 comments on commit 9e58b81

Please sign in to comment.