Extended interfaces are not expanded for subschemas

[gmac]

Revised issue:

An interface may be extended in the gateway schema to include cross-schema fields. However, those extended fields are not expanded into concrete type fragments when delegating down to a subschema that only contains a subset of the interface, resulting in an error.

Original issue:

I believe there's a practical limitation in type merging around fields that must be present to fulfill an interface, but are not present in the service and have no way to be marked as "external" so they get fetched from elsewhere (similar to Federation's @external directive). Here's an example:

# -- Posts schema --

type Post {
  id: ID!
  title: String!
  url: String!
}

# -- Layouts schema --

interface HomepageSlot {
  id: ID!
  title: String!
  url: String!
}

type Post implements HomepageSlot {
  id: ID!
  title: String! # external value
  url: String! # external value
}

type Section implements HomepageSlot {
  id: ID!
  title: String!
  url: String!
  posts: [Post!]!
}

type Homepage {
  slots: [HomepageSlot]!
}

So the issue here is that Post is required to have title and url fields in the layouts schema to fulfill an interface, however it doesn't actually have the data for those fields.

I believe this is one of the functions of the Federation @external directive: the value of a field marked as external will be ignored and always fetch from another service. I could imaging type merging implementing something similar like this:

const layoutsSubschema = {
  schema:  layoutsSchema,
  merge: {
    Post: {
      fields: {
        title: { external: true },
        url: { external: true },
      },
      selectionSet: '{ id }',
      fieldName: 'postById',
      args: ({ id }) => ({ id }),
    }
  }
}

What those external flags would do is indicate that the title and url values returned from the subschema should be ignored (they may be empty strings simply provided to fulfill an interface), and the gateway should proceed to fetch the real values from another service.

[yaacovCR]

Interesting! Why do you need the interface to include the external fields?

By the way, not sure this is the same issue, but I am hopeful that merging interfaces also works, have not had time to test.

[gmac]

Well, “Homepage” wants to be able to provide HomepageSlot abstracts, and the only way to do that with the data available (in layouts service) is to reduce HomepageSlot interface down to nothing but the “id” field—which eliminates a lot of the value of the interface. That begs the question: how does an interface allow cross-service data to exist? While you can try to extend the interface in the gateway schema to include title and url at the top, an interface-level query for those fields fails in the subschema where the interface does not include them.

Studying federation for hints, my interpretation is that their system allows interfaces to remain intact within the service that defines them, and simply offers the @external mechanism to indicate that certain fields in this service should be disregarded and the final value fetched from elsewhere.

[yaacovCR]

The driving idea is that each schema should only "know" about itself, and so if an interface includes fields, it should be able to resolve them.

While it is true that both Post and Section have title and url fields, that is known only to the gateway.

I think best would be to compare to new interfaces on gateway unknown to subschemas which are expanded prior to delegation to all implementing types.

I think you are making an argument to expand even interfaces known on subschemas. Then you could modify the interface on gateway to add additional fields...

[yaacovCR]

Relevant:

https://github.com/ardatan/graphql-tools/blob/master/packages/delegate/src/transforms/ExpandAbstractTypes.ts#L51

[yaacovCR]

You could also right now add a new interface and extend the merged types to implement that new interface, and then add a new root field delegating to the layout schema...

[yaacovCR]

But I think to enable merging of interfaces more generally, we may need to do former, will have to run through a few test cases when I have time, would love for there to be another way.

[gmac]

So with this notion of expanding abstract types... if I understand correctly, that will convert selections on a gateway-level interface into selections on concrete types sent to the subschema? Interesting. Would that ExpandAbtractTypes transform get installed in the layout subschema, or elsewhere?

This sounds like a decent strategy. Is there an opportunity here for automation, where delegation would recognize a gateway-level interface and automatically expand it? That’d be huge if there was a simple documented way to add a top-level interface... no one really seems to have a clear opinion of how to achieve this at the moment.

[gmac]

I think you are making an argument to expand even interfaces known on subschemas. Then you could modify the interface on gateway to add additional fields...

Oh. This just clicked as to what you’re saying here. Yes, that’s prefect. Looks like ExpandAbstractTypes is already run, but it doesn’t do anything for an extended interface already present in the subschema. Seems like if it always operated, or else also took effect when a present interface is extended, that would work perfectly.

[gmac]

Did some poking on this last night and it looks like ExpandAbstractTypes only converts existing fragments into expanded fragments. Would the best approach here be to write a new transform that expands extended interface fields from a root selection into fragments, or attempt to modify ExpandAbstractTypes to handle this?

[yaacovCR]

I think the latter.

[gmac]

Got a fix for this here... #1912. Will have a look at the test failures in the morning.

[gmac]

Interesting... just setup a quick test for merged interfaces. Is this the interface merging pattern you had in mind? It fails due to a GraphQLError [Object]: Type Item can only implement Placement once. in graphql/type/validate...

test('merges interfaces across subschemas', async () => {
  const namedItemSchema = makeExecutableSchema({
    typeDefs: `
      interface Placement {
        id: ID!
        name: String!
      }
      type Item implements Placement {
        id: ID!
        name: String!
      }
      type Query {
        itemById(id: ID!): Item
      }
    `,
    resolvers: {
      Query: {
        itemById(obj, args, context, info) {
          return { id: args.id, name: `Item ${args.id}` };
        }
      }
    }
  });

  const indexedItemSchema = makeExecutableSchema({
    typeDefs: `
      interface Placement {
        id: ID!
        index: Int!
      }
      type Item implements Placement {
        id: ID!
        index: Int!
      }
      type Query {
        placementById(id: ID!): Placement
      }
    `,
    resolvers: {
      Query: {
        placementById(obj, args, context, info) {
          return { __typename: 'Item', id: args.id, index: Number(args.id) };
        }
      }
    }
  });

  const stitchedSchema = stitchSchemas({
    subschemas: [
      {
        schema: namedItemSchema,
        merge: {
          Item: {
            selectionSet: '{ id }',
            fieldName: 'itemById',
            args: ({ id }) => ({ id }),
          }
        }
      },
      { schema: indexedItemSchema },
    ]
  });

  const result = await graphql(stitchedSchema, `
    query {
      placement: placementById(id: 23) {
        id
        index
        name
      }
    }
  `);

  expect(result.data.placement).toEqual({ id: '23', index: 23, name: 'Item 23' });
});

That said, this is a fairly contrived example and I can't think of a reason I'd actually want to do this type of merging. I think with gateway-level extension support (adding a test for that into #1912), that handles all the common cases I'd want to accomplish.

[yaacovCR]

Ah, that should be a quick fix, but what you get for not testing.

[yaacovCR]

I mean, I should say that I think so, considering that I have not tested it thoroughly and can't say for sure what's wrong, but it looks like I'm just concatenating the interfaces that are implemented and not merging them so that there are duplicates...

[gmac]

That sounds about right. I tried removing implements Placement from one of the subschema types so that the interfaces could merge even if the type wasn't able to reflect it that schema, but the interface didn't seem to resolve with the consolidated fields.

[yaacovCR]

graphql-tools/packages/stitch/src/mergeCandidates.ts

Line 165 in eeb993c

return interfaces != null ? acc.concat(interfaces) : acc;

[yaacovCR]

Similar bug in unions

graphql-tools/packages/stitch/src/mergeCandidates.ts

Line 199 in eeb993c

const types = configs.reduce((acc, config) => acc.concat(config.types), []);

[ardatan]

Available in 6.2.0!