Concrete types implementing interface referenced in schema should be automatically added

[ctoomey]

(This is a follow-up to google-group thread).

Unless the concrete types implementing an interface are referenced in the query or mutation fields of a Schema object or added explicitly via the additionalTypes field, they will cause a "Can't find appropriate subtype for field" error when returned via a query field that's declared to return the interface type.

For example, in the sangria-playground schema, if you comment out the human and droid query fields like so

  val Query = ObjectType(
    "Query", fields[CharacterRepo, Unit](
      Field("hero", Character,
        arguments = EpisodeArg :: Nil,
        deprecationReason = Some("Use `human` or `droid` fields instead"),
        resolve = (ctx) ⇒ ctx.ctx.getHero(ctx.arg(EpisodeArg)))
//      Field("human", OptionType(Human),
//        arguments = ID :: Nil,
//        resolve = ctx ⇒ ctx.ctx.getHuman(ctx arg ID)),
//      Field("droid", Droid,
//        arguments = ID :: Nil,
//        resolve = Projector((ctx, f) ⇒ ctx.ctx.getDroid(ctx arg ID).get))
    ))

then attempting to query the hero field will produce this error:

query HeroAndFriends {
  hero {
    name
  }
}

=>

{
  "data": null,
  "errors": [
    {
      "message": "Can't find appropriate subtype for field at path hero (line 2, column 3):\n  hero {\n  ^",
      "path": [
        "hero"
      ],
      "locations": [
        {
          "line": 2,
          "column": 3
        }
      ]
    }
  ]
}

Ideally the types implementing an interface referenced in the schema would be automatically discovered and added to the schema. Failing that, it'd be good to mention this "gotcha" in the documentation.

[OlegIlyenko]

Just to make it easier to follow, I will repeat my reply on mailing list here as well:

I think this kind of issue can be compensated with better documentation and error reporting, but it would be quite challenging to fix. Given and IFType there is no way to know that there is an AType if there no explicit reference to it.

One possible solution I'm aware of is to mutate some JVM-global state in a constructor of the AType and register it in some kind of registry. Later on, Schema can access this registry and discover AType. I don't think that it is an appropriate solution for a library though, so I would prefer to avoid introducing it for obvious reasons.

[ctoomey]

Why not track the implementing concrete types in the interface type, and add those to the schema when adding the interface type? It's already necessary to declare the interfaces that a concrete type implements (via the interfaces field of ObjectType or an Interfaces config. to the deriveObjectType macro) in order to use it in fields that return the interface type.

It doesn't strike me that doing this would be all that difficult, though of course I could be wrong. Is there any other reason not to do it and address this limitation (and not have to worry about where to document it)?

[OlegIlyenko]

This would be an option, but this would also mean that InterfaceType will become a mutable object. So far, the most of the library library and schema definition DSL in particular is just a composition of immutable data structures. Making the InterfaceType a mutable object would be quite a dramatic change. I was thinking about, but I'm still not convinced that add value would be worth of such a big tradeoff.

[ctoomey]

Ya, I can see that'd be a meaningful change, but on the flip side, have you ever used a language where you had to declare each implementation of an interface in more than 1 place? It seems like a design bug in the type system to have types declare they implement an interface yet to have the type system not keep track of this and require the user to tell it about that a 2nd time to get it to work.

In other words, if the language is going to support interfaces, it should go all the way and not half way.

[OlegIlyenko]

I think a good example would be the Node interface from relay. Since all identifiable objects in the schema should implement this interface, it is quite common to distribute implementations of the Node interface across codebase (in fact I know several project that do this because keeping all implementations in a single file is just not maintainable).

In general, finding all subtypes of an interface is more complex problem than is looks on the first sight. For example, Java reflection API does not support this feature to this date. Some libraries are trying to address this, but the way they are doing it is by loading all classes in the classpath from JARs, iterating over them and filtering only these that implement the interface. Scala reflection and macro API do support this feature in a limited form, but it is extremely buggy and works only in limited set of scenarios (for example for sealed class hierarchies).

Also, the fact that an object type implements an interface somewhere in codebase does not necessarily represent an intent to expose this type publicly. Exposing such types in all GraphQL endpoints of an application without clear indication of the intent can be quite dangerous (e.g. from security perspective where normal relay /graphql API suddenly exposes information about types that supposed to be visible only though admin API or for users with admin privileges).

[ctoomey]

Ok, I guess doing it manually will have to suffice then. Thanks for thinking through it and considering the ramifications.

As far as documentation, I just now saw the Providing Additional Types, which explains pretty well the limitation and workaround. Curious, did you just add that section? If not I completely missed it, though I likely would have still wanted to understand the reasons for not doing the automatic resolution.