Inherit types

[dwwoelfel]

I'm working on implementing a Node interface for OneGraph. Each of our underlying APIs implements its own node interface, so we have GitHubNode, SalesforceNode, IntercomNode, etc.

To implement the resolver for the OneGraphNode, I just need to determine which service the nodeId is for, then utilize the existing machinery for resolving a node.

It would look something like:

field(
  "node",
  ~args=Arg.[arg("oneGraphId", ~typ=non_null(guid))],
  ~typ=oneGraphNodeInterface,
  ~resolve=(resolveInfo, (), id) =>
  switch (decodeNodeId(id)) {
  | Ok({service, serviceNodeId}) =>
    switch (service) {
    | `GitHub => resolveGitHubNode(serviceNodeId) |> gitHubNodeToOneGraphNode
    | `Salesforce => resolveSalesforceNode(serviceNodeId) |> salesforceNodeToOneGraphNode
    }
  }
);

It would be really handy if I had a way to convert from the underlying service's node interface to the OneGraph node interface.

Would you be open to a new inherit_types function that copied all the types from one interface to another and created a function to coerce from one interface to another?

It would look like:

  let inherit_types :
    'a 'b.
    ('ctx, 'a) abstract_typ ->
    ('ctx, 'b) abstract_typ ->
    ('ctx, 'a) abstract_value -> ('ctx, 'b) abstract_value =
    fun from_abstract_typ to_abstract_typ ->
    match (from_abstract_typ, to_abstract_typ) with
    | (Abstract from_typ), (Abstract to_typ) ->
       to_typ.types <- List.concat [from_typ.types; to_typ.types];
       List.iter (fun o ->
                  match o with
                  | AnyTyp (Object o) -> o.abstracts := to_typ :: !(o.abstracts)
                  | _ -> invalid_arg "Types for Abstract type must be objects"
                 ) to_typ.types;
       fun (AbstractValue (from_typ, src)) -> (AbstractValue (from_typ, src))
    | _ -> invalid_arg "Arguments must both be Interface/Union"

With that function, I could easily construct gitHubNodeToOneGraphNode and salesforceNodeToOneGraphNode above by doing:

let gitHubNodeToOneGraphNode = Schema.inherit_types(gitHubNode, oneGraphNodeInterface);
let salesforceNodeToOneGraphNode = Schema.inherit_types(salesforceNode, oneGraphNodeInterface);

[andreas]

I'll try to rephrase your problem more abstractly to make sure I understand correctly:

You have two interfaces A and B, where A defines at least the same fields as B. Rather than specify for all objects that implement A also implement B, you would like to indicate with inherit_types that B is a subtype of A.

Does that sound right?

I wonder whether it would be better to store the subtyping relationship rather than simply copy types from from_typ to to_typ. In particular, the proposed implementation will be incorrect if more types are latter added to from_typ.

Maybe your use case could be solved by supporting interfaces implementing other interfaces (an addition to the spec that was recently merged)?

[dwwoelfel]

where A defines at least the same fields as B ... B is a subtype of A

My use case is a little different than that. In my case, A defines different fields than B, so it's not a relationship that could be defined by interfaces implementing interfaces.

To make it a bit more concrete, these are my interfaces:

interface A {
  oneGraphId: ID!
}

interface B {
  id: ID!
}

What I'm looking for is more of a helper to reduce some of the boilerplate of creating my new interface.

Since I've already done all of the work to create coercers for interface B and I know that anything that implements B will implement A, it would be nice to use the existing machinery I've created to coerce it to AbstractValue(B,src) and then just coerce that to AbstractValue(A,src).

[andreas]

where A defines at least the same fields as B ... B is a subtype of A

In my case, A defines different fields than B, so it's not a relationship that could be defined by interfaces implementing interfaces. [...] I know that anything that implements B will implement A

Ah, I see. It sounds like there's some domain knowledge that is not expressed in the schema: B does not implement A, but anything that implements B will implement A. To explore the idea, could B implement A?

I wonder if this is a use case that can be generalised so it's more broadly applicable -- any thoughts on this? I would prefer adding a general capability over a very specific one.

[dwwoelfel]

I wonder if this is a use case that can be generalised so it's more broadly applicable -- any thoughts on this? I would prefer adding a general capability over a very specific one.

Do you think it would be possible to give user code more access to the internals for abstract types?

I could probably do everything I needed if I had two functions, one that gave me all of the types for an abstract type and another that allowed me to construct an AbstractValue from an abstract type and a src.

[andreas]

I wonder if this is a use case that can be generalised so it's more broadly applicable -- any thoughts on this? I would prefer adding a general capability over a very specific one.

Do you think it would be possible to give user code more access to the internals for abstract types?

I could probably do everything I needed if I had two functions, one that gave me all of the types for an abstract type and another that allowed me to construct an AbstractValue from an abstract type and a src.

Can you try provide suggested types for these two functions? Just to make sure I understand correctly. I haven't thought deeply about it, but I think this approach might run into some typing issues.

[andreas]

Another way to address this issue is by relaxing the type guarantees around unions and interfaces. It's generally hard to capture these concepts well without a clunky API, and the current one has its warts too. Interfaces implementing other interfaces will only make this harder.

I've created a proof of concept on a branch, which relaxes the type guarantees around unions and interfaces. In particular, the type system no longer tries to enforce a correct return value from fields with an interface/union as the output type. This can cause errors at query time if you make a mistake. On the other hand the API is (much?) simpler.

Example:

let cat = Schema.(obj "Cat" ~fields:(fun _ -> (* ... *)))
let dog = Schema.(obj "Dog" ~fields:(fun _ -> (* ... *)))

let pet = Schema.(union "Pet" Type.[dog; cat])

let named =
  Schema.(interface "Named"
    Type.[dog; cat]
    ~fields:(fun _ -> [
      abstract_field "name" ~typ:(non_null string) ~args:Arg.[]
    ]))

let schema =
  Schema.(schema [
    field "pets"
      ~typ:(non_null (list (non_null pet)))
      ~args:Arg.[]
      ~resolve:(fun _ () -> [ abstract_value cat meow; abstract_value dog fido ])
    ;
    field "named_objects"
      ~typ:(non_null (list (non_null named)))
      ~args:Arg.[]
      ~resolve:(fun _ () -> [ abstract_value cat meow; abstract_value dog fido ]);
    ])

Would the loss of type safety be worth the simplified interface for you, @dwwoelfel?

[dwwoelfel]

Sorry for the delay. I tried to write the functions I mentioned, but couldn't figure out a way to do it without an error about types escaping their scope. In the end, I just ended up implementing it without any changes to ocaml-graphql-server. https://www.onegraph.com/schema/interface/OneGraphNode

Would the loss of type safety be worth the simplified interface for you, @dwwoelfel?

This looks really nice! We're not really taking advantage of the current type safety, anyway.

Seems like it should work with mutually recursive objects?

let rec cat = lazy Schema.(obj "Cat" ~fields:(fun _ -> (* ... *)))
  and dog = lazy Schema.(obj "Dog" ~fields:(fun _ -> (* ... *)))
  and pet = lazy Schema.(union "Pet" Type.[(Lazy.force dog); (Lazy.force cat)])

One problem we notice with the current interface/union definers is that we have to remember to call Lazy.force on them before they'll show up in the introspection query.

[andreas]

Would the loss of type safety be worth the simplified interface for you, @dwwoelfel?

This looks really nice! We're not really taking advantage of the current type safety, anyway.

Cool, I'll consider moving forward with this 😎

Seems like it should work with mutually recursive objects?

Yes.

One problem we notice with the current interface/union definers is that we have to remember to call Lazy.force on them before they'll show up in the introspection query.

Yeah, I've definitely run into that as well. I have an idea for a design that gets rid of lazy for recursive objects.