graphql-java: Reference Equality Check in VariableTypesMatchRule breaks if GraphQLInputObjectType is created multiple times.

Normally in schema creation, it is OK to create multiple instances of the same type object. However, VariableTypesMatchRule uses reference equality, so if I create my schema with multiple instances of an equivalent GraphQLObjectInputType then I will get validation failures. This was tricky to track down because I create input types in methods (e.g. private GraphQLInputObjectType createPropertyMatchType()) and then call those methods when I need it them.

I think the ideal solution is to use deep equality instead of reference equality in VariableTypeMatchRule.

Otherwise some process in schema creation should canonicalize the types within the schema.

About this issue

Original URL
State: closed
Created 8 years ago
Reactions: 1
Comments: 26 (21 by maintainers)

Most upvoted comments

From this, I infer that two types with the same name must be equal or the schema is invalid.

Right: they must be equal according to the spec. But what if due to say, a bug, they are not?

Say I have one spot in my code where I have:

newObject
  .name("Obj")
  .field(newFieldDefinition()
    .name("field")
    .type(GraphQLString)
    .build())
  .build()

and another where:

newObject
  .name("Obj")
  .field(newFieldDefinition()
    .name("field")
    .type(GraphQLInt)
    .build())
  .build()

then, say, in some other code I compare these definitions and the result is they are equivalent. Then how much longer down the line will this bug manifest?

My point is—and sorry truly if I am missing something or am just being too pedantic—that yes, the spec says a type’s name should be unique. But from a runtime Java perspective, two object definitions with the same name can be totally different, and the usefulness of an equivalence relation that only compares their names seems, well, not useful, and even worse, bug prone?

dminkovsky on Oct 20, 2016

Nothing is stopping them to do so. What we can do is to refuse adding two unequal types with the same name to the schema, proactively, so this type of mistake can be caught early.

On Friday, 6 May 2016, danielkwinsor notifications@github.com wrote:

In terms of my graphql-java-type-generator, it stores types with key == type name, so there cannot be duplicate GraphQL types. Further, the default type name is the FQN with underscores, also with the ability to override the name. (But actually I did realize something I could do better).

So actually I am thinking in the more general case. What is to stop a user from hand coding 2 AccountTypes, which then may or may not be equal if we make an equality change?

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/graphql-java/graphql-java/issues/85#issuecomment-217508577

yrashk on May 6, 2016

The graphql schema was designed to be based on the principle that for each type there is exactly one Java Instance representing this type. That’s the reason the ckecks are using reference equality. This is just simpler.

Of course we could change that, but I am not sure about the overall consequences.

andimarek on May 6, 2016

I think I ran into a similar case with fragments (PossibleFragmentSpreads), particularly this https://github.com/andimarek/graphql-java/blob/master/src/main/java/graphql/validation/rules/PossibleFragmentSpreads.java#L76 makes use of reference equality.

yrashk on May 4, 2016