Querying fragments result in incorrect downstream requests

[hrkfdn]

Describe the bug
It appears that while fragments in queries are resolved correctly, they are substituted inside the query while the fragment is still sent to the downstream service. This will result in an UnusedFragment validation error in the downstream service.

This appears to be a regression in Apollo Router 0.16.0, as the problem does not occur with 0.15.1.

To Reproduce
Steps to reproduce the behavior:

1. Setup services

type EMSLicense {
  licenseNumber: String
}

type Employee @key(fields : "id") {
  driverLicense: EMSLicense
  passengerTransportLicense: EMSLicense
}

2. Submit request

query Employee($id: ID!) {
  employee(id: $id) {
    ...Employee
  }
}

fragment Employee on Employee {
  driverLicense {
    ...EMSLicense
  }
  passengerTransportLicense {
    ...EMSLicense
  }
}

fragment EMSLicense on EMSLicense {
  licenseNumber
}

3. Results in the following downstream query

query Employee__driver_ems__0($id: ID!) {
  employee(id: $id) {
    driverLicense {
      ... on EMSLicense {
        licenseNumber
      }
    }
    passengerTransportLicense {
      ... on EMSLicense {
        licenseNumber
      }
    }
  }
}
fragment EMSLicense on EMSLicense {
  licenseNumber
}

Desktop (please complete the following information):

• Apollo Router Version 0.16.0

[abernix]

Thanks for reporting this! This may be an upstream bug as it aligns with an update to our federation v2.1.0 query planner. I'm going to check with another set of people and circle back to this.

[abernix]

I've transferred this to the federation repository where it can be looked at by the query planner team. We'll still need to update the Router (where this was opened originally) if a fix is applied, so let's be sure to communicate when that's ready if that happens. Thanks!

[pcmanus]

That was indeed an issue with the query planner code, and more precisely due to an issue with #1911.

Tl;dr, that earlier PR make it so that we now re-use query fragments in subgraph fetches when it make sense, so the fact that the EMSLicense fragment in the example above is present is now expected, but the unexpected part is that this fragment is not actually used in the query, and that was due to some code path re-expanding fragments mistakenly. Pushed #2098 with the fix.