graphql-java-kickstart/graphql-java-tools

Expected object type [...] to exist for union

marcus-nl opened this issue · 1 comments

Description

Suppose we have a union U = Foo | Bar and GraphQL type Bar is mapped to the Java interface Bar. In this case the concrete value for type Bar must be a class that implements Bar, i.e.:

public interface Bar { ... }
public class BarImpl implements Bar { ... }

// and then a resolver method somewhere (GraphQL signature is `fooOrBar: U!`):
public Object getFooOrBar() { return new BarImpl(...); }

The UnionTypeResolver supports this scenario due to PR 140, which looks at the direct interfaces of the value.

However, if the type hierarchy is a bit more complex it will fail. Suppose we have an additional intermediate interface:

public interface Bar { ... }
public interface BarEntity { ... }
public class BarEntityImpl implements BarEntity { ... }

// and then a resolver method somewhere (GraphQL signature is `fooOrBar: U!`)
public Object getFooOrBar() { return new BarEntityImpl(...); }

In this case the resolver will fail to resolve the type, since GraphQL type Bar is mapped to interface Bar, and that's not a direct superinterface of BarEntityImpl.

Expected behavior

I would expect class BarEntityImpl to succesfully resolve to GraphQL type Bar, since it implements (indirectly) interface Bar.

Actual behavior

Resolving fails with error "Expected object type with name 'BarEntityImpl' to exist for union 'U', but it doesn't!".

Steps to reproduce the bug

Pull request #559 reproduces this issue.

I've created PR #559 with a test case and proposed fix for this issue.

Some additional thoughts:

  1. I wonder why the return type for getType is GraphQLObjectType? (i.e. nullable) instead of GraphQLObjectType (non-nullable) as it is in TypeResolver. Currently getType never returns null right? Note that I am new to Kotlin, so I may be misunderstanding something.
  2. An alternative approach specifically for resolving union types could be to maintain a collection of mappings from the union's element types to the corresponding classes. getType would then only have to iterate over the entries and use the class' isAssignableFrom to select a matching one. This should be simpler and more efficient.