Source level resolver compositions are not applied (transform-resolvers-composition)

[ntziolis]

Resolver compositions on source level are ignored, at least for a graphql source.

To Reproduce
Steps to reproduce the behavior:

• start this codesandbox
• the config sets up resolver compositions for all queries on both:
  • root level of config
  • source level of config
• the composition resolver code does nothing besides add a console log:
  • ----- GLOBAL composer executed ----
  • and ----- SOURCE composer executed ---- respectively
• However when executing the following query in graphiql:

  query {
    countries{
      code
    }
  }

• Only ----- GLOBAL composer executed ---- entry is logged into the console window

Expected behavior
Both composers should be executed. The expectation would be that

• the composition resolver on root level is executed first then
• then the composition resolver on source level
• and then the actual resolver that points to the graphql source

Environment:

• OS: Linux
• @graphql-mesh/transform-resolvers-composition:
• NodeJS: v10.23.0

Additional context
While debugging we found that while the resolvers[typeName][fieldName] key does exist, the data returned doesn't match any of the conditions in the following code block, which prevents matches to be found:
https://github.com/ardatan/graphql-tools/blob/master/packages/resolvers-composition/src/resolvers-composition.ts#L57

[ntziolis]

I think I just found the reason for the issue we were seeing, as outlined here:
https://www.graphql-mesh.com/docs/getting-started/mesh-transforms/#implications

NOTE: "wrap" is the only approach that works with data sources that already "speaks" GraphQL, or when you want to transform >at all-sources (root) level, unless you're using merger-bare. If you want to remove the possible runtime implications, consider >either moving your renames at the data source level, or opting into merger-bare; in order to take advantage of "bare" mode.

A bit further down it is outline that compositions do not support wrap mode:

Based on the above, are we correct in the assumption that there is currently no way to use composition on a graphql data source level?

Our goal:
Apply compositions to all queries from a downstream graphql datasource, but not to any other query exposed on our graphql api.

[santino]

I don't have experience with resolvers composition transform, but I believe the fact that it doesn't support wrap mode is most likely the issue.

Out of interest, I wonder why you have closed the issue, if you found an alternative solution it might be worth sharing with future readers.
I thought you might try to use Resolvers Composition directly from graphql-tools, doc available here. BTW graphql-tools is already a dependency of Mesh, so you wouldn't actually introduce any new library to your server.

I think this is an interesting case, it might actually make sense to introduce "wrap" mode to resolvers composition to make it easy to have this transform on GraphQL sources as well.

[ntziolis]

@santino Thank you for the link I will research this with my team. I closed the issue as I realized this wasn't a bug, but was working as designed and documented.

That said I reopened since you mentioned an interest in potentially supporting this use case. In our opinion it would be very useful to be able to apply compositions on any type of source.

The larger use case is the following:

• In todays environment a lot of times third party apis are used in applications, often with parts of the data model and functionality being exposed to the client
  • Think stripe express accounts where you want to provide users with access to the current balance, payment receipts etc. (which for security reasons should stay on the stripe servers)
• In the past securing these external apis with app specific auth and re-exposing parts of the data model and functionality through the applications api has been anything but DRY as well as error prone
• With mesh we can solve this with mostly configuration, we:
  • use resolver composition to ensure the client accessing the downstream api is allowed to access specific resolvers
  • filter the resolvers we want the client to be able to access
  • filter the fields on the types we want to be returned to the client
  • filter the args that we don't want the client to be able to provide (like the stripe account id)
    • we then use resolver composition to set these args server side

In combination this allows us to integrate external apis 5-10x faster than previously already. With reduction in potential bugs as well as an ability to adapt to changes in the providers api in a much more timely fashion.

Some more details can be found in these previous issues and PRs:

• Question: Is it possible to filter args? #1481
• Filter arguments #1498

Additional notes:

• We could further cut down on configuration if resolver composition would support glob patterns similar to the filter transform (eg Query.{first,second}. Or alternatively allow for multiple resolver patters to be provided for a single composition entry.
• We would also favor being able to apply any transform on any level for the sake of adaptability. Me and my team would be happy to assist to make the remaining resolvers wrap compatible. As far as we can tell this was already the case before the introduction of bare transforms.

[ntziolis]

@santino If the goal is to apply all kind of transforms on handler / source level (incl. graphql data soruces) there I see 2 options:

• Either implement both warp and bare for all transforms so they can also be used with graphql sources
  • This would likely entail breaking changes to the config format (eg in case of composite resolver)
  • As well as mean that any future or custom built transform would need to support both modes
• Or apply special handling for bare transforms on graphql handlers / sources
  • Since its possible to apply the bare transforms on root level, even if the underlying handler / source is graphql
  • There already is a way to "transform" the graphql source so that a bare transform can be applied to it
  • Currently this happens on root level
  • But it should be possible to copy this transformation logic and execute it
    • after the source level handling is through (incl. applying wrap transforms)
    • but before the root level logic is executed
  • Again this would only be done for graphql data sources
    • The additional "transformation" could be conditional on if there are any bare transforms
    • To not have any negative performance impact in case no bare transforms are applied

The advantage of solution 2 are:

• reduced maintenance as there is no need to have 2 implementations for each transformer
• reduced user error since a transform might not be supported on the configured source

[santino]

Thanks for your comments @ntziolis

I personally believe we should stick with "wrap" mode when it comes to handling GraphQL sources and I found applying transforms at the source level preferable, with the benefit of delivering polished sources ready for the stitching process.
So the solution I would personally pick up would be along those lines.

I will review the conversation during the week (right now I'm on a bank holiday).
Having said all this, please note that I am just an external contributor to this repository. I have been working on several bits and I am very keen to continue to support the growth of this library and the people using it, with the knowledge I gained but is important to know that I am not an authority responsible for the decision-making here, nor even someone with full knowledge on Mesh and all its dependencies (such as graphql-tools).
However, I have been engaging The Guild team, and I will discuss this with them, so we can hopefully agree on the next steps.

Surely you will get more updates here :)

[yaacovCR]

Also not a contributor to graphql-mesh, but I help out here and there with the upstream @graphql-tools/wrap and @graphql-tools/stitch libraries.

I can comment on why we don't use bare transforms (or at least my understanding of bare transforms) in those packages (which are all graphql sources). One reason is ok-ish, and one is so-so, neither is insurmountable as it relates to graphql-mesh, at least as far as I understand, but things might get more complicated, because other consumers of @graphql-tools, i.e. @graphql-tools/stitch have other constraints. I am going to start with some background that will help me understand the problem. It's mostly for my benefit, so please indulge me.

Let's use the term transform to refer to a manipulation of the graphql pipeline that alters a certain desired characteristic (such as a field or type name), but otherwise maintains operational parity with the original schema. This can be done within the execution layer (see n1ru4l/envelop#94) or within the schema itself -- upstream @graphql-tools and this library, graphql-mesh perform the transforms within the schema.

Now, in terms of the schema layer -- given an arbitrary schema, we can't quite know whether this is even possible. Let's limit our discussion for a second to a single transform -- renaming a non-root field. Resolver for a given field are quite diverse. Let's a couple of scenarios:

(A) A custom resolver may having nothing to do with its field name. In this case, the schema can be safely altered without any worry about modifying the resover.

(B) The default resolver does depend on its field name -- that's how it resolves itself, based on the entry under that field name within the parent source. This resolver must be replaced with a resolver that does the lookup based on the original field name.

(C) An arbitary resolver may perversely check its field name, and, if not equal to the original field name, always return undefined. This resolver could not be wrapped or altered in any way.

What if we limit our discussion of transforms to a certain class of schemas, however? Specifically, schemas that have resolvers in which the source already contains the entire result, because, for example, the root resolver performed a look ahead of the required fieldNodes and queried a database, REST API or external GraphQL endpoint, or what have you?

The root field resolvers for these schemas do not depend on the field name (type A) and the non-root field resolvers can be modified to refer to the original field name (B) or may depend on the alias anyway (type A). This is essentially my understanding of "bare" transforms, in which we bake the transformation into the individual field resolvers, and I will call that resolver based transforms from now on.

Another option is to traverse the result returned by our database, REST API, or external GraphQL endpoint, and modify it prior to it being returned to GraphQL execution. I will call that ROOT resolver based transforms from now on. This is what some of the graphql-tools transforms do. Why?

Two reasons:

1. Historically, schema stitching supports adding additional resolvers on the modified schema (i.e. the gateway) and those schemas would theoretically expect types and fields to be renamed within the parent source object. This may or may not be relevant in the mesh environment, depending on (a) whether you have additional resolvers, and (b) whether this is some way to manually be aware of the original field names or automatically convert them as needed for the custom resolvers.
2. New versions of schema stitching allow fields to come from arbitrarily different schemas, which may have different renaming transforms applied. This further complicates the prior point, but also would require some straightforward but necessary logic to perform the lookup based on the originating subschema and the required transforms. We have not worked on that necessary logic because of the prior point, but it is certainly doable.

Resolver based transforms are more efficient than root resolver based transforms because the latter hide another layer of result traversal. The latter is basically equivalent to the execution layer transform discussed above, which adds another round of result traversal, it just does it within the root resolver rather than after execution.

Now, you may have some GraphQLSchema (de novo, delegating to a database, REST API etc) and wish to transform it, and you may be aware of the many transforms in the @graphql-tools/wrap package, and so you may be tempted to just create your schema locally, and then wrap it with the transform. This creates two additional layers of result traversal, one within our root resolver, as mentioned above, but also a whole additional unnecessary (?) round of GraphQL execution/traversal. That has the most overhead, and that's basically -- if I understand correctly - that's what the original wrap transforms do, and what @santino has been trying to avoid.

I'm not sure if the above is helpful to anyone else, but it was helpful to me! I think there possible ways forward we can work within the graphql-tools library to implement individual field-level resolver transforms, as the 2 issues above do not seem insurmountable. It will probably take a while.

[Note parenthetically, that field renaming itself doesn't actually traverse the result at all -- we use alias tricks to rename the fields.]

[ntziolis]

@yaacovCR Thank you for these insights. Haven yet fully digested all the information will have another look tmr. bright and early.

@santino Before I saw your reply I already went ahead and did a quick and dirty version of what I was trying to convey before. Key part piece is that handlers have a new warpOnly property, based on which the getMesh method has different handling.

Have a look here:
ntziolis/graphql-mesh@master...ntziolis:bare-support-for-graphql-source

Please note:

• This already works as expected and allows having bare transformers on graphql source level (I tested with the same setup as was in the codesandbox)
• All unit tests still pass
• I assume that if there is a rename done on source level (wrap mode) that the new name must be used in the transform

Again this was just a quick shoot in the dark. Me and my team are happy to invest significant time to any solution that gets us to be able to apply all transforms on all levels.

[santino]

Didn't manage to discuss this with the Mesh team yet, but I am going ahead and ​just taking a look at your codesandbox.
I think the solution should be simple, let me try to come up with a draft.

[ntziolis]

@santino Just to ensure there is no misunderstanding. The codesandbox uses the currently released graphql-mesh libs. In order to tests the changed code above I downloaded the codesandbox and used npm link to use the changed graphql-mesh libs to run it.

Please let me know if I can aide this effort in any way, as mentioned before me and my team are willing to contribute significant time to this project moving forward.

[santino]

Yes, that was clear 🙂

I have now raised #1928 as a proper solution to this issue.
Let's see how the team receives it 😉

[ntziolis]

@santino Looking forward to hearing what they think. With that in wrap being the default, but bare being the more performant option, it would be nice to:

• globally override this default
• while at the same time on datasource level fall back to warp mode if the data source doesn't support bare transforms

This would keep the amount of configuration to a minimum in regards to having to specify modes. As well as reduce configuration mistakes by users.

Should I open another issue for this as feature request?

[santino]

I like the idea, potentially we could just add a new property to the root of mesh config that sets the default transform mode.
Then within each transform, you can override it as you wish.

It sounds good, but obviously, right now only two plugins support both modes, and with resolvers-composition it would be three.
Anyway, as transforms are applied across several sources I suppose it would still make sense.
We'd just need to make sure people don't misinterpret that their set default mode will work across all transforms; as it can obviously only work on transforms that accept the "mode" property.

Yes, feel free to open another issue.
When I have time I will be happy to look into this or feel free to jump at it if you prefer 🙂

[ntziolis]

Sry of not clear the above only applies when all transforms actually support both modes. Will open another issue to cover both:

• expand dual mode support to remaining transforms
• allow changing the default mode (incl. handling fallback for data sources that don't support bare)

[santino]

Hey @ntziolis ,
my PR has been merged and released.

Can you confirm the latest releases solved the issue you reported?
In that case, we could close this issue.

[ntziolis]

@santino I can confirm that this now works as expected (I used the code sandbox with updated deps to verify).