Skip to content

Commit

Permalink
Let known FeatureDefinition subclasses implement `validateSubgraphS…
Browse files Browse the repository at this point in the history
…chema` method (#2910)
  • Loading branch information
benjamn authored Jan 19, 2024
1 parent 56a7816 commit 66833fb
Show file tree
Hide file tree
Showing 8 changed files with 716 additions and 6 deletions.
6 changes: 6 additions & 0 deletions .changeset/popular-mirrors-move.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@apollo/federation-internals": minor
"@apollo/composition": patch
---

Allow known `FeatureDefinition` subclasses to define custom subgraph schema validation rules
143 changes: 143 additions & 0 deletions composition-js/src/__tests__/compose.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4961,4 +4961,147 @@ describe('@source* directives', () => {
}`
)
});

describe('validation errors', () => {
const goodSchema = gql`
extend schema
@link(url: "https://specs.apollo.dev/federation/v2.7", import: ["@key"])
@link(url: "https://specs.apollo.dev/source/v0.1", import: [
"@sourceAPI"
"@sourceType"
"@sourceField"
])
@sourceAPI(
name: "A"
http: { baseURL: "https://api.a.com/v1" }
)
{
query: Query
}
type Query {
resources: [Resource!]! @sourceField(
api: "A"
http: { GET: "/resources" }
)
}
type Resource @key(fields: "id") @sourceType(
api: "A"
http: { GET: "/resources/{id}" }
selection: "id description"
) {
id: ID!
description: String!
}
`;

const badSchema = gql`
extend schema
@link(url: "https://specs.apollo.dev/federation/v2.7", import: ["@key"])
@link(url: "https://specs.apollo.dev/source/v0.1", import: [
"@sourceAPI"
"@sourceType"
"@sourceField"
])
@sourceAPI(
name: "A?!" # Should be valid GraphQL identifier
http: { baseURL: "https://api.a.com/v1" }
)
{
query: Query
}
type Query {
resources: [Resource!]! @sourceField(
api: "A"
http: { GET: "/resources" }
)
}
type Resource @key(fields: "id") @sourceType(
api: "A"
http: { GET: "/resources/{id}" }
selection: "id description"
) {
id: ID!
description: String!
}
`;

it('good schema composes without validation errors', () => {
const result = composeServices([{
name: 'good',
typeDefs: goodSchema,
}]);
expect(result.errors ?? []).toEqual([]);
});

it('bad schema composes with validation errors', () => {
const result = composeServices([{
name: 'bad',
typeDefs: badSchema,
}]);

const messages = result.errors!.map(e => e.message);

expect(messages).toContain(
'[bad] @sourceAPI(name: "A?!") must specify valid GraphQL name'
);

expect(messages).toContain(
'[bad] @sourceType specifies unknown api A'
);

expect(messages).toContain(
'[bad] @sourceField specifies unknown api A'
);
});

const renamedSchema = gql`
extend schema
@link(url: "https://specs.apollo.dev/federation/v2.7", import: ["@key"])
@link(url: "https://specs.apollo.dev/source/v0.1", import: [
{ name: "@sourceAPI", as: "@api" }
{ name: "@sourceType", as: "@type" }
{ name: "@sourceField", as: "@field" }
])
@api(
name: "not an identifier"
http: { baseURL: "https://api.a.com/v1" }
)
{
query: Query
}
type Query {
resources: [Resource!]! @field(
api: "not an identifier"
http: { GET: "/resources" }
)
}
type Resource @key(fields: "id") @type(
api: "not an identifier"
http: { GET: "/resources/{id}" }
selection: "id description"
) {
id: ID!
description: String!
}
`;

it('can handle the @source* directives being renamed', () => {
const result = composeServices([{
name: 'renamed',
typeDefs: renamedSchema,
}]);

const messages = result.errors!.map(e => e.message);

expect(messages).toContain(
'[renamed] @api(name: "not an identifier") must specify valid GraphQL name'
);
});
});
});
18 changes: 18 additions & 0 deletions docs/source/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,24 @@ The following errors might be raised during composition:
| `ROOT_SUBSCRIPTION_USED` | A subgraph's schema defines a type with the name `subscription`, while also specifying a _different_ type name as the root query object. This is not allowed. | 0.x | |
| `SATISFIABILITY_ERROR` | Subgraphs can be merged, but the resulting supergraph API would have queries that cannot be satisfied by those subgraphs. | 2.0.0 | |
| `SHAREABLE_HAS_MISMATCHED_RUNTIME_TYPES` | A shareable field return type has mismatched possible runtime types in the subgraphs in which the field is declared. As shared fields must resolve the same way in all subgraphs, this is almost surely a mistake. | 2.0.0 | |
| `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_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 | |
| `SOURCE_FIELD_HTTP_PATH_INVALID` | The `@sourceField` directive must specify a valid URL template for `http.{GET,POST,PUT,PATCH,DELETE}` | 2.7.0 | |
| `SOURCE_FIELD_NOT_ON_ROOT_OR_ENTITY_FIELD` | The `@sourceField` directive must be applied to a field of the `Query` or `Mutation` types, or of an entity type | 2.7.0 | |
| `SOURCE_FIELD_PROTOCOL_INVALID` | If `@sourceField` specifies a protocol, it must match the corresponding `@sourceAPI` protocol | 2.7.0 | |
| `SOURCE_FIELD_SELECTION_INVALID` | The `selection` argument of the `@sourceField` directive must be a valid `JSONSelection` that outputs fields of the GraphQL type | 2.7.0 | |
| `SOURCE_HTTP_HEADERS_INVALID` | The `http.headers` argument of `@source*` directives must specify valid HTTP headers | 2.7.0 | |
| `SOURCE_TYPE_API_ERROR` | The `api` argument of the `@sourceType` directive must match a valid `@sourceAPI` name | 2.7.0 | |
| `SOURCE_TYPE_HTTP_BODY_INVALID` | If the `@sourceType` specifies `http.body`, it must be a valid `JSONSelection` | 2.7.0 | |
| `SOURCE_TYPE_HTTP_METHOD_INVALID` | The `@sourceType` directive must specify exactly one of `http.GET` or `http.POST` | 2.7.0 | |
| `SOURCE_TYPE_HTTP_PATH_INVALID` | The `@sourceType` directive must specify a valid URL template for `http.GET` or `http.POST` | 2.7.0 | |
| `SOURCE_TYPE_ON_NON_OBJECT_OR_NON_ENTITY` | The `@sourceType` directive must be applied to an object or interface type that also has `@key` | 2.7.0 | |
| `SOURCE_TYPE_PROTOCOL_INVALID` | The `@sourceType` directive must specify the same protocol as its corresponding `@sourceAPI` | 2.7.0 | |
| `SOURCE_TYPE_SELECTION_INVALID` | The `selection` argument of the `@sourceType` directive must be a valid `JSONSelection` that outputs fields of the GraphQL type | 2.0.0 | |
| `TYPE_DEFINITION_INVALID` | A built-in or federation type has an invalid definition in the schema. | 2.0.0 | |
| `TYPE_KIND_MISMATCH` | A type has the same name in different subgraphs, but a different kind. For instance, one definition is an object type but another is an interface. | 2.0.0 | Replaces: `VALUE_TYPE_KIND_MISMATCH`, `EXTENSION_OF_WRONG_KIND`, `ENUM_MISMATCH_TYPE` |
| `TYPE_WITH_ONLY_UNUSED_EXTERNAL` | A federation 1 schema has a composite type comprised only of unused external fields. Note that this error can _only_ be raised for federation 1 schema as federation 2 schema do not allow unused external fields (and errors with code EXTERNAL_UNUSED will be raised in that case). But when federation 1 schema are automatically migrated to federation 2 ones, unused external fields are automatically removed, and in rare case this can leave a type empty. If that happens, an error with this code will be raised | 2.0.0 | |
Expand Down
125 changes: 125 additions & 0 deletions internals-js/src/error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -555,6 +555,112 @@ const INTERFACE_KEY_MISSING_IMPLEMENTATION_TYPE = makeCodeDefinition(
{ addedIn: '2.3.0' },
)

const SOURCE_API_NAME_INVALID = makeCodeDefinition(
'SOURCE_API_NAME_INVALID',
'Each `@sourceAPI` directive must take a unique and valid name as an argument',
{ addedIn: '2.7.0' },
);

const SOURCE_API_PROTOCOL_INVALID = makeCodeDefinition(
'SOURCE_API_PROTOCOL_INVALID',
'Each `@sourceAPI` directive must specify exactly one of the known protocols',
{ addedIn: '2.7.0' },
);

const SOURCE_API_HTTP_BASE_URL_INVALID = makeCodeDefinition(
'SOURCE_API_HTTP_BASE_URL_INVALID',
'The `@sourceAPI` directive must specify a valid http.baseURL',
{ addedIn: '2.7.0' },
);

const SOURCE_HTTP_HEADERS_INVALID = makeCodeDefinition(
'SOURCE_HTTP_HEADERS_INVALID',
'The `http.headers` argument of `@source*` directives must specify valid HTTP headers',
{ addedIn: '2.7.0' },
);

const SOURCE_TYPE_API_ERROR = makeCodeDefinition(
'SOURCE_TYPE_API_ERROR',
'The `api` argument of the `@sourceType` directive must match a valid `@sourceAPI` name',
{ addedIn: '2.7.0' },
);

const SOURCE_TYPE_PROTOCOL_INVALID = makeCodeDefinition(
'SOURCE_TYPE_PROTOCOL_INVALID',
'The `@sourceType` directive must specify the same protocol as its corresponding `@sourceAPI`',
{ addedIn: '2.7.0' },
);

const SOURCE_TYPE_HTTP_METHOD_INVALID = makeCodeDefinition(
'SOURCE_TYPE_HTTP_METHOD_INVALID',
'The `@sourceType` directive must specify exactly one of `http.GET` or `http.POST`',
{ addedIn: '2.7.0' },
);

const SOURCE_TYPE_HTTP_PATH_INVALID = makeCodeDefinition(
'SOURCE_TYPE_HTTP_PATH_INVALID',
'The `@sourceType` directive must specify a valid URL template for `http.GET` or `http.POST`',
{ addedIn: '2.7.0' },
);

const SOURCE_TYPE_HTTP_BODY_INVALID = makeCodeDefinition(
'SOURCE_TYPE_HTTP_BODY_INVALID',
'If the `@sourceType` specifies `http.body`, it must be a valid `JSONSelection`',
{ addedIn: '2.7.0' },
);

const SOURCE_TYPE_ON_NON_OBJECT_OR_NON_ENTITY = makeCodeDefinition(
'SOURCE_TYPE_ON_NON_OBJECT_OR_NON_ENTITY',
'The `@sourceType` directive must be applied to an object or interface type that also has `@key`',
{ addedIn: '2.7.0' },
);

const SOURCE_TYPE_SELECTION_INVALID = makeCodeDefinition(
'SOURCE_TYPE_SELECTION_INVALID',
'The `selection` argument of the `@sourceType` directive must be a valid `JSONSelection` that outputs fields of the GraphQL type',
);

const SOURCE_FIELD_API_ERROR = makeCodeDefinition(
'SOURCE_FIELD_API_ERROR',
'The `api` argument of the `@sourceField` directive must match a valid `@sourceAPI` name',
{ addedIn: '2.7.0' },
);

const SOURCE_FIELD_PROTOCOL_INVALID = makeCodeDefinition(
'SOURCE_FIELD_PROTOCOL_INVALID',
'If `@sourceField` specifies a protocol, it must match the corresponding `@sourceAPI` protocol',
{ addedIn: '2.7.0' },
);

const SOURCE_FIELD_HTTP_METHOD_INVALID = makeCodeDefinition(
'SOURCE_FIELD_HTTP_METHOD_INVALID',
'The `@sourceField` directive must specify at most one of `http.{GET,POST,PUT,PATCH,DELETE}`',
{ addedIn: '2.7.0' },
);

const SOURCE_FIELD_HTTP_PATH_INVALID = makeCodeDefinition(
'SOURCE_FIELD_HTTP_PATH_INVALID',
'The `@sourceField` directive must specify a valid URL template for `http.{GET,POST,PUT,PATCH,DELETE}`',
{ addedIn: '2.7.0' },
);

const SOURCE_FIELD_HTTP_BODY_INVALID = makeCodeDefinition(
'SOURCE_FIELD_HTTP_BODY_INVALID',
'If `@sourceField` specifies http.body, it must be a valid `JSONSelection` matching available arguments and fields',
{ addedIn: '2.7.0' },
);

const SOURCE_FIELD_SELECTION_INVALID = makeCodeDefinition(
'SOURCE_FIELD_SELECTION_INVALID',
'The `selection` argument of the `@sourceField` directive must be a valid `JSONSelection` that outputs fields of the GraphQL type',
{ addedIn: '2.7.0' },
);

const SOURCE_FIELD_NOT_ON_ROOT_OR_ENTITY_FIELD = makeCodeDefinition(
'SOURCE_FIELD_NOT_ON_ROOT_OR_ENTITY_FIELD',
'The `@sourceField` directive must be applied to a field of the `Query` or `Mutation` types, or of an entity type',
{ addedIn: '2.7.0' },
);

export const ERROR_CATEGORIES = {
DIRECTIVE_FIELDS_MISSING_EXTERNAL,
Expand Down Expand Up @@ -643,6 +749,25 @@ export const ERRORS = {
INTERFACE_OBJECT_USAGE_ERROR,
INTERFACE_KEY_NOT_ON_IMPLEMENTATION,
INTERFACE_KEY_MISSING_IMPLEMENTATION_TYPE,
// Errors related to @sourceAPI, @sourceType, and/or @sourceField
SOURCE_API_NAME_INVALID,
SOURCE_API_PROTOCOL_INVALID,
SOURCE_API_HTTP_BASE_URL_INVALID,
SOURCE_HTTP_HEADERS_INVALID,
SOURCE_TYPE_API_ERROR,
SOURCE_TYPE_PROTOCOL_INVALID,
SOURCE_TYPE_HTTP_METHOD_INVALID,
SOURCE_TYPE_HTTP_PATH_INVALID,
SOURCE_TYPE_HTTP_BODY_INVALID,
SOURCE_TYPE_ON_NON_OBJECT_OR_NON_ENTITY,
SOURCE_TYPE_SELECTION_INVALID,
SOURCE_FIELD_API_ERROR,
SOURCE_FIELD_PROTOCOL_INVALID,
SOURCE_FIELD_HTTP_METHOD_INVALID,
SOURCE_FIELD_HTTP_PATH_INVALID,
SOURCE_FIELD_HTTP_BODY_INVALID,
SOURCE_FIELD_SELECTION_INVALID,
SOURCE_FIELD_NOT_ON_ROOT_OR_ENTITY_FIELD,
};

const codeDefByCode = Object.values(ERRORS).reduce((obj: {[code: string]: ErrorCodeDefinition}, codeDef: ErrorCodeDefinition) => { obj[codeDef.code] = codeDef; return obj; }, {});
Expand Down
10 changes: 7 additions & 3 deletions internals-js/src/federation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ import {
import { defaultPrintOptions, PrintOptions as PrintOptions, printSchema } from "./print";
import { createObjectTypeSpecification, createScalarTypeSpecification, createUnionTypeSpecification } from "./directiveAndTypeSpecification";
import { didYouMean, suggestionList } from "./suggestions";
import { coreFeatureDefinitionIfKnown } from "./knownCoreFeatures";
import { coreFeatureDefinitionIfKnown, validateKnownFeatures } from "./knownCoreFeatures";
import { joinIdentity } from "./specs/joinSpec";
import {
SourceAPIDirectiveArgs,
Expand Down Expand Up @@ -583,8 +583,7 @@ export class FederationMetadata {
private _fieldUsedPredicate?: (field: FieldDefinition<CompositeType>) => boolean;
private _isFed2Schema?: boolean;

constructor(readonly schema: Schema) {
}
constructor(readonly schema: Schema) {}

private onInvalidate() {
this._externalTester = undefined;
Expand Down Expand Up @@ -1081,6 +1080,11 @@ export class FederationBlueprint extends SchemaBlueprint {
validateKeyOnInterfacesAreAlsoOnAllImplementations(metadata, errorCollector);
validateInterfaceObjectsAreOnEntities(metadata, errorCollector);

// FeatureDefinition objects passed to registerKnownFeature can register
// validation functions for subgraph schemas by overriding the
// validateSubgraphSchema method.
validateKnownFeatures(schema, errorCollector);

// If tag is redefined by the user, make sure the definition is compatible with what we expect
const tagDirective = metadata.tagDirective();
if (tagDirective) {
Expand Down
17 changes: 16 additions & 1 deletion internals-js/src/knownCoreFeatures.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { GraphQLError } from "graphql";
import { Schema } from "./definitions";
import { FeatureDefinition, FeatureDefinitions, FeatureUrl } from "./specs/coreSpec";

const registeredFeatures: Map<string, FeatureDefinitions> = new Map();
const registeredFeatures = new Map<string, FeatureDefinitions>();

export function registerKnownFeature(definitions: FeatureDefinitions) {
if (!registeredFeatures.has(definitions.identity)) {
Expand All @@ -12,6 +14,19 @@ export function coreFeatureDefinitionIfKnown(url: FeatureUrl): FeatureDefinition
return registeredFeatures.get(url.identity)?.find(url.version);
}

export function validateKnownFeatures(
schema: Schema,
errorCollector: GraphQLError[] = [],
): GraphQLError[] {
registeredFeatures.forEach(definitions => {
const feature = definitions.latest();
if (feature.validateSubgraphSchema !== FeatureDefinition.prototype.validateSubgraphSchema) {
errorCollector.push(...feature.validateSubgraphSchema(schema));
}
});
return errorCollector;
}

/**
* Removes a feature from the set of known features.
*
Expand Down
5 changes: 5 additions & 0 deletions internals-js/src/specs/coreSpec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,11 @@ export abstract class FeatureDefinition {
.concat(this.typeSpecs().map((spec) => spec.name));
}

// No-op implementation that can be overridden by subclasses.
validateSubgraphSchema(_schema: Schema): GraphQLError[] {
return [];
}

protected nameInSchema(schema: Schema): string | undefined {
const feature = this.featureInSchema(schema);
return feature?.nameInSchema;
Expand Down
Loading

0 comments on commit 66833fb

Please sign in to comment.