Graphql-tools: Stacked object merges fail in v7 (regression)

[gmac]

Upgrading to v7, my API test suite shows an outstanding failure possibly similar to https://github.com/ardatan/graphql-tools/issues/2137, though distinct. While https://github.com/ardatan/graphql-tools/issues/2137 was a newly-discovered issue that existed in v6, this is demonstrably a regression that worked in v6 and is failing in v7.

In this scenario, objects merging atop other merged objects now appear to fail. Here's another three-part test that demonstrates:

• Test 1: two versions of Post merge together with unique fields from each service. This works fine.
• Test 2: Post.network in Posts service merges with Network from Layouts service. This also works fine.
• Test 3: now the merge patterns from tests 1&2 are combined, at which time they fail when requested together.

Of note, I'm running the alpha stitching branch from https://github.com/ardatan/graphql-tools/pull/2154. The test suite had several failures with 7.0.0, though running with that fix has brought it down to just this one (so significant progress!)

cc @yaacovCR @DrewML

import { makeExecutableSchema } from '@graphql-tools/schema';
import { stitchSchemas } from '@graphql-tools/stitch';
import { graphql } from 'graphql';

describe('Merged associations', () => {
  const layoutSchema = makeExecutableSchema({
    typeDefs: `
      type Network {
        id: ID!
        domain: String!
      }
      type Post {
        id: ID!
        sections: [String]!
      }
      type Query {
        networks(ids: [ID!]!): [Network]!
        _posts(ids: [ID!]!): [Post]!
      }
    `,
    resolvers: {
      Query: {
        networks: (root, { ids }) => ids.map(id => ({ id, domain: `network${id}.com` })),
        _posts: (root, { ids }) => ids.map(id => ({
          id,
          sections: ['News']
        })),
      }
    }
  });

  const postsSchema = makeExecutableSchema({
    typeDefs: `
      type Network {
        id: ID!
      }
      type Post {
        id: ID!
        title: String!
        network: Network
      }
      type Query {
        posts(ids: [ID!]!): [Post]!
      }
    `,
    resolvers: {
      Query: {
        posts: (root, { ids }) => ids.map(id => ({
          id,
          title: `Post ${id}`,
          network: { id: Number(id)+2 }
        })),
      }
    }
  });

  const gatewaySchema = stitchSchemas({
    subschemas: [
      {
        schema: layoutSchema,
        merge: {
          Network: {
            selectionSet: '{ id }',
            fieldName: 'networks',
            key: ({ id }) => id,
            argsFromKeys: (ids) => ({ ids }),
          },
          Post: {
            selectionSet: '{ id }',
            fieldName: '_posts',
            key: ({ id }) => id,
            argsFromKeys: (ids) => ({ ids }),
          },
        },
      },
      {
        schema: postsSchema,
        merge: {
          Post: {
            selectionSet: '{ id }',
            fieldName: 'posts',
            key: ({ id }) => id,
            argsFromKeys: (ids) => ({ ids }),
          },
        },
      },
    ]
  });

  it('merges object with own remote type', async () => {
    const { data, errors } = await graphql(gatewaySchema, `
      query {
        posts(ids: [55]) {
          title
          sections
        }
      }
    `);

    expect(data.posts).toEqual([{
      title: 'Post 55',
      sections: ['News']
    }]);
  });

  it('merges object association with associated remote type', async () => {
    const { data, errors } = await graphql(gatewaySchema, `
      query {
        posts(ids: [55]) {
          title
          network { domain }
        }
      }
    `);

    expect(data.posts).toEqual([{
      title: 'Post 55',
      network: { domain: 'network57.com' },
    }]);
  });

  it('merges object with own remote type and association with associated remote type', async () => {
    const { data } = await graphql(gatewaySchema, `
      query {
        posts(ids: [55]) {
          title
          network { domain }
          sections
        }
      }
    `);

    expect(data.posts).toEqual([{
      title: 'Post 55',
      network: { domain: 'network57.com' },
      sections: ['News']
    }]);
  });
});

[gmac]

(Also worth noting, this is a fairly contrived layout of objects and fields that look a little strange in this microcosm test configuration... the error is real, even if the combination of merge patterns look a little odd here when taken literally)

[yaacovCR]

Blork.

[yaacovCR]

Are you able to again share the actual requests/responses to the different microservices. My suspicion is that delegation is occurring successfully, but that this is a merge failure... But not sure

[gmac]

Sure thing. Focusing just on test 3 (the failure)–the initial queries are as I would expect, but it fails without querying for the merged network:

1. initial query to PostsService.posts – looks correct

posts {
  posts(ids: [55]) {
    title
    network {
      __typename
      id
    }
    __typename
    id
  }
}

# returns:
# [ { id: '55', title: 'Post 55', network: { id: 57 } } ]

1. subsequent query to LayoutsService._posts – looks correct

_posts query ($_v0_ids: [ID!]!) {
  _posts(ids: $_v0_ids) {
    sections
    __typename
    id
  }
}

# returns:
# [ { id: '55', sections: [ 'News' ] } ]

1. LayoutsService.networks query – missing

2. Final result:

{
  "errors": [
    {
      "message": "Cannot return null for non-nullable field Network.domain.",
      "locations": [
        {
          "line": 5,
          "column": 21
        }
      ],
      "path": [
        "posts",
        0,
        "network",
        "domain"
      ]
    }
  ],
  "data": {
    "posts": [
      {
        "title": "Post 55",
        "network": null,
        "sections": [
          "News"
        ]
      }
    ]
  }
}

[yaacovCR]

Awesome, so not a merge failure it's a problem with the delegation plan are you able to get some logging into the get Fields not in subschema function again

[yaacovCR]

Sorry, traveling

[yaacovCR]

I think problem here

https://github.com/ardatan/graphql-tools/blob/master/packages/delegate/src/externalObjects.ts#L76

I am overriding instead of updating the field subschema map

[yaacovCR]

Not always start with an empty object

[yaacovCR]

Sometimes target[FIELD_SUBSCHEMA_MAP_SYMBOL]

[gmac]

What you're describing, it's that this...? https://github.com/ardatan/graphql-tools/blob/master/packages/delegate/src/externalObjects.ts#L79-L81

I just tried changing the linked line to target[FIELD_SUBSCHEMA_MAP_SYMBOL] ?? {}, but no effect.

[yaacovCR]

Actually that code needs a lot of improvement but I just did a quick refactor and was able to fix locally. Well hopefully get a fix out this afternoon

[DrewML]

I'm still working my way through another similar bug in my project. I'll open up a separate issue with a minimal repo just in case it's a diff issue, and can merge later if the root cause ends up being the same as @gmac's report.

[Grmiade]

Hi @gmac @yaacovCR 👋
Are we sure the issue is fixed? We still have some issues in our side and it seems the test 3 still fails.

We receive

{
  title: 'Post 55',
  network: null,
  sections: ['News'],
}

instead of

{
  title: 'Post 55',
  network: { domain: 'network57.com' },
  sections: ['News'],
}

"@graphql-tools/batch-delegate": "^7.0.0",
"@graphql-tools/delegate": "^7.0.2",
"@graphql-tools/load": "^6.2.5",
"@graphql-tools/links": "^7.0.2",
"@graphql-tools/schema": "^7.0.0",
"@graphql-tools/stitch": "^7.0.3",
"@graphql-tools/wrap": "^7.0.1",
"@graphql-tools/url-loader": "^6.3.2",

[gmac]

Have you run a manual upgrade on all of those packages? Specifically: batch-delegate, which didn’t get an update but must be upgraded to lock onto a new delegate version. I had the same issue while trying to grab the latest. Manually running yarn upgrade on everything sorted it out.

All that is to say, yes, I’m pretty confident the fixes made are viable. We’ve got a pretty rigorous stitching test suite on our gateway and the latest stitch package passes it

[Grmiade]

You right! My lock file mixed some version. Thanks a lot @gmac and @yaacovCR 💪
But I still have an issue with the v7, since I'm not sure it's linked to this one, I will open you another issue with a tests suite.