apollographql / router

A configurable, high-performance routing runtime for Apollo Federation 🚀
https://www.apollographql.com/docs/router/
Other
813 stars 271 forks source link

Corrected overwritten fields in `remove_conditions_from_selection_set` #6116

Closed TylerBloom closed 1 month ago

TylerBloom commented 1 month ago

In SelectionSet::remove_conditions_from_selection_set, a SelectionMap was being constructed directly from an existing selection set. Each selection had its conditions removed and inserted into the new map; however, it is possible that removing the directives from the selection would now cause a key collision. An example of this can be seen in the tests that were added:

query Test($b: Boolean!) {
  foo @include(if: $b) {
    bar {
      name
    }
    bar @include(if: $b) {
      thing
    }
  }
}

When the selection set was finalized, the selection bar { name } was getting overwritten because, after the next selection had its @include directive removed, the two selections' keys collided. In the JS code, they used SelectionSet::lazyMap, so now the Rust code does too.


Checklist

Complete the checklist (and note appropriate exceptions) before the PR is marked ready-for-review.

Exceptions

Note any exceptions here

Notes

[^1]: It may be appropriate to bring upcoming changes to the attention of other (impacted) groups. Please endeavour to do this before seeking PR approval. The mechanism for doing this will vary considerably, so use your judgement as to how and when to do this. [^2]: Configuration is an important part of many changes. Where applicable please try to document configuration examples. [^3]: Tick whichever testing boxes are applicable. If you are adding Manual Tests, please document the manual testing (extensively) in the Exceptions.

svc-apollo-docs commented 1 month ago

✅ Docs Preview Ready

No new or changed pages found.

github-actions[bot] commented 1 month ago

@TylerBloom, please consider creating a changeset entry in /.changesets/. These instructions describe the process and tooling.

router-perf[bot] commented 1 month ago

CI performance tests