Skip to content

Failed validation for alias clashing with another field #3332

Closed

Description

@martinbonnin

The following test fails:

    @Test
    fun test() {
        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.

Edit: this is on version 21.1

Activity

  1. bbakerman commented on Sep 21, 2023

    @bbakerman
    Member

    I would expect this to be the case as well

  2. bbakerman commented on Sep 21, 2023

    @bbakerman
    Member
  3. bbakerman commented on Sep 21, 2023

    @bbakerman
    Member

    graphql.validation.rules.OverlappingFieldsCanBeMergedTest#Alias masking direct field access seems to have test for this

    fragment aliasMaskingDirectFieldAccess on Dog {
                name: nickname
                name
            }
            ```
    
    but maybe there is a fragement specific bit of code.  Checking now...
    
  4. bbakerman commented on Sep 21, 2023

    @bbakerman
    Member

    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

  5. martinbonnin commented on Sep 21, 2023

    @martinbonnin
    ContributorAuthor

    The plot thickens... Thanks for the reproducer!

  6. bbakerman commented on Sep 21, 2023

    @bbakerman
    Member

    More observations as I test (and so I dont lose them)

            { dog {
                name: nickname
                name
            }}
    

    This ordering passes the test and kit produces errors as expected . However

            { dog {
                name
                name: nickname
            }}
    
    

    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

  7. bbakerman commented on Sep 21, 2023

    @bbakerman
    Member

    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)

  8. martinbonnin commented on Sep 21, 2023

    @martinbonnin
    ContributorAuthor

    I'm relived that GraphQL isn't biased for 🐶 over 🐱 :). Thanks for the thorough investigation!

  9. dondonz commented on Oct 17, 2023

    @dondonz
    Member

    Fixed by #3334

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions