From abc7594b9bcfd0c880f94a2121f6f731341b234c Mon Sep 17 00:00:00 2001 From: Franklin Wang Date: Wed, 20 Dec 2023 12:11:12 +1300 Subject: [PATCH 1/2] Prevent multiple source fields even if list is not leaf field --- .../validation/NadelHydrationValidation.kt | 37 ++++---- .../NadelHydrationValidationTest2.kt | 91 +++++++++++++++++++ 2 files changed, 111 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..47297bd22 100644 --- a/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt +++ b/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt @@ -3,11 +3,13 @@ package graphql.nadel.validation import graphql.GraphQLContext import graphql.nadel.Service import graphql.nadel.dsl.RemoteArgumentDefinition +import graphql.nadel.dsl.RemoteArgumentSource import graphql.nadel.dsl.RemoteArgumentSource.SourceType.FieldArgument 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 +122,31 @@ internal class NadelHydrationValidation( hydrations: List, ): List { if (hydrations.size > 1) { - val anyListSourceInputField = hydrations - .any { hydration -> - val parentType = parent.underlying as GraphQLFieldsContainer + val sourceFields = 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 = sourceFields + .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 = sourceFields + .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( From 4c3b82037d70a09c5b531a3452003ee9bbfeb7c3 Mon Sep 17 00:00:00 2001 From: Franklin Wang Date: Wed, 20 Dec 2023 12:15:36 +1300 Subject: [PATCH 2/2] Rename var --- .../graphql/nadel/validation/NadelHydrationValidation.kt | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt b/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt index 47297bd22..3c61d5e9e 100644 --- a/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt +++ b/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt @@ -3,7 +3,6 @@ package graphql.nadel.validation import graphql.GraphQLContext import graphql.nadel.Service import graphql.nadel.dsl.RemoteArgumentDefinition -import graphql.nadel.dsl.RemoteArgumentSource import graphql.nadel.dsl.RemoteArgumentSource.SourceType.FieldArgument import graphql.nadel.dsl.RemoteArgumentSource.SourceType.ObjectField import graphql.nadel.dsl.RemoteArgumentSource.SourceType.StaticArgument @@ -122,7 +121,7 @@ internal class NadelHydrationValidation( hydrations: List, ): List { if (hydrations.size > 1) { - val sourceFields = hydrations + val pathsToSourceFields = hydrations .asSequence() .flatMap { hydration -> hydration @@ -133,7 +132,7 @@ internal class NadelHydrationValidation( .toList() val parentType = parent.underlying as GraphQLFieldsContainer - val anyListSourceInputField = sourceFields + val anyListSourceInputField = pathsToSourceFields .any { pathToField -> parentType .getFieldsAlong(pathToField) @@ -143,7 +142,7 @@ internal class NadelHydrationValidation( } if (anyListSourceInputField) { - val uniqueSourceFieldPaths = sourceFields + val uniqueSourceFieldPaths = pathsToSourceFields .toSet() if (uniqueSourceFieldPaths.size > 1) {