From 257053677b8bae2d6d4a8d3d5f0859a858506445 Mon Sep 17 00:00:00 2001 From: Mike Marcacci Date: Sun, 11 Aug 2019 18:26:28 -0700 Subject: [PATCH] RFC: Allow interfaces to implement other interfaces This is an initial implementation of the RFC described by spec PR #373 https://github.com/graphql/graphql-spec/pull/373 --- src/__fixtures__/github-schema.json | 2 +- src/__fixtures__/schema-kitchen-sink.graphql | 6 + .../__tests__/union-interface-test.js | 64 ++- src/language/__tests__/schema-parser-test.js | 168 ++++++- src/language/__tests__/schema-printer-test.js | 6 + src/language/ast.js | 2 + src/language/parser.js | 15 +- src/language/printer.js | 27 +- src/language/visitor.js | 10 +- src/type/__tests__/definition-test.js | 44 ++ src/type/__tests__/introspection-test.js | 2 +- src/type/__tests__/schema-test.js | 37 +- src/type/__tests__/validation-test.js | 425 ++++++++++++++++++ src/type/definition.js | 16 +- src/type/introspection.js | 4 +- src/type/schema.js | 44 +- src/type/validate.js | 144 +++--- .../__tests__/buildASTSchema-test.js | 38 ++ .../__tests__/buildClientSchema-test.js | 30 ++ src/utilities/__tests__/extendSchema-test.js | 87 +++- .../__tests__/findBreakingChanges-test.js | 45 ++ .../__tests__/lexicographicSortSchema-test.js | 4 +- src/utilities/__tests__/schemaPrinter-test.js | 59 ++- .../__tests__/typeComparators-test.js | 27 +- src/utilities/buildASTSchema.js | 10 + src/utilities/buildClientSchema.js | 8 + src/utilities/extendSchema.js | 8 + src/utilities/findBreakingChanges.js | 26 +- src/utilities/introspectionQuery.js | 3 + src/utilities/lexicographicSortSchema.js | 1 + src/utilities/schemaPrinter.js | 6 +- src/utilities/typeComparators.js | 11 + .../FragmentsOnCompositeTypes-test.js | 10 + src/validation/__tests__/harness.js | 29 +- 34 files changed, 1299 insertions(+), 119 deletions(-) diff --git a/src/__fixtures__/github-schema.json b/src/__fixtures__/github-schema.json index 7352a87fa38..186e4864e89 100644 --- a/src/__fixtures__/github-schema.json +++ b/src/__fixtures__/github-schema.json @@ -56052,7 +56052,7 @@ }, { "name": "INTERFACE", - "description": "Indicates this type is an interface. `fields` and `possibleTypes` are valid fields.", + "description": "Indicates this type is an interface. `fields`, `interfaces`, and `possibleTypes` are valid fields.", "isDeprecated": false, "deprecationReason": null }, diff --git a/src/__fixtures__/schema-kitchen-sink.graphql b/src/__fixtures__/schema-kitchen-sink.graphql index 648f9200f66..05135d97df6 100644 --- a/src/__fixtures__/schema-kitchen-sink.graphql +++ b/src/__fixtures__/schema-kitchen-sink.graphql @@ -56,6 +56,12 @@ extend interface Bar { extend interface Bar @onInterface +interface Baz implements Bar { + one: Type + two(argument: InputType!): Type + four(argument: String = "string"): String +} + union Feed = | Story | Article diff --git a/src/execution/__tests__/union-interface-test.js b/src/execution/__tests__/union-interface-test.js index ace2a7b8d49..1fbba64f917 100644 --- a/src/execution/__tests__/union-interface-test.js +++ b/src/execution/__tests__/union-interface-test.js @@ -55,23 +55,46 @@ const NamedType = new GraphQLInterfaceType({ }, }); +const LifeType = new GraphQLInterfaceType({ + name: 'Life', + fields: () => ({ + progeny: { type: GraphQLList(LifeType) }, + }), +}); + +const MammalType = new GraphQLInterfaceType({ + name: 'Mammal', + interfaces: [LifeType], + fields: () => ({ + progeny: { type: GraphQLList(MammalType) }, + mother: { type: MammalType }, + father: { type: MammalType }, + }), +}); + const DogType = new GraphQLObjectType({ name: 'Dog', - interfaces: [NamedType], - fields: { + interfaces: [MammalType, LifeType, NamedType], + fields: () => ({ name: { type: GraphQLString }, barks: { type: GraphQLBoolean }, - }, + progeny: { type: GraphQLList(DogType) }, + mother: { type: DogType }, + father: { type: DogType }, + }), isTypeOf: value => value instanceof Dog, }); const CatType = new GraphQLObjectType({ name: 'Cat', - interfaces: [NamedType], - fields: { + interfaces: [MammalType, LifeType, NamedType], + fields: () => ({ name: { type: GraphQLString }, meows: { type: GraphQLBoolean }, - }, + progeny: { type: GraphQLList(CatType) }, + mother: { type: CatType }, + father: { type: CatType }, + }), isTypeOf: value => value instanceof Cat, }); @@ -90,12 +113,15 @@ const PetType = new GraphQLUnionType({ const PersonType = new GraphQLObjectType({ name: 'Person', - interfaces: [NamedType], - fields: { + interfaces: [NamedType, MammalType, LifeType], + fields: () => ({ name: { type: GraphQLString }, pets: { type: GraphQLList(PetType) }, friends: { type: GraphQLList(NamedType) }, - }, + progeny: { type: GraphQLList(PersonType) }, + mother: { type: PersonType }, + father: { type: PersonType }, + }), isTypeOf: value => value instanceof Person, }); @@ -122,6 +148,15 @@ describe('Execute: Union and intersection types', () => { enumValues { name } inputFields { name } } + Mammal: __type(name: "Mammal") { + kind + name + fields { name } + interfaces { name } + possibleTypes { name } + enumValues { name } + inputFields { name } + } Pet: __type(name: "Pet") { kind name @@ -140,7 +175,16 @@ describe('Execute: Union and intersection types', () => { kind: 'INTERFACE', name: 'Named', fields: [{ name: 'name' }], - interfaces: null, + interfaces: [], + possibleTypes: [{ name: 'Person' }, { name: 'Dog' }, { name: 'Cat' }], + enumValues: null, + inputFields: null, + }, + Mammal: { + kind: 'INTERFACE', + name: 'Mammal', + fields: [{ name: 'progeny' }, { name: 'mother' }, { name: 'father' }], + interfaces: [{ name: 'Life' }], possibleTypes: [{ name: 'Person' }, { name: 'Dog' }, { name: 'Cat' }], enumValues: null, inputFields: null, diff --git a/src/language/__tests__/schema-parser-test.js b/src/language/__tests__/schema-parser-test.js index fb48b7c71c8..07fb41e0baf 100644 --- a/src/language/__tests__/schema-parser-test.js +++ b/src/language/__tests__/schema-parser-test.js @@ -171,7 +171,7 @@ describe('Schema Parser', () => { }); }); - it('Extension without fields', () => { + it('Object extension without fields', () => { const doc = parse('extend type Hello implements Greeting'); expect(toJSONDeep(doc)).to.deep.equal({ @@ -190,7 +190,25 @@ describe('Schema Parser', () => { }); }); - it('Extension without fields followed by extension', () => { + it('Interface extension without fields', () => { + const doc = parse('extend interface Hello implements Greeting'); + expect(toJSONDeep(doc)).to.deep.equal({ + kind: 'Document', + definitions: [ + { + kind: 'InterfaceTypeExtension', + name: nameNode('Hello', { start: 17, end: 22 }), + interfaces: [typeNode('Greeting', { start: 34, end: 42 })], + directives: [], + fields: [], + loc: { start: 0, end: 42 }, + }, + ], + loc: { start: 0, end: 42 }, + }); + }); + + it('Object extension without fields followed by extension', () => { const doc = parse(` extend type Hello implements Greeting @@ -221,14 +239,51 @@ describe('Schema Parser', () => { }); }); - it('Extension without anything throws', () => { + it('Interface extension without fields followed by extension', () => { + const doc = parse(` + extend interface Hello implements Greeting + + extend interface Hello implements SecondGreeting + `); + expect(toJSONDeep(doc)).to.deep.equal({ + kind: 'Document', + definitions: [ + { + kind: 'InterfaceTypeExtension', + name: nameNode('Hello', { start: 24, end: 29 }), + interfaces: [typeNode('Greeting', { start: 41, end: 49 })], + directives: [], + fields: [], + loc: { start: 7, end: 49 }, + }, + { + kind: 'InterfaceTypeExtension', + name: nameNode('Hello', { start: 74, end: 79 }), + interfaces: [typeNode('SecondGreeting', { start: 91, end: 105 })], + directives: [], + fields: [], + loc: { start: 57, end: 105 }, + }, + ], + loc: { start: 0, end: 110 }, + }); + }); + + it('Object extension without anything throws', () => { expectSyntaxError('extend type Hello', 'Unexpected ', { line: 1, column: 18, }); }); - it('Extension do not include descriptions', () => { + it('Interface extension without anything throws', () => { + expectSyntaxError('extend interface Hello', 'Unexpected ', { + line: 1, + column: 23, + }); + }); + + it('Object extension do not include descriptions', () => { expectSyntaxError( ` "Description" @@ -249,6 +304,27 @@ describe('Schema Parser', () => { ); }); + it('Interface extension do not include descriptions', () => { + expectSyntaxError( + ` + "Description" + extend interface Hello { + world: String + }`, + 'Unexpected Name "extend"', + { line: 3, column: 7 }, + ); + + expectSyntaxError( + ` + extend "Description" interface Hello { + world: String + }`, + 'Unexpected String "Description"', + { line: 2, column: 14 }, + ); + }); + it('Schema extension', () => { const body = ` extend schema { @@ -341,6 +417,31 @@ describe('Schema Parser', () => { }); }); + it('Simple interface inheriting interface', () => { + const doc = parse('interface Hello implements World { field: String }'); + expect(toJSONDeep(doc)).to.deep.equal({ + kind: 'Document', + definitions: [ + { + kind: 'InterfaceTypeDefinition', + name: nameNode('Hello', { start: 10, end: 15 }), + description: undefined, + interfaces: [typeNode('World', { start: 27, end: 32 })], + directives: [], + fields: [ + fieldNode( + nameNode('field', { start: 35, end: 40 }), + typeNode('String', { start: 42, end: 48 }), + { start: 35, end: 48 }, + ), + ], + loc: { start: 0, end: 50 }, + }, + ], + loc: { start: 0, end: 50 }, + }); + }); + it('Simple type inheriting interface', () => { const doc = parse('type Hello implements World { field: String }'); @@ -396,6 +497,34 @@ describe('Schema Parser', () => { }); }); + it('Simple interface inheriting multiple interfaces', () => { + const doc = parse('interface Hello implements Wo & rld { field: String }'); + expect(toJSONDeep(doc)).to.deep.equal({ + kind: 'Document', + definitions: [ + { + kind: 'InterfaceTypeDefinition', + name: nameNode('Hello', { start: 10, end: 15 }), + description: undefined, + interfaces: [ + typeNode('Wo', { start: 27, end: 29 }), + typeNode('rld', { start: 32, end: 35 }), + ], + directives: [], + fields: [ + fieldNode( + nameNode('field', { start: 38, end: 43 }), + typeNode('String', { start: 45, end: 51 }), + { start: 38, end: 51 }, + ), + ], + loc: { start: 0, end: 53 }, + }, + ], + loc: { start: 0, end: 53 }, + }); + }); + it('Simple type inheriting multiple interfaces with leading ampersand', () => { const doc = parse('type Hello implements & Wo & rld { field: String }'); @@ -425,6 +554,36 @@ describe('Schema Parser', () => { }); }); + it('Simple interface inheriting multiple interfaces with leading ampersand', () => { + const doc = parse( + 'interface Hello implements & Wo & rld { field: String }', + ); + expect(toJSONDeep(doc)).to.deep.equal({ + kind: 'Document', + definitions: [ + { + kind: 'InterfaceTypeDefinition', + name: nameNode('Hello', { start: 10, end: 15 }), + description: undefined, + interfaces: [ + typeNode('Wo', { start: 29, end: 31 }), + typeNode('rld', { start: 34, end: 37 }), + ], + directives: [], + fields: [ + fieldNode( + nameNode('field', { start: 40, end: 45 }), + typeNode('String', { start: 47, end: 53 }), + { start: 40, end: 53 }, + ), + ], + loc: { start: 0, end: 55 }, + }, + ], + loc: { start: 0, end: 55 }, + }); + }); + it('Single value enum', () => { const doc = parse('enum Hello { WORLD }'); @@ -480,6 +639,7 @@ describe('Schema Parser', () => { kind: 'InterfaceTypeDefinition', name: nameNode('Hello', { start: 10, end: 15 }), description: undefined, + interfaces: [], directives: [], fields: [ fieldNode( diff --git a/src/language/__tests__/schema-printer-test.js b/src/language/__tests__/schema-printer-test.js index 407445f2162..776dee863a7 100644 --- a/src/language/__tests__/schema-printer-test.js +++ b/src/language/__tests__/schema-printer-test.js @@ -92,6 +92,12 @@ describe('Printer: SDL document', () => { extend interface Bar @onInterface + interface Baz implements Bar { + one: Type + two(argument: InputType!): Type + four(argument: String = "string"): String + } + union Feed = Story | Article | Advert union AnnotatedUnion @onUnion = A | B diff --git a/src/language/ast.js b/src/language/ast.js index c32a199fd79..20a6529df34 100644 --- a/src/language/ast.js +++ b/src/language/ast.js @@ -488,6 +488,7 @@ export type InterfaceTypeDefinitionNode = { +loc?: Location, +description?: StringValueNode, +name: NameNode, + +interfaces?: $ReadOnlyArray, +directives?: $ReadOnlyArray, +fields?: $ReadOnlyArray, ... @@ -589,6 +590,7 @@ export type InterfaceTypeExtensionNode = { +kind: 'InterfaceTypeExtension', +loc?: Location, +name: NameNode, + +interfaces?: $ReadOnlyArray, +directives?: $ReadOnlyArray, +fields?: $ReadOnlyArray, ... diff --git a/src/language/parser.js b/src/language/parser.js index 01d84252666..016d6e7ad50 100644 --- a/src/language/parser.js +++ b/src/language/parser.js @@ -964,12 +964,14 @@ class Parser { const description = this.parseDescription(); this.expectKeyword('interface'); const name = this.parseName(); + const interfaces = this.parseImplementsInterfaces(); const directives = this.parseDirectives(true); const fields = this.parseFieldsDefinition(); return { kind: Kind.INTERFACE_TYPE_DEFINITION, description, name, + interfaces, directives, fields, loc: this.loc(start), @@ -1215,22 +1217,29 @@ class Parser { /** * InterfaceTypeExtension : - * - extend interface Name Directives[Const]? FieldsDefinition - * - extend interface Name Directives[Const] + * - extend interface Name ImplementsInterfaces? Directives[Const]? FieldsDefinition + * - extend interface Name ImplementsInterfaces? Directives[Const] + * - extend interface Name ImplementsInterfaces */ parseInterfaceTypeExtension(): InterfaceTypeExtensionNode { const start = this._lexer.token; this.expectKeyword('extend'); this.expectKeyword('interface'); const name = this.parseName(); + const interfaces = this.parseImplementsInterfaces(); const directives = this.parseDirectives(true); const fields = this.parseFieldsDefinition(); - if (directives.length === 0 && fields.length === 0) { + if ( + interfaces.length === 0 && + directives.length === 0 && + fields.length === 0 + ) { throw this.unexpected(); } return { kind: Kind.INTERFACE_TYPE_EXTENSION, name, + interfaces, directives, fields, loc: this.loc(start), diff --git a/src/language/printer.js b/src/language/printer.js index a96ff3f3d88..c66cbc0d96a 100644 --- a/src/language/printer.js +++ b/src/language/printer.js @@ -148,8 +148,18 @@ const printDocASTReducer: any = { ), ), - InterfaceTypeDefinition: addDescription(({ name, directives, fields }) => - join(['interface', name, join(directives, ' '), block(fields)], ' '), + InterfaceTypeDefinition: addDescription( + ({ name, interfaces, directives, fields }) => + join( + [ + 'interface', + name, + wrap('implements ', join(interfaces, ' & ')), + join(directives, ' '), + block(fields), + ], + ' ', + ), ), UnionTypeDefinition: addDescription(({ name, directives, types }) => @@ -206,8 +216,17 @@ const printDocASTReducer: any = { ' ', ), - InterfaceTypeExtension: ({ name, directives, fields }) => - join(['extend interface', name, join(directives, ' '), block(fields)], ' '), + InterfaceTypeExtension: ({ name, interfaces, directives, fields }) => + join( + [ + 'extend interface', + name, + wrap('implements ', join(interfaces, ' & ')), + join(directives, ' '), + block(fields), + ], + ' ', + ), UnionTypeExtension: ({ name, directives, types }) => join( diff --git a/src/language/visitor.js b/src/language/visitor.js index 21189bc56fb..c54c2580079 100644 --- a/src/language/visitor.js +++ b/src/language/visitor.js @@ -113,7 +113,13 @@ export const QueryDocumentKeys = { 'defaultValue', 'directives', ], - InterfaceTypeDefinition: ['description', 'name', 'directives', 'fields'], + InterfaceTypeDefinition: [ + 'description', + 'name', + 'interfaces', + 'directives', + 'fields', + ], UnionTypeDefinition: ['description', 'name', 'directives', 'types'], EnumTypeDefinition: ['description', 'name', 'directives', 'values'], EnumValueDefinition: ['description', 'name', 'directives'], @@ -125,7 +131,7 @@ export const QueryDocumentKeys = { ScalarTypeExtension: ['name', 'directives'], ObjectTypeExtension: ['name', 'interfaces', 'directives', 'fields'], - InterfaceTypeExtension: ['name', 'directives', 'fields'], + InterfaceTypeExtension: ['name', 'interfaces', 'directives', 'fields'], UnionTypeExtension: ['name', 'directives', 'types'], EnumTypeExtension: ['name', 'directives', 'values'], InputObjectTypeExtension: ['name', 'directives', 'fields'], diff --git a/src/type/__tests__/definition-test.js b/src/type/__tests__/definition-test.js index 658c26a9747..3dd0f2eacb1 100644 --- a/src/type/__tests__/definition-test.js +++ b/src/type/__tests__/definition-test.js @@ -415,6 +415,50 @@ describe('Type System: Interfaces', () => { ).not.to.throw(); }); + it('accepts an Interface type with an array of interfaces', () => { + const implementing = new GraphQLInterfaceType({ + name: 'AnotherInterface', + fields: {}, + interfaces: [InterfaceType], + }); + expect(implementing.getInterfaces()).to.deep.equal([InterfaceType]); + }); + + it('accepts an Interface type with interfaces as a function returning an array', () => { + const implementing = new GraphQLInterfaceType({ + name: 'AnotherInterface', + fields: {}, + interfaces: () => [InterfaceType], + }); + expect(implementing.getInterfaces()).to.deep.equal([InterfaceType]); + }); + + it('rejects an Interface type with incorrectly typed interfaces', () => { + const objType = new GraphQLInterfaceType({ + name: 'AnotherInterface', + fields: {}, + // $DisableFlowOnNegativeTest + interfaces: {}, + }); + expect(() => objType.getInterfaces()).to.throw( + 'AnotherInterface interfaces must be an Array or a function which returns an Array.', + ); + }); + + it('rejects an Interface type with interfaces as a function returning an incorrect type', () => { + const objType = new GraphQLInterfaceType({ + name: 'AnotherInterface', + fields: {}, + // $DisableFlowOnNegativeTest + interfaces() { + return {}; + }, + }); + expect(() => objType.getInterfaces()).to.throw( + 'AnotherInterface interfaces must be an Array or a function which returns an Array.', + ); + }); + it('rejects an Interface type with an incorrect type for resolveType', () => { expect( () => diff --git a/src/type/__tests__/introspection-test.js b/src/type/__tests__/introspection-test.js index fd5473a65d8..dc16d2ae80b 100644 --- a/src/type/__tests__/introspection-test.js +++ b/src/type/__tests__/introspection-test.js @@ -1357,7 +1357,7 @@ describe('Introspection', () => { }, { description: - 'Indicates this type is an interface. `fields` and `possibleTypes` are valid fields.', + 'Indicates this type is an interface. `fields`, `interfaces`, and `possibleTypes` are valid fields.', name: 'INTERFACE', }, { diff --git a/src/type/__tests__/schema-test.js b/src/type/__tests__/schema-test.js index 31746249372..427b30bd2a1 100644 --- a/src/type/__tests__/schema-test.js +++ b/src/type/__tests__/schema-test.js @@ -157,12 +157,18 @@ describe('Type System: Schema', () => { fields: {}, }); - const SomeSubtype = new GraphQLObjectType({ - name: 'SomeSubtype', + const SomeImplementingInterface = new GraphQLInterfaceType({ + name: 'SomeImplementingInterface', fields: {}, interfaces: [SomeInterface], }); + const SomeImplementingObject = new GraphQLObjectType({ + name: 'SomeImplementingObject', + fields: {}, + interfaces: [SomeImplementingInterface, SomeInterface], + }); + const schema = new GraphQLSchema({ query: new GraphQLObjectType({ name: 'Query', @@ -170,11 +176,13 @@ describe('Type System: Schema', () => { iface: { type: SomeInterface }, }, }), - types: [SomeSubtype], + types: [SomeImplementingObject], }); expect(schema.getType('SomeInterface')).to.equal(SomeInterface); - expect(schema.getType('SomeSubtype')).to.equal(SomeSubtype); + expect(schema.getType('SomeImplementingObject')).to.equal( + SomeImplementingObject, + ); }); it("includes interfaces' thunk subtypes in the type map", () => { @@ -183,24 +191,35 @@ describe('Type System: Schema', () => { fields: {}, }); - const SomeSubtype = new GraphQLObjectType({ - name: 'SomeSubtype', + const SomeImplementingInterface = new GraphQLInterfaceType({ + name: 'SomeImplementingInterface', fields: {}, interfaces: () => [SomeInterface], }); + const SomeImplementingObject = new GraphQLObjectType({ + name: 'SomeImplementingObject', + fields: {}, + interfaces: () => [SomeInterface, SomeImplementingInterface], + }); + const schema = new GraphQLSchema({ query: new GraphQLObjectType({ name: 'Query', fields: { - iface: { type: SomeInterface }, + iface: { type: SomeImplementingInterface }, }, }), - types: [SomeSubtype], + types: [SomeImplementingObject], }); expect(schema.getType('SomeInterface')).to.equal(SomeInterface); - expect(schema.getType('SomeSubtype')).to.equal(SomeSubtype); + expect(schema.getType('SomeImplementingInterface')).to.equal( + SomeImplementingInterface, + ); + expect(schema.getType('SomeImplementingObject')).to.equal( + SomeImplementingObject, + ); }); it('includes nested input objects in the map', () => { diff --git a/src/type/__tests__/validation-test.js b/src/type/__tests__/validation-test.js index 10dcd8453ea..32c56ae7f36 100644 --- a/src/type/__tests__/validation-test.js +++ b/src/type/__tests__/validation-test.js @@ -1946,4 +1946,429 @@ describe('Objects must adhere to Interface they implement', () => { }, ]); }); + + it('rejects an Object missing a transitive interface', () => { + const schema = buildSchema(` + type Query { + test: AnotherObject + } + + interface SuperInterface { + field: String! + } + + interface AnotherInterface implements SuperInterface { + field: String! + } + + type AnotherObject implements AnotherInterface { + field: String! + } + `); + expect(validateSchema(schema)).to.deep.equal([ + { + message: + 'Object type AnotherObject must implement SuperInterface because it is implemented by AnotherInterface.', + locations: [{ line: 14, column: 37 }], + }, + ]); + }); +}); + +describe('Interfaces must adhere to Interface they implement', () => { + it('accepts an Interface which implements an Interface', () => { + const schema = buildSchema(` + type Query { + test: ChildInterface + } + + interface ParentInterface { + field(input: String): String + } + + interface ChildInterface implements ParentInterface { + field(input: String): String + } + `); + expect(validateSchema(schema)).to.deep.equal([]); + }); + + it('accepts an Interface which implements an Interface along with more fields', () => { + const schema = buildSchema(` + type Query { + test: ChildInterface + } + + interface ParentInterface { + field(input: String): String + } + + interface ChildInterface implements ParentInterface { + field(input: String): String + anotherField: String + } + `); + expect(validateSchema(schema)).to.deep.equal([]); + }); + + it('accepts an Interface which implements an Interface field along with additional optional arguments', () => { + const schema = buildSchema(` + type Query { + test: ChildInterface + } + + interface ParentInterface { + field(input: String): String + } + + interface ChildInterface implements ParentInterface { + field(input: String, anotherInput: String): String + } + `); + expect(validateSchema(schema)).to.deep.equal([]); + }); + + it('rejects an Interface missing an Interface field', () => { + const schema = buildSchema(` + type Query { + test: ChildInterface + } + + interface ParentInterface { + field(input: String): String + } + + interface ChildInterface implements ParentInterface { + anotherField: String + } + `); + expect(validateSchema(schema)).to.deep.equal([ + { + message: + 'Interface field ParentInterface.field expected but ChildInterface does not provide it.', + locations: [{ line: 7, column: 9 }, { line: 10, column: 7 }], + }, + ]); + }); + + it('rejects an Interface with an incorrectly typed Interface field', () => { + const schema = buildSchema(` + type Query { + test: ChildInterface + } + + interface ParentInterface { + field(input: String): String + } + + interface ChildInterface implements ParentInterface { + field(input: String): Int + } + `); + expect(validateSchema(schema)).to.deep.equal([ + { + message: + 'Interface field ParentInterface.field expects type String but ChildInterface.field is type Int.', + locations: [{ line: 7, column: 31 }, { line: 11, column: 31 }], + }, + ]); + }); + + it('rejects an Interface with a differently typed Interface field', () => { + const schema = buildSchema(` + type Query { + test: ChildInterface + } + + type A { foo: String } + type B { foo: String } + + interface ParentInterface { + field: A + } + + interface ChildInterface implements ParentInterface { + field: B + } + `); + expect(validateSchema(schema)).to.deep.equal([ + { + message: + 'Interface field ParentInterface.field expects type A but ChildInterface.field is type B.', + locations: [{ line: 10, column: 16 }, { line: 14, column: 16 }], + }, + ]); + }); + + it('accepts an Interface with a subtyped Interface field (interface)', () => { + const schema = buildSchema(` + type Query { + test: ChildInterface + } + + interface ParentInterface { + field: ParentInterface + } + + interface ChildInterface implements ParentInterface { + field: ChildInterface + } + `); + expect(validateSchema(schema)).to.deep.equal([]); + }); + + it('accepts an Interface with a subtyped Interface field (union)', () => { + const schema = buildSchema(` + type Query { + test: ChildInterface + } + + type SomeObject { + field: String + } + + union SomeUnionType = SomeObject + + interface ParentInterface { + field: SomeUnionType + } + + interface ChildInterface implements ParentInterface { + field: SomeObject + } + `); + expect(validateSchema(schema)).to.deep.equal([]); + }); + + it('rejects an Interface missing an Interface argument', () => { + const schema = buildSchema(` + type Query { + test: ChildInterface + } + + interface ParentInterface { + field(input: String): String + } + + interface ChildInterface implements ParentInterface { + field: String + } + `); + expect(validateSchema(schema)).to.deep.equal([ + { + message: + 'Interface field argument ParentInterface.field(input:) expected but ChildInterface.field does not provide it.', + locations: [{ line: 7, column: 15 }, { line: 11, column: 9 }], + }, + ]); + }); + + it('rejects an Interface with an incorrectly typed Interface argument', () => { + const schema = buildSchema(` + type Query { + test: ChildInterface + } + + interface ParentInterface { + field(input: String): String + } + + interface ChildInterface implements ParentInterface { + field(input: Int): String + } + `); + expect(validateSchema(schema)).to.deep.equal([ + { + message: + 'Interface field argument ParentInterface.field(input:) expects type String but ChildInterface.field(input:) is type Int.', + locations: [{ line: 7, column: 22 }, { line: 11, column: 22 }], + }, + ]); + }); + + it('rejects an Interface with both an incorrectly typed field and argument', () => { + const schema = buildSchema(` + type Query { + test: ChildInterface + } + + interface ParentInterface { + field(input: String): String + } + + interface ChildInterface implements ParentInterface { + field(input: Int): Int + } + `); + expect(validateSchema(schema)).to.deep.equal([ + { + message: + 'Interface field ParentInterface.field expects type String but ChildInterface.field is type Int.', + locations: [{ line: 7, column: 31 }, { line: 11, column: 28 }], + }, + { + message: + 'Interface field argument ParentInterface.field(input:) expects type String but ChildInterface.field(input:) is type Int.', + locations: [{ line: 7, column: 22 }, { line: 11, column: 22 }], + }, + ]); + }); + + it('rejects an Interface which implements an Interface field along with additional required arguments', () => { + const schema = buildSchema(` + type Query { + test: ChildInterface + } + + interface ParentInterface { + field(baseArg: String): String + } + + interface ChildInterface implements ParentInterface { + field( + baseArg: String, + requiredArg: String! + optionalArg1: String, + optionalArg2: String = "", + ): String + } + `); + expect(validateSchema(schema)).to.deep.equal([ + { + message: + 'Object field ChildInterface.field includes required argument requiredArg that is missing from the Interface field ParentInterface.field.', + locations: [{ line: 13, column: 11 }, { line: 7, column: 9 }], + }, + ]); + }); + + it('accepts an Interface with an equivalently wrapped Interface field type', () => { + const schema = buildSchema(` + type Query { + test: ChildInterface + } + + interface ParentInterface { + field: [String]! + } + + interface ChildInterface implements ParentInterface { + field: [String]! + } + `); + expect(validateSchema(schema)).to.deep.equal([]); + }); + + it('rejects an Interface with a non-list Interface field list type', () => { + const schema = buildSchema(` + type Query { + test: ChildInterface + } + + interface ParentInterface { + field: [String] + } + + interface ChildInterface implements ParentInterface { + field: String + } + `); + expect(validateSchema(schema)).to.deep.equal([ + { + message: + 'Interface field ParentInterface.field expects type [String] but ChildInterface.field is type String.', + locations: [{ line: 7, column: 16 }, { line: 11, column: 16 }], + }, + ]); + }); + + it('rejects an Interface with a list Interface field non-list type', () => { + const schema = buildSchema(` + type Query { + test: ChildInterface + } + + interface ParentInterface { + field: String + } + + interface ChildInterface implements ParentInterface { + field: [String] + } + `); + expect(validateSchema(schema)).to.deep.equal([ + { + message: + 'Interface field ParentInterface.field expects type String but ChildInterface.field is type [String].', + locations: [{ line: 7, column: 16 }, { line: 11, column: 16 }], + }, + ]); + }); + + it('accepts an Interface with a subset non-null Interface field type', () => { + const schema = buildSchema(` + type Query { + test: ChildInterface + } + + interface ParentInterface { + field: String + } + + interface ChildInterface implements ParentInterface { + field: String! + } + `); + expect(validateSchema(schema)).to.deep.equal([]); + }); + + it('rejects an Interface with a superset nullable Interface field type', () => { + const schema = buildSchema(` + type Query { + test: ChildInterface + } + + interface ParentInterface { + field: String! + } + + interface ChildInterface implements ParentInterface { + field: String + } + `); + expect(validateSchema(schema)).to.deep.equal([ + { + message: + 'Interface field ParentInterface.field expects type String! but ChildInterface.field is type String.', + locations: [{ line: 7, column: 16 }, { line: 11, column: 16 }], + }, + ]); + }); + + it('rejects an Object missing a transitive interface', () => { + const schema = buildSchema(` + type Query { + test: ChildInterface + } + + interface SuperInterface { + field: String! + } + + interface ParentInterface implements SuperInterface { + field: String! + } + + interface ChildInterface implements ParentInterface { + field: String! + } + `); + expect(validateSchema(schema)).to.deep.equal([ + { + message: + 'Interface type ChildInterface must implement SuperInterface because it is implemented by ParentInterface.', + locations: [{ line: 14, column: 43 }], + }, + ]); + }); }); diff --git a/src/type/definition.js b/src/type/definition.js index ef1f61cb4f3..c20b8ad3d8f 100644 --- a/src/type/definition.js +++ b/src/type/definition.js @@ -727,7 +727,9 @@ defineToStringTag(GraphQLObjectType); defineToJSON(GraphQLObjectType); function defineInterfaces( - config: GraphQLObjectTypeConfig, + config: + | GraphQLObjectTypeConfig + | GraphQLInterfaceTypeConfig, ): Array { const interfaces = resolveThunk(config.interfaces) || []; devAssert( @@ -949,6 +951,7 @@ export class GraphQLInterfaceType { resolveType: ?GraphQLTypeResolver<*, *>; _fields: Thunk>; + _interfaces: Thunk>; constructor(config: GraphQLInterfaceTypeConfig<*, *>): void { this.name = config.name; @@ -957,6 +960,7 @@ export class GraphQLInterfaceType { this.extensionASTNodes = undefineIfEmpty(config.extensionASTNodes); this.resolveType = config.resolveType; this._fields = defineFieldMap.bind(undefined, config); + this._interfaces = defineInterfaces.bind(undefined, config); devAssert(typeof config.name === 'string', 'Must provide name.'); devAssert( config.resolveType == null || typeof config.resolveType === 'function', @@ -972,14 +976,23 @@ export class GraphQLInterfaceType { return this._fields; } + getInterfaces(): Array { + if (typeof this._interfaces === 'function') { + this._interfaces = this._interfaces(); + } + return this._interfaces; + } + toConfig(): {| ...GraphQLInterfaceTypeConfig<*, *>, + interfaces: Array, fields: GraphQLFieldConfigMap<*, *>, extensionASTNodes: $ReadOnlyArray, |} { return { name: this.name, description: this.description, + interfaces: this.getInterfaces(), resolveType: this.resolveType, fields: fieldsToFieldsConfig(this.getFields()), astNode: this.astNode, @@ -998,6 +1011,7 @@ defineToJSON(GraphQLInterfaceType); export type GraphQLInterfaceTypeConfig = {| name: string, + interfaces?: Thunk>, fields: Thunk>, /** * Optionally provide a custom type resolver function. If one is not provided, diff --git a/src/type/introspection.js b/src/type/introspection.js index eff1e0c42e3..1431417327b 100644 --- a/src/type/introspection.js +++ b/src/type/introspection.js @@ -240,7 +240,7 @@ export const __Type = new GraphQLObjectType({ interfaces: { type: GraphQLList(GraphQLNonNull(__Type)), resolve(type) { - if (isObjectType(type)) { + if (isObjectType(type) || isInterfaceType(type)) { return type.getInterfaces(); } }, @@ -398,7 +398,7 @@ export const __TypeKind = new GraphQLEnumType({ INTERFACE: { value: TypeKind.INTERFACE, description: - 'Indicates this type is an interface. `fields` and `possibleTypes` are valid fields.', + 'Indicates this type is an interface. `fields`, `interfaces`, and `possibleTypes` are valid fields.', }, UNION: { value: TypeKind.UNION, diff --git a/src/type/schema.js b/src/type/schema.js index cb7c9924596..9a874f792bd 100644 --- a/src/type/schema.js +++ b/src/type/schema.js @@ -27,6 +27,7 @@ import { type GraphQLNamedType, type GraphQLAbstractType, type GraphQLObjectType, + type GraphQLInterfaceType, isAbstractType, isObjectType, isInterfaceType, @@ -123,7 +124,8 @@ export class GraphQLSchema { _subscriptionType: ?GraphQLObjectType; _directives: $ReadOnlyArray; _typeMap: TypeMap; - _implementations: ObjMap>; + _implementations: ObjMap>; + _objectImplementations: ObjMap>; _possibleTypeMap: ObjMap>; // Used as a cache for validateSchema(). __validationErrors: ?$ReadOnlyArray; @@ -190,16 +192,28 @@ export class GraphQLSchema { // Keep track of all implementations by interface name. this._implementations = Object.create(null); + this._objectImplementations = Object.create(null); for (const type of objectValues(this._typeMap)) { - if (isObjectType(type)) { + if (isObjectType(type) || isInterfaceType(type)) { for (const iface of type.getInterfaces()) { if (isInterfaceType(iface)) { + // Store implementations by objects and interfaces. const impls = this._implementations[iface.name]; if (impls) { impls.push(type); } else { this._implementations[iface.name] = [type]; } + + // Store implementations by objects only. + if (isObjectType(type)) { + const objImpls = this._objectImplementations[iface.name]; + if (objImpls) { + objImpls.push(type); + } else { + this._objectImplementations[iface.name] = [type]; + } + } } } } else if (isAbstractType(type) && !this._implementations[type.name]) { @@ -234,7 +248,24 @@ export class GraphQLSchema { if (isUnionType(abstractType)) { return abstractType.getTypes(); } - return this._implementations[abstractType.name]; + + const types = this._objectImplementations[abstractType.name]; + if (!types) { + return (this._objectImplementations[abstractType.name] = []); + } + + return types; + } + + getImplementations( + interfaceType: GraphQLInterfaceType, + ): $ReadOnlyArray { + const implementations = this._implementations[interfaceType.name]; + if (!implementations) { + return (this._implementations[interfaceType.name] = []); + } + + return implementations; } isPossibleType( @@ -254,6 +285,13 @@ export class GraphQLSchema { return Boolean(possibleTypeMap[abstractType.name][possibleType.name]); } + isImplementation( + interfaceType: GraphQLInterfaceType, + possibleType: GraphQLObjectType | GraphQLInterfaceType, + ): boolean { + return this.getImplementations(interfaceType).includes(possibleType); + } + getDirectives(): $ReadOnlyArray { return this._directives; } diff --git a/src/type/validate.js b/src/type/validate.js index c4e9b0dc6fe..2425333b9e4 100644 --- a/src/type/validate.js +++ b/src/type/validate.js @@ -241,10 +241,13 @@ function validateTypes(context: SchemaValidationContext): void { validateFields(context, type); // Ensure objects implement the interfaces they claim to. - validateObjectInterfaces(context, type); + validateInterfaces(context, type); } else if (isInterfaceType(type)) { // Ensure fields are valid. validateFields(context, type); + + // Ensure interfaces implement the interfaces they claim to. + validateInterfaces(context, type); } else if (isUnionType(type)) { // Ensure Unions include valid member types. validateUnionMembers(context, type); @@ -319,78 +322,94 @@ function validateFields( } } -function validateObjectInterfaces( +function validateInterfaces( context: SchemaValidationContext, - object: GraphQLObjectType, + implementing: GraphQLObjectType | GraphQLInterfaceType, ): void { const implementedTypeNames = Object.create(null); - for (const iface of object.getInterfaces()) { - if (!isInterfaceType(iface)) { + for (const implemented of implementing.getInterfaces()) { + if (!isInterfaceType(implemented)) { context.reportError( - `Type ${inspect(object)} must only implement Interface types, ` + - `it cannot implement ${inspect(iface)}.`, - getAllImplementsInterfaceNodes(object, iface), + `Type ${inspect(implementing)} must only implement Interface types, ` + + `it cannot implement ${inspect(implemented)}.`, + getAllImplementsInterfaceNodes(implementing, implemented), ); continue; } - if (implementedTypeNames[iface.name]) { + if (implementedTypeNames[implemented.name]) { context.reportError( - `Type ${object.name} can only implement ${iface.name} once.`, - getAllImplementsInterfaceNodes(object, iface), + `Type ${implementing.name} can only implement ${implemented.name} once.`, + getAllImplementsInterfaceNodes(implementing, implemented), ); continue; } - implementedTypeNames[iface.name] = true; - validateObjectImplementsInterface(context, object, iface); + implementedTypeNames[implemented.name] = true; + validateImplementsInterface(context, implementing, implemented); } } -function validateObjectImplementsInterface( +function validateImplementsInterface( context: SchemaValidationContext, - object: GraphQLObjectType, - iface: GraphQLInterfaceType, + implementing: GraphQLObjectType | GraphQLInterfaceType, + implemented: GraphQLInterfaceType, ): void { - const objectFieldMap = object.getFields(); - const ifaceFieldMap = iface.getFields(); + const implementingFieldMap = implementing.getFields(); + const implementedFieldMap = implemented.getFields(); + + // Assert each ancestor interface is explicitly implemented. + validateImplementsAncestors(context, implementing, implemented); // Assert each interface field is implemented. - for (const [fieldName, ifaceField] of objectEntries(ifaceFieldMap)) { - const objectField = objectFieldMap[fieldName]; + for (const [fieldName, implementedField] of objectEntries( + implementedFieldMap, + )) { + const implementingField = implementingFieldMap[fieldName]; - // Assert interface field exists on object. - if (!objectField) { + // Assert interface field exists on implementing. + if (!implementingField) { context.reportError( - `Interface field ${iface.name}.${fieldName} expected but ${object.name} does not provide it.`, - [ifaceField.astNode, ...getAllNodes(object)], + `Interface field ${implemented.name}.${fieldName} expected but ${implementing.name} does not provide it.`, + [implementedField.astNode, ...getAllNodes(implementing)], ); continue; } - // Assert interface field type is satisfied by object field type, by being + // Assert interface field type is satisfied by implementing field type, by being // a valid subtype. (covariant) - if (!isTypeSubTypeOf(context.schema, objectField.type, ifaceField.type)) { + if ( + !isTypeSubTypeOf( + context.schema, + implementingField.type, + implementedField.type, + ) + ) { context.reportError( - `Interface field ${iface.name}.${fieldName} expects type ` + - `${inspect(ifaceField.type)} but ${object.name}.${fieldName} ` + - `is type ${inspect(objectField.type)}.`, + `Interface field ${implemented.name}.${fieldName} expects type ` + + `${inspect(implementedField.type)} but ${ + implementing.name + }.${fieldName} ` + + `is type ${inspect(implementingField.type)}.`, [ - ifaceField.astNode && ifaceField.astNode.type, - objectField.astNode && objectField.astNode.type, + implementedField.astNode && implementedField.astNode.type, + implementingField.astNode && implementingField.astNode.type, ], ); } // Assert each interface field arg is implemented. - for (const ifaceArg of ifaceField.args) { - const argName = ifaceArg.name; - const objectArg = find(objectField.args, arg => arg.name === argName); + for (const implementedArg of implementedField.args) { + const argName = implementedArg.name; + const implementingArg = find( + implementingField.args, + arg => arg.name === argName, + ); // Assert interface field arg exists on object field. - if (!objectArg) { + if (!implementingArg) { context.reportError( - `Interface field argument ${iface.name}.${fieldName}(${argName}:) expected but ${object.name}.${fieldName} does not provide it.`, - [ifaceArg.astNode, objectField.astNode], + `Interface field argument ${implemented.name}.${fieldName}(${argName}:) expected but ${implementing.name}.${fieldName} does not provide it.`, + [implementedArg.astNode, implementingField.astNode], ); continue; } @@ -398,15 +417,15 @@ function validateObjectImplementsInterface( // Assert interface field arg type matches object field arg type. // (invariant) // TODO: change to contravariant? - if (!isEqualType(ifaceArg.type, objectArg.type)) { + if (!isEqualType(implementedArg.type, implementingArg.type)) { context.reportError( - `Interface field argument ${iface.name}.${fieldName}(${argName}:) ` + - `expects type ${inspect(ifaceArg.type)} but ` + - `${object.name}.${fieldName}(${argName}:) is type ` + - `${inspect(objectArg.type)}.`, + `Interface field argument ${implemented.name}.${fieldName}(${argName}:) ` + + `expects type ${inspect(implementedArg.type)} but ` + + `${implementing.name}.${fieldName}(${argName}:) is type ` + + `${inspect(implementingArg.type)}.`, [ - ifaceArg.astNode && ifaceArg.astNode.type, - objectArg.astNode && objectArg.astNode.type, + implementedArg.astNode && implementedArg.astNode.type, + implementingArg.astNode && implementingArg.astNode.type, ], ); } @@ -415,19 +434,42 @@ function validateObjectImplementsInterface( } // Assert additional arguments must not be required. - for (const objectArg of objectField.args) { - const argName = objectArg.name; - const ifaceArg = find(ifaceField.args, arg => arg.name === argName); - if (!ifaceArg && isRequiredArgument(objectArg)) { + for (const implementingArg of implementingField.args) { + const argName = implementingArg.name; + const implementedArg = find( + implementedField.args, + arg => arg.name === argName, + ); + if (!implementedArg && isRequiredArgument(implementingArg)) { context.reportError( - `Object field ${object.name}.${fieldName} includes required argument ${argName} that is missing from the Interface field ${iface.name}.${fieldName}.`, - [objectArg.astNode, ifaceField.astNode], + `Object field ${implementing.name}.${fieldName} includes required argument ${argName} that is missing from the Interface field ${implemented.name}.${fieldName}.`, + [implementingArg.astNode, implementedField.astNode], ); } } } } +function validateImplementsAncestors( + context: SchemaValidationContext, + implementing: GraphQLObjectType | GraphQLInterfaceType, + implemented: GraphQLInterfaceType, +): void { + const implementingInterfaces = implementing.getInterfaces(); + implemented.getInterfaces().forEach(transitive => { + if (!implementingInterfaces.includes(transitive)) { + context.reportError( + `${isObjectType(implementing) ? 'Object' : 'Interface'} type ${ + implementing.name + } must implement ${transitive.name} because it is implemented by ${ + implemented.name + }.`, + getAllImplementsInterfaceNodes(implementing, implemented), + ); + } + }); +} + function validateUnionMembers( context: SchemaValidationContext, union: GraphQLUnionType, @@ -594,7 +636,7 @@ function getAllSubNodes( } function getAllImplementsInterfaceNodes( - type: GraphQLObjectType, + type: GraphQLObjectType | GraphQLInterfaceType, iface: GraphQLInterfaceType, ): $ReadOnlyArray { return getAllSubNodes(type, typeNode => typeNode.interfaces).filter( diff --git a/src/utilities/__tests__/buildASTSchema-test.js b/src/utilities/__tests__/buildASTSchema-test.js index abcbd91063d..4b71535d7c6 100644 --- a/src/utilities/__tests__/buildASTSchema-test.js +++ b/src/utilities/__tests__/buildASTSchema-test.js @@ -301,6 +301,27 @@ describe('Schema Builder', () => { expect(cycleSDL(sdl)).to.equal(sdl); }); + it('Simple interface heirarchy', () => { + const sdl = dedent` + schema { + query: Child + } + + interface Child implements Parent { + str: String + } + + type Hello implements Parent & Child { + str: String + } + + interface Parent { + str: String + } + `; + expect(cycleSDL(sdl)).to.equal(sdl); + }); + it('Empty enum', () => { const sdl = dedent` enum EmptyEnum @@ -632,6 +653,23 @@ describe('Schema Builder', () => { expect(cycleSDL(sdl)).to.equal(sdl); }); + it('Unreferenced interface implementing referenced interface', () => { + const sdl = dedent` + interface Child implements Parent { + key: String + } + + interface Parent { + key: String + } + + type Query { + iface: Parent + } + `; + expect(cycleSDL(sdl)).to.equal(sdl); + }); + it('Unreferenced type implementing referenced union', () => { const sdl = dedent` type Concrete { diff --git a/src/utilities/__tests__/buildClientSchema-test.js b/src/utilities/__tests__/buildClientSchema-test.js index 3eca5702feb..a7f758bc189 100644 --- a/src/utilities/__tests__/buildClientSchema-test.js +++ b/src/utilities/__tests__/buildClientSchema-test.js @@ -211,6 +211,36 @@ describe('Type System: build schema from introspection', () => { expect(cycleIntrospection(sdl)).to.equal(sdl); }); + it('builds a schema with an interface heirarchy', () => { + const sdl = dedent` + type Dog implements Friendly & Named { + bestFriend: Friendly + name: String + } + + interface Friendly implements Named { + """The best friend of this friendly thing""" + bestFriend: Friendly + name: String + } + + type Human implements Friendly & Named { + bestFriend: Friendly + name: String + } + + interface Named { + name: String + } + + type Query { + friendly: Friendly + } + `; + + expect(cycleIntrospection(sdl)).to.equal(sdl); + }); + it('builds a schema with an implicit interface', () => { const sdl = dedent` type Dog implements Friendly { diff --git a/src/utilities/__tests__/extendSchema-test.js b/src/utilities/__tests__/extendSchema-test.js index 360cd82d34e..0b8badfb455 100644 --- a/src/utilities/__tests__/extendSchema-test.js +++ b/src/utilities/__tests__/extendSchema-test.js @@ -54,17 +54,25 @@ const SomeScalarType = new GraphQLScalarType({ name: 'SomeScalar' }); const SomeInterfaceType = new GraphQLInterfaceType({ name: 'SomeInterface', fields: () => ({ - name: { type: GraphQLString }, some: { type: SomeInterfaceType }, }), }); +const AnotherInterfaceType = new GraphQLInterfaceType({ + name: 'AnotherInterface', + interfaces: [SomeInterfaceType], + fields: () => ({ + name: { type: GraphQLString }, + some: { type: AnotherInterfaceType }, + }), +}); + const FooType = new GraphQLObjectType({ name: 'Foo', - interfaces: [SomeInterfaceType], + interfaces: [AnotherInterfaceType, SomeInterfaceType], fields: () => ({ name: { type: GraphQLString }, - some: { type: SomeInterfaceType }, + some: { type: AnotherInterfaceType }, tree: { type: GraphQLNonNull(GraphQLList(FooType)) }, }), }); @@ -73,7 +81,6 @@ const BarType = new GraphQLObjectType({ name: 'Bar', interfaces: [SomeInterfaceType], fields: () => ({ - name: { type: GraphQLString }, some: { type: SomeInterfaceType }, foo: { type: FooType }, }), @@ -259,9 +266,9 @@ describe('extendSchema', () => { } `); expect(printTestSchemaChanges(extendedSchema)).to.equal(dedent` - type Foo implements SomeInterface { + type Foo implements AnotherInterface & SomeInterface { name: String - some: SomeInterface + some: AnotherInterface tree: [Foo]! newField: String } @@ -698,9 +705,9 @@ describe('extendSchema', () => { } `); expect(printTestSchemaChanges(extendedSchema)).to.equal(dedent` - type Foo implements SomeInterface { + type Foo implements AnotherInterface & SomeInterface { name: String - some: SomeInterface + some: AnotherInterface tree: [Foo]! newField(arg1: String, arg2: NewInputObj!): String } @@ -720,9 +727,9 @@ describe('extendSchema', () => { } `); expect(printTestSchemaChanges(extendedSchema)).to.equal(dedent` - type Foo implements SomeInterface { + type Foo implements AnotherInterface & SomeInterface { name: String - some: SomeInterface + some: AnotherInterface tree: [Foo]! newField(arg1: SomeEnum!): SomeEnum } @@ -778,9 +785,9 @@ describe('extendSchema', () => { } `); expect(printTestSchemaChanges(extendedSchema)).to.equal(dedent` - type Foo implements SomeInterface { + type Foo implements AnotherInterface & SomeInterface { name: String - some: SomeInterface + some: AnotherInterface tree: [Foo]! newObject: NewObject newInterface: NewInterface @@ -824,9 +831,9 @@ describe('extendSchema', () => { } `); expect(printTestSchemaChanges(extendedSchema)).to.equal(dedent` - type Foo implements SomeInterface & NewInterface { + type Foo implements AnotherInterface & SomeInterface & NewInterface { name: String - some: SomeInterface + some: AnotherInterface tree: [Foo]! baz: String } @@ -930,6 +937,10 @@ describe('extendSchema', () => { newField: String } + extend interface AnotherInterface { + newField: String + } + extend type Bar { newField: String } @@ -939,28 +950,66 @@ describe('extendSchema', () => { } `); expect(printTestSchemaChanges(extendedSchema)).to.equal(dedent` - type Bar implements SomeInterface { + interface AnotherInterface implements SomeInterface { name: String + some: AnotherInterface + newField: String + } + + type Bar implements SomeInterface { some: SomeInterface foo: Foo newField: String } - type Foo implements SomeInterface { + type Foo implements AnotherInterface & SomeInterface { name: String - some: SomeInterface + some: AnotherInterface tree: [Foo]! newField: String } interface SomeInterface { - name: String some: SomeInterface newField: String } `); }); + it('extends interfaces by adding new implemted interfaces', () => { + const extendedSchema = extendTestSchema(` + interface NewInterface { + newField: String + } + + extend interface AnotherInterface implements NewInterface { + newField: String + } + + extend type Foo implements NewInterface { + newField: String + } + `); + expect(printTestSchemaChanges(extendedSchema)).to.equal(dedent` + interface AnotherInterface implements SomeInterface & NewInterface { + name: String + some: AnotherInterface + newField: String + } + + type Foo implements AnotherInterface & SomeInterface & NewInterface { + name: String + some: AnotherInterface + tree: [Foo]! + newField: String + } + + interface NewInterface { + newField: String + } + `); + }); + it('allows extension of interface with missing Object fields', () => { const extendedSchema = extendTestSchema(` extend interface SomeInterface { @@ -973,7 +1022,6 @@ describe('extendSchema', () => { expect(printTestSchemaChanges(extendedSchema)).to.equal(dedent` interface SomeInterface { - name: String some: SomeInterface newField: String } @@ -992,7 +1040,6 @@ describe('extendSchema', () => { `); expect(printTestSchemaChanges(extendedSchema)).to.equal(dedent` interface SomeInterface { - name: String some: SomeInterface newFieldA: Int newFieldB(test: Boolean): String diff --git a/src/utilities/__tests__/findBreakingChanges-test.js b/src/utilities/__tests__/findBreakingChanges-test.js index 56bfc056611..828821d5677 100644 --- a/src/utilities/__tests__/findBreakingChanges-test.js +++ b/src/utilities/__tests__/findBreakingChanges-test.js @@ -572,6 +572,27 @@ describe('findBreakingChanges', () => { ]); }); + it('should detect interfaces removed from types', () => { + const oldSchema = buildSchema(` + interface Interface1 + + interface Interface2 implements Interface1 + `); + + const newSchema = buildSchema(` + interface Interface1 + + interface Interface2 + `); + + expect(findBreakingChanges(oldSchema, newSchema)).to.deep.equal([ + { + type: BreakingChangeType.INTERFACE_REMOVED_FROM_OBJECT, + description: 'Interface2 no longer implements interface Interface1.', + }, + ]); + }); + it('should ignore changes in order of interfaces', () => { const oldSchema = buildSchema(` interface FirstInterface @@ -968,6 +989,30 @@ describe('findDangerousChanges', () => { ]); }); + it('should detect interfaces added to interfaces', () => { + const oldSchema = buildSchema(` + interface OldInterface + interface NewInterface + + interface Interface1 implements OldInterface + `); + + const newSchema = buildSchema(` + interface OldInterface + interface NewInterface + + interface Interface1 implements OldInterface & NewInterface + `); + + expect(findDangerousChanges(oldSchema, newSchema)).to.deep.equal([ + { + type: DangerousChangeType.INTERFACE_ADDED_TO_OBJECT, + description: + 'NewInterface added to interfaces implemented by Interface1.', + }, + ]); + }); + it('should detect if a type was added to a union type', () => { const oldSchema = buildSchema(` type Type1 diff --git a/src/utilities/__tests__/lexicographicSortSchema-test.js b/src/utilities/__tests__/lexicographicSortSchema-test.js index 01839ce88be..705428a5c53 100644 --- a/src/utilities/__tests__/lexicographicSortSchema-test.js +++ b/src/utilities/__tests__/lexicographicSortSchema-test.js @@ -75,7 +75,7 @@ describe('lexicographicSortSchema', () => { dummy: String } - interface FooC { + interface FooC implements FooB & FooA { dummy: String } @@ -93,7 +93,7 @@ describe('lexicographicSortSchema', () => { dummy: String } - interface FooC { + interface FooC implements FooA & FooB { dummy: String } diff --git a/src/utilities/__tests__/schemaPrinter-test.js b/src/utilities/__tests__/schemaPrinter-test.js index 850e47f94f3..17578e9581d 100644 --- a/src/utilities/__tests__/schemaPrinter-test.js +++ b/src/utilities/__tests__/schemaPrinter-test.js @@ -340,6 +340,61 @@ describe('Type System Printer', () => { `); }); + it('Print Hierarchical Interface', () => { + const FooType = new GraphQLInterfaceType({ + name: 'Foo', + fields: { str: { type: GraphQLString } }, + }); + + const BaazType = new GraphQLInterfaceType({ + name: 'Baaz', + interfaces: [FooType], + fields: { + int: { type: GraphQLInt }, + str: { type: GraphQLString }, + }, + }); + + const BarType = new GraphQLObjectType({ + name: 'Bar', + fields: { + str: { type: GraphQLString }, + int: { type: GraphQLInt }, + }, + interfaces: [FooType, BaazType], + }); + + const Query = new GraphQLObjectType({ + name: 'Query', + fields: { bar: { type: BarType } }, + }); + + const Schema = new GraphQLSchema({ + query: Query, + types: [BarType], + }); + const output = printForTest(Schema); + expect(output).to.equal(dedent` + interface Baaz implements Foo { + int: Int + str: String + } + + type Bar implements Foo & Baaz { + str: String + int: Int + } + + interface Foo { + str: String + } + + type Query { + bar: Bar + } + `); + }); + it('Print Unions', () => { const FooType = new GraphQLObjectType({ name: 'Foo', @@ -709,7 +764,7 @@ describe('Type System Printer', () => { OBJECT """ - Indicates this type is an interface. \`fields\` and \`possibleTypes\` are valid fields. + Indicates this type is an interface. \`fields\`, \`interfaces\`, and \`possibleTypes\` are valid fields. """ INTERFACE @@ -915,7 +970,7 @@ describe('Type System Printer', () => { # Indicates this type is an object. \`fields\` and \`interfaces\` are valid fields. OBJECT - # Indicates this type is an interface. \`fields\` and \`possibleTypes\` are valid fields. + # Indicates this type is an interface. \`fields\`, \`interfaces\`, and \`possibleTypes\` are valid fields. INTERFACE # Indicates this type is a union. \`possibleTypes\` is a valid field. diff --git a/src/utilities/__tests__/typeComparators-test.js b/src/utilities/__tests__/typeComparators-test.js index 12b19f58e21..f2123c576b2 100644 --- a/src/utilities/__tests__/typeComparators-test.js +++ b/src/utilities/__tests__/typeComparators-test.js @@ -110,7 +110,7 @@ describe('typeComparators', () => { expect(isTypeSubTypeOf(schema, member, union)).to.equal(true); }); - it('implementation is subtype of interface', () => { + it('implementing object is subtype of interface', () => { const iface = new GraphQLInterfaceType({ name: 'Interface', fields: { @@ -127,5 +127,30 @@ describe('typeComparators', () => { const schema = testSchema({ field: { type: impl } }); expect(isTypeSubTypeOf(schema, impl, iface)).to.equal(true); }); + + it('implementing interface is subtype of interface', () => { + const iface = new GraphQLInterfaceType({ + name: 'Interface', + fields: { + field: { type: GraphQLString }, + }, + }); + const iface2 = new GraphQLInterfaceType({ + name: 'Interface2', + interfaces: [iface], + fields: { + field: { type: GraphQLString }, + }, + }); + const impl = new GraphQLObjectType({ + name: 'Object', + interfaces: [iface2, iface], + fields: { + field: { type: GraphQLString }, + }, + }); + const schema = testSchema({ field: { type: impl } }); + expect(isTypeSubTypeOf(schema, iface2, iface)).to.equal(true); + }); }); }); diff --git a/src/utilities/buildASTSchema.js b/src/utilities/buildASTSchema.js index c3d4b350d80..5931e20440c 100644 --- a/src/utilities/buildASTSchema.js +++ b/src/utilities/buildASTSchema.js @@ -347,8 +347,17 @@ export class ASTDefinitionBuilder { } _makeInterfaceDef(astNode: InterfaceTypeDefinitionNode) { + const interfaceNodes = astNode.interfaces; const fieldNodes = astNode.fields; + // Note: While this could make assertions to get the correctly typed + // values below, that would throw immediately while type system + // validation with validateSchema() will produce more actionable results. + const interfaces = + interfaceNodes && interfaceNodes.length > 0 + ? () => interfaceNodes.map(ref => (this.getNamedType(ref): any)) + : []; + const fields = fieldNodes && fieldNodes.length > 0 ? () => keyByNameNode(fieldNodes, field => this.buildField(field)) @@ -357,6 +366,7 @@ export class ASTDefinitionBuilder { return new GraphQLInterfaceType({ name: astNode.name.value, description: getDescription(astNode, this._options), + interfaces, fields, astNode, }); diff --git a/src/utilities/buildClientSchema.js b/src/utilities/buildClientSchema.js index 6796f8ffd2a..26c088335a4 100644 --- a/src/utilities/buildClientSchema.js +++ b/src/utilities/buildClientSchema.js @@ -253,9 +253,17 @@ export function buildClientSchema( function buildInterfaceDef( interfaceIntrospection: IntrospectionInterfaceType, ): GraphQLInterfaceType { + if (!interfaceIntrospection.interfaces) { + throw new Error( + 'Introspection result missing interfaces: ' + + inspect(interfaceIntrospection), + ); + } + return new GraphQLInterfaceType({ name: interfaceIntrospection.name, description: interfaceIntrospection.description, + interfaces: () => interfaceIntrospection.interfaces.map(getInterfaceType), fields: () => buildFieldDefMap(interfaceIntrospection), }); } diff --git a/src/utilities/extendSchema.js b/src/utilities/extendSchema.js index 4d1778e87c6..8b86b29ff04 100644 --- a/src/utilities/extendSchema.js +++ b/src/utilities/extendSchema.js @@ -356,10 +356,18 @@ export function extendSchema( ): GraphQLInterfaceType { const config = type.toConfig(); const extensions = typeExtsMap[config.name] || []; + const interfaceNodes = flatMap(extensions, node => node.interfaces || []); const fieldNodes = flatMap(extensions, node => node.fields || []); return new GraphQLInterfaceType({ ...config, + interfaces: () => [ + ...type.getInterfaces().map(replaceNamedType), + // Note: While this could make early assertions to get the correctly + // typed values, that would throw immediately while type system + // validation with validateSchema() will produce more actionable results. + ...interfaceNodes.map(node => (astBuilder.getNamedType(node): any)), + ], fields: () => ({ ...mapValue(config.fields, extendField), ...keyValMap( diff --git a/src/utilities/findBreakingChanges.js b/src/utilities/findBreakingChanges.js index 3d48314c509..3f1c34d5f1a 100644 --- a/src/utilities/findBreakingChanges.js +++ b/src/utilities/findBreakingChanges.js @@ -189,7 +189,7 @@ function findTypeChanges( } else if (isObjectType(oldType) && isObjectType(newType)) { schemaChanges.push(...findObjectTypeChanges(oldType, newType)); } else if (isInterfaceType(oldType) && isInterfaceType(newType)) { - schemaChanges.push(...findFieldChanges(oldType, newType)); + schemaChanges.push(...findInterfaceTypeChanges(oldType, newType)); } else if (oldType.constructor !== newType.constructor) { schemaChanges.push({ type: BreakingChangeType.TYPE_CHANGED_KIND, @@ -324,6 +324,30 @@ function findObjectTypeChanges( return schemaChanges; } +function findInterfaceTypeChanges( + oldType: GraphQLInterfaceType, + newType: GraphQLInterfaceType, +): Array { + const schemaChanges = findFieldChanges(oldType, newType); + const interfacesDiff = diff(oldType.getInterfaces(), newType.getInterfaces()); + + for (const newInterface of interfacesDiff.added) { + schemaChanges.push({ + type: DangerousChangeType.INTERFACE_ADDED_TO_OBJECT, + description: `${newInterface.name} added to interfaces implemented by ${oldType.name}.`, + }); + } + + for (const oldInterface of interfacesDiff.removed) { + schemaChanges.push({ + type: BreakingChangeType.INTERFACE_REMOVED_FROM_OBJECT, + description: `${oldType.name} no longer implements interface ${oldInterface.name}.`, + }); + } + + return schemaChanges; +} + function findFieldChanges( oldType: GraphQLObjectType | GraphQLInterfaceType, newType: GraphQLObjectType | GraphQLInterfaceType, diff --git a/src/utilities/introspectionQuery.js b/src/utilities/introspectionQuery.js index ca55e8a1f13..4895acb237a 100644 --- a/src/utilities/introspectionQuery.js +++ b/src/utilities/introspectionQuery.js @@ -167,6 +167,9 @@ export type IntrospectionInterfaceType = { +name: string, +description?: ?string, +fields: $ReadOnlyArray, + +interfaces: $ReadOnlyArray< + IntrospectionNamedTypeRef, + >, +possibleTypes: $ReadOnlyArray< IntrospectionNamedTypeRef, >, diff --git a/src/utilities/lexicographicSortSchema.js b/src/utilities/lexicographicSortSchema.js index 16723a86ca6..c6a18cb52c0 100644 --- a/src/utilities/lexicographicSortSchema.js +++ b/src/utilities/lexicographicSortSchema.js @@ -115,6 +115,7 @@ export function lexicographicSortSchema(schema: GraphQLSchema): GraphQLSchema { const config = type.toConfig(); return new GraphQLInterfaceType({ ...config, + interfaces: () => sortTypes(config.interfaces), fields: () => sortFields(config.fields), }); } else if (isUnionType(type)) { diff --git a/src/utilities/schemaPrinter.js b/src/utilities/schemaPrinter.js index 3f35e96623c..56a198cad00 100644 --- a/src/utilities/schemaPrinter.js +++ b/src/utilities/schemaPrinter.js @@ -194,9 +194,13 @@ function printObject(type: GraphQLObjectType, options): string { } function printInterface(type: GraphQLInterfaceType, options): string { + const interfaces = type.getInterfaces(); + const implementedInterfaces = interfaces.length + ? ' implements ' + interfaces.map(i => i.name).join(' & ') + : ''; return ( printDescription(options, type) + - `interface ${type.name}` + + `interface ${type.name}${implementedInterfaces}` + printFields(options, type) ); } diff --git a/src/utilities/typeComparators.js b/src/utilities/typeComparators.js index 9b70895bf04..c58cb73c69a 100644 --- a/src/utilities/typeComparators.js +++ b/src/utilities/typeComparators.js @@ -4,6 +4,7 @@ import { type GraphQLSchema } from '../type/schema'; import { type GraphQLType, type GraphQLCompositeType, + isInterfaceType, isObjectType, isListType, isNonNullType, @@ -71,6 +72,16 @@ export function isTypeSubTypeOf( return false; } + // If superType and maybeSubType are both interfaces, check if the latter + // implements the former. + if ( + isInterfaceType(superType) && + isInterfaceType(maybeSubType) && + schema.isImplementation(superType, maybeSubType) + ) { + return true; + } + // If superType type is an abstract type, maybeSubType type may be a currently // possible object type. if ( diff --git a/src/validation/__tests__/FragmentsOnCompositeTypes-test.js b/src/validation/__tests__/FragmentsOnCompositeTypes-test.js index 76eef0a8af8..d990bda29f3 100644 --- a/src/validation/__tests__/FragmentsOnCompositeTypes-test.js +++ b/src/validation/__tests__/FragmentsOnCompositeTypes-test.js @@ -52,6 +52,16 @@ describe('Validate: Fragments on composite types', () => { `); }); + it('interface is valid inline fragment type', () => { + expectValid(` + fragment validFragment on Mammal { + ... on Canine { + name + } + } + `); + }); + it('inline fragment without type is valid', () => { expectValid(` fragment validFragment on Pet { diff --git a/src/validation/__tests__/harness.js b/src/validation/__tests__/harness.js index c4f313504de..96561c19793 100644 --- a/src/validation/__tests__/harness.js +++ b/src/validation/__tests__/harness.js @@ -46,8 +46,22 @@ const Being = new GraphQLInterfaceType({ }), }); +const Mammal = new GraphQLInterfaceType({ + name: 'Mammal', + interfaces: [], + fields: () => ({ + mother: { + type: Mammal, + }, + father: { + type: Mammal, + }, + }), +}); + const Pet = new GraphQLInterfaceType({ name: 'Pet', + interfaces: [Being], fields: () => ({ name: { type: GraphQLString, @@ -58,11 +72,18 @@ const Pet = new GraphQLInterfaceType({ const Canine = new GraphQLInterfaceType({ name: 'Canine', + interfaces: [Mammal, Being], fields: () => ({ name: { type: GraphQLString, args: { surname: { type: GraphQLBoolean } }, }, + mother: { + type: Canine, + }, + father: { + type: Canine, + }, }), }); @@ -77,6 +98,7 @@ const DogCommand = new GraphQLEnumType({ const Dog = new GraphQLObjectType({ name: 'Dog', + interfaces: [Being, Pet, Mammal, Canine], fields: () => ({ name: { type: GraphQLString, @@ -104,8 +126,13 @@ const Dog = new GraphQLObjectType({ type: GraphQLBoolean, args: { x: { type: GraphQLInt }, y: { type: GraphQLInt } }, }, + mother: { + type: Dog, + }, + father: { + type: Dog, + }, }), - interfaces: [Being, Pet, Canine], }); const Cat = new GraphQLObjectType({