diff --git a/.changeset/shiny-forks-happen.md b/.changeset/shiny-forks-happen.md new file mode 100644 index 000000000..9e305c180 --- /dev/null +++ b/.changeset/shiny-forks-happen.md @@ -0,0 +1,9 @@ +--- +"@apollo/query-graphs": patch +--- + +fix: handle directive conditions on fragments when building query graphs + +This fix addresses issues with handling fragments when they specify directive conditions: +* when exploding the types we were not propagating directive conditions +* when processing fragment that specifies super type of an existing type and also specifies directive condition, we were incorrectly preserving the unnecessary type condition. This type condition was problematic as it could be referencing types from supergraph that were not available in the local schema. Instead, we now drop the redundant type condition and only preserve the directives (if specified). diff --git a/query-graphs-js/src/graphPath.ts b/query-graphs-js/src/graphPath.ts index 6fb38ba55..0ff74f8d1 100644 --- a/query-graphs-js/src/graphPath.ts +++ b/query-graphs-js/src/graphPath.ts @@ -2509,7 +2509,7 @@ function advanceWithOperation( const optionsByImplems: OpGraphPath[][][] = []; for (const tName of intersection) { debug.group(() => `Trying ${tName}`); - const castOp = new FragmentElement(currentType, tName); + const castOp = new FragmentElement(currentType, tName, operation.appliedDirectives); const implemOptions = advanceSimultaneousPathsWithOperation( supergraphSchema, new SimultaneousPathsWithLazyIndirectPaths([path], context, conditionResolver), @@ -2548,8 +2548,13 @@ function advanceWithOperation( const conditionType = supergraphSchema.type(typeName)!; if (isAbstractType(conditionType) && possibleRuntimeTypes(conditionType).some(t => t.name == currentType.name)) { debug.groupEnd(() => `${typeName} is a super-type of current type ${currentType}: no edge to take`); + // Operation type condition is applicable on the current type, so the types are already exploded but the + // condition can reference types from the supergraph that are not present in the local subgraph. + // + // If operation has applied directives we need to convert to inline fragment without type condition, otherwise + // we ignore the fragment altogether. const updatedPath = operation.appliedDirectives.length > 0 - ? path.add(operation, null, noConditionsResolution, operation.deferDirectiveArgs()) + ? path.add(operation.withUpdatedTypes(currentType, undefined), null, noConditionsResolution, operation.deferDirectiveArgs()) : path; return { options: [[ updatedPath ]] }; } diff --git a/query-planner-js/src/__tests__/buildPlan.test.ts b/query-planner-js/src/__tests__/buildPlan.test.ts index f0717e03d..80ca229a7 100644 --- a/query-planner-js/src/__tests__/buildPlan.test.ts +++ b/query-planner-js/src/__tests__/buildPlan.test.ts @@ -7973,3 +7973,185 @@ describe('@requires references external field indirectly', () => { `); }); }); + +describe('handles fragments with directive conditions', () => { + test('fragment with intersecting parent type and directive condition', () => { + const subgraphA = { + typeDefs: gql` + directive @test on FRAGMENT_SPREAD + type Query { + i: I + } + interface I { + _id: ID + } + type T1 implements I @key(fields: "id") { + _id: ID + id: ID + } + type T2 implements I @key(fields: "id") { + _id: ID + id: ID + } + `, + name: 'A', + }; + + const subgraphB = { + typeDefs: gql` + directive @test on FRAGMENT_SPREAD + type Query { + i2s: [I2] + } + interface I2 { + id: ID + title: String + } + type T1 implements I2 @key(fields: "id") { + id: ID + title: String + } + type T2 implements I2 @key(fields: "id") { + id: ID + title: String + } + `, + name: 'B', + }; + + const [api, queryPlanner] = composeAndCreatePlanner(subgraphA, subgraphB); + + const operation = operationFromDocument( + api, + gql` + query { + i { + _id + ... on I2 @test { + id + } + } + } + `, + ); + + const queryPlan = queryPlanner.buildQueryPlan(operation); + expect(queryPlan).toMatchInlineSnapshot(` + QueryPlan { + Fetch(service: "A") { + { + i { + __typename + _id + ... on T1 @test { + id + } + ... on T2 @test { + id + } + } + } + }, + } + `); + }); + + test('nested fragment with interseting parent type and directive condition', () => { + const subgraphA = { + typeDefs: gql` + directive @test on FRAGMENT_SPREAD + type Query { + i: I + } + interface I { + _id: ID + } + type T1 implements I @key(fields: "id") { + _id: ID + id: ID + } + type T2 implements I @key(fields: "id") { + _id: ID + id: ID + } + `, + name: 'A', + }; + + const subgraphB = { + typeDefs: gql` + directive @test on FRAGMENT_SPREAD + type Query { + i2s: [I2] + } + interface I2 { + id: ID + title: String + } + type T1 implements I2 @key(fields: "id") { + id: ID + title: String + } + type T2 implements I2 @key(fields: "id") { + id: ID + title: String + } + `, + name: 'B', + }; + + const [api, queryPlanner] = composeAndCreatePlanner(subgraphA, subgraphB); + + const operation = operationFromDocument( + api, + gql` + query { + i { + _id + ... on I2 { + ... on I2 @test { + id + } + } + } + } + `, + ); + + expect(operation.expandAllFragments().toString()).toMatchInlineSnapshot(` + "{ + i { + _id + ... on I2 { + ... on I2 @test { + id + } + } + } + }" + `); + const queryPlan = queryPlanner.buildQueryPlan(operation); + expect(queryPlan).toMatchInlineSnapshot(` + QueryPlan { + Fetch(service: "A") { + { + i { + __typename + _id + ... on T1 { + ... @test { + id + } + } + ... on T2 { + ... @test { + id + } + } + } + } + }, + } + `); + }); +});