Add equals/hashCode to schema types

[kroepke]

In order to compose a schema from a set of independently constructed types and interfaces, all GraphQLType implementations should provide a equals/hashCode method.

I've chosen the name property here, because:

• according to the spec no two types can have the same name
• looking up possible implementations of interfaces relies on .equals, when used with DI reference equality breaks
• several places in SchemaUtil already identify types by their names

SchemaUtil could be converted to use Set<GraphQLType> in several places instead of Map<String, GraphQLType> but this is not part of this PR.

[andimarek]

Thanks for this PR!

I will comment on specific code issues.

[andimarek]

We talked about this, the real question is here, if we have to return null values.

[kroepke]

I don't think that GraphQLModifiedType implementations are ever asked for their name.
Of course that's a little bit hard to see from the interface hierarchy, because the base interface declares getName().

Technically we could return anything, it's never used anywhere. Of course that does not hinder any client to ask for name. The reference implementation uses GraphQLNamedType for this, I guess if we did that it would be clearer. Not sure if it's really worth it, though.

[yrashk]

It looks like #85 is somewhat related to this PR, unless I am missing something?

[andimarek]

Yes, this is the "compare the names in equals" version implementation of #85.

I would suggest to add two tests for every type, to show that equals and hashCode are working as intended.

Also equals uses getName() while hashCode uses name directly. This I think should be changed.

And I think it is only a first step because there might code which uses == to compare types instead equals.

[metacosm]

This fix (or a similar one that properly implements hashCode and equals) is actually needed to properly resolve fragments on union types when using graphql-java-servlet. Currently, FieldCollector.doesFragmentConditionMatch calls FieldCollector.checkTypeCondition where the resolved type is instantiated by my type resolver whereas the condition type is resolved from the schema. They are semantically identical but different instances thus resulting in conditionType.equals(type) failing.

[kroepke]

Just popping in, because I received an email today about a comment I can't find now:

I'm currently not actively using/trying to use GraphQL and have no spare cycles do maintain this.
So if someone wants to take this over, please feel free. I'm not even sure of what the state of Java-GraphQL is at the moment.

[dminkovsky]

@kroepke cool, thanks for checking in!

@metacosm

calls FieldCollector.checkTypeCondition where the resolved type is instantiated by my type resolver:

Yes, this issue is mentioned at the bottom of #205 (comment): it would be useful if the TypeResolver interface provided an instance of the schema in the resolution method. Otherwise, you need the schema somehow available in the TypeResolver's scope, which might require more work on your part.

But yes, as you see, the schema definition system as it stands is all about simplicity and therefore uses reference equality/identity. Not great, but at least simple.

I, too, would prefer a value-based schema system. But, I am not really comfortable with a hashCode/equals that's based entirely on one field like "name".

[andimarek]

I am closing this Issue: The main problem here is that a schema contains not only data but also logic (DataFetcher, TypeResolver). This is something we can't easily change for now, so we will not add equals/hashcode to them.