Skip to content

Add type reference support to interface builders#2602

Merged
bbakerman merged 2 commits into
graphql-java:masterfrom
dondonz:2546-interface-visitor
Nov 15, 2021
Merged

Add type reference support to interface builders#2602
bbakerman merged 2 commits into
graphql-java:masterfrom
dondonz:2546-interface-visitor

Conversation

@dondonz
Copy link
Copy Markdown
Member

@dondonz dondonz commented Nov 1, 2021

Fixes #2546.

An interface (or object) can contain a type reference to itself. The referenced type is replaced with the real type object when the schema is built.

This PR adds support for type references in GraphQLInterfaceType builders, making these equivalent to builders in GraphQLObjectType.

return this;
}

public Builder withInterface(GraphQLInterfaceType interfaceType) {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Shifting this down to be adjacent to the other withInterface method

Copy link
Copy Markdown
Member

@bbakerman bbakerman left a comment

Choose a reason for hiding this comment

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

See the comments on how to avoid a breaking change

}

public Builder replaceInterfaces(List<GraphQLInterfaceType> interfaces) {
public Builder replaceInterfaces(List<? extends GraphQLNamedOutputType> interfaces) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is an API breaking change - albeit a low likelyhood one - but a breaks a break so if we can NOT do it - let not do it.

This can be rewritten like this to avoid the breaking change

        public Builder replaceInterfaces(List<GraphQLInterfaceType> interfaces) {
            return replaceInterfacesOrReferences(interfaces);
        }

        public Builder replaceInterfacesOrReferences(List<? extends GraphQLNamedOutputType> interfacesOrReferences) {

and then up above you can change the impl to be

    public GraphQLInterfaceType withNewChildren(SchemaElementChildrenContainer newChildren) {
        return transform(builder ->
                builder.replaceDirectives(newChildren.getChildren(CHILD_DIRECTIVES))
                        .replaceFields(newChildren.getChildren(CHILD_FIELD_DEFINITIONS))
                        .replaceInterfacesOrReferences(newChildren.getChildren(CHILD_INTERFACES))
        );
    }

return this;
}

public Builder withInterfaces(GraphQLTypeReference... references) {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Add for completeness

@dondonz dondonz requested a review from bbakerman November 14, 2021 22:09
@bbakerman bbakerman added this to the 18.0 milestone Nov 15, 2021
@bbakerman bbakerman merged commit 8c7249b into graphql-java:master Nov 15, 2021
@dondonz dondonz deleted the 2546-interface-visitor branch November 15, 2021 22:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ClassCastException when changing fields with visitor

2 participants