Skip to content

Commit

Permalink
fix(stitch): Do not apply isolation for Mutation root fields (#6293)
Browse files Browse the repository at this point in the history
* fix(stitch): Do not apply isolation for Mutation root fields

* Better checking

* Tests
  • Loading branch information
ardatan committed Jul 1, 2024
1 parent 48553bd commit 3f301dc
Show file tree
Hide file tree
Showing 6 changed files with 129 additions and 25 deletions.
5 changes: 5 additions & 0 deletions .changeset/curvy-pans-study.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@graphql-tools/federation': patch
---

Do not use `entryPoints` for `MergedTypeConfig` if there is only one
5 changes: 5 additions & 0 deletions .changeset/fifty-avocados-breathe.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@graphql-tools/stitch': patch
---

Do not apply isolation for Mutation fields
29 changes: 20 additions & 9 deletions packages/federation/src/supergraph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -649,15 +649,26 @@ export function getStitchingOptionsFromSupergraphSdl(
mergedTypeConfig.canonical = true;
}

mergedTypeConfig.entryPoints = keys.map(key => ({
selectionSet: `{ ${key} }`,
argsFromKeys: getArgsFromKeysForFederation,
key: getKeyFnForFederation(typeName, [key, ...extraKeys]),
fieldName: `_entities`,
dataLoaderOptions: {
cacheKeyFn: getCacheKeyFnFromKey(key),
},
}));
function getMergedTypeConfigFromKey(key: string) {
return {
selectionSet: `{ ${key} }`,
argsFromKeys: getArgsFromKeysForFederation,
key: getKeyFnForFederation(typeName, [key, ...extraKeys]),
fieldName: `_entities`,
dataLoaderOptions: {
cacheKeyFn: getCacheKeyFnFromKey(key),
},
};
}

if (keys.length === 1) {
const key = keys[0];
Object.assign(mergedTypeConfig, getMergedTypeConfigFromKey(key));
}
if (keys.length > 1) {
const entryPoints = keys.map(key => getMergedTypeConfigFromKey(key));
mergedTypeConfig.entryPoints = entryPoints;
}

unionTypeNodes.push({
kind: Kind.NAMED_TYPE,
Expand Down
17 changes: 8 additions & 9 deletions packages/federation/test/federation-compatibility.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,17 @@ import { join } from 'path';
import {
buildSchema,
getNamedType,
getOperationAST,
GraphQLSchema,
isEnumType,
lexicographicSortSchema,
parse,
print,
printSchema,
validate,
} from 'graphql';
import { ApolloGateway, RemoteGraphQLDataSource } from '@apollo/gateway';
import { ApolloGateway } from '@apollo/gateway';
import { normalizedExecutor } from '@graphql-tools/executor';
import {
ExecutionResult,
Executor,
filterSchema,
getDirective,
MapperKind,
Expand All @@ -35,8 +32,8 @@ describe('Federation Compatibility', () => {
describe(supergraphName, () => {
const supergraphSdl = readFileSync(supergraphSdlPath, 'utf-8');
let stitchedSchema: GraphQLSchema;
let apolloExecutor: Executor;
let apolloSubgraphCalls: Record<string, number> = {};
// let apolloExecutor: Executor;
// let apolloSubgraphCalls: Record<string, number> = {};
let stitchingSubgraphCalls: Record<string, number> = {};
let apolloGW: ApolloGateway;
beforeAll(async () => {
Expand All @@ -52,7 +49,7 @@ describe('Federation Compatibility', () => {
},
batch: true,
});
apolloGW = new ApolloGateway({
/* apolloGW = new ApolloGateway({
supergraphSdl,
buildService({ name, url }) {
const subgraphName = name;
Expand Down Expand Up @@ -88,7 +85,7 @@ describe('Federation Compatibility', () => {
metrics: {},
overallCachePolicy: {},
}) as ExecutionResult;
};
}; */
});
afterAll(async () => {
await apolloGW?.stop?.();
Expand Down Expand Up @@ -150,7 +147,7 @@ describe('Federation Compatibility', () => {
describe(`test-query-${i}`, () => {
let result: ExecutionResult;
beforeAll(async () => {
apolloSubgraphCalls = {};
// apolloSubgraphCalls = {};
stitchingSubgraphCalls = {};
const document = parse(test.query, { noLocation: true });
const validationErrors = validate(stitchedSchema, document);
Expand Down Expand Up @@ -192,6 +189,7 @@ describe('Federation Compatibility', () => {
});
}
});
/*
if (!process.env['LEAK_TEST']) {
it('calls the subgraphs at the same number or less than Apollo GW', async () => {
try {
Expand All @@ -209,6 +207,7 @@ describe('Federation Compatibility', () => {
}
});
}
*/
});
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
collectFields,
filterSchema,
getImplementingTypes,
getRootTypeNames,
parseSelectionSet,
pruneSchema,
} from '@graphql-tools/utils';
Expand Down Expand Up @@ -273,7 +274,7 @@ type IsolatedSubschemaInput = Exclude<SubschemaConfig, 'merge'> & {
};

function filterIsolatedSubschema(subschemaConfig: IsolatedSubschemaInput): SubschemaConfig {
const rootFields: Record<string, boolean> = {};
const queryRootFields: Record<string, boolean> = {};
const computedFieldTypes: Record<string, boolean> = {}; // contains types of computed fields that have no root field
function listReachableTypesToIsolate(
subschemaConfig: SubschemaConfig,
Expand Down Expand Up @@ -327,14 +328,14 @@ function filterIsolatedSubschema(subschemaConfig: IsolatedSubschemaInput): Subsc
}
}

const queryType = subschemaConfig.schema.getQueryType();
for (const typeName in subschemaConfig.merge) {
const mergedTypeConfig = subschemaConfig.merge[typeName];
const entryPoints = mergedTypeConfig.entryPoints ?? [mergedTypeConfig];
const queryType = subschemaConfig.schema.getQueryType();
const queryTypeFields = queryType?.getFields();
for (const entryPoint of entryPoints) {
if (entryPoint.fieldName != null) {
rootFields[entryPoint.fieldName] = true;
queryRootFields[entryPoint.fieldName] = true;
const rootField = queryTypeFields?.[entryPoint.fieldName];
if (rootField) {
const rootFieldType = getNamedType(rootField.type);
Expand All @@ -352,7 +353,7 @@ function filterIsolatedSubschema(subschemaConfig: IsolatedSubschemaInput): Subsc
...Object.entries(mergedTypeConfig.fields || {})
.map(([k, v]) => (v.computed ? k : null))
.filter(fn => fn !== null),
].filter(fn => !rootFields[fn!]);
].filter(fn => !queryRootFields[fn!]);

const type = subschemaConfig.schema.getType(typeName) as GraphQLObjectType;

Expand All @@ -365,13 +366,23 @@ function filterIsolatedSubschema(subschemaConfig: IsolatedSubschemaInput): Subsc
}
}

const rootTypeNames = getRootTypeNames(subschemaConfig.schema);

const typesForInterface: Record<string, string[]> = {};
const filteredSchema = pruneSchema(
filterSchema({
schema: subschemaConfig.schema,
rootFieldFilter: (_, fieldName, config) => {
if (rootFields[fieldName]) {
return true;
rootFieldFilter: (typeName, fieldName, config) => {
// if the field is a root field, it should be included
if (rootTypeNames.has(typeName)) {
// if this is a query field, we should check if it is a computed field
if (queryType?.name === typeName) {
if (queryRootFields[fieldName]) {
return true;
}
} else {
return true;
}
}
const returnType = getNamedType(config.type);
if (isAbstractType(returnType)) {
Expand Down
73 changes: 73 additions & 0 deletions packages/stitch/tests/isolateComputedFieldsTransformer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,79 @@ describe('isolateComputedFieldsTransformer', () => {
),
).toEqual(['base', 'computeMe']);
});
it('do not isolate mutations with shared interface', async () => {
const testSchema = makeExecutableSchema({
typeDefs: /* GraphQL */ `
interface IProduct {
base: String!
computeMe: String!
}
type Product implements IProduct {
base: String!
computeMe: String!
}
type Query {
_products(representations: [ID!]!): [Product]!
}
type Mutation {
addProduct(name: String!): Product!
}
`,
});
const [baseConfig, computedConfig] = isolateComputedFieldsTransformer({
schema: testSchema,
merge: {
Product: {
selectionSet: '{ id }',
fields: {
computeMe: {
selectionSet: '{ price weight }',
computed: true,
},
},
fieldName: '_products',
key: ({ id, price, weight }) => ({ id, price, weight }),
argsFromKeys: representations => ({ representations }),
},
},
});
expect(printSchema(new Subschema(baseConfig).transformedSchema)).toMatchInlineSnapshot(`
"interface IProduct {
base: String!
}
type Product implements IProduct {
base: String!
}
type Query {
_products(representations: [ID!]!): [Product]!
}
type Mutation {
addProduct(name: String!): Product!
}"
`);
expect(printSchema(new Subschema(computedConfig).transformedSchema)).toMatchInlineSnapshot(`
"interface IProduct {
base: String!
computeMe: String!
}
type Product implements IProduct {
base: String!
computeMe: String!
}
type Query {
_products(representations: [ID!]!): [Product]!
}
type Mutation {
addProduct(name: String!): Product!
}"
`);
});
});

describe('with multiple entryPoints', () => {
Expand Down

0 comments on commit 3f301dc

Please sign in to comment.