Polymorphic Type Handling

[CaptnBlubber]

Currently the Codegeneration puts Type Instances as fields to an Generic Object.
To make it more clear here is an Example:

For a query like:

query HeroForEpisode($ep: Episode!) {
  hero(episode: $ep) {
    ... on Droid {
        name      
    }
    ... on Human {
      name      
    }
  }
}

we do get the following Generated as Java Code:

public static class Hero {
  private @Nullable AsHuman AsHuman;

  private @Nullable AsDroid AsDroid;

  public Hero(@Nullable AsHuman AsHuman,
    @Nullable AsDroid AsDroid) {
    this.AsHuman = AsHuman;
    this.AsDroid = AsDroid;
  }

  public @Nullable AsHuman AsHuman() {
    return this.AsHuman;
  }

  public @Nullable AsDroid AsDroid() {
    return this.AsDroid;
  }

  public static class AsHuman {
    private final @Nullable String name;

    public AsHuman(@Nullable String name) {
      this.name = name;
    }

    public @Nullable String name() {
      return this.name;
    }
  }

  public static class AsDroid {
    private final @Nullable String name;

    public AsDroid(@Nullable String name) {
      this.name = name;
    }

    public @Nullable String name() {
      return this.name;
    }
  }
}

This makes detection of Subtypes not convenient as you always have to check weather the return type of as<type>()is null. In my opinion using thing.instanceOf(Clazz.class) produces better code.

My suggestion is to change the generic class to abstract and create instances of the mapped subtypes, so the generated Java Classes would look something like this:

public abstract static class Hero {
  public Hero() {
    
    }

  public static class AsHuman extends Hero {
    private final @Nullable String name;

    public AsHuman(@Nullable String name) {
      this.name = name;
    }

    public @Nullable String name() {
      return this.name;
    }
  }

  public static class AsDroid extends Hero{
    private final @Nullable String name;

    public AsDroid(@Nullable String name) {
      this.name = name;
    }

    public @Nullable String name() {
      return this.name;
    }
  }
}

[sav007]

@s3xy4ngyc making abstraction still won't save you from null as hero can be null:

public static class Hero {
  private @Nullable Hero hero;

but this check is still valid if (hero.hero() instanceof Hero.Human) even for nulls.
Only one concern the name looks weird hero.hero() vs hero.asHuman()

@martijnwalraven Any thoughts regarding this?

[CaptnBlubber]

@sav007 there is no need for having Hero as field in the Hero class. You actually just call hero instanceof Hero.Human

It gets clearer when we are talking about Lists.
For now a List would be like:
[“@Hero", “@Hero", “@Hero", “@Hero", “@Hero"]
Lets just assume we implement something simple by printing the class name (As subtype):

(for Hero h in response.data().movie().heroes()) {
    if(h.asHuman() != null { print(h.asHuman().getClass().getSimpleName());}
    if(h.asDroid() != null { print(h.asDroid().getClass().getSimpleName());}
}

As of when we would directly instantiate the Subtypes we would end up having something like this:
[“@Human", “@Droid", “@Droid", “@Droid", “@Human"]

(for Hero h in response.data().movie().heroes()) {
    print(h.getClass().getSimpleName());
}

[sav007]

@s3xy4ngyc unfortunately this idea is not valid for Unions.

So there are 2 ways to get heterogeneous list like we have for Droids, Humans.
You can use unions or interfaces.

With interfaces everything seems ok, we have parent interface Character that we use as a parent class for generated classes Droid and Human.

But for unions you can mix any objects, that don't require to have the same parent interface. So the parent for such classes will be Object that not nice (to have response.data().movie().heroes() as List<Object>).

[sav007]

https://medium.com/the-graphqlhub/graphql-tour-interfaces-and-unions-7dd5be35de0d#.pd22ebukm

[AlexWih]

@sav007 Checked with the interface types:
For a query like:

query HeroAndFriends($episode: Episode) {
  hero(episode: $episode) {
    name
      ... on Droid {
        primaryFunction
      }
      ... on Human {
        height
      }
  }
}

Where Droid and Human declared with parent interface Character the generated code has classes AsDroid and AsHuman and they do not implement Character.
So, it means we still can't put create heterogeneous list like List with them.
The base interface Character is not even generated.

[sav007]

@AlexWih sorry for confusion but I described the possible scenarios
Yes, right now we don't generated parent class for types with interfaces.

But the whole point is there is no way we can make solution that will work for both cases: with interfaces and with unions.

[martijnwalraven]

@sav007: Maybe I don't understand the issue, but couldn't you just generate an abstract class for a union and have the possible types extend it, just as you would for a field of an interface type?

In the code you proposed above, I don't think it matters whether the type of hero is an interface or a union. In both cases, you could generate a Hero abstract class. The only difference is that Hero will never contain any fields of its own in the union case, because the possible types don't share any fields.

Another way of looking at it is that you wouldn't want to generate an interface for Character either, just an abstract class Hero which contains a subset of the fields from Character as specified in the selection set for hero.

[sav007]

@martijnwalraven

The only difference is that Hero will never contain any fields of its own in the union case, because the possible types don't share any fields.

Agree, makes sense.

Question: our apollo-codegen doesn't provide any information regarding parent interface

"inlineFragments": [
	{
		"typeCondition": "Human",
		"possibleTypes": [
			"Human"
		],
		"fields": [
			{
				"responseName": "name",
				"fieldName": "name",
				"type": "String!"
			},
			{
				"responseName": "height",
				"fieldName": "height",
				"type": "Float"
			}
		],
		"fragmentSpreads": []
	},
	{
		"typeCondition": "Droid",
		"possibleTypes": [
			"Droid"
		],
		"fields": [
			{
				"responseName": "name",
				"fieldName": "name",
				"type": "String!"
			},
			{
				"responseName": "primaryFunction",
				"fieldName": "primaryFunction",
				"type": "String"
			}
		],
		"fragmentSpreads": []
	}
]

To support correctly polymorphic types with parent interface we need to update code gen module, right?

[martijnwalraven]

@sav007: The IR should also contain the parent field type and fields that are not part of an inline fragment.

So for a query like:

query HeroForEpisode($episode: Episode) {
  hero(episode: $episode) {
    name
    ... on Human {
      height
    }
    ... on Droid {
      primaryFunction
    }
  }
}

The IR should contain:

responseName: 'hero',
fieldName: 'hero',
type: 'Character',
fields: [
  {
    responseName: 'name',
    fieldName: 'name',
    type: 'String!'
  },
],

Note that this is different than the query you specified, where name is only queried on Droid or Human, not as part of the parent:

query HeroForEpisode($ep: Episode!) {
  hero(episode: $ep) {
    ... on Droid {
        name      
    }
    ... on Human {
      name      
    }
  }
}

Something else to think about is that we currently only expose inlineFragments for concrete object types, with all merged fields from all parent interfaces. We may potentially also want to expose other interface types specified as inline fragments in the original selection set.

[sav007]

Do you have an issue for this? or should we create new one under codegen project?

[sav007]

Per our today discussion, we decided to put this issue on hold, as to implement it right we need to rethink from architecture of apollo codegen perspective to support GraphQL interfaces as well