Skip to content

Commit

Permalink
Allow use of legacy names for schema validation (#1194)
Browse files Browse the repository at this point in the history
This offers an escape hatch for schema that are defined with legacy field names that were valid in older versions of the spec and no longer are via an explicit white-list.

Fixes #1184
  • Loading branch information
leebyron committed Jan 8, 2018
1 parent 740b7fa commit 50d499e
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 3 deletions.
37 changes: 37 additions & 0 deletions src/type/__tests__/validation-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,43 @@ describe('Type System: Objects must have fields', () => {
},
]);
});

it('accepts an Object type with explicitly allowed legacy named fields', () => {
const schemaBad = new GraphQLSchema({
query: new GraphQLObjectType({
name: 'Query',
fields: { __badName: { type: GraphQLString } },
}),
});
const schemaOk = new GraphQLSchema({
query: new GraphQLObjectType({
name: 'Query',
fields: { __badName: { type: GraphQLString } },
}),
allowedLegacyNames: ['__badName'],
});
expect(validateSchema(schemaBad)).to.containSubset([
{
message:
'Name "__badName" must not begin with "__", which is reserved by ' +
'GraphQL introspection.',
},
]);
expect(validateSchema(schemaOk)).to.deep.equal([]);
});

it('throws with bad value for explicitly allowed legacy names', () => {
expect(
() =>
new GraphQLSchema({
query: new GraphQLObjectType({
name: 'Query',
fields: { __badName: { type: GraphQLString } },
}),
allowedLegacyNames: true,
}),
).to.throw('"allowedLegacyNames" must be Array if provided but got: true.');
});
});

describe('Type System: Fields args must be properly named', () => {
Expand Down
17 changes: 17 additions & 0 deletions src/type/schema.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ export class GraphQLSchema {
_possibleTypeMap: ?ObjMap<ObjMap<boolean>>;
// Used as a cache for validateSchema().
__validationErrors: ?$ReadOnlyArray<GraphQLError>;
// Referenced by validateSchema().
__allowedLegacyNames: ?$ReadOnlyArray<string>;

constructor(config: GraphQLSchemaConfig): void {
// If this schema was built from a source known to be valid, then it may be
Expand All @@ -103,6 +105,12 @@ export class GraphQLSchema {
'"directives" must be Array if provided but got: ' +
`${String(config.directives)}.`,
);
invariant(
!config.allowedLegacyNames || Array.isArray(config.allowedLegacyNames),
'"allowedLegacyNames" must be Array if provided but got: ' +
`${String(config.allowedLegacyNames)}.`,
);
this.__allowedLegacyNames = config.allowedLegacyNames;
}

this._queryType = config.query;
Expand Down Expand Up @@ -228,6 +236,15 @@ export type GraphQLSchemaConfig = {
directives?: ?Array<GraphQLDirective>,
astNode?: ?SchemaDefinitionNode,
assumeValid?: boolean,
/**
* If provided, the schema will consider fields or types with names included
* in this list valid, even if they do not adhere to the specification's
* schema validation rules.
*
* This option is provided to ease adoption and may be removed in a future
* major release.
*/
allowedLegacyNames?: ?Array<string>,
};

function typeMapReducer(map: TypeMap, type: ?GraphQLType): TypeMap {
Expand Down
16 changes: 13 additions & 3 deletions src/type/validate.js
Original file line number Diff line number Diff line change
Expand Up @@ -215,9 +215,17 @@ function validateName(
context: SchemaValidationContext,
node: { +name: string, +astNode: ?ASTNode },
): void {
// If a schema explicitly allows some legacy name which is no longer valid,
// allow it to be assumed valid.
if (
context.schema.__allowedLegacyNames &&
context.schema.__allowedLegacyNames.indexOf(node.name) !== -1
) {
return;
}
// Ensure names are valid, however introspection types opt out.
const error = isValidNameError(node.name, node.astNode || undefined);
if (error && !isIntrospectionType((node: any))) {
if (error) {
context.addError(error);
}
}
Expand All @@ -236,8 +244,10 @@ function validateTypes(context: SchemaValidationContext): void {
return;
}

// Ensure they are named correctly.
validateName(context, type);
// Ensure it is named correctly (excluding introspection types).
if (!isIntrospectionType(type)) {
validateName(context, type);
}

if (isObjectType(type)) {
// Ensure fields are valid
Expand Down

0 comments on commit 50d499e

Please sign in to comment.