fix(gateway): merge selection sets of @requires fields

[jakeblaxon]

This fixes a bug in the Apollo Gateway, where @requires fields are not provided to resolvers for fields nested more than one level deep.

What this is trying to fix

This aims to fix issue #3848: [Federation Gateway] @requires directive sometimes does not provide all required fields

Scenario

When requiring fields nested more than one level deep, they are not getting merged together into one selection set in the query plan. Instead, the query plan will contain duplicate sets like the following:

A {
    nested1 {
        nested2 {
            requiredValue
        }
    }
    nested1 {
        nested2 {
            userSpecifiedValue
        }
    }
}

The required value would be fetched but overwritten by the second selection set in the javascript object. This PR solves this issue by merging the selection sets together, to arbitrary depth, like the following:

A {
    nested1 {
        nested2 {
            requiredValue
            userSpecifiedValue
        }
    }
}

Fixes #3848
Closes #4017

[apollo-cla]

@jakeblaxon: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/

[jakeblaxon]

Just noticed that #4017 appears to solve the same issue. I'll leave this PR open for now, but I'll go ahead and close it if #4017 gets merged in.

[trevor-scheer]

Thanks so much for taking the time to work on this @jakeblaxon! I've spoken with the author of the PR you linked, I'd like to go forward with your implementation if you're comfortable with the changes I'm suggesting.

If you're busy or unable to carry this forward for any reason please let me know and I'd be more than happy to wrap this up. Thanks again! 🎉

[trevor-scheer]

Also, please add a changelog entry to the gateway changelog. Note: there's a separate changelog for gateway from the rest of the server monorepo.

[jakeblaxon]

Thanks, @trevor-scheer! I can get these changes in soon. One concern I want to bring up though is that this only combines selection sets for field nodes. If we were to get a fragment node that itself had nested subfields, that fragment node's selection set would still potentially override the corresponding field node's.

It currently appears that we expand any fragments into fields before calling selectionSetFromFieldSet(), so I haven't run into this issue, even when testing with fragments in the user query. I added the second test case, however, to catch this issue in case something changes in the future and we end up not expanding fragments before calling selectionSetFromFieldSet(). (You can see an example of this potential issue with the second test case that I added).

I noticed that in the #4017 implementation, since we pass context into selectionSetFromFieldSet(), we can potentially solve this issue, since we would then have access to that fragment node's corresponding selection set. (The reason I didn't address the fragment case was because I didn't have access to the selection set directly from the fragment node). If we continue to expand fragments before building the query plan, then this won't be necessary. If we don't continue this, however, we may want to consider going with #4017 so we can address the fragment case.

Curious to hear your thoughts. Thanks!

[trevor-scheer]

@jakeblaxon the insight is very much appreciated. Let's go forward with this implementation. If we ever do go down that path, we can revisit Lenny's solution or adapt things as needed. Let me know if you need anything from me!

[trevor-scheer]

Two minor changes within this review and this looks good to go! Thanks for the timely turnaround on these updates.

When I land this PR, do you mind if I attribute co-authorship to Lenny as well? I appreciate the effort that went into this from both of you!

FWIW, this is what that would look like in the final commit:
https://help.github.com/en/github/committing-changes-to-your-project/creating-a-commit-with-multiple-authors#creating-co-authored-commits-on-github

[jakeblaxon]

@trevor-scheer Of course! Please add @lennyburdette as a co-author. Thank you!

[trevor-scheer]

Thank you both for your time and efforts here, I'll have this released as a patch fix early next week!

[abernix]

Published in @apollo/[email protected]! 🎉 Thanks for this contribution!