Skip to content

Commit

Permalink
Spec compliance: Validation error on multi-field subscription (#882)
Browse files Browse the repository at this point in the history
As per the spec, a subscription should simply use the first field defined, and not make any other assertions during the Subscribe operation. Instead, a validation rule should detect this and report it.
  • Loading branch information
leebyron committed May 26, 2017
1 parent 0fe2972 commit 1c4477c
Show file tree
Hide file tree
Showing 7 changed files with 156 additions and 14 deletions.
1 change: 1 addition & 0 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,7 @@ export {
PossibleFragmentSpreadsRule,
ProvidedNonNullArgumentsRule,
ScalarLeafsRule,
SingleFieldSubscriptionsRule,
UniqueArgumentNamesRule,
UniqueDirectivesPerLocationRule,
UniqueFragmentNamesRule,
Expand Down
36 changes: 26 additions & 10 deletions src/subscription/__tests__/subscribe-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -211,12 +211,27 @@ describe('Subscribe', () => {
}).not.to.throw();
});

it('should throw when querying for multiple fields', async () => {
it('should only resolve the first field of invalid multi-field', async () => {
let didResolveImportantEmail = false;
let didResolveNonImportantEmail = false;

const SubscriptionTypeMultiple = new GraphQLObjectType({
name: 'Subscription',
fields: {
importantEmail: { type: EmailEventType },
nonImportantEmail: { type: EmailEventType },
importantEmail: {
type: EmailEventType,
subscribe() {
didResolveImportantEmail = true;
return eventEmitterAsyncIterator(new EventEmitter(), 'event');
}
},
nonImportantEmail: {
type: EmailEventType,
subscribe() {
didResolveNonImportantEmail = true;
return eventEmitterAsyncIterator(new EventEmitter(), 'event');
}
},
}
});

Expand All @@ -232,13 +247,14 @@ describe('Subscribe', () => {
}
`);

expect(() => {
subscribe(
testSchema,
ast
);
}).to.throw(
'A subscription operation must contain exactly one root field.');
const subscription = subscribe(testSchema, ast);
subscription.next(); // Ask for a result, but ignore it.

expect(didResolveImportantEmail).to.equal(true);
expect(didResolveNonImportantEmail).to.equal(false);

// Close subscription
subscription.return();
});

it('produces payload for multiple subscribe in same subscription',
Expand Down
4 changes: 0 additions & 4 deletions src/subscription/subscribe.js
Original file line number Diff line number Diff line change
Expand Up @@ -158,10 +158,6 @@ export function createSourceEventStream(
Object.create(null)
);
const responseNames = Object.keys(fields);
invariant(
responseNames.length === 1,
'A subscription operation must contain exactly one root field.'
);
const responseName = responseNames[0];
const fieldNodes = fields[responseName];
const fieldNode = fieldNodes[0];
Expand Down
81 changes: 81 additions & 0 deletions src/validation/__tests__/SingleFieldSubscriptions-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
/**
* Copyright (c) 2015, Facebook, Inc.
* All rights reserved.
*
* This source code is licensed under the BSD-style license found in the
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*/

import { describe, it } from 'mocha';
import { expectPassesRule, expectFailsRule } from './harness';
import {
SingleFieldSubscriptions,
singleFieldOnlyMessage,
} from '../rules/SingleFieldSubscriptions';


describe('Validate: Subscriptions with single field', () => {

it('valid subscription', () => {
expectPassesRule(SingleFieldSubscriptions, `
subscription ImportantEmails {
importantEmails
}
`);
});

it('fails with more than one root field', () => {
expectFailsRule(SingleFieldSubscriptions, `
subscription ImportantEmails {
importantEmails
notImportantEmails
}
`, [ {
message: singleFieldOnlyMessage('ImportantEmails'),
locations: [ { line: 4, column: 9 } ],
path: undefined,
} ]);
});

it('fails with more than one root field including introspection', () => {
expectFailsRule(SingleFieldSubscriptions, `
subscription ImportantEmails {
importantEmails
__typename
}
`, [ {
message: singleFieldOnlyMessage('ImportantEmails'),
locations: [ { line: 4, column: 9 } ],
path: undefined,
} ]);
});

it('fails with many more than one root field', () => {
expectFailsRule(SingleFieldSubscriptions, `
subscription ImportantEmails {
importantEmails
notImportantEmails
spamEmails
}
`, [ {
message: singleFieldOnlyMessage('ImportantEmails'),
locations: [ { line: 4, column: 9 }, { line: 5, column: 9 } ],
path: undefined,
} ]);
});

it('fails with more than one root field in anonymous subscriptions', () => {
expectFailsRule(SingleFieldSubscriptions, `
subscription {
importantEmails
notImportantEmails
}
`, [ {
message: singleFieldOnlyMessage(null),
locations: [ { line: 4, column: 9 } ],
path: undefined,
} ]);
});

});
5 changes: 5 additions & 0 deletions src/validation/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,11 @@ export {
ScalarLeafs as ScalarLeafsRule
} from './rules/ScalarLeafs';

// Spec Section: "Subscriptions with Single Root Field"
export {
SingleFieldSubscriptions as SingleFieldSubscriptionsRule
} from './rules/SingleFieldSubscriptions';

// Spec Section: "Argument Uniqueness"
export {
UniqueArgumentNames as UniqueArgumentNamesRule
Expand Down
39 changes: 39 additions & 0 deletions src/validation/rules/SingleFieldSubscriptions.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/* @flow */
/**
* Copyright (c) 2015, Facebook, Inc.
* All rights reserved.
*
* This source code is licensed under the BSD-style license found in the
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*/

import type { ValidationContext } from '../index';
import { GraphQLError } from '../../error';
import type { OperationDefinitionNode } from '../../language/ast';


export function singleFieldOnlyMessage(name: ?string): string {
return (name ? `Subscription "${name}" ` : 'Anonymous Subscription ') +
'must select only one top level field.';
}

/**
* Subscriptions must only include one field.
*
* A GraphQL subscription is valid only if it contains a single root field.
*/
export function SingleFieldSubscriptions(context: ValidationContext): any {
return {
OperationDefinition(node: OperationDefinitionNode) {
if (node.operation === 'subscription') {
if (node.selectionSet.selections.length !== 1) {
context.reportError(new GraphQLError(
singleFieldOnlyMessage(node.name && node.name.value),
node.selectionSet.selections.slice(1)
));
}
}
}
};
}
4 changes: 4 additions & 0 deletions src/validation/specifiedRules.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ import { UniqueOperationNames } from './rules/UniqueOperationNames';
// Spec Section: "Lone Anonymous Operation"
import { LoneAnonymousOperation } from './rules/LoneAnonymousOperation';

// Spec Section: "Subscriptions with Single Root Field"
import { SingleFieldSubscriptions } from './rules/SingleFieldSubscriptions';

// Spec Section: "Fragment Spread Type Existence"
import { KnownTypeNames } from './rules/KnownTypeNames';

Expand Down Expand Up @@ -98,6 +101,7 @@ import type { ValidationContext } from './index';
export const specifiedRules: Array<(context: ValidationContext) => any> = [
UniqueOperationNames,
LoneAnonymousOperation,
SingleFieldSubscriptions,
KnownTypeNames,
FragmentsOnCompositeTypes,
VariablesAreInputTypes,
Expand Down

0 comments on commit 1c4477c

Please sign in to comment.