Skip to content

Conversation

@tinnou
Copy link
Contributor

@tinnou tinnou commented Sep 27, 2020

Description

When using the AstTransformer, and working on extensions, I noticed in some cases when changing a child node on an extension, the resulting node would wrongly be converted to its base type.

Here is an example, given:

extend type MyType {
  login: String
}

and changing the field login to say signin using the AstTransformer will have the effect of changing MyType extension object type to an object type.
Resulting document would be:

type MyType {
  signin: String
}

Fix

This bug happens because the AstTransformer uses the withNewChildren() method on each definition to reattach the children and this method being missing in extensions would defer to using withNewChildren() on the base definition returning a new instance of the base instead of the extensions.

Adding the appropriate override in each extension definition solves the issue.

I also noticed while writing the test, the AstPrinter was missing knowledge on how to print schema extensions.

Testing

Unit test in AstTransformerTest covers all types of extensions.
Unit test in AstPrinterTest covers the missed schema extension printing.

(I can backport on 14.x once it merges)

@bbakerman bbakerman added this to the 16.0 milestone Oct 1, 2020
@bbakerman
Copy link
Member

Thanks for this

@bbakerman
Copy link
Member

Excellent code as always

@bbakerman bbakerman added the needs to be backported a bugfix that still needs to be backported label Oct 1, 2020
@bbakerman bbakerman merged commit 3998b0d into graphql-java:master Oct 1, 2020
tinnou added a commit to tinnou/graphql-java that referenced this pull request Oct 1, 2020
bbakerman pushed a commit that referenced this pull request Oct 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs to be backported a bugfix that still needs to be backported

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants