Skip to content

Commit

Permalink
Merge branch 'main' into f2485-composition-error-itfObject
Browse files Browse the repository at this point in the history
  • Loading branch information
Sylvain Lebresne committed Mar 29, 2023
2 parents 6ada88c + 5cc7c29 commit 4a3b499
Show file tree
Hide file tree
Showing 6 changed files with 220 additions and 152 deletions.
7 changes: 7 additions & 0 deletions .changeset/lovely-walls-sneeze.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@apollo/federation-internals": patch
---

Fix assertion error during query planning in some cases where queries has some unsatisfiable branches (a part of the
query goes through type conditions that no runtime types satisfies).

10 changes: 8 additions & 2 deletions .cspell/cspell.yml
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
$schema: https://raw.githubusercontent.com/streetsidesoftware/cspell/main/cspell.schema.json
# Configuration for the cspell command-line tool and the Code Spell Checker
# VSCode extension
# (https://marketplace.visualstudio.com/items?itemName=streetsidesoftware.code-spell-checker).
Expand All @@ -19,9 +20,14 @@ allowCompoundWords: true
# by hand. Note that we do want to check, say, JSON files (as package.json
# contains English text like package descriptions).
useGitignore: true

# Need to set this since globs are relative to the cspell config file.
globRoot: '..'
files:
- '**'
- '.changeset/**'
ignorePaths:
- .cspell/cspell.yml
- '**/__generated__/**'
- 'gateway-js/src/__generated__/**'

overrides:
# Ignore anything in a changelog file that looks like a GitHub username.
Expand Down
58 changes: 57 additions & 1 deletion internals-js/src/__tests__/operations.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,6 @@ describe('basic operations', () => {
})
});


describe('MutableSelectionSet', () => {
test('memoizer', () => {
const schema = parseSchema(`
Expand Down Expand Up @@ -546,3 +545,60 @@ describe('MutableSelectionSet', () => {
expect(sets).toStrictEqual(['{}', '{ t { v1 } }', '{ t { v1 v3 } }', '{ t { v1 v3 v2 } }', '{ t { v1 v3 v4 } }']);
});
});

describe('unsatisfiable branches removal', () => {
const schema = parseSchema(`
type Query {
i: I
j: J
}
interface I {
a: Int
b: Int
}
interface J {
b: Int
}
type T1 implements I & J {
a: Int
b: Int
c: Int
}
type T2 implements I {
a: Int
b: Int
d: Int
}
type T3 implements J {
a: Int
b: Int
d: Int
}
`);

const withoutUnsatisfiableBranches = (op: string) => {
return parseOperation(schema, op).trimUnsatisfiableBranches().toString(false, false)
};


it.each([
'{ i { a } }',
'{ i { ... on T1 { a b c } } }',
])('is identity if there is no unsatisfiable branches', (op) => {
expect(withoutUnsatisfiableBranches(op)).toBe(op);
});

it.each([
{ input: '{ i { ... on I { a } } }', output: '{ i { a } }' },
{ input: '{ i { ... on T1 { ... on I { a b } } } }', output: '{ i { ... on T1 { a b } } }' },
{ input: '{ i { ... on I { a ... on T2 { d } } } }', output: '{ i { a ... on T2 { d } } }' },
{ input: '{ i { ... on T2 { ... on I { a ... on J { b } } } } }', output: '{ i { ... on T2 { a } } }' },
])('removes unsatisfiable branches', ({input, output}) => {
expect(withoutUnsatisfiableBranches(input)).toBe(output);
});
});
9 changes: 5 additions & 4 deletions internals-js/src/operations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2275,10 +2275,12 @@ class InlineFragmentSelection extends FragmentSelection {
// If the current type is an object, then we never need to keep the current fragment because:
// - either the fragment is also an object, but we've eliminated the case where the 2 types are the same,
// so this is just an unsatisfiable branch.
// - or it's not an object, but then the current type is more precise and no poitn in "casting" to a
// less precise interface/union.
// - or it's not an object, but then the current type is more precise and no point in "casting" to a
// less precise interface/union. And if the current type is not even a valid runtime of said interface/union,
// then we should completely ignore the branch (or, since we're eliminating `thisCondition`, we would be
// building an invalid selection).
if (isObjectType(currentType)) {
if (isObjectType(thisCondition)) {
if (isObjectType(thisCondition) || !possibleRuntimeTypes(thisCondition).includes(currentType)) {
return undefined;
} else {
const trimmed = this.selectionSet.trimUnsatisfiableBranches(currentType);
Expand Down Expand Up @@ -2343,7 +2345,6 @@ class InlineFragmentSelection extends FragmentSelection {
return this.selectionSet === trimmedSelectionSet ? this : this.withUpdatedSelectionSet(trimmedSelectionSet);
}


expandAllFragments(): FragmentSelection {
return this.mapToSelectionSet((s) => s.expandAllFragments());
}
Expand Down

0 comments on commit 4a3b499

Please sign in to comment.