andreas / ocaml-graphql-server

GraphQL servers in OCaml
MIT License
624 stars 60 forks source link

Interfaces don't enforce fields being implemented. #151

Open phated opened 5 years ago

phated commented 5 years ago

I have a schema similar to:

let card: Schema.abstract_typ(unit, [ | `Pet]) =
  Schema.(
    interface(
      "Pet", ~doc="Interface which all pets must conform to", ~fields=_typ =>
      [
        abstract_field(
          "id",
          ~typ=non_null(string),
          ~args=Arg.[],
        ),
      ]
    )
  );

let dog =
  Schema.(
    obj("Dog", ~fields=_typ =>
      [
        field(
          "name",
          ~typ=non_null(string),
          ~args=Arg.[],
          ~resolve=(ctx, dog) => dog.name,
        ),
      ]
    )
  );

let cat =
  Schema.(
    obj("Cat", ~fields=_typ =>
      [
        field(
          "name",
          ~typ=non_null(string),
          ~args=Arg.[],
          ~resolve=(ctx, cat) => cat.name,
        ),
      ]
    )
  );

let dogAsPet = Schema.(add_type(pet, dog));
let catAsPet = Schema.(add_type(pet, cat));

But there are no errors produced unless I try to access the id field in a query. So this query actually succeeds:

query {
  pets {
    ... on Dog {
      name
    }
    ... on Cat {
      name
    }
  }
}
andreas commented 5 years ago

This is a known limitation, but unfortunately not documented 😞Whether an object correctly implements an interface is one of the things that cannot be captured in the type system. My current plan is to "just" implement it as an exception raising runtime check when calling Schema.add_type. Although inferior to a type error, I think it's a reasonable trade-off to have an exception at schema-construction time to be exception-free at query-execution time.

Here's the TODO in the source code:

https://github.com/andreas/ocaml-graphql-server/blob/9ec8c84a3921a3b906a4db775cdf57bf8215d292/graphql/src/graphql_schema.ml#L510

I'm happy to accept contributions, even if it's just a note in the README 😀