movio / bramble

A federated GraphQL API gateway
https://movio.github.io/bramble/
MIT License
497 stars 55 forks source link

Fix fragment spread elimination during result merge + format #153

Closed lucianjon closed 2 years ago

lucianjon commented 2 years ago

FIxes: #151

There was an issue multiple top level named fragment spreads under an interface type were not being correctly eliminated during the merge and format stages of query execution.

The behaviour should be that for both inline and named fragment spreads, only the spread that matches the concrete type in the response is kept. This was working for inline spreads but not named spreads.

The problem was made a little harder to fix due to an existing problem with evaluateSkipAndInclude that would overwrite the ObjectDefinition for a FragmentSpread with the wrong value.

This PR fixes both issues.

codecov-commenter commented 2 years ago

Codecov Report

Merging #153 (9e0455f) into main (6cfb4cd) will increase coverage by 0.09%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #153      +/-   ##
==========================================
+ Coverage   70.29%   70.38%   +0.09%     
==========================================
  Files          25       25              
  Lines        2626     2634       +8     
==========================================
+ Hits         1846     1854       +8     
  Misses        660      660              
  Partials      120      120              
Impacted Files Coverage Δ
execution.go 77.85% <100.00%> (ø)
query_execution.go 77.38% <100.00%> (+0.38%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 6cfb4cd...9e0455f. Read the comment docs.