Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Allow interface type extensions #1222

Merged
merged 5 commits into from
Feb 8, 2018
Merged

Allow interface type extensions #1222

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d35badd
Allow interface type extensions
lillyfwang Feb 2, 2018
5fb61bc
Do not extend object types that implement the interface
lillyfwang Feb 6, 2018
6f1afca
Added more tests in schema validation after interface extensions
lillyfwang Feb 6, 2018
f6d7eeb
Added more test cases to extendSchema, removed some tests from validate
lillyfwang Feb 8, 2018
0b58284
Merge branch 'master' into extend-interface-types
leebyron Feb 8, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
111 changes: 111 additions & 0 deletions src/type/__tests__/validation-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -896,6 +896,117 @@ describe('Type System: Objects can only implement unique interfaces', () => {
});
});

describe('Type System: Interface extensions should be valid', () => {
it('rejects an Object implementing the extended interface due to missing field', () => {
const schema = buildSchema(`
type Query {
test: AnotherObject
}

interface AnotherInterface {
field: String
}

type AnotherObject implements AnotherInterface {
field: String
}
`);
const extendedSchema = extendSchema(
schema,
parse(`
extend interface AnotherInterface {
newField: String
}
`),
);
expect(validateSchema(extendedSchema)).to.containSubset([
{
message:
'Interface field AnotherInterface.newField expected but AnotherObject does not provide it.',
locations: [{ line: 3, column: 11 }, { line: 10, column: 7 }],
},
]);
});

it('rejects an Object implementing the extended interface due to missing field args', () => {
const schema = buildSchema(`
type Query {
test: AnotherObject
}

interface AnotherInterface {
field: String
}

type AnotherObject implements AnotherInterface {
field: String
}
`);
const extendedSchema = extendSchema(
schema,
parse(`
extend interface AnotherInterface {
newField(test: Boolean): String
}

extend type AnotherObject {
newField: String
}
`),
);
expect(validateSchema(extendedSchema)).to.containSubset([
{
message:
'Interface field argument AnotherInterface.newField(test:) expected but AnotherObject.newField does not provide it.',
locations: [{ line: 3, column: 20 }, { line: 7, column: 11 }],
},
]);
});

it('rejects Objects implementing the extended interface due to mismatching interface type', () => {
const schema = buildSchema(`
type Query {
test: AnotherObject
}

interface AnotherInterface {
field: String
}

type AnotherObject implements AnotherInterface {
field: String
}
`);
const extendedSchema = extendSchema(
schema,
parse(`
extend interface AnotherInterface {
newInterfaceField: NewInterface
}

interface NewInterface {
newField: String
}

interface MismatchingInterface {
newField: String
}

extend type AnotherObject {
newInterfaceField: MismatchingInterface
}
`),
);
expect(validateSchema(extendedSchema)).to.containSubset([
{
message:
'Interface field AnotherInterface.newInterfaceField expects type NewInterface but AnotherObject.newInterfaceField is type MismatchingInterface.',
locations: [{ line: 3, column: 30 }, { line: 15, column: 30 }],
},
]);
});
});

describe('Type System: Interface fields must have output types', () => {
function schemaWithInterfaceFieldOfType(fieldType) {
const BadInterfaceType = new GraphQLInterfaceType({
Expand Down
239 changes: 238 additions & 1 deletion src/utilities/__tests__/extendSchema-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -801,6 +801,220 @@ describe('extendSchema', () => {
`);
});

it('extends interfaces by adding new fields', () => {
const ast = parse(`
extend interface SomeInterface {
newField: String
}

extend type Bar {
newField: String
}

extend type Foo {
newField: String
}
`);
const originalPrint = printSchema(testSchema);
const extendedSchema = extendSchema(testSchema, ast);
expect(extendedSchema).to.not.equal(testSchema);
expect(printSchema(testSchema)).to.equal(originalPrint);
expect(printSchema(extendedSchema)).to.equal(dedent`
type Bar implements SomeInterface {
name: String
some: SomeInterface
foo: Foo
newField: String
}

type Biz {
fizz: String
}

type Foo implements SomeInterface {
name: String
some: SomeInterface
tree: [Foo]!
newField: String
}

type Query {
foo: Foo
someUnion: SomeUnion
someEnum: SomeEnum
someInterface(id: ID!): SomeInterface
}

enum SomeEnum {
ONE
TWO
}

interface SomeInterface {
name: String
some: SomeInterface
newField: String
}

union SomeUnion = Foo | Biz
`);
});

it('allows extension of interface with missing Object fields', () => {
const ast = parse(`
extend interface SomeInterface {
newObject: NewObject
newInterface: NewInterface
newUnion: NewUnion
newScalar: NewScalar
newTree: [Foo]!
newField(arg1: String, arg2: NewEnum!): String
}

type NewObject implements NewInterface {
baz: String
}

type NewOtherObject {
fizz: Int
}

interface NewInterface {
baz: String
}

union NewUnion = NewObject | NewOtherObject

scalar NewScalar

enum NewEnum {
OPTION_A
OPTION_B
}
`);
const originalPrint = printSchema(testSchema);
const extendedSchema = extendSchema(testSchema, ast);
expect(extendedSchema).to.not.equal(testSchema);
expect(printSchema(testSchema)).to.equal(originalPrint);
expect(printSchema(extendedSchema)).to.equal(dedent`
type Bar implements SomeInterface {
name: String
some: SomeInterface
foo: Foo
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably bug here, should fail because both Foo and Bar are missing additional fields:

 newObject: NewObject
 newInterface: NewInterface
 newUnion: NewUnion
 newScalar: NewScalar
 newTree: [Foo]!
 newField(arg1: String, arg2: NewEnum!): String

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well printSchema should not fail here: we should be able to parse, print and extend invalid schemas. Only validateSchema(extendedSchema) should fail.

But if that's what you're testing, you should be explicit "can parse and print schema extension with missing Object fields", then prove that it's invalid by testing validateSchema fails (don't test the actual error message though, as we don't wan to force people to update this test when they update that error message).

Copy link
Member

@IvanGoncharov IvanGoncharov Feb 7, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mjmahone You are right, I completely forget about that.
But I still think this particular test should contain valid SDL examples to not confuse readers.

But if that's what you're testing, you should be explicit "can parse and print schema extension with missing Object fields", then prove that it's invalid by testing validateSchema fails (don't test the actual error message though, as we don't wan to force people to update this test when they update that error message).

Totally make sense to add one such test for buildShema and extendSchema functions 👍

}

type Biz {
fizz: String
}

type Foo implements SomeInterface {
name: String
some: SomeInterface
tree: [Foo]!
}

enum NewEnum {
OPTION_A
OPTION_B
}

interface NewInterface {
baz: String
}

type NewObject implements NewInterface {
baz: String
}

type NewOtherObject {
fizz: Int
}

scalar NewScalar

union NewUnion = NewObject | NewOtherObject

type Query {
foo: Foo
someUnion: SomeUnion
someEnum: SomeEnum
someInterface(id: ID!): SomeInterface
}

enum SomeEnum {
ONE
TWO
}

interface SomeInterface {
name: String
some: SomeInterface
newObject: NewObject
newInterface: NewInterface
newUnion: NewUnion
newScalar: NewScalar
newTree: [Foo]!
newField(arg1: String, arg2: NewEnum!): String
}

union SomeUnion = Foo | Biz
`);
});

it('extends interfaces multiple times', () => {
const ast = parse(`
extend interface SomeInterface {
newFieldA: Int
}

extend interface SomeInterface {
newFieldB(test: Boolean): String
}
`);
const originalPrint = printSchema(testSchema);
const extendedSchema = extendSchema(testSchema, ast);
expect(extendedSchema).to.not.equal(testSchema);
expect(printSchema(testSchema)).to.equal(originalPrint);
expect(printSchema(extendedSchema)).to.equal(dedent`
type Bar implements SomeInterface {
name: String
some: SomeInterface
foo: Foo
}

type Biz {
fizz: String
}

type Foo implements SomeInterface {
name: String
some: SomeInterface
tree: [Foo]!
}

type Query {
foo: Foo
someUnion: SomeUnion
someEnum: SomeEnum
someInterface(id: ID!): SomeInterface
}

enum SomeEnum {
ONE
TWO
}

interface SomeInterface {
name: String
some: SomeInterface
newFieldA: Int
newFieldB(test: Boolean): String
}

union SomeUnion = Foo | Biz
`);
});

it('may extend mutations and subscriptions', () => {
const mutationSchema = new GraphQLSchema({
query: new GraphQLObjectType({
Expand Down Expand Up @@ -994,8 +1208,20 @@ describe('extendSchema', () => {
);
});

it('does not allow extending an unknown interface type', () => {
const ast = parse(`
extend interface UnknownInterfaceType {
baz: String
}
`);
expect(() => extendSchema(testSchema, ast)).to.throw(
'Cannot extend interface "UnknownInterfaceType" because it does not ' +
'exist in the existing schema.',
);
});

describe('does not allow extending a non-object type', () => {
it('not an interface', () => {
it('not an object', () => {
const ast = parse(`
extend type SomeInterface {
baz: String
Expand All @@ -1006,6 +1232,17 @@ describe('extendSchema', () => {
);
});

it('not an interface', () => {
const ast = parse(`
extend interface Foo {
baz: String
}
`);
expect(() => extendSchema(testSchema, ast)).to.throw(
'Cannot extend non-interface type "Foo".',
);
});

it('not a scalar', () => {
const ast = parse(`
extend type String {
Expand Down
Loading