Skip to content

Conversation

@jhaber
Copy link
Contributor

@jhaber jhaber commented Feb 7, 2019

This PR is a fix for #228.

As far as I can tell, the mapClass getting passed in is never a ParameterizedType so PropertyMapResolver#getMapGenericType always returns Object::class.java (and even if it was generic, the code is looking at the type parameters on the wrong class). This leads to the limitation documented here:

Note: If one of the values of a type backed by a java.util.Map is non-scalar then this type will need to be added to the type dictionary (see below).

This PR attempts to fix the issue by using ClassMate to introspect the Map type parameters. This fixes the issue described in #228 and makes map resolvers work without the need to add to the dictionary.

However, this PR doesn't fix the issue with mapClass losing it's generics. This means that the return type of your method can't be Map, otherwise you'll get the previous behavior of detecting Object::class.java (to be safe, the return type shouldn't be generic at all). For example, this code gets detected properly and doesn't need to use the dictionary:

public class PropertiesMap implements Map<String, PropertyValue> {}

public PropertiesMap getProperties() {
  return new PropertiesMap(...);
}

However this code would detect Object::class.java and need to still use the dictionary:

public class PropertiesMap implements Map<String, PropertyValue> {}

public Map<String, PropertyValue> getProperties() {
  return new PropertiesMap(...);
}

@oliemansm oliemansm merged commit b4b2627 into graphql-java-kickstart:master Feb 16, 2019
@oliemansm
Copy link
Member

Thanks @jhaber!

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.

2 participants