From 9e58b81851f1b6614b3160443b519c19823b4b54 Mon Sep 17 00:00:00 2001 From: Franklin Wang <9077461+gnawf@users.noreply.github.com> Date: Thu, 21 Dec 2023 15:29:48 +1300 Subject: [PATCH] Prevent multiple source fields even if list is not leaf field (#503) * Prevent multiple source fields even if list is not leaf field * Rename var --- .../validation/NadelHydrationValidation.kt | 36 ++++---- .../NadelHydrationValidationTest2.kt | 91 +++++++++++++++++++ 2 files changed, 110 insertions(+), 17 deletions(-) diff --git a/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt b/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt index fe68ff685..3c61d5e9e 100644 --- a/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt +++ b/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt @@ -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 @@ -120,30 +121,31 @@ internal class NadelHydrationValidation( hydrations: List, ): List { 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), ) diff --git a/lib/src/test/kotlin/graphql/nadel/validation/NadelHydrationValidationTest2.kt b/lib/src/test/kotlin/graphql/nadel/validation/NadelHydrationValidationTest2.kt index d8c2265d2..fe6627343 100644 --- a/lib/src/test/kotlin/graphql/nadel/validation/NadelHydrationValidationTest2.kt +++ b/lib/src/test/kotlin/graphql/nadel/validation/NadelHydrationValidationTest2.kt @@ -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(