graphql-tools icon indicating copy to clipboard operation
graphql-tools copied to clipboard

enhance(stitch) support multiple merge keys

Open gmac opened this issue 4 years ago • 14 comments

Stitching does not currently allow for multiple key formats distributed across services, for example:

  • Catalog: Product @key(selectionSet: "{ upc }") ... @merge(keyField: "upc")
  • Vendor: Product @key(selectionSet: "{ id upc }") ... @merge
  • Reviews: Product @key(selectionSet: "{ id }") ... @merge(keyField: "id")

The theory here is that the central Vendor service holds a join of keys used by the other two services, so records should be able to be merged through this intermediary, ie:

  • Catalog < Vendor > Reviews (works)
  • Catalog > Vendor > Reviews (unresolvable)
  • Reviews > Vendor > Catalog (unresolvable)

The problem here (I believe) is that singular selection sets act as blocking dependencies. The intermediary requires { id upc } to resolve its record, but the services that provide those fields each require { id } and { upc } respectively. It seems like what would be needed here is for the intermediary to support multiple merge paths, ie:

{
  schema: reviewsSchema,
  merge: {
    Product: [
      { selectionSet: '{ id }', fieldName: 'productById', ... },
      { selectionSet: '{ upc }', fieldName: 'productByUpc', ... },
    ]
  }
}

With these multiple paths, the service should only need to require one key at a time, not both in order to fulfill its record. Or maybe I'm misinterpreting something here? Test demonstrating the problem is here:

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

describe('merge on multiple keys', () => {
  const catalogSchema = makeExecutableSchema({
    typeDefs: `
      type Product {
        upc: ID!
        name: String
      }

      type Query {
        productsByUpc(upcs: [ID!]!): [Product]!
      }
    `,
    resolvers: {
      Query: {
        productsByUpc: () => [{ upc: '1', name: 'Table' }],
      }
    }
  });

  const vendorSchema = makeExecutableSchema({
    typeDefs: `
      type Product {
        id: ID!
        upc: ID!
        price: Int
      }

      input ProductKey {
        id: ID
        upc: ID
      }

      type Query {
        productsByKey(keys: [ProductKey!]!): [Product]!
      }
    `,
    resolvers: {
      Query: {
        productsByKey: () => [{ id: '101', upc: '1', price: 879 }],
      }
    }
  });

  const reviewsSchema = makeExecutableSchema({
    typeDefs: `
      type Product {
        id: ID!
        reviews: [String]
      }

      type Query {
        productsById(ids: [ID!]!): [Product]!
      }
    `,
    resolvers: {
      Query: {
        productsById: () => [{ id: '101', reviews: ['works!'] }],
      }
    }
  });

  const gatewaySchema = stitchSchemas({
    subschemas: [
      {
        schema: catalogSchema,
        merge: {
          Product: {
            selectionSet: '{ upc }',
            fieldName: 'productsByUpc',
            key: ({ upc }) => upc,
            argsFromKeys: (upcs) => ({ upcs }),
          }
        }
      },
      {
        schema: vendorSchema,
        merge: {
          Product: {
            selectionSet: '{ upc id }',
            fieldName: 'productsByKey',
            key: ({ upc, id }) => ({ upc, id }),
            argsFromKeys: (keys) => ({ keys }),
          }
        }
      },
      {
        schema: reviewsSchema,
        merge: {
          Product: {
            selectionSet: '{ id }',
            fieldName: 'productsById',
            key: ({ id }) => id,
            argsFromKeys: (ids) => ({ ids }),
          }
        }
      }
    ]
  });

  const result = [{
    id: '101',
    upc: '1',
    name: 'Table',
    price: 879,
    reviews: ['works!'],
  }];

  test('works from upc <- join -> id', async () => {
    const { data } = await graphql(gatewaySchema, `
      query {
        productsByKey(keys: [{ id: "101" }]) {
          id
          upc
          name
          price
          reviews
        }
      }
    `);

    expect(data.productsByKey).toEqual(result);
  });

  test('works from upc -> join -> id', async () => {
    const { data } = await graphql(gatewaySchema, `
      query {
        productsByUpc(upcs: ["1"]) {
          # id
          upc
          name
          price
          reviews
        }
      }
    `);

    expect(data.productsByUpc).toEqual(result);
  });

  test('works from id -> join -> upc', async () => {
    const { data } = await graphql(gatewaySchema, `
      query {
        productsById(ids: ["101"]) {
          id
          # upc
          name
          price
          reviews
        }
      }
    `);

    expect(data.productsByUpc).toEqual(result);
  });
});

gmac avatar Jan 26 '21 14:01 gmac

Hmmm. Gave it another think and I am not sure that this should work. The vendorSchema his declaring that it requires both ID and UPC to resolve records, not either one. Basically I am coming to the same conclusion I believe you already have. This is working as designed now, but there could possibly be a feature request to allow even more merging patterns.

yaacovCR avatar Jan 26 '21 14:01 yaacovCR

I wonder if you can get the either or functionality by setting up two subschema configuration objects with different merge parameters but the same actual endpoint

yaacovCR avatar Jan 26 '21 15:01 yaacovCR

🤔

gmac avatar Jan 26 '21 15:01 gmac

wellwuddayaknow. That works. Might have to explore an SDL wrapper around that pattern... a la @key(selectionSet: "{ id }" @key(selectionSet: "{ upc }"). Admittedly, formally supporting separate merge paths on the one schema may avoid some other complications.

  const gatewaySchema = stitchSchemas({
    subschemas: [
      {
        schema: catalogSchema,
        merge: {
          Product: {
            selectionSet: '{ upc }',
            fieldName: 'productsByUpc',
            key: ({ upc }) => upc,
            argsFromKeys: (upcs) => ({ upcs }),
          }
        }
      },
      {
        schema: vendorSchema,
        merge: {
          Product: {
            selectionSet: '{ upc }',
            fieldName: 'productsByKey',
            key: ({ upc }) => ({ upc }),
            argsFromKeys: (keys) => ({ keys }),
          }
        }
      },
      {
        schema: vendorSchema,
        merge: {
          Product: {
            selectionSet: '{ id }',
            fieldName: 'productsByKey',
            key: ({ id }) => ({ id }),
            argsFromKeys: (keys) => ({ keys }),
          }
        }
      },
      {
        schema: reviewsSchema,
        merge: {
          Product: {
            selectionSet: '{ id }',
            fieldName: 'productsById',
            key: ({ id }) => id,
            argsFromKeys: (ids) => ({ ids }),
          }
        }
      }
    ]
  });

gmac avatar Jan 26 '21 15:01 gmac

Also, delegation planning is smart enough to still see those as the same schema, right? So requests are consolidated?

gmac avatar Jan 26 '21 15:01 gmac

Requests consolidation is function of executors so yes they should share the same batching executor automatically

yaacovCR avatar Jan 26 '21 15:01 yaacovCR

Note that the @computed directive already does something similar in creating a new subschema config, so we already have a pattern in which directives duplicate the subschemaConfig object...

So the basic plumbing to support merge config options or new directives that do something like this should basically exist...although perhaps it could be improved...

yaacovCR avatar Jan 26 '21 17:01 yaacovCR

Indeed, my general thoughts as well.

gmac avatar Jan 26 '21 17:01 gmac

Hello, I'm having the same issue, i want to define multiple endpoints that loads the same type but using different arguments. I understood this works now with type merging subschemas configuration, but i'm using SDL directives. For me is mandatory to use directives, i'm using stitching in a bigger platform and i want to limit changes to the gateway. How can this be done with stitching directives?

Thank you!

alexandra-c avatar Apr 02 '21 07:04 alexandra-c

@alexandra-c there is a PR open by @gmac to add this in #2554

are you able to comment there in terms of whether that is what you need

yaacovCR avatar Apr 02 '21 10:04 yaacovCR

FWIW, there’s a demo available in the schema stitching handbook that demonstrates how to do this using only present capabilities. https://github.com/gmac/schema-stitching-handbook/tree/master/type-merging-multiple-keys

Note that SDL and static JavaScript config are not mutually exclusive, therefore you may setup your graph using SDL directives and hard-code in an alternate entrypoint for a schema using that static config pattern that is presently available.

gmac avatar Apr 02 '21 11:04 gmac

FWIW, there’s a demo available in the schema stitching handbook that demonstrates how to do this using only present capabilities. https://github.com/gmac/schema-stitching-handbook/tree/master/type-merging-multiple-keys

Note that SDL and static JavaScript config are not mutually exclusive, therefore you may setup your graph using SDL directives and hard-code in an alternate entrypoint for a schema using that static config pattern that is presently available.

Yes, didn't tried it yet but i assumed it works with both SDL and static configs. I need to convince my team that stitching is the right way to go with our platform and that's why i think i tried to avoid mixing SDL with static configs, which at some point it may become a bit hard to understand. I'll try to see if i can wrap my head around what implementing the SDL syntax implies but my ts skills are not that good 🙈 i'll see what i can do. Thanks

alexandra-c avatar Apr 02 '21 12:04 alexandra-c

@alexandra-c I started a branch for the SDL work a few months back and got a ways into it, although nowhere near completion. It's definitely a tricky problem that takes some advanced stitching knowledge to build upon.

My best advice would be to not worry TOO much about the cosmetics in a proof of concept... hopefully you can sway your team on the other merits that you presumably like (subscriptions? cross-service interfaces?) without getting hung up on one rough edge. If not, then the rough edge didn't matter much anyway. And if it does all come down to that one factor, well – that's an honest reflection of where the library is today. Best to build it into your team's decision making.

gmac avatar Apr 02 '21 18:04 gmac

@alexandra-c – multiple entry points PR has been merged and the feature is available in [email protected]. I've also updated the schema stitching handbook example, along with some information about working around the SDL limitation here: https://github.com/gmac/schema-stitching-handbook/tree/master/type-merging-multiple-keys#with-sdl-directives.

gmac avatar Apr 09 '21 13:04 gmac

Closing this issue. It seems @gmac already gave some information there. Let us know if this is still an issue.

ardatan avatar Mar 29 '23 05:03 ardatan