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

Align @external with @shareable for how it behaves on a type #2214

Merged
merged 5 commits into from
Oct 26, 2022
Merged
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 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 @@ -3,6 +3,7 @@
## vNext

- 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 @@ -1594,11 +1594,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 @@ -1637,8 +1639,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