Scaffold input type inheritance

[chillu]

Related to Scaffold object type inheritance

Overview

Object types in queries should represent the SilverStripe inheritance structure, in order to make it clear which fields a developer can expect to be returned from a query, and to which part of the PHP class inheritance chain they belong (e.g. Page vs. RedirectorPage).

The same rationale applies to "input types" used for mutations. But as opposed to "object types", they don't support polymorphism through unions. While there's a graphql-js plugin we could port to graphql-php for this, we'd be going outside of the GraphQL spec for this. It's unlikely that this would be accepted into the graphql-php project.

Option 1: One mutation, separate fields for input types

Input types can be nested. One solution would be creating a field for each possible subclass, and then create a specific input type for it. This allows us to retain strong typeing, e.g. making fields mandatory on certain subclasses. One downside here is that we can't enforce that any of these fields is passed through GraphQL, and have to raise an exception in our own resolvers instead.

type SiteTree {
    id
    created
    title
    content
}

type Page {
    id
    created
    title
    content
    myField
}

type RedirectorPage {
    id
    created
    title
    content
    myField
    redirectionType
    externalUrl
}

union SiteTreeTypes = SiteTree | Page | RedirectorPage

input SiteTreeInputType {
    created
    title
    content
}

input PageInputType {
    created
    title
    content
    myField
}

input RedirectorPageInputType {
    created
    title
    content
    myField
    redirectionType
    externalUrl
}

input AllSiteTreeInputTypes {
    SiteTree: SiteTreeInputType
    Page: PageInputType
    RedirectorPage: RedirectorPageInputType
}

mutation createSiteTree(ID:ID!, AllSiteTreeInputTypes) {
  SiteTreeTypes
}

Option 2: One mutation, flattened input type

Alternatively, we could flatten the structure, and indicate the desired class through a className field. This isn't ideal because we can't enforce non-null fields through GraphQL, and have to use exceptions in our own logic instead. It also makes it less clear which fields belong to which subclass to developers navigating the GraphQL API surface. Given that we split up object types by subclass, that's a relevant concern for this target group.

input SiteTreeInputType {
    className
    created
    title
    content
    myField
    redirectionType
    externalUrl
}

mutation createSiteTree(ID:ID!, SiteTreeInputType) {
  SiteTreeTypes
}

Option 3: Multiple mutations, each with one input type

Using the same types as above, we could create specialised mutations for each subclass. This increases the API surface, but makes the input and return arguments much clearer (1:1 mapping with types).

mutation createSiteTree(ID:ID!, SiteTreeInputType) {
  SiteTreeType
}

mutation createPage(ID:ID!, PageInputType) {
  PageType
}

mutation createRedirectorPage(ID:ID!, RedirectorPageInputType) {
  RedirectorPageType
}

[sminnee]

Why would createSiteTree() take a RedirectorPage as an input?
Wouldn’t it be createRedirectorPage()?

If you're expecting createSiteTree() to create an instance of any one of its subclasses, why not just make it createDataObject()?

[sminnee]

More generally, it seems like a mutation would either be designed for specific subclass, or if it were generic it would only make use of the fields provided by a parent class.

For operations that aren't "create" or "update" — e.g. "delete or publish" — I'd expect that we use something narrower, like and ID or a BaseClass + ID pair, to identify objects.

[sminnee]

If you wanted something truly generic I would probably just admit the type weakness and go for something of the form:

{
  Fields: [
    { Name: "Title", Value: "Sam" },
    { Name: "Content", Value: "blarg" }
  ]
} 

[chillu]

I've added @sminnee feedback as "option 3", and agree that this is the best way forward.

[chillu]

Aaron's done this in #20 now, hooray!