Skip to content

Commit

Permalink
fix(federation): Aggregate interfaces for types and interfaces (apoll…
Browse files Browse the repository at this point in the history
…ographql/apollo-server#4497)

For value types which implement interfaces, composition currently
exhibits some unexpected behavior. Value types can implement whatever
interfaces they'd like within their respective service, but that won't
necessarily be represented within the composed schema.

To resolve this, we need to aggregate all interfaces that an object
or interface might implement and ensure those are represented in the
composed schema.

It's not important for every service to implement every interface
that the others do, so in this sense we can be more permissive and
allow each service to choose which interfaces a type or interface
should implement locally.
Apollo-Orig-Commit-AS: apollographql/apollo-server@3081569
  • Loading branch information
trevor-scheer authored Aug 21, 2020
1 parent 0bc0806 commit 12b6241
Show file tree
Hide file tree
Showing 3 changed files with 135 additions and 6 deletions.
1 change: 1 addition & 0 deletions federation-js/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
- __FIX__: CSDL complex `@key`s shouldn't result in an unparseable document [PR #4490](https://github.com/apollographql/apollo-server/pull/4490)
- __FIX__: Value type validations - restrict unions, scalars, enums [PR #4496](https://github.com/apollographql/apollo-server/pull/4496)
- __FIX__: Composition - aggregate interfaces for types and interfaces in composed schema [PR #4497](https://github.com/apollographql/apollo-server/pull/4497)

## v0.19.1

Expand Down
92 changes: 87 additions & 5 deletions federation-js/src/composition/__tests__/composeAndValidate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
DocumentNode,
GraphQLScalarType,
specifiedDirectives,
printSchema,
} from 'graphql';
import {
astSerializer,
Expand Down Expand Up @@ -555,6 +556,83 @@ describe('composition of value types', () => {
`);
});
});

it('composed type implements ALL interfaces that value types implement', () => {
const serviceA = {
typeDefs: gql`
interface Node {
id: ID!
}
interface Named {
name: String
}
type Product implements Named & Node {
id: ID!
name: String
}
type Query {
node(id: ID!): Node
}
`,
name: 'serviceA',
};

const serviceB = {
typeDefs: gql`
interface Node {
id: ID!
}
type Product implements Node {
id: ID!
name: String
}
`,
name: 'serviceB',
};

const serviceC = {
typeDefs: gql`
interface Named {
name: String
}
type Product implements Named {
id: ID!
name: String
}
`,
name: 'serviceC',
};

const serviceD = {
typeDefs: gql`
type Product {
id: ID!
name: String
}
`,
name: 'serviceD',
};

const { schema, errors, composedSdl } = composeAndValidate([
serviceA,
serviceB,
serviceC,
serviceD,
]);

expect(errors).toHaveLength(0);
expect((schema.getType('Product') as GraphQLObjectType).getInterfaces())
.toHaveLength(2);

expect(printSchema(schema)).toContain('type Product implements Named & Node');
expect(composedSdl).toContain('type Product implements Named & Node');

});
});

describe('composition of schemas with directives', () => {
Expand Down Expand Up @@ -630,7 +708,7 @@ describe('composition of schemas with directives', () => {
});

it(`doesn't strip the special case @deprecated and @specifiedBy type-system directives`, () => {
const specUrl = "http://my-spec-url.com";
const specUrl = 'http://my-spec-url.com';
const deprecationReason = "Don't remove me please";

// Detecting >15.1.0 by the new addition of the `specifiedBy` directive
Expand All @@ -641,9 +719,11 @@ describe('composition of schemas with directives', () => {
typeDefs: gql`
# This directive needs to be conditionally added depending on the testing
# environment's version of graphql (>= 15.1.0 includes this new directive)
${isAtLeastGraphqlVersionFifteenPointOne
? `scalar MyScalar @specifiedBy(url: "${specUrl}")`
: ''}
${
isAtLeastGraphqlVersionFifteenPointOne
? `scalar MyScalar @specifiedBy(url: "${specUrl}")`
: ''
}
type EarthConcern {
environmental: String!
Expand Down Expand Up @@ -673,7 +753,9 @@ describe('composition of schemas with directives', () => {
const specifiedBy = schema.getDirective('specifiedBy');
expect(specifiedBy).toMatchInlineSnapshot(`"@specifiedBy"`);
const customScalar = schema.getType('MyScalar');
expect((customScalar as GraphQLScalarType).specifiedByUrl).toEqual(specUrl);
expect((customScalar as GraphQLScalarType).specifiedByUrl).toEqual(
specUrl,
);
}
});
});
Expand Down
48 changes: 47 additions & 1 deletion federation-js/src/composition/compose.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ import {
TypeDefinitionNode,
DirectiveDefinitionNode,
TypeExtensionNode,
ObjectTypeDefinitionNode,
NamedTypeNode,
} from 'graphql';
import { transformSchema } from 'apollo-graphql';
import federationDirectives from '../directives';
Expand Down Expand Up @@ -348,11 +350,55 @@ export function buildSchemaFromDefinitionsAndExtensions({
directives: [...specifiedDirectives, ...federationDirectives],
});

// This interface and predicate is a TS / graphql-js workaround for now while
// we're using a local graphql version < v15. This predicate _could_ be:
// `node is ObjectTypeDefinitionNode | InterfaceTypeDefinitionNode` in the
// future to be more semantic. However this gives us type safety and flexibility
// for now.
interface HasInterfaces {
interfaces?: ObjectTypeDefinitionNode['interfaces'];
}

function nodeHasInterfaces(node: any): node is HasInterfaces {
return 'interfaces' in node;
}

// Extend the blank schema with the base type definitions (as an AST node)
const definitionsDocument: DocumentNode = {
kind: Kind.DOCUMENT,
definitions: [
...Object.values(typeDefinitionsMap).flat(),
...Object.values(typeDefinitionsMap).flatMap(typeDefinitions => {
// See if any of our Objects or Interfaces implement any interfaces at all.
// If not, we can return early.
if (!typeDefinitions.some(nodeHasInterfaces)) return typeDefinitions;

const uniqueInterfaces: Map<
string,
NamedTypeNode
> = (typeDefinitions as HasInterfaces[]).reduce(
(map, objectTypeDef) => {
objectTypeDef.interfaces?.forEach((iface) =>
map.set(iface.name.value, iface),
);
return map;
},
new Map(),
);

// No interfaces, no aggregation - just return what we got.
if (uniqueInterfaces.size === 0) return typeDefinitions;

const [first, ...rest] = typeDefinitions;

return [
...rest,
{
...first,
interfaces: Array.from(uniqueInterfaces.values()),
},
];

}),
...Object.values(directiveDefinitionsMap).map(
definitions => Object.values(definitions)[0],
),
Expand Down

0 comments on commit 12b6241

Please sign in to comment.