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

Additional SourceSpecDefinition validations #2914

Merged
merged 13 commits into from
Jan 24, 2024
Merged
Show file tree
Hide file tree
Changes from 10 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
5 changes: 5 additions & 0 deletions .changeset/selfish-cooks-nail.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@apollo/federation-internals": patch
---

Additional `SourceSpecDefinition` validations and bug fixes
104 changes: 100 additions & 4 deletions composition-js/src/__tests__/compose.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4996,9 +4996,30 @@ describe('@source* directives', () => {
}
`;

// TODO Test the following errors using badSchema:
// - [x] SOURCE_FEDERATION_VERSION_REQUIRED,
// - [x] SOURCE_API_NAME_INVALID,
// - [x] SOURCE_API_PROTOCOL_INVALID,
// - [x] SOURCE_API_HTTP_BASE_URL_INVALID,
// - [x] SOURCE_HTTP_HEADERS_INVALID,
// - [x] SOURCE_TYPE_API_ERROR,
// - [x] SOURCE_TYPE_PROTOCOL_INVALID,
// - [x] SOURCE_TYPE_HTTP_METHOD_INVALID,
// - [ ] SOURCE_TYPE_HTTP_PATH_INVALID,
// - [ ] SOURCE_TYPE_HTTP_BODY_INVALID,
// - [x] SOURCE_TYPE_ON_NON_OBJECT_OR_NON_ENTITY,
// - [ ] SOURCE_TYPE_SELECTION_INVALID,
// - [x] SOURCE_FIELD_API_ERROR,
// - [ ] SOURCE_FIELD_PROTOCOL_INVALID,
// - [x] SOURCE_FIELD_HTTP_METHOD_INVALID,
// - [ ] SOURCE_FIELD_HTTP_PATH_INVALID,
// - [ ] SOURCE_FIELD_HTTP_BODY_INVALID,
// - [ ] SOURCE_FIELD_SELECTION_INVALID,
// - [x] SOURCE_FIELD_NOT_ON_ROOT_OR_ENTITY_FIELD,

const badSchema = gql`
extend schema
@link(url: "https://specs.apollo.dev/federation/v2.7", import: ["@key"])
@link(url: "https://specs.apollo.dev/federation/v2.5", import: ["@key"])
@link(url: "https://specs.apollo.dev/source/v0.1", import: [
"@sourceAPI"
"@sourceType"
Expand All @@ -5008,25 +5029,60 @@ describe('@source* directives', () => {
name: "A?!" # Should be valid GraphQL identifier
http: { baseURL: "https://api.a.com/v1" }
)
@sourceAPI(
name: "Bogus"
http: {
baseURL: "not a url"
headers: [
{ name: "i n v a l i d", value: "header value", as: "re|named" }
]
}
)
@sourceAPI(
name: "NoProtocol"
)
{
query: Query
}

type Query {
resources: [Resource!]! @sourceField(
api: "A"
http: { GET: "/resources" }
http: {
GET: "/resources"
DELETE: "/resources"
}
)
}

type Resource @key(fields: "id") @sourceType(
api: "A"
http: { GET: "/resources/{id}" }
selection: "id description"
)
@sourceType(
api: "Bogus"
http: {
GET: "/resources/{id}"
POST: "/resources"
}
selection: "id"
) {
id: ID!
description: String!
}

type NonEntity @sourceType(
api: "A"
# http: { GET: "/nonentities/{id}" }
selection: "id some_field"
) {
id: ID!
someField: String! @sourceField(
api: "A"
selection: ".some_field"
)
}
`;

it('good schema composes without validation errors', () => {
Expand All @@ -5046,7 +5102,15 @@ describe('@source* directives', () => {
const messages = result.errors!.map(e => e.message);

expect(messages).toContain(
'[bad] @sourceAPI(name: "A?!") must specify valid GraphQL name'
'[bad] Schemas that @link to https://specs.apollo.dev/source must also @link to federation version v2.7 or later (found v2.5)'
);

expect(messages).toContain(
'[bad] @sourceAPI(name: "A?!") must specify name using only [a-zA-Z0-9-_] characters'
);

expect(messages).toContain(
'[bad] @sourceAPI must specify one protocol from the set {http}'
);

expect(messages).toContain(
Expand All @@ -5056,6 +5120,38 @@ describe('@source* directives', () => {
expect(messages).toContain(
'[bad] @sourceField specifies unknown api A'
);

expect(messages).toContain(
'[bad] @sourceAPI http.baseURL \"not a url\" must be valid URL (error: Invalid URL)'
);

expect(messages).toContain(
'[bad] @sourceAPI header {\"name\":\"i n v a l i d\",\"value\":\"header value\",\"as\":\"re|named\"} specifies invalid name'
);

expect(messages).toContain(
'[bad] @sourceAPI header {\"name\":\"i n v a l i d\",\"value\":\"header value\",\"as\":\"re|named\"} specifies invalid \'as\' name'
);

expect(messages).toContain(
'[bad] @sourceType must specify exactly one of http.GET or http.POST'
);

expect(messages).toContain(
'[bad] @sourceField allows at most one of http.{GET,POST,PUT,PATCH,DELETE}'
);

expect(messages).toContain(
'[bad] @sourceType must be applied to an entity type that also has a @key directive'
);

expect(messages).toContain(
'[bad] @sourceType must specify same http argument as corresponding @sourceAPI for api A'
);

expect(messages).toContain(
'[bad] @sourceField must be applied to root Query or Mutation field or field of entity type'
);
});

const renamedSchema = gql`
Expand Down Expand Up @@ -5100,7 +5196,7 @@ describe('@source* directives', () => {
const messages = result.errors!.map(e => e.message);

expect(messages).toContain(
'[renamed] @api(name: "not an identifier") must specify valid GraphQL name'
'[renamed] @api(name: "not an identifier") must specify name using only [a-zA-Z0-9-_] characters'
);
});
});
Expand Down
1 change: 1 addition & 0 deletions docs/source/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ The following errors might be raised during composition:
| `SOURCE_API_HTTP_BASE_URL_INVALID` | The `@sourceAPI` directive must specify a valid http.baseURL | 2.7.0 | |
| `SOURCE_API_NAME_INVALID` | Each `@sourceAPI` directive must take a unique and valid name as an argument | 2.7.0 | |
| `SOURCE_API_PROTOCOL_INVALID` | Each `@sourceAPI` directive must specify exactly one of the known protocols | 2.7.0 | |
| `SOURCE_FEDERATION_VERSION_REQUIRED` | Schemas using `@source{API,Type,Field}` directives must @link-import v2.7 or later of federation | 2.7.1 | |
| `SOURCE_FIELD_API_ERROR` | The `api` argument of the `@sourceField` directive must match a valid `@sourceAPI` name | 2.7.0 | |
| `SOURCE_FIELD_HTTP_BODY_INVALID` | If `@sourceField` specifies http.body, it must be a valid `JSONSelection` matching available arguments and fields | 2.7.0 | |
| `SOURCE_FIELD_HTTP_METHOD_INVALID` | The `@sourceField` directive must specify at most one of `http.{GET,POST,PUT,PATCH,DELETE}` | 2.7.0 | |
Expand Down
7 changes: 7 additions & 0 deletions internals-js/src/error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -561,6 +561,12 @@ const INTERFACE_KEY_MISSING_IMPLEMENTATION_TYPE = makeCodeDefinition(
{ addedIn: '2.3.0' },
)

const SOURCE_FEDERATION_VERSION_REQUIRED = makeCodeDefinition(
'SOURCE_FEDERATION_VERSION_REQUIRED',
'Schemas using `@source{API,Type,Field}` directives must @link-import v2.7 or later of federation',
benjamn marked this conversation as resolved.
Show resolved Hide resolved
{ addedIn: '2.7.1' },
);

const SOURCE_API_NAME_INVALID = makeCodeDefinition(
'SOURCE_API_NAME_INVALID',
'Each `@sourceAPI` directive must take a unique and valid name as an argument',
Expand Down Expand Up @@ -757,6 +763,7 @@ export const ERRORS = {
INTERFACE_KEY_NOT_ON_IMPLEMENTATION,
INTERFACE_KEY_MISSING_IMPLEMENTATION_TYPE,
// Errors related to @sourceAPI, @sourceType, and/or @sourceField
SOURCE_FEDERATION_VERSION_REQUIRED,
SOURCE_API_NAME_INVALID,
SOURCE_API_PROTOCOL_INVALID,
SOURCE_API_HTTP_BASE_URL_INVALID,
Expand Down
Loading