Skip to content

Commit

Permalink
fix #1277 ensure interface has at least 1 concrete type (#1280)
Browse files Browse the repository at this point in the history
* fix #1277 ensure interface has at least 1 concrete type

* fix tests

* update error message

* fix lint

* remove impossible schema invariant and test

* lint

* test for unimplemented interfaces
  • Loading branch information
mattkrick authored and leebyron committed Apr 4, 2018
1 parent 6f16ba2 commit 43ff3e3
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 30 deletions.
19 changes: 0 additions & 19 deletions src/type/__tests__/schema-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,6 @@ const InterfaceType = new GraphQLInterfaceType({
fields: { fieldName: { type: GraphQLString } },
});

const ImplementingType = new GraphQLObjectType({
name: 'Object',
interfaces: [InterfaceType],
fields: { fieldName: { type: GraphQLString, resolve: () => '' } },
});

const DirectiveInputType = new GraphQLInputObjectType({
name: 'DirInput',
fields: {
Expand Down Expand Up @@ -76,19 +70,6 @@ const Schema = new GraphQLSchema({
});

describe('Type System: Schema', () => {
describe('Getting possible types', () => {
it('throws human-reable error if schema.types is not defined', () => {
const checkPossible = () => {
return Schema.isPossibleType(InterfaceType, ImplementingType);
};
expect(checkPossible).to.throw(
'Could not find possible implementing types for Interface in schema. ' +
'Check that schema.types is defined and is an array of all possible ' +
'types in the schema.',
);
});
});

describe('Type Map', () => {
it('includes input types only used in directives', () => {
expect(Schema.getTypeMap()).to.include.key('DirInput');
Expand Down
40 changes: 35 additions & 5 deletions src/type/__tests__/validation-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,21 +31,22 @@ const SomeScalarType = new GraphQLScalarType({
parseLiteral() {},
});

const SomeInterfaceType = new GraphQLInterfaceType({
name: 'SomeInterface',
fields: { f: { type: GraphQLString } },
});

const SomeObjectType = new GraphQLObjectType({
name: 'SomeObject',
fields: { f: { type: GraphQLString } },
interfaces: [SomeInterfaceType],
});

const SomeUnionType = new GraphQLUnionType({
name: 'SomeUnion',
types: [SomeObjectType],
});

const SomeInterfaceType = new GraphQLInterfaceType({
name: 'SomeInterface',
fields: { f: { type: GraphQLString } },
});

const SomeEnumType = new GraphQLEnumType({
name: 'SomeEnum',
values: {
Expand Down Expand Up @@ -772,6 +773,7 @@ describe('Type System: Object fields must have output types', () => {
f: { type: BadObjectType },
},
}),
types: [SomeObjectType],
});
}

Expand Down Expand Up @@ -1032,13 +1034,22 @@ describe('Type System: Interface fields must have output types', () => {
},
});

const BadImplementingType = new GraphQLObjectType({
name: 'BadImplementing',
interfaces: [BadInterfaceType],
fields: {
badField: { type: fieldType },
},
});

return new GraphQLSchema({
query: new GraphQLObjectType({
name: 'Query',
fields: {
f: { type: BadInterfaceType },
},
}),
types: [BadImplementingType, SomeObjectType],
});
}

Expand Down Expand Up @@ -1092,6 +1103,25 @@ describe('Type System: Interface fields must have output types', () => {
},
]);
});

it('rejects an interface not implemented by at least one object', () => {
const schema = buildSchema(`
type Query {
test: SomeInterface
}
interface SomeInterface {
foo: String
}
`);
expect(validateSchema(schema)).to.containSubset([
{
message:
'Interface SomeInterface must be implemented by at least one Object type.',
locations: [{ line: 6, column: 7 }],
},
]);
});
});

describe('Type System: Field arguments must have input types', () => {
Expand Down
9 changes: 3 additions & 6 deletions src/type/schema.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
*/

import {
isAbstractType,
isObjectType,
isInterfaceType,
isUnionType,
Expand Down Expand Up @@ -160,6 +161,8 @@ export class GraphQLSchema {
}
}
});
} else if (isAbstractType(type) && !this._implementations[type.name]) {
this._implementations[type.name] = [];
}
});
}
Expand Down Expand Up @@ -204,12 +207,6 @@ export class GraphQLSchema {

if (!possibleTypeMap[abstractType.name]) {
const possibleTypes = this.getPossibleTypes(abstractType);
invariant(
Array.isArray(possibleTypes),
`Could not find possible implementing types for ${abstractType.name} ` +
'in schema. Check that schema.types is defined and is an array of ' +
'all possible types in the schema.',
);
possibleTypeMap[abstractType.name] = possibleTypes.reduce(
(map, type) => ((map[type.name] = true), map),
Object.create(null),
Expand Down
18 changes: 18 additions & 0 deletions src/type/validate.js
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,9 @@ function validateTypes(context: SchemaValidationContext): void {
} else if (isInterfaceType(type)) {
// Ensure fields are valid.
validateFields(context, type);

// Ensure Interfaces include at least 1 Object type.
validateInterfaces(context, type);
} else if (isUnionType(type)) {
// Ensure Unions include valid member types.
validateUnionMembers(context, type);
Expand Down Expand Up @@ -364,6 +367,21 @@ function validateObjectInterfaces(
});
}

function validateInterfaces(
context: SchemaValidationContext,
iface: GraphQLInterfaceType,
): void {
const possibleTypes = context.schema.getPossibleTypes(iface);

if (possibleTypes.length === 0) {
context.reportError(
`Interface ${iface.name} must be implemented ` +
`by at least one Object type.`,
iface.astNode,
);
}
}

function validateObjectImplementsInterface(
context: SchemaValidationContext,
object: GraphQLObjectType,
Expand Down

0 comments on commit 43ff3e3

Please sign in to comment.