You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Failed validation for alias clashing with another field #3332
@Test
funtest() {
val schemaString =""" type Query { cat: Cat! } type Cat { foo1: String! foo2: String! }""".trimIndent()
val documentString =""" query GetCat { cat { foo1 foo1: foo2 } }""".trimIndent()
val typeDefinitionRegistry =SchemaParser().parse(schemaString)
val document =Parser().parseDocument(documentString)
val graphQLSchema =SchemaGenerator().makeExecutableSchema(typeDefinitionRegistry, RuntimeWiring.newRuntimeWiring().build())
val result =Validator().validateDocument(graphQLSchema, document, Locale.US)
// There should be a validation error at this point.
check(result.isNotEmpty())
}
I would expect a validation error because foo1 and foo1: foo2 cannot be merged.
I think I found the problem - we failed to use unwrapped parent types in the code. The earlier idea that it was ordering if aliases was a red herring.
But in the existing tests we have a simple type dog : Dog while the provided example above is cat : Cat! - that is its a modified nonnull type wrapping a Cat type.
The code failed to store the unwrapped part when it associates it per field per parent type. Then later it does compositions on what it expects to be unwrapped and it does match and hence it thinks it's ok.
This has been like this since maybe Aug 2021 based on git history... maybe even longer.
To hit this you need to have a "clashing field name / field alias" AND it needed to be inside a modified type (eg non null or list)
Activity
bbakerman commentedon Sep 21, 2023
I would expect this to be the case as well
bbakerman commentedon Sep 21, 2023
https://spec.graphql.org/June2018/#sec-Field-Selection-Merging says as much
bbakerman commentedon Sep 21, 2023
graphql.validation.rules.OverlappingFieldsCanBeMergedTest#Alias masking direct field accessseems to have test for thisbbakerman commentedon Sep 21, 2023
I made a reproduction PR : #3334
Its failing BUT I have another case that is passing. Curious - I haven't debugged it further but this one looks like a curly one
martinbonnin commentedon Sep 21, 2023
The plot thickens... Thanks for the reproducer!
bbakerman commentedon Sep 21, 2023
More observations as I test (and so I dont lose them)
This ordering passes the test and kit produces errors as expected . However
does not pass the test and it produces NO validation errors as reported above.
It seems the ordering of the alias is confusing the validation code. Not sure what because I haven't got that far
bbakerman commentedon Sep 21, 2023
I think I found the problem - we failed to use unwrapped parent types in the code. The earlier idea that it was ordering if aliases was a red herring.
But in the existing tests we have a simple type
dog : Dogwhile the provided example above iscat : Cat!- that is its a modified nonnull type wrapping aCattype.The code failed to store the unwrapped part when it associates it per field per parent type. Then later it does compositions on what it expects to be unwrapped and it does match and hence it thinks it's ok.
This has been like this since maybe Aug 2021 based on git history... maybe even longer.
To hit this you need to have a "clashing field name / field alias" AND it needed to be inside a modified type (eg non null or list)
martinbonnin commentedon Sep 21, 2023
I'm relived that GraphQL isn't biased for 🐶 over 🐱 :). Thanks for the thorough investigation!
dondonz commentedon Oct 17, 2023
Fixed by #3334