Add fragment parameter to delegateToSchema

[fernandobandeira]

I'd like to propose the addition of a fragment parameter to the delegateToSchema method, usecase:

Suppose that we are using DataLoader to batch a relationship query like the following:

const transactions = (parent, args, context, info) => {
    if (!context.loaders.transactions) {
      context.loaders.transactions = new DataLoader(ids => info.mergeInfo.delegateToSchema({
        schema: schemas.transactions,
        operation: 'query',
        fieldName: 'transactionsByUsers',
        args: {
          users: ids,
          filter: args.filter
        },
        context,
        info
      })
        .then(transactions => {
          const transactionsByUser = _.groupBy(transactions, 'userId')
          return ids.map(id => transactionsByUser[id])
        })
      )
    }

    return context.loaders.transactions.load(parent.id)
  }

I can get the ID of the user even if it's not requested by the client, by passing it through the fragment parameter:

    User: {
      transactions: {
        fragment: '... on User { id }',
        resolve: transactions
      }
    }

But currently, I can't get the userId field from the batch query, thus, the client must specify it in the query. The idea is that we could pass a fragment that would add this field to the query and remove it when sending back to the client.
Maybe I can achieve this by using the transformRequest but it's not really clear in the docs how I would do that.

[mfix22]

You can add a fragment to transactionsByUsers no? 😃

[fernandobandeira]

@mfix22 I can't because transactionsByUsers is in a remote schema (a different service) if I add it to the gateway I get an error message: Invalid options provided to ApolloServer: Query.transactionsByUsers defined in resolvers, but not in schema.

And if I add to the remote schema it doesn't work too, the result in the variable transactionsByUser still misses the userId field.

However if #724 gets merged I can send all of the queries on the same request and then data load on the service and do a single database query inside the service and return a separate graphql query for each userId this way I wouldn't need to access the userId in the gateway since the result would already be grouped by the userId. And I would be able to get rid of the transactionsByUsers query altogether.

[mfix22]

You can create a mergedSchema on top of your remote, creating a new resolver that defines the fragment you need, and delegates to transactionsByUsers. You can then filter this new field out of your top level schema using transforms or just a simple forEach filter (:

[gigouni]

Hi @mfix22 ,

I think I'm facing the same issue but not sure to understand your workaround. Could you provide an example to create a new resolver with the needed fragment?

Thanks

[hansololai]

I had the same issue before, my workaround is to manually modify the Info object. The info object contains the fields that the user requested. I see your problem is if user did not request for "userId" field, then after data loader get back, you'll have no way to map it to the original query.

My idea was to manually add this field into Info. If you print out the Info object, you could see that there's an attribute called fieldNotes, in which it resides selectionSet of what the user requested. This object is a AST, More about this in here. Basically once you figure out the structure, you can manually add one Node into it so that the delegateToSchema will return the userId.

[BrendanBall]

Is this likely to be implemented? I'm facing the same issue. I just realized that delegateToSchema works in dataloaders with the ReplaceFieldWithFragment which is really awesome. There are currently just 2 issues that I know of with using dataloaders with delegate to schema, which are

1. Using the info object and args from the first key in the dataloader can cause problems as it's possible for different keys in the dataloader to have the same id but different selection sets and different args. This is solvable by manually grouping the keys based on their selection sets and args into separate batches although it's kind of horrible so would be nice to have a better abstraction.
2. This issue where you're relying on a specific field to be in the response to properly dataload. There are 2 workarounds, the first being what @hansololai said, the second being that the downstream service should return results in the correct order according to the list of IDs you sent it. This can work in some situations where you control the downstream service but is still pretty dodge to rely on this. I think being able to add the fragment in delegateToSchema is the best solution to this problem.

[BrendanBall]

I've started working on a PR for implementing this feature of adding a fragment to delegateToSchema. I'm having some trouble on writing a test for this because the current delegateToSchema tests only look at the final graphql response which wouldn't include the extra field. The actual code looks fairly easy to implement though as I can just look at ReplaceFieldWithFragment transformer on how to do it.

In the meantime I've had to compromise and implement a custom request transformer as I need a fix urgently. Here is the fairly generic transformer which @fernandobandeira should be able to use if you haven't solved this yet:

import { visit, Kind, visitWithTypeInfo, TypeInfo } from 'graphql'
import { append, uniqBy } from 'ramda'

function requestTransform (schema, typeName, fieldName) {
  return originalRequest => {
    const typeInfo = new TypeInfo(schema)
    let document = visit(originalRequest.document, visitWithTypeInfo(typeInfo, {
      [Kind.SELECTION_SET]: function (node) {
        let parentType = typeInfo.getParentType()
        if (parentType && parentType.name === typeName) {
          let fieldNode = { kind: Kind.FIELD, name: { kind: Kind.NAME, value: fieldName } }
          let selections = uniqBy(f => f.name.value, append(fieldNode, node.selections))
          return { ...node, selections }
        }
        return node
      }
    }))
    return { ...originalRequest, document }
  }
}

const transactions = (parent, args, context, info) => {
    if (!context.loaders.transactions) {
      context.loaders.transactions = new DataLoader(ids => info.mergeInfo.delegateToSchema({
        schema: schemas.transactions,
        operation: 'query',
        fieldName: 'transactionsByUsers',
        args: {
          users: ids,
          filter: args.filter
        },
        context,
        info,
        transforms: [{ transformRequest: requestTransform(schemas.transactions, 'Transaction', 'userId') }]
      })
        .then(transactions => {
          const transactionsByUser = _.groupBy(transactions, 'userId')
          return ids.map(id => transactionsByUser[id])
        })
      )
    }

    return context.loaders.transactions.load(parent.id)
  }

This should theoretically work for you. I've copy pasted the requestTransform function as is from my code and it works for me

EDIT: fixed function to work with fragments

[Jalle19]

We really need a solution for this

[Jalle19]

Ended up with https://gist.github.com/Jalle19/1ca5081f220e83e1015fd661ee4e877c, works for my purposes. Use like this:

const users = await info.mergeInfo.delegateToSchema({
  schema,
  operation: 'query',
  fieldName: 'users',
  args: {
    ids: userIds,
  },
  context,
  info,
  transforms: [createSelectionSetAlteringTransform('users', 'id')]
})

[BrendanBall]

I've given up on this whole fragment in delegateToSchema thing . @Jalle19 that is pretty much what I did. I was pretty happy when I found the WrapQuery transform. It just simplifies my transform above in the previous comment in this case, but it's very useful in other transforms. After stitching a lot more of our schema, I've run into a lot more complicated use cases where the only thing you can do is use a very custom transform. I think this particular problem should rather be solved by adding a generic transform to the library, instead of trying to add this fragment solution as it solves only 1 simple use case of stitching.

Also, stitching with dataloaders is actually pretty complicated because the key in the dataloader cannot only be the id, it also needs to include the selection set and any arguments if applicable. Otherwise you quickly run into bugs where 2 queries with the same id but different selection sets get grouped together and then the 1 query doesn't resolve correctly. We have mostly solved this internally, but I think there is not enough resources on integrating stitching and dataloaders. I could maybe write a blog post about it sometime if people are interested, but I don't really do that kind of thing so might be difficult to find the motivation 😜

[Jalle19]

I think this particular problem should rather be solved by adding a generic transform to the library, instead of trying to add this fragment solution as it solves only 1 simple use case of stitching.

Sure, an out-of-the-box solution would be preferred. Like I wrote earlier, this did the trick for me, that's why it's a gist and not a pull request.

Also, stitching with dataloaders is actually pretty complicated because the key in the dataloader cannot only be the id, it also needs to include the selection set and any arguments if applicable. Otherwise you quickly run into bugs where 2 queries with the same id but different selection sets get grouped together and then the 1 query doesn't resolve correctly.

I see how that can indeed be an issue, however it could perhaps be solved by altering the queries the client makes so that the situation never arises. If the queries are generated dynamically though this can be tough.

I could maybe write a blog post about it sometime if people are interested, but I don't really do that kind of thing so might be difficult to find the motivation

Please do, there are way too few resources available on schema stitching in practice, at least when you get past the todoapp stuff.

[Jalle19]

@BrendanBall you don't happen to have any metrics on how much transforms affect performance?

[BrendanBall]

@Jalle19 nah, but a simple query transform shouldn't be significant? I imagine that's very dependent on your schema and amount of data.

[yaacovCR]

See yaacovCR#33 as well as createRequest function exported in v5, rolling into #1306