Skip to content

Commit

Permalink
Align @external with @Shareable for how it behaves on a type (#2214)
Browse files Browse the repository at this point in the history
* Align @external with @Shareable for how it behaves on a type.

i.e. marking the type external makes all fields external.

Fixes #2156
  • Loading branch information
clenfest authored Oct 26, 2022
1 parent bd4fae9 commit 1e2dab3
Show file tree
Hide file tree
Showing 8 changed files with 211 additions and 106 deletions.
1 change: 1 addition & 0 deletions composition-js/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ This CHANGELOG pertains only to Apollo Federation packages in the 2.x range. The
## vNext

- Improves error message to help with misspelled source of an `@override` [PR #2181](https://github.com/apollographql/federation/pull/2181).
- Provide support for marking @external on object type [PR #2214](https://github.com/apollographql/federation/pull/2214)

## 2.1.2

Expand Down
177 changes: 177 additions & 0 deletions composition-js/src/__tests__/compose.external.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,177 @@
import gql from 'graphql-tag';
import './matchers';
import {
assertCompositionSuccess,
errors,
schemas,
composeAsFed2Subgraphs,
} from "./testHelper";
import {
printSchema,
} from '@apollo/federation-internals';

describe('tests related to @external', () => {
it('errors on incompatible types with @external', () => {
const subgraphA = {
name: 'subgraphA',
typeDefs: gql`
type Query {
T: T! @provides(fields: "f")
}
type T @key(fields: "id") {
id: ID!
f: String @external
}
`,
};

const subgraphB = {
name: 'subgraphB',
typeDefs: gql`
type T @key(fields: "id") {
id: ID!
f: Int @shareable
}
`,
};

const result = composeAsFed2Subgraphs([subgraphA, subgraphB]);
expect(result.errors).toBeDefined();
expect(errors(result)).toStrictEqual([
['EXTERNAL_TYPE_MISMATCH', 'Type of field "T.f" is incompatible across subgraphs (where marked @external): it has type "Int" in subgraph "subgraphB" but type "String" in subgraph "subgraphA"'],
]);
});

it('errors on missing arguments to @external declaration', () => {
const subgraphA = {
name: 'subgraphA',
typeDefs: gql`
type Query {
T: T! @provides(fields: "f")
}
type T @key(fields: "id") {
id: ID!
f: String @external
}
`,
};

const subgraphB = {
name: 'subgraphB',
typeDefs: gql`
type T @key(fields: "id") {
id: ID!
f(x: Int): String @shareable
}
`,
};

const result = composeAsFed2Subgraphs([subgraphA, subgraphB]);
expect(result.errors).toBeDefined();
expect(errors(result)).toStrictEqual([
['EXTERNAL_ARGUMENT_MISSING', 'Field "T.f" is missing argument "T.f(x:)" in some subgraphs where it is marked @external: argument "T.f(x:)" is declared in subgraph "subgraphB" but not in subgraph "subgraphA" (where "T.f" is @external).'],
]);
});

it('errors on incompatible argument types in @external declaration', () => {
const subgraphA = {
name: 'subgraphA',
typeDefs: gql`
type Query {
T: T!
}
interface I {
f(x: String): String
}
type T implements I @key(fields: "id") {
id: ID!
f(x: String): String @external
}
`,
};

const subgraphB = {
name: 'subgraphB',
typeDefs: gql`
type T @key(fields: "id") {
id: ID!
f(x: Int): String
}
`,
};

const result = composeAsFed2Subgraphs([subgraphA, subgraphB]);
expect(result.errors).toBeDefined();
expect(errors(result)).toStrictEqual([
['EXTERNAL_ARGUMENT_TYPE_MISMATCH', 'Type of argument "T.f(x:)" is incompatible across subgraphs (where "T.f" is marked @external): it has type "Int" in subgraph "subgraphB" but type "String" in subgraph "subgraphA"'],
]);
});

it('@external marked on type', () => {
const subgraphA = {
name: 'subgraphA',
typeDefs: gql`
type Query {
T: T!
}
type T @key(fields: "id") {
id: ID!
x: X @external
y: Int @requires(fields: "x { a b c d }")
}
type X @external {
a: Int
b: Int
c: Int
d: Int
}
`,
};

const subgraphB = {
name: 'subgraphB',
typeDefs: gql`
type T @key(fields: "id") {
id: ID!
x: X
}
type X {
a: Int
b: Int
c: Int
d: Int
}
`,
};

const result = composeAsFed2Subgraphs([subgraphA, subgraphB]);
assertCompositionSuccess(result);

const [_, api] = schemas(result);
expect(printSchema(api)).toMatchString(`
type Query {
T: T!
}
type T {
id: ID!
x: X
y: Int
}
type X {
a: Int
b: Int
c: Int
d: Int
}
`);
});
});
100 changes: 0 additions & 100 deletions composition-js/src/__tests__/compose.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -426,38 +426,6 @@ describe('composition', () => {
]);
});

it('errors on incompatible types with @external', () => {
const subgraphA = {
name: 'subgraphA',
typeDefs: gql`
type Query {
T: T! @provides(fields: "f")
}
type T @key(fields: "id") {
id: ID!
f: String @external
}
`,
};

const subgraphB = {
name: 'subgraphB',
typeDefs: gql`
type T @key(fields: "id") {
id: ID!
f: Int @shareable
}
`,
};

const result = composeAsFed2Subgraphs([subgraphA, subgraphB]);
expect(result.errors).toBeDefined();
expect(errors(result)).toStrictEqual([
['EXTERNAL_TYPE_MISMATCH', 'Type of field "T.f" is incompatible across subgraphs (where marked @external): it has type "Int" in subgraph "subgraphB" but type "String" in subgraph "subgraphA"'],
]);
});

it('errors on merging a list type with a non-list version', () => {
const subgraphA = {
name: 'subgraphA',
Expand Down Expand Up @@ -1043,74 +1011,6 @@ describe('composition', () => {
]);
});

it('errors on missing arguments to @external declaration', () => {
const subgraphA = {
name: 'subgraphA',
typeDefs: gql`
type Query {
T: T! @provides(fields: "f")
}
type T @key(fields: "id") {
id: ID!
f: String @external
}
`,
};

const subgraphB = {
name: 'subgraphB',
typeDefs: gql`
type T @key(fields: "id") {
id: ID!
f(x: Int): String @shareable
}
`,
};

const result = composeAsFed2Subgraphs([subgraphA, subgraphB]);
expect(result.errors).toBeDefined();
expect(errors(result)).toStrictEqual([
['EXTERNAL_ARGUMENT_MISSING', 'Field "T.f" is missing argument "T.f(x:)" in some subgraphs where it is marked @external: argument "T.f(x:)" is declared in subgraph "subgraphB" but not in subgraph "subgraphA" (where "T.f" is @external).'],
]);
});

it('errors on incompatible argument types in @external declaration', () => {
const subgraphA = {
name: 'subgraphA',
typeDefs: gql`
type Query {
T: T!
}
interface I {
f(x: String): String
}
type T implements I @key(fields: "id") {
id: ID!
f(x: String): String @external
}
`,
};

const subgraphB = {
name: 'subgraphB',
typeDefs: gql`
type T @key(fields: "id") {
id: ID!
f(x: Int): String
}
`,
};

const result = composeAsFed2Subgraphs([subgraphA, subgraphB]);
expect(result.errors).toBeDefined();
expect(errors(result)).toStrictEqual([
['EXTERNAL_ARGUMENT_TYPE_MISMATCH', 'Type of argument "T.f(x:)" is incompatible across subgraphs (where "T.f" is marked @external): it has type "Int" in subgraph "subgraphB" but type "String" in subgraph "subgraphA"'],
]);
});

it('errors on incompatible argument default', () => {
const subgraphA = {
name: 'subgraphA',
Expand Down
11 changes: 10 additions & 1 deletion docs/source/federated-types/federated-directives.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ For more information, see [Migrating entities and fields](../entities-advanced/#
### `@external`

```graphql
directive @external on FIELD_DEFINITION
directive @external on FIELD_DEFINITION | OBJECT
```

Indicates that this subgraph usually _can't_ resolve a particular object field, but it still needs to define that field for other purposes.
Expand All @@ -356,6 +356,15 @@ type Query {

This example subgraph _usually_ can't resolve the `Product.name` field, but it _can_ at the `Query.outOfStockProducts` query path (indicated by the [`@provides` directive](#provides)).

If applied to an object type definition, _all_ of that type's fields are considered `@external`:

```graphql {1}
type Position @external {
x: Int!
y: Int!
}
```


### `@provides`

Expand Down
2 changes: 1 addition & 1 deletion docs/source/subgraph-spec.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ extend type Query {
_service: _Service!
}

directive @external on FIELD_DEFINITION
directive @external on FIELD_DEFINITION | OBJECT
directive @requires(fields: FieldSet!) on FIELD_DEFINITION
directive @provides(fields: FieldSet!) on FIELD_DEFINITION
directive @key(fields: FieldSet!, resolvable: Boolean = true) repeatable on OBJECT | INTERFACE
Expand Down
1 change: 1 addition & 0 deletions internals-js/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

- Ensures supergraph `@defer`/`@stream` definitions of supergraph are not included in the API schema [PR #2212](https://github.com/apollographql/federation/pull/2212).
- Fix validation of variable on input field not taking default into account [PR #2176](https://github.com/apollographql/federation/pull/2176).
- Provide support for marking @external on object type [PR #2214](https://github.com/apollographql/federation/pull/2214)

## 2.1.0

Expand Down
14 changes: 13 additions & 1 deletion internals-js/src/federation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1642,11 +1642,13 @@ class ExternalTester {
private readonly fakeExternalFields = new Set<string>();
private readonly providedFields = new Set<string>();
private readonly externalDirective: DirectiveDefinition<{}>;
private readonly externalFieldsOnType = new Set<string>();

constructor(readonly schema: Schema) {
this.externalDirective = this.metadata().externalDirective();
this.collectFakeExternals();
this.collectProvidedFields();
this.collectExternalsOnType();
}

private metadata(): FederationMetadata {
Expand Down Expand Up @@ -1685,8 +1687,18 @@ class ExternalTester {
}
}

private collectExternalsOnType() {
for (const type of this.schema.objectTypes()) {
if (type.hasAppliedDirective(this.externalDirective)) {
for (const field of type.fields()) {
this.externalFieldsOnType.add(field.coordinate);
}
}
}
}

isExternal(field: FieldDefinition<any> | InputFieldDefinition) {
return field.hasAppliedDirective(this.externalDirective) && !this.isFakeExternal(field);
return (field.hasAppliedDirective(this.externalDirective) || this.externalFieldsOnType.has(field.coordinate)) && !this.isFakeExternal(field);
}

isFakeExternal(field: FieldDefinition<any> | InputFieldDefinition) {
Expand Down
Loading

0 comments on commit 1e2dab3

Please sign in to comment.