Skip to content

Issue 3332 - overlapping aliases should be prevented #3334

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dondonz merged 2 commits into from
Oct 17, 2023
Merged

Issue 3332 - overlapping aliases should be prevented #3334

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3c50b22
Issue 3332 - overlapping aliases should be prevented
bbakerman Sep 21, 2023
2f81ac2
Issue 3332 - the problem was unwrapped types
bbakerman Sep 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/main/java/graphql/validation/rules/OverlappingFieldsCanBeMerged.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ private void collectFieldsForField(Map<String, Set<FieldAndType>> fieldMap, Grap
GraphQLFieldDefinition fieldDefinition = getVisibleFieldDefinition(fieldsContainer, field);
fieldType = fieldDefinition != null ? fieldDefinition.getType() : null;
}
fieldMap.get(responseName).add(new FieldAndType(field, fieldType, parentType));
fieldMap.get(responseName).add(new FieldAndType(field, fieldType, unwrappedParent));
Copy link
Member Author

@bbakerman bbakerman Sep 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the bug. When the parent type is a non null (or list) wrapped type - it was not being held in the field+ parent type construct as that BUT it was being compared as if it was a fully unwrapped type

}

private GraphQLFieldDefinition getVisibleFieldDefinition(GraphQLFieldsContainer fieldsContainer, Field field) {
Expand Down
53 changes: 53 additions & 0 deletions src/test/groovy/graphql/validation/rules/OverlappingFieldsCanBeMergedTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,7 @@ class OverlappingFieldsCanBeMergedTest extends Specification {
def schema = schema('''
type Dog {
nickname: String
name : String
}
type Query { dog: Dog }
''')
Expand All @@ -349,6 +350,58 @@ class OverlappingFieldsCanBeMergedTest extends Specification {
errorCollector.getErrors()[0].locations == [new SourceLocation(3, 13), new SourceLocation(4, 13)]
}

def 'issue 3332 - Alias masking direct field access non fragment'() {
given:
def query = """
{ dog {
name: nickname
name
}}
"""
def schema = schema('''
type Dog {
name : String
nickname: String
}
type Query { dog: Dog }
''')
when:
traverse(query, schema)

then:
errorCollector.getErrors().size() == 1
errorCollector.getErrors()[0].message == "Validation error (FieldsConflict@[dog]) : 'name' : 'nickname' and 'name' are different fields"
errorCollector.getErrors()[0].locations == [new SourceLocation(3, 13), new SourceLocation(4, 13)]
Copy link
Member Author

@bbakerman bbakerman Sep 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does NOT fail ; wat ??

Copy link
Member Author

@bbakerman bbakerman Sep 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dog parent field was nullable

}

def 'issue 3332 -Alias masking direct field access non fragment with non null parent type'() {
given:
def query = """
query GetCat {
cat {
foo1
foo1: foo2
}
}
"""
def schema = schema('''
type Query {
cat: Cat! # non null parent type
}
type Cat {
foo1: String!
foo2: String!
}
''')
when:
traverse(query, schema)

then:
errorCollector.getErrors().size() == 1
errorCollector.getErrors()[0].message == "Validation error (FieldsConflict@[cat]) : 'foo1' : 'foo1' and 'foo2' are different fields"
errorCollector.getErrors()[0].locations == [new SourceLocation(4, 17), new SourceLocation(5, 17)]
}
Copy link
Member Author

@bbakerman bbakerman Sep 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fails!

Copy link
Member Author

@bbakerman bbakerman Sep 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and is now fixed


def 'conflicting args'() {
given:
def query = """
Expand Down