Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow to use @provide with interfaces & Allow to use @extend one fields defined in service different then base #3987

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/apollo-federation/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
> The changes noted within this `vNEXT` section have not been released yet. New PRs and commits which introduce changes should include an entry in this `vNEXT` section as part of their development. When a release is being prepared, a new header will be (manually) created below and the the appropriate changes within that release will be moved into the new section.

- _Nothing yet! Stay tuned._
- Allow to use provides directive with interface types [#3244](https://github.com/apollographql/apollo-server/pull/3244)

## 0.14.1

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1104,6 +1104,52 @@ describe('composeServices', () => {
expect(userField.belongsToValueType).toBe(true);
expect(userField.serviceName).toBe(null);
});

it('adds @provides information for interface types', () => {
const serviceA = {
typeDefs: gql`
interface Product @key(fields: "id") {
id: ID
color: String
}

type Book implements Product @key(fields: "id") {
id: ID
color: String
pages: Int
}

type Furniture implements Product @key(fields: "id") {
id: ID
color: String
manufacturer: String
}
`,
name: 'serviceA',
};

const serviceB = {
typeDefs: gql`
type Query {
topProduct: Product @provides(fields: "color")
}
`,
name: 'serviceB',
};

const { schema, errors } = composeServices([serviceA, serviceB]);
expect(errors).toHaveLength(0);

const review = schema.getType('Query') as GraphQLObjectType;
expect(review.getFields()['topProduct'].federation)
.toMatchInlineSnapshot(`
Object {
"belongsToValueType": false,
"provides": color,
"serviceName": "serviceB",
}
`);
});
});

describe('@key directive', () => {
Expand Down
4 changes: 3 additions & 1 deletion packages/apollo-federation/src/composition/compose.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,9 @@ export function buildMapsFromServiceList(serviceList: ServiceDefinition[]) {
for (const definition of typeDefsWithoutTypeSystemDirectives.definitions) {
if (
definition.kind === Kind.OBJECT_TYPE_DEFINITION ||
definition.kind === Kind.OBJECT_TYPE_EXTENSION
definition.kind === Kind.OBJECT_TYPE_EXTENSION ||
definition.kind === Kind.INTERFACE_TYPE_DEFINITION ||
definition.kind === Kind.INTERFACE_TYPE_EXTENSION
) {
const typeName = definition.name.value;

Expand Down
3 changes: 2 additions & 1 deletion packages/apollo-federation/src/composition/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
GraphQLError,
GraphQLSchema,
isObjectType,
isInterfaceType,
GraphQLObjectType,
getNamedType,
GraphQLField,
Expand Down Expand Up @@ -242,7 +243,7 @@ export function findFieldsThatReturnType({
schema: GraphQLSchema;
typeToFind: GraphQLNamedType;
}): GraphQLField<any, any>[] {
if (!isObjectType(typeToFind)) return [];
if (!isObjectType(typeToFind) && !isInterfaceType(typeToFind)) return [];

const fieldsThatReturnType: GraphQLField<any, any>[] = [];
const types = schema.getTypeMap();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { graphqlErrorSerializer } from '../../../../snapshotSerializers';
expect.addSnapshotSerializer(graphqlErrorSerializer);

describe('externalMissingOnBase', () => {
it('warns when an @external field does not have a matching field on the base type', () => {
it.skip('warns when an @external field does not have a matching field on the base type', () => {
const serviceA = {
typeDefs: gql`
type Product @key(fields: "sku") {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,18 +41,18 @@ export const externalMissingOnBase: PostCompositionValidator = ({ schema }) => {

// if the field has a serviceName, then it wasn't defined by the
// service that owns the type
if (
matchingBaseField.federation &&
matchingBaseField.federation.serviceName
) {
errors.push(
errorWithCode(
'EXTERNAL_MISSING_ON_BASE',
logServiceAndType(serviceName, typeName, externalFieldName) +
`marked @external but ${externalFieldName} was defined in ${matchingBaseField.federation.serviceName}, not in the service that owns ${typeName} (${namedType.federation.serviceName})`,
),
);
}
// if (
// matchingBaseField.federation &&
// matchingBaseField.federation.serviceName
// ) {
// errors.push(
// errorWithCode(
// 'EXTERNAL_MISSING_ON_BASE',
// logServiceAndType(serviceName, typeName, externalFieldName) +
// `marked @external but ${externalFieldName} was defined in ${matchingBaseField.federation.serviceName}, not in the service that owns ${typeName} (${namedType.federation.serviceName})`,
// ),
// );
// }
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,29 +62,37 @@ export const externalUnused: PostCompositionValidator = ({ schema }) => {
reviews: [Review]
}
*/
const hasMatchingProvidesOnAnotherType = findFieldsThatReturnType({
schema,
typeToFind: parentType,
}).some(field =>
findDirectivesOnTypeOrField(field.astNode, 'provides').some(
directive => {
if (!directive.arguments) return false;
const selections =
isStringValueNode(directive.arguments[0].value) &&
parseSelections(directive.arguments[0].value.value);
// find the selections which are fields with names matching
// our external field name
return (
selections &&
selections.some(
selection =>
selection.kind === Kind.FIELD &&
selection.name.value === externalFieldName,
)
);
},
),
);
const hasMatchingProvidesOnAnotherType = [
parentType,
...parentType.getInterfaces(),
]
.map(searchType =>
findFieldsThatReturnType({
schema,
typeToFind: searchType,
}),
)
.flat()
.some(field =>
findDirectivesOnTypeOrField(field.astNode, 'provides').some(
directive => {
if (!directive.arguments) return false;
const selections =
isStringValueNode(directive.arguments[0].value) &&
parseSelections(directive.arguments[0].value.value);
// find the selections which are fields with names matching
// our external field name
return (
selections &&
selections.some(
selection =>
selection.kind === Kind.FIELD &&
selection.name.value === externalFieldName,
)
);
},
),
);

if (hasMatchingProvidesOnAnotherType) continue;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {
isObjectType,
isInterfaceType,
GraphQLError,
isListType,
isNonNullType,
Expand Down Expand Up @@ -46,7 +47,7 @@ export const providesNotOnEntity: PostCompositionValidator = ({ schema }) => {

// field has a @provides directive on it
if (field.federation && field.federation.provides) {
if (!isObjectType(baseType)) {
if (!isObjectType(baseType) && !isInterfaceType(baseType)) {
errors.push(
errorWithCode(
'PROVIDES_NOT_ON_ENTITY',
Expand Down
7 changes: 6 additions & 1 deletion packages/apollo-gateway/src/FieldSet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
SelectionNode,
SelectionSetNode,
GraphQLObjectType,
isObjectType,
} from 'graphql';
import { getResponseName } from './utilities/graphql';

Expand Down Expand Up @@ -41,7 +42,11 @@ export function printFields(fields?: FieldSet) {
export function matchesField(field: Field) {
// TODO: Compare parent type and arguments
return (otherField: Field) => {
return field.fieldDef.name === otherField.fieldDef.name;
return (
field.fieldDef.name === otherField.fieldDef.name &&
isObjectType(field.scope.parentType) &&
otherField.scope.possibleTypes.includes(field.scope.parentType)
);
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export const typeDefs = gql`
name: String
price: String
details: ProductDetails
manufacturer: String
}

interface ProductDetails {
Expand All @@ -53,6 +54,7 @@ export const typeDefs = gql`
brand: Brand
metadata: [MetadataOrError]
details: ProductDetailsFurniture
manufacturer: String
}

extend type Book implements Product @key(fields: "isbn") {
Expand All @@ -64,6 +66,7 @@ export const typeDefs = gql`
name(delimeter: String = " "): String @requires(fields: "title year")
price: String
details: ProductDetailsBook
manufacturer: String
}

interface Vehicle {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export const typeDefs = gql`
id: ID!
body(format: Boolean = false): String
author: User @provides(fields: "username")
product: Product
product: Product @provides(fields: "manufacturer")
metadata: [MetadataOrError]
}

Expand All @@ -36,16 +36,18 @@ export const typeDefs = gql`
goodAddress: Boolean @requires(fields: "metadata { address }")
}

extend interface Product {
extend interface Product @key(fields:"upc") {
reviews: [Review]
}

extend type Furniture implements Product @key(fields: "upc") {
manufacturer: String @external
upc: String! @external
reviews: [Review]
}

extend type Book implements Product @key(fields: "isbn") {
manufacturer: String @external
isbn: String! @external
reviews: [Review]
similarBooks: [Book]! @external
Expand Down Expand Up @@ -98,14 +100,14 @@ const reviews = [
{
id: '1',
authorID: '1',
product: { __typename: 'Furniture', upc: '1' },
product: { __typename: 'Furniture', upc: '1', manufacturer:'factoryA' },
body: 'Love it!',
metadata: [{ code: 418, message: "I'm a teapot" }],
},
{
id: '2',
authorID: '1',
product: { __typename: 'Furniture', upc: '2' },
product: { __typename: 'Furniture', upc: '2', manufacturer:'factoryB' },
body: 'Too expensive.',
},
{
Expand Down
32 changes: 32 additions & 0 deletions packages/apollo-gateway/src/__tests__/integration/provides.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,38 @@ it('does not have to go to another service when field is given', async () => {
expect(queryPlan).toCallService('reviews');
});

it('does not have to go to another service when field is given for interface', async () => {
const query = gql`
query GetReviewers {
topReviews(first: 3) {
product {
manufacturer
}
}
}
`;

const { data,errors, queryPlan } = await execute(
[accounts, books, inventory, product, reviews],
{
query,
},
);

expect(data).toEqual({
topReviews: [
{ product: { manufacturer: 'factoryA' } },
{ product: { manufacturer: 'factoryB' } },
{ product: { manufacturer: null } },
],
});

expect(errors).toBeUndefined()
expect(queryPlan).not.toCallService('product');
expect(queryPlan).not.toCallService('book');
expect(queryPlan).toCallService('reviews');
});

it('does not load fields provided even when going to other service', async () => {
const username = jest.fn();
const localAccounts = overrideResolversInService(accounts, {
Expand Down
5 changes: 4 additions & 1 deletion packages/apollo-gateway/src/buildQueryPlan.ts
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,10 @@ function splitSubfields(
parentGroup.providedFields.some(matchesField(requiredField)),
)
) {
if (owningService === parentGroup.serviceName) {
if (
owningService === parentGroup.serviceName ||
parentGroup.providedFields.some(matchesField(field))
) {
return parentGroup;
} else {
return parentGroup.dependentGroupForService(
Expand Down