Skip to content

Conversation

@dondonz
Copy link
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
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
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
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
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