From 43ff3e35c699d1b6260126465b84810678d4543e Mon Sep 17 00:00:00 2001 From: Matt Krick Date: Tue, 3 Apr 2018 20:53:13 -0700 Subject: [PATCH] fix #1277 ensure interface has at least 1 concrete type (#1280) * 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 --- src/type/__tests__/schema-test.js | 19 ------------- src/type/__tests__/validation-test.js | 40 +++++++++++++++++++++++---- src/type/schema.js | 9 ++---- src/type/validate.js | 18 ++++++++++++ 4 files changed, 56 insertions(+), 30 deletions(-) diff --git a/src/type/__tests__/schema-test.js b/src/type/__tests__/schema-test.js index 1f8a5563e1..e3f14ada41 100644 --- a/src/type/__tests__/schema-test.js +++ b/src/type/__tests__/schema-test.js @@ -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: { @@ -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'); diff --git a/src/type/__tests__/validation-test.js b/src/type/__tests__/validation-test.js index 19f13a336b..0452fffd3d 100644 --- a/src/type/__tests__/validation-test.js +++ b/src/type/__tests__/validation-test.js @@ -31,9 +31,15 @@ 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({ @@ -41,11 +47,6 @@ const SomeUnionType = new GraphQLUnionType({ types: [SomeObjectType], }); -const SomeInterfaceType = new GraphQLInterfaceType({ - name: 'SomeInterface', - fields: { f: { type: GraphQLString } }, -}); - const SomeEnumType = new GraphQLEnumType({ name: 'SomeEnum', values: { @@ -772,6 +773,7 @@ describe('Type System: Object fields must have output types', () => { f: { type: BadObjectType }, }, }), + types: [SomeObjectType], }); } @@ -1032,6 +1034,14 @@ 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', @@ -1039,6 +1049,7 @@ describe('Type System: Interface fields must have output types', () => { f: { type: BadInterfaceType }, }, }), + types: [BadImplementingType, SomeObjectType], }); } @@ -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', () => { diff --git a/src/type/schema.js b/src/type/schema.js index 988493015f..4a6f917890 100644 --- a/src/type/schema.js +++ b/src/type/schema.js @@ -8,6 +8,7 @@ */ import { + isAbstractType, isObjectType, isInterfaceType, isUnionType, @@ -160,6 +161,8 @@ export class GraphQLSchema { } } }); + } else if (isAbstractType(type) && !this._implementations[type.name]) { + this._implementations[type.name] = []; } }); } @@ -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), diff --git a/src/type/validate.js b/src/type/validate.js index 812d869fc4..96a477df78 100644 --- a/src/type/validate.js +++ b/src/type/validate.js @@ -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); @@ -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,