Skip to content
This repository has been archived by the owner on Sep 11, 2019. It is now read-only.

Context differences between top-level and stitching resolvers #68

Closed
grxy opened this issue Dec 26, 2017 · 17 comments · Fixed by #73
Closed

Context differences between top-level and stitching resolvers #68

grxy opened this issue Dec 26, 2017 · 17 comments · Fixed by #73

Comments

@grxy
Copy link

grxy commented Dec 26, 2017

I have the following test data source:

// context keys resolver
const context = (root, args, context) => JSON.stringify(Object.keys(context))

const TestDataSource = {
    context: {
        test1: true,
        test2: true,
    },
    namespace: 'test',
    resolvers: {
        Query: {
            context,
        },
    },
    stitching: {
        linkTypeDefs: `
            extend type Query {
                test: Test
            }
        `,
        resolvers: () => ({
            Query: {
                test: () => ({}),
            },
            Test: {
                context,
            },
        })
    },
    typeDefs: `
        type Query {
            context: String
        }

        type Test {
            context: String
        }
    `
}

export default TestDataSource

My GrAMPS config is as follows:

gramps({
    dataSources: [TestDataSource],
    extraContext: (req) => ({
        req
    })
})

When I run the following query:

query {
  context
  test {
    context
  }
}

...I get the following output:

{
  "data": {
    "context": "[\"test1\",\"test2\"]",
    "test": {
      "context": "[\"req\",\"test\",\"_extensionStack\"]"
    }
  }
}

Is it intentional that the GraphQL context for stitching resolvers is different from the top-level resolvers in that the top-level resolvers merge the current data source context into the main context object and the stitching resolvers add the namespace context at context.<namespace>?

Another thing to note is that extraContext only seems to add my req key for stitching resolvers and not top-level resolvers. Is this a known issue or by-design?

@timrs2998
Copy link

Yeah I see there's an issue with extraContext not being passed to the top-level resolvers.

Internally, there's a context object like:

const context = {
  ..extraContext,
  YourNamespaceOne,
  YourNamespaceTwo,
  ..
}

but in mapResolvers regular resolvers get passed context[namespace], while stitching resolvers get passed the full context.

The resolvers should get passed something like: {..extraContext, ..context[namespace] } while the stitching resolver behavior seems debatable to me.

My concern with changing the stitching resolvers is that we're using the other contexts for our stitching resolvers. IMO it makes sense to have access to other contexts when stitching multiple data-sources together. If that behavior is changed, it should be a major release and ideally [for me] there would be an "escape hatch" to access other contexts.

@ecwyne
Copy link
Collaborator

ecwyne commented Jan 4, 2018

FWIW extraContext is passed into every data-source resolver context here

    const extra = extraContext(req);
    return sources.reduce((allContext, source) => {
      const sourceContext =
        typeof source.context === 'function'
          ? source.context(req)
          : source.context;

      return {
        ...allContext,
        [source.namespace]: { ...extra, ...sourceContext },
      };
    }, {});

My thoughts on context in stitching resolvers aren't well defined. This makes sense to me though.

IMO it makes sense to have access to other contexts when stitching multiple data-sources together

@jlengstorf
Copy link
Member

This is a good question. Should we leave all data source contexts available in stitching?

I don't have a strong opinion or use case either way — but we should probably decide on something to stick with and add documentation.

@timrs2998
Copy link

timrs2998 commented Jan 4, 2018

Another thing to note is that extraContext only seems to add my req key for stitching resolvers and not top-level resolvers. Is this a known issue or by-design?

I can reproduce this behavior with a gist. With latest GrAMPS 1.1.0, I do not see fields from extraContext (ie: req in your example) inside resolvers.

I have a hard time following. Can we split this into two separate issues?

  • extraContext
  • schema stitching

@ecwyne
Copy link
Collaborator

ecwyne commented Jan 4, 2018

https://github.com/gramps-graphql/gramps/commits/master

@jlengstorf I can confirm that the current npm version v1.1.0 does not have 3be3006

Not sure how this happened because the manual release happened after 3be3006

Currently published:

  const getContext = req => sources.reduce((allContext, source) => {
    const sourceContext = typeof source.context === 'function' ? source.context(req) : source.context;

    return _extends({}, allContext, {
      [source.namespace]: sourceContext
    });
  }, extraContext(req));

Should be:

  const getContext = req => {
    const extra = extraContext(req);
    return sources.reduce((allContext, source) => {
      const sourceContext = typeof source.context === 'function' ? source.context(req) : source.context;

      return _extends({}, allContext, {
        [source.namespace]: _extends({}, extra, sourceContext)
      });
    }, {});
  };

@ecwyne
Copy link
Collaborator

ecwyne commented Jan 4, 2018

@grxy
Copy link
Author

grxy commented Jan 5, 2018

@ecwyne I think the published release is missing #59 as well. Can't see it here: https://unpkg.com/@gramps/[email protected]/dist/lib/mapResolvers.js

@grxy
Copy link
Author

grxy commented Jan 5, 2018

@jlengstorf @ecwyne @timrs2998


Extra Context

My thoughts: unless we have a separate config option for stitching extraContext vs top-level extraContext, both resolvers should have access to that context.


Data Source Context

I can support an argument for either of the possible solutions:

  1. Top level resolvers have single data source context + extraContext, stitching resolvers have all contexts + extraContext
    This makes the most sense semantically. Since it would be impossible to rely on any existing types in the top level schema, maybe it should be impossible to access context from other data sources.
  2. All resolvers have access to all data source contexts + extraContext
    This provides for a consistent shape for all contexts, regardless of location. This could allow for simpler documentation for context in general.

Personally, I think I lean toward consistency across all contexts.

An alternative to this would be to make it configurable via a simple either or option or via something like mapContext, mapDataSourceContext, mapStitchingContext option(s) that could allow a use to control how context is provided to resolvers. This, of course, adds to the complexity of this package.

@timrs2998
Copy link

I like the consistency from option 2. However, there is no valid use case for non-stitching resolvers to have access to other contexts. How about a third option:

  1. Non-stitching resolvers have single data source context + extraContext, stitching resolvers have data-source context + extraContext + externalContext

@timrs2998
Copy link

related: gramps-graphql/gramps#66

thinking about scenarios to use stitching on the GrAMPS-level, and the only use case I can come up with is when I want to stitch to another datasource

so I'm thinking the contexts should be setup to enable stitching other data-sources, but only for stitching resolvers

@grxy
Copy link
Author

grxy commented Jan 5, 2018

@timrs2998 It seems to me the whole point of stitching is to extend another data source, so what you say makes total sense.

So you're suggesting context be structured something like...:

const topLevelContext = {
    [dataSource.namespace]: dataSource.context
    ...extraContext
}

const stitchingContext = {
    [dataSource.namespace]: dataSource.context
    ...otherDataSourceContextsKeyedByNamespace
    ...extraContext
}

Is that correct?

@timrs2998
Copy link

No, I'm thinking keys should be more explicit for extraContext and stitched data sources, but consistent and convenient for the current data-source:

const topLevelContext = {
    [dataSource.namespace]: dataSource.context,
    extraContext,
}

const stitchingContext = {
    [dataSource.namespace]: dataSource.context,
    extraContext,
    externalContexts: otherDataSourceContextsKeyedByNamespace,
}

@jlengstorf
Copy link
Member

I'm not sure what happened with 1.1.0 — sorry about that. I just manually released 1.1.1, which has the extra contexts: https://unpkg.com/@gramps/[email protected]/dist/gramps.js

RE: stitching, I'm leaning toward @timrs2998 suggestion that we explicitly place external contexts under a property.

We've got it so data sources only have their own context + extra context now, so it's like this:

{
  ...[dataSource.namespace].context,
  ...extraContext,
}

Adding external data source contexts explicitly would look something like:

{
  ...[dataSource.namespace].context,
  ...extraContext,
  external: allOtherDataSourceContexts,
}

I think we can use external vs. externalContexts since it would be context.external.OtherDataSource, which seems pretty clear to me.

@grxy
Copy link
Author

grxy commented Jan 10, 2018

@jlengstorf Thanks for the release. Will try to test and reach out if I have any other issues.

Maybe in keeping with the fact that the context from other data sources is accessible only in stitching resolvers, we could call that key stitching instead of external or externalContexts. We could then in docs refer to things as stitching context and stitching resolvers. Other than that, what you propose makes sense and I'm 100% on board.

@jlengstorf
Copy link
Member

I'm alright with stitching. @kbrandwijk and @ecwyne, any objections to this format for stitching resolvers?

The final format would be:

{
  ...[dataSource.namespace].context,
  ...extraContext,
  stitching: allOtherDataSourceContexts,
}

@jlengstorf
Copy link
Member

I actually started digging into this, and it turns out that the reason we didn't do this originally is because it's not possible. 😄

So, stitching is done when we bring all of the data sources together. This means that — during stitching — all data sources share the same context.

That means that our context object during stitching should be something like this, assuming we have DSOne and DSTwo:

const context = {
  DSOne: { /* ... */ },
  DSTwo: { /* ... */ },
  ...extraContext,
};

Our stitching resolvers will need to use the correct namespaces. Ultimately, this is probably a good thing for clarity:

  stitching: {
    linkTypeDefs: `
      extend type Query {
        test: String
      }
    `,
    resolvers: () => ({
      Query: {
        test: (_, { id }, context) => context.DSOne.getStringValue(id),
      },
    }),
  },

How it should work:

Here’s a slight variation on @grxy's original example:

const context = (_, __, ctx) => JSON.stringify(Object.keys(ctx));

/*
 * For more information on the building GrAMPS data sources, see
 * https://gramps.js.org/data-source/data-source-overview/
 */
export default {
  // TODO: Rename the context to describe the data source.
  namespace: 'StitchingTest',
  context: {
    test1: true,
    test2: true,
  },
  typeDefs: `
    type Query {
      context: String
    }
    type Test {
      context: String
    }
  `,
  resolvers: {
    Query: {
      context,
    },
  },
  stitching: {
    linkTypeDefs: `
      extend type Query {
        test: Test
      }
    `,
    resolvers: () => ({
      Query: {
        test: () => ({}),
      },
      Test: {
        context,
      },
    }),
  },
};

The extra context was missing, so I've tweaked the contexts so we end up with this:

{
  "data": {
    "context": "[\"addedFromExtraContext\",\"test1\",\"test2\"]",
    "test": {
      "context": "[\"addedFromExtraContext\",\"StitchingTest\"]"
    }
  }
}

I'm also working on an example to show data stitching actually happening inside of GrAMPS, because so far I haven't seen anyone actually using it. Will follow up once I have it working.

jlengstorf added a commit that referenced this issue Jan 11, 2018
Extra context is now included both _inside_ data source namespaced
contexts and _at the top level_ to ensure it’s available for schema
stitching.

close #68
jlengstorf added a commit that referenced this issue Jan 11, 2018
Extra context is now included both _inside_ data source namespaced
contexts and _at the top level_ to ensure it’s available for schema
stitching.

close #68
@jlengstorf
Copy link
Member

Okay — I've got schema stitching working using the GrAMPS pattern. I've written up a walkthrough on how to do this, which I'll add to the docs shortly. Anyone have a few minutes to run through this and let me know if you hit any snags or need extra clarification along the way?

Thanks!

Part 1: Create a Data Source

To get started, let's create our first data source using the GraphQL CLI

# Use npx to run the command without having to install anything globally
npx graphql-cli create -b gramps-graphql/data-source-base data-source-stitchingtest

# Move into the folder that was just created
cd $_

In src/index.js, we can declare the entire data source for the sake of simplicity:

const getContext = (_, __, ctx) => Object.keys(ctx);

export default {
  namespace: 'StitchingTest',
  typeDefs: `
    type Query {
      getContext: [String]
      getById(id: ID!): STX_Test
    }
    type STX_Test {
      id: ID
      value: String
    }
  `,
  context: {
    getValue: id => ({
      id,
      value: `from StitchingTest with ID “${id}”`,
    }),
  },
  resolvers: {
    Query: {
      getContext,
      getById: (_, { id }, ctx) => ctx.getValue(id),
    },
  },
};

This data source exposes two queries:

  • getContext — returns an array of object keys that are present in the data source’s context object
  • getById — exposes id and value fields

Let's test this out by running the data source:

yarn dev

At http://localhost:8080/playground, run the following query:

{
  getContext
  getById(id: 3) {
    value
  }
}

We should see the following return value:

{
  "data": {
    "getContext": [
      "getValue"
    ],
    "getById": {
      "value": "from StitchingTest with ID “3”"
    }
  }
}

So far so good.

Add Local Schema Stitching

Next, let's add some schema stitching to the existing data source, just to make sure it's working the way we expected.

In src/index.js, add a stitching property with the following definitions:

export default {
  namespace: 'StitchingTest',
  typeDefs: `...`,
  context: { /* ... */ },
  resolvers: { /* ... */ },
  stitching: {
    linkTypeDefs: `
      extend type Query {
        getStitchingContext: [String]
      }
    `,
    resolvers: () => ({
      Query: {
        getStitchingContext: getContext,
      },
    }),
  },
};

Restart the data source in your terminal (ctrl + C to stop, yarn dev to start), head to http://localhost:8080/playground, and run the following query:

  {
    getContext
+   getStitchingContext
    getById(id: 3) {
      value
    }
  }

We should see the following return value:

{
  "data": {
    "getContext": [
      "getValue"
    ],
    "getStitchingContext": [
      "StitchingTest"
    ],
    "getById": {
      "value": "from StitchingTest with ID “3”"
    }
  }
}

NOTE: Notice that the contexts are different in getStitchingContext. This happens because each data source scopes its context to its own namespace to prevent accidentally relying on another data source's context. However, schema stitching does rely on multiple data source's contexts, so we include all of the data sources' contexts.

Add a Second Data Source

Next, let's create a second data source so we can set up more realistic schema stitching.

In your terminal, move into the same directory where your first data source was created, then run the following to create a second data source:

# Use npx to run the command without having to install anything globally
npx graphql-cli create -b gramps-graphql/data-source-base data-source-stitchingtwo

# Move into the folder that was just created
cd $_

In src/index.js, create the second data source all in one place:

export default {
  namespace: 'StitchingTwo',
  typeDefs: `
    type Query {
      getSomeValues(val: ID): ST2_Values
    }
    type ST2_Values {
      foo: String
      bar: String
      bat: String
    }
  `,
  context: {
    getSomeValues: val => ({
      foo: `Schema (val: ${val})`,
      bar: `Stitching (val: ${val})`,
      bat: `Rules (val: ${val})`,
    }),
  },
  resolvers: {
    Query: {
      getSomeValues: (_, { val }, ctx) => ctx.getSomeValues(val),
    },
  },
};

This data source is pretty bare bones: it has a single query — getSomeValues — that exposes three fields that have text and echo the val the query was called with.

To test it, let's fire up the new data source along with the original data source:

yarn dev --data-source ../data-source-stitchingtest

NOTE: yarn dev is shorthand for gramps dev --data-source ., so what we're doing here is effectively running gramps dev --data-source . --data-source ../data-source-stitchingtest

Open http://localhost:8080/playground and update the query to call getSomeValues:

  {
    getContext
    getStitchingContext
    getById(id: 3) {
      value
    }
+   getSomeValues(val: 2) {
+     foo
+     bar
+     bat
+   }
  }

The output should be:

{
  "data": {
    "getContext": [
      "getValue"
    ],
    "getStitchingContext": [
      "StitchingTwo",
      "StitchingTest"
    ],
    "getById": {
      "value": "from StitchingTest with ID “3”"
    },
    "getSomeValues": {
      "foo": "Schema (val: 2)",
      "bar": "Stitching (val: 2)",
      "bat": "Rules (val: 2)"
    }
  }
}

Use Schema Stitching to Combine the Two Data Sources

Finally, let's add stitching config to tie the two data source together. In the second data source, add the following:

export default {
  namespace: 'StitchingTwo',
  typeDefs: `...`,
  context: { /* ... */ },
  resolvers: { /* ... */ },
  stitching: {
    linkTypeDefs: `
      extend type STX_Test {
        stitched: ST2_Values
      }
    `,
    resolvers: mergeInfo => ({
      STX_Test: {
        stitched: {
          fragment: 'fragment StitchingTestField on STX_Test { id }',
          resolve: ({ id }, args, context, info) =>
            mergeInfo.delegate(
              'query',
              'getSomeValues',
              { val: id },
              context,
              info,
            ),
        },
      },
    }),
  },
};

First, we use linkTypeDefs to extend the STX_Test type by adding a new field called stitched.

Then, in resolvers, we set up stitched — which is a field on our first data source, remember — to get its value from the getSomeValues query, which is in the second data source.

Under the hood, this is done using mergeSchemas. Be sure to check that out for additional information about how schema stitching happens, and some of the different ways you can work with it.

With the stitching config in place, let's fire it up and test it.

Run the following command to start a gateway with both data sources:

yarn dev --data-source ../data-source-stitchingtest

Then, open http://localhost:8080/playground and add the stitching field to the query:

  {
    getContext
    getStitchingContext
    getById(id: 3) {
      value
+     stitched {
+       foo
+       bar
+       bat
+     }
    }
    getSomeValues(val: 2) {
      foo
      bar
      bat
    }
  }

Once executed, we'll see the following:

{
  "data": {
    "getContext": [
      "getValue"
    ],
    "getStitchingContext": [
      "StitchingTwo",
      "StitchingTest"
    ],
    "getById": {
      "value": "from StitchingTest with ID “3”",
      "stitched": {
        "foo": "Schema (val: 3)",
        "bar": "Stitching (val: 3)",
        "bat": "Rules (val: 3)"
      }
    },
    "getSomeValues": {
      "foo": "Schema (val: 2)",
      "bar": "Stitching (val: 2)",
      "bat": "Rules (val: 2)"
    }
  }
}

And that's it! We now have one data source including data from a second data source as part of its own queries.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants