diff --git a/src/__tests__/starWarsStreamQuery-test.js b/src/__tests__/starWarsStreamQuery-test.js index b2cfe7174b9..d82017ad26d 100644 --- a/src/__tests__/starWarsStreamQuery-test.js +++ b/src/__tests__/starWarsStreamQuery-test.js @@ -69,20 +69,134 @@ describe('Star Wars Query Stream Tests', () => { }); }); - describe('Basic Queries', () => { - it('Can @stream an array field', async () => { + describe('@stream conflict validation', () => { + it('Does not allow a mix of @stream and no @stream on the same field', async () => { const query = ` query HeroFriendsQuery { hero { - friends @stream(initial_count: 2, label: "HeroFriends") { + friends { + id + } + ...FriendsName + } + } + fragment FriendsName on Character { + friends @stream(label: "nameLabel", initial_count: 1) { + name + } + } + `; + const result = await graphql(StarWarsSchemaDeferStreamEnabled, query); + expect(result).to.deep.equal({ + errors: [ + { + message: + 'Fields "friends" conflict because they have differing stream directives. Use different aliases on the fields to fetch both if this was intentional.', + locations: [ + { + line: 4, + column: 13, + }, + { + line: 11, + column: 11, + }, + ], + }, + ], + }); + }); + it('Does not allow multiple @stream with different initial_count', async () => { + const query = ` + query HeroFriendsQuery { + hero { + friends @stream(label: "sameLabel", initial_count: 3) { + id + } + ...FriendsName + } + } + fragment FriendsName on Character { + friends @stream(label: "sameLabel", initial_count: 1) { + name + } + } + `; + const result = await graphql(StarWarsSchemaDeferStreamEnabled, query); + expect(result).to.deep.equal({ + errors: [ + { + message: + 'Fields "friends" conflict because they have differing stream directives. Use different aliases on the fields to fetch both if this was intentional.', + locations: [ + { + line: 4, + column: 13, + }, + { + line: 11, + column: 11, + }, + ], + }, + ], + }); + }); + it('Does not allow multiple @stream with different label', async () => { + const query = ` + query HeroFriendsQuery { + hero { + friends @stream(label: "idLabel", initial_count: 1) { id - name } + ...FriendsName + } + } + fragment FriendsName on Character { + friends @stream(label: "nameLabel", initial_count: 1) { + name } } `; + const result = await graphql(StarWarsSchemaDeferStreamEnabled, query); + expect(result).to.deep.equal({ + errors: [ + { + message: + 'Fields "friends" conflict because they have differing stream directives. Use different aliases on the fields to fetch both if this was intentional.', + locations: [ + { + line: 4, + column: 13, + }, + { + line: 11, + column: 11, + }, + ], + }, + ], + }); + }); + it('Does allow multiple @stream with same label and initial_count', async () => { + const query = ` + query HeroFriendsQuery { + hero { + friends @stream(label: "sameLabel", initial_count: 2) { + id + } + ...FriendsName + } + } + fragment FriendsName on Character { + friends @stream(label: "sameLabel", initial_count: 2) { + name + } + } + `; const result = await graphql(StarWarsSchemaDeferStreamEnabled, query); const { patches: patchesIterable, ...initial } = result; + expect(initial).to.deep.equal({ data: { hero: { @@ -99,44 +213,99 @@ describe('Star Wars Query Stream Tests', () => { }, }, }); - const patches = []; - if (patchesIterable) { await forAwaitEach(patchesIterable, patch => { patches.push(patch); }); } - expect(patches).to.have.lengthOf(1); expect(patches[0]).to.deep.equal({ - label: 'HeroFriends', + data: { + id: '1003', + name: 'Leia Organa', + }, path: ['hero', 'friends', 2], + label: 'sameLabel', + }); + }); + it('Does allow multiple @stream with different label and initial_count when fields are aliased', async () => { + const query = ` + query HeroFriendsQuery { + hero { + friends @stream(label: "idLabel", initial_count: 2) { + id + } + ...FriendsName + } + } + fragment FriendsName on Character { + namedFriends: friends @stream(label: "nameLabel", initial_count: 1) { + name + } + } + `; + const result = await graphql(StarWarsSchemaDeferStreamEnabled, query); + const { patches: patchesIterable, ...initial } = result; + + expect(initial).to.deep.equal({ + data: { + hero: { + friends: [ + { + id: '1000', + }, + { + id: '1002', + }, + ], + namedFriends: [ + { + name: 'Luke Skywalker', + }, + ], + }, + }, + }); + const patches = []; + if (patchesIterable) { + await forAwaitEach(patchesIterable, patch => { + patches.push(patch); + }); + } + expect(patches).to.have.lengthOf(3); + expect(patches[0]).to.deep.equal({ data: { id: '1003', + }, + path: ['hero', 'friends', 2], + label: 'idLabel', + }); + expect(patches[1]).to.deep.equal({ + data: { + name: 'Han Solo', + }, + path: ['hero', 'namedFriends', 1], + label: 'nameLabel', + }); + expect(patches[2]).to.deep.equal({ + data: { name: 'Leia Organa', }, + path: ['hero', 'namedFriends', 2], + label: 'nameLabel', }); }); - it('Can @stream multiple selections on the same field', async () => { + }); + describe('Basic Queries', () => { + it('Can @stream an array field', async () => { const query = ` query HeroFriendsQuery { hero { - friends { + friends @stream(initial_count: 2, label: "HeroFriends") { id + name } - ...FriendsName - ...FriendsAppearsIn - } - } - fragment FriendsName on Character { - friends @stream(label: "nameLabel", initial_count: 1) { - name - } - } - fragment FriendsAppearsIn on Character { - friends @stream(label: "appearsInLabel", initial_count: 2) { - appearsIn } } `; @@ -148,15 +317,11 @@ describe('Star Wars Query Stream Tests', () => { friends: [ { id: '1000', - appearsIn: ['NEW_HOPE', 'EMPIRE', 'JEDI'], name: 'Luke Skywalker', }, { id: '1002', - appearsIn: ['NEW_HOPE', 'EMPIRE', 'JEDI'], - }, - { - id: '1003', + name: 'Han Solo', }, ], }, @@ -171,29 +336,103 @@ describe('Star Wars Query Stream Tests', () => { }); } - expect(patches).to.have.lengthOf(3); + expect(patches).to.have.lengthOf(1); expect(patches[0]).to.deep.equal({ + label: 'HeroFriends', + path: ['hero', 'friends', 2], data: { - name: 'Han Solo', + id: '1003', + name: 'Leia Organa', + }, + }); + }); + it('Errors are added to the correct patch', async () => { + const query = ` + query HeroFriendsQuery { + hero { + friends @stream(initial_count: 0, label: "HeroFriends") { + ... on Human { + secretFriend + } + } + } + } + `; + const result = await graphql(StarWarsSchemaDeferStreamEnabled, query); + const { patches: patchesIterable, ...initial } = result; + expect(initial).to.deep.equal({ + data: { + hero: { + friends: [], + }, }, - path: ['hero', 'friends', 1], - label: 'nameLabel', }); + const patches = []; + + if (patchesIterable) { + await forAwaitEach(patchesIterable, patch => { + patches.push(patch); + }); + } + + expect(patches).to.have.lengthOf(3); + expect(patches[0]).to.deep.equal({ + data: { + secretFriend: null, + }, + path: ['hero', 'friends', 0], + label: 'HeroFriends', + errors: [ + { + message: 'secretFriend is secret.', + locations: [ + { + line: 6, + column: 17, + }, + ], + path: ['hero', 'friends', 0, 'secretFriend'], + }, + ], + }); expect(patches[1]).to.deep.equal({ data: { - name: 'Leia Organa', + secretFriend: null, }, - path: ['hero', 'friends', 2], - label: 'nameLabel', + path: ['hero', 'friends', 1], + label: 'HeroFriends', + errors: [ + { + message: 'secretFriend is secret.', + locations: [ + { + line: 6, + column: 17, + }, + ], + path: ['hero', 'friends', 1, 'secretFriend'], + }, + ], }); - expect(patches[2]).to.deep.equal({ data: { - appearsIn: ['NEW_HOPE', 'EMPIRE', 'JEDI'], + secretFriend: null, }, path: ['hero', 'friends', 2], - label: 'appearsInLabel', + label: 'HeroFriends', + errors: [ + { + message: 'secretFriend is secret.', + locations: [ + { + line: 6, + column: 17, + }, + ], + path: ['hero', 'friends', 2, 'secretFriend'], + }, + ], }); }); }); diff --git a/src/execution/execute.js b/src/execution/execute.js index 2e505111a85..300263854e5 100644 --- a/src/execution/execute.js +++ b/src/execution/execute.js @@ -1062,6 +1062,14 @@ function completeListValue( ); } + // validation only allows equivalent streams on multiple fields, so it is + // safe to only check the first fieldNode for the stream directive + const stream = getDirectiveValues( + GraphQLStreamDirective, + fieldNodes[0], + exeContext.variableValues, + ); + // This is specified as a simple map, however we're optimizing the path // where the list contains no Promises by avoiding creating another Promise. const itemType = returnType.ofType; @@ -1071,58 +1079,46 @@ function completeListValue( // No need to modify the info object containing the path, // since from here on it is not ever accessed by resolver functions. const fieldPath = addPath(path, index); - const initialFieldNodes = []; - for (const fieldNode of fieldNodes) { - const stream = getDirectiveValues( - GraphQLStreamDirective, - fieldNode, - exeContext.variableValues, + if ( + exeContext.schema.__experimentalStream && + stream && + stream.if !== false && + typeof stream.label === 'string' && + typeof stream.initial_count === 'number' && + index >= stream.initial_count + ) { + const patchErrors = []; + exeContext.dispatcher.add( + stream.label, + fieldPath, + () => + completeValueCatchingError( + exeContext, + itemType, + fieldNodes, + info, + fieldPath, + item, + patchErrors, + ), + patchErrors, + ); + } else { + const completedItem = completeValueCatchingError( + exeContext, + itemType, + fieldNodes, + info, + fieldPath, + item, + errors, ); - if ( - exeContext.schema.__experimentalStream && - stream && - stream.if !== false && - typeof stream.label === 'string' && - typeof stream.initial_count === 'number' && - index >= stream.initial_count - ) { - const patchErrors = []; - exeContext.dispatcher.add( - stream.label, - fieldPath, - () => - completeValueCatchingError( - exeContext, - itemType, - [fieldNode], - info, - fieldPath, - item, - patchErrors, - ), - patchErrors, - ); - } else { - initialFieldNodes.push(fieldNode); - } - } - if (!initialFieldNodes.length) { - return; - } - const completedItem = completeValueCatchingError( - exeContext, - itemType, - initialFieldNodes, - info, - fieldPath, - item, - errors, - ); - if (!containsPromise && isPromise(completedItem)) { - containsPromise = true; + if (!containsPromise && isPromise(completedItem)) { + containsPromise = true; + } + completedResults.push(completedItem); } - completedResults.push(completedItem); }); return containsPromise ? Promise.all(completedResults) : completedResults; diff --git a/src/validation/rules/OverlappingFieldsCanBeMerged.js b/src/validation/rules/OverlappingFieldsCanBeMerged.js index ad5571a57bb..a4ed334fbc6 100644 --- a/src/validation/rules/OverlappingFieldsCanBeMerged.js +++ b/src/validation/rules/OverlappingFieldsCanBeMerged.js @@ -16,6 +16,7 @@ import { type FieldNode, type ArgumentNode, type FragmentDefinitionNode, + type DirectiveNode, } from '../../language/ast'; import { @@ -597,6 +598,13 @@ function findConflict( [node2], ]; } + if (!sameStreams(node1.directives || [], node2.directives || [])) { + return [ + [responseName, 'they have differing stream directives'], + [node1], + [node2], + ]; + } } if (type1 && type2 && doTypesConflict(type1, type2)) { @@ -651,6 +659,60 @@ function sameArguments( }); } +function sameDirectiveArgument( + directive1: DirectiveNode, + directive2: DirectiveNode, + argumentName: string, +): boolean { + if (!directive1.arguments) { + return false; + } + const argument1 = find( + directive1.arguments, + argument => argument.name.value === argumentName, + ); + if (!argument1) { + return false; + } + if (!directive2.arguments) { + return false; + } + const argument2 = find( + directive2.arguments, + argument => argument.name.value === argumentName, + ); + if (!argument2) { + return false; + } + return sameValue(argument1.value, argument2.value); +} + +function getStreamDirective( + directives: $ReadOnlyArray, +): ?DirectiveNode { + return find(directives, directive => directive.name.value === 'stream'); +} + +function sameStreams( + directives1: $ReadOnlyArray, + directives2: $ReadOnlyArray, +): boolean { + const stream1 = getStreamDirective(directives1); + const stream2 = getStreamDirective(directives2); + if (!stream1 && !stream2) { + // both fields do not have streams + return true; + } else if (stream1 && stream2) { + // check if both fields have equivalent streams + return ( + sameDirectiveArgument(stream1, stream2, 'initial_count') && + sameDirectiveArgument(stream1, stream2, 'label') + ); + } + // fields have a mix of stream and no stream + return false; +} + function sameValue(value1, value2) { return (!value1 && !value2) || print(value1) === print(value2); }