From 227f0b867d469836c8d472ae222e58cf10204ebd Mon Sep 17 00:00:00 2001 From: Vladimir Razuvaev Date: Wed, 22 Aug 2018 15:10:10 +0700 Subject: [PATCH] Serial execution should support sync execution (+tests for the serial execution) --- src/Executor/Executor.php | 145 ++++++++++++++----------- tests/Executor/SyncTest.php | 207 ++++++++++++++++++++++++++++++++++++ 2 files changed, 291 insertions(+), 61 deletions(-) create mode 100644 tests/Executor/SyncTest.php diff --git a/src/Executor/Executor.php b/src/Executor/Executor.php index 4251554..2a94e38 100644 --- a/src/Executor/Executor.php +++ b/src/Executor/Executor.php @@ -285,29 +285,31 @@ class Executor // field and its descendants will be omitted, and sibling fields will still // be executed. An execution which encounters errors will still result in a // resolved Promise. - $result = $this->exeContext->promises->create(function (callable $resolve) { - return $resolve($this->executeOperation($this->exeContext->operation, $this->exeContext->rootValue)); - }); + $data = $this->executeOperation($this->exeContext->operation, $this->exeContext->rootValue); + $result = $this->buildResponse($data); - return $result - ->then( - null, - function ($error) { - // Errors from sub-fields of a NonNull type may propagate to the top level, - // at which point we still log the error and null the parent field, which - // in this case is the entire response. - $this->exeContext->addError($error); + // Note: we deviate here from the reference implementation a bit by always returning promise + // But for the "sync" case it is always fulfilled + return $this->isPromise($result) + ? $result + : $this->exeContext->promises->createFulfilled($result); + } - return null; - } - ) - ->then(function ($data) { - if ($data !== null) { - $data = (array) $data; - } - - return new ExecutionResult($data, $this->exeContext->errors); + /** + * @param mixed|null|Promise $data + * @return ExecutionResult|Promise + */ + private function buildResponse($data) + { + if ($this->isPromise($data)) { + return $data->then(function ($resolved) { + return $this->buildResponse($resolved); }); + } + if ($data !== null) { + $data = (array) $data; + } + return new ExecutionResult($data, $this->exeContext->errors); } /** @@ -333,14 +335,12 @@ class Executor $this->executeFieldsSerially($type, $rootValue, $path, $fields) : $this->executeFields($type, $rootValue, $path, $fields); - $promise = $this->getPromise($result); - if ($promise) { - return $promise->then( + if ($this->isPromise($result)) { + return $result->then( null, function ($error) { $this->exeContext->addError($error); - - return null; + return $this->exeContext->promises->createFulfilled(null); } ); } @@ -348,7 +348,6 @@ class Executor return $result; } catch (Error $error) { $this->exeContext->addError($error); - return null; } } @@ -555,44 +554,34 @@ class Executor */ private function executeFieldsSerially(ObjectType $parentType, $sourceValue, $path, $fields) { - $prevPromise = $this->exeContext->promises->createFulfilled([]); - - $process = function ($results, $responseName, $path, $parentType, $sourceValue, $fieldNodes) { - $fieldPath = $path; - $fieldPath[] = $responseName; - $result = $this->resolveField($parentType, $sourceValue, $fieldNodes, $fieldPath); - if ($result === self::$UNDEFINED) { - return $results; - } - $promise = $this->getPromise($result); - if ($promise) { - return $promise->then(function ($resolvedResult) use ($responseName, $results) { - $results[$responseName] = $resolvedResult; - + $result = $this->promiseReduce( + array_keys($fields->getArrayCopy()), + function ($results, $responseName) use ($path, $parentType, $sourceValue, $fields) { + $fieldNodes = $fields[$responseName]; + $fieldPath = $path; + $fieldPath[] = $responseName; + $result = $this->resolveField($parentType, $sourceValue, $fieldNodes, $fieldPath); + if ($result === self::$UNDEFINED) { return $results; - }); - } - $results[$responseName] = $result; - - return $results; - }; - - foreach ($fields as $responseName => $fieldNodes) { - $prevPromise = $prevPromise->then(function ($resolvedResults) use ( - $process, - $responseName, - $path, - $parentType, - $sourceValue, - $fieldNodes - ) { - return $process($resolvedResults, $responseName, $path, $parentType, $sourceValue, $fieldNodes); + } + $promise = $this->getPromise($result); + if ($promise) { + return $promise->then(function ($resolvedResult) use ($responseName, $results) { + $results[$responseName] = $resolvedResult; + return $results; + }); + } + $results[$responseName] = $result; + return $results; + }, + [] + ); + if ($this->isPromise($result)) { + return $result->then(function ($resolvedResults) { + return self::fixResultsIfEmptyArray($resolvedResults); }); } - - return $prevPromise->then(function ($resolvedResults) { - return self::fixResultsIfEmptyArray($resolvedResults); - }); + return self::fixResultsIfEmptyArray($result); } /** @@ -962,6 +951,15 @@ class Executor throw new \RuntimeException(sprintf('Cannot complete value of unexpected type "%s".', $returnType)); } + /** + * @param mixed $value + * @return bool + */ + private function isPromise($value) + { + return $value instanceof Promise || $this->exeContext->promises->isThenable($value); + } + /** * Only returns the value if it acts like a Promise, i.e. has a "then" function, * otherwise returns null. @@ -990,6 +988,31 @@ class Executor return null; } + /** + * Similar to array_reduce(), however the reducing callback may return + * a Promise, in which case reduction will continue after each promise resolves. + * + * If the callback does not return a Promise, then this function will also not + * return a Promise. + * + * @param mixed[] $values + * @param \Closure $callback + * @param Promise|mixed|null $initialValue + * @return mixed[] + */ + private function promiseReduce(array $values, \Closure $callback, $initialValue) + { + return array_reduce($values, function ($previous, $value) use ($callback) { + $promise = $this->getPromise($previous); + if ($promise) { + return $promise->then(function ($resolved) use ($callback, $value) { + return $callback($resolved, $value); + }); + } + return $callback($previous, $value); + }, $initialValue); + } + /** * Complete a list value by completing each item in the list with the * inner type diff --git a/tests/Executor/SyncTest.php b/tests/Executor/SyncTest.php new file mode 100644 index 0000000..da32b2f --- /dev/null +++ b/tests/Executor/SyncTest.php @@ -0,0 +1,207 @@ +schema = new Schema([ + 'query' => new ObjectType([ + 'name' => 'Query', + 'fields' => [ + 'syncField' => [ + 'type' => Type::string(), + 'resolve' => function ($rootValue) { + return $rootValue; + } + ], + 'asyncField' => [ + 'type' => Type::string(), + 'resolve' => function ($rootValue) { + return new Deferred(function () use ($rootValue) { + return $rootValue; + }); + } + ] + ] + ]), + 'mutation' => new ObjectType([ + 'name' => 'Mutation', + 'fields' => [ + 'syncMutationField' => [ + 'type' => Type::string(), + 'resolve' => function ($rootValue) { + return $rootValue; + } + ] + ] + ]) + ]); + $this->promiseAdapter = new SyncPromiseAdapter(); + } + + // Describe: Execute: synchronously when possible + + /** + * @it does not return a Promise for initial errors + */ + public function testDoesNotReturnAPromiseForInitialErrors() + { + $doc = 'fragment Example on Query { syncField }'; + $result = $this->execute( + $this->schema, + Parser::parse($doc), + 'rootValue' + ); + $this->assertSync(['errors' => [['message' => 'Must provide an operation.']]], $result); + } + + /** + * @it does not return a Promise if fields are all synchronous + */ + public function testDoesNotReturnAPromiseIfFieldsAreAllSynchronous() + { + $doc = 'query Example { syncField }'; + $result = $this->execute( + $this->schema, + Parser::parse($doc), + 'rootValue' + ); + $this->assertSync(['data' => ['syncField' => 'rootValue']], $result); + } + + /** + * @it does not return a Promise if mutation fields are all synchronous + */ + public function testDoesNotReturnAPromiseIfMutationFieldsAreAllSynchronous() + { + $doc = 'mutation Example { syncMutationField }'; + $result = $this->execute( + $this->schema, + Parser::parse($doc), + 'rootValue' + ); + $this->assertSync(['data' => ['syncMutationField' => 'rootValue']], $result); + } + + /** + * @it returns a Promise if any field is asynchronous + */ + public function testReturnsAPromiseIfAnyFieldIsAsynchronous() + { + $doc = 'query Example { syncField, asyncField }'; + $result = $this->execute( + $this->schema, + Parser::parse($doc), + 'rootValue' + ); + $this->assertAsync(['data' => ['syncField' => 'rootValue', 'asyncField' => 'rootValue']], $result); + } + + // Describe: graphqlSync + + /** + * @it does not return a Promise for syntax errors + */ + public function testDoesNotReturnAPromiseForSyntaxErrors() + { + $doc = 'fragment Example on Query { { { syncField }'; + $result = $this->graphqlSync( + $this->schema, + $doc + ); + $this->assertSync([ + 'errors' => [ + ['message' => 'Syntax Error: Expected Name, found {', + 'locations' => [['line' => 1, 'column' => 29]]] + ] + ], $result); + } + + /** + * @it does not return a Promise for validation errors + */ + public function testDoesNotReturnAPromiseForValidationErrors() + { + $doc = 'fragment Example on Query { unknownField }'; + $validationErrors = DocumentValidator::validate($this->schema, Parser::parse($doc)); + $result = $this->graphqlSync( + $this->schema, + $doc + ); + $expected = [ + 'errors' => Utils::map($validationErrors, function ($e) { + return FormattedError::createFromException($e); + }) + ]; + $this->assertSync($expected, $result); + } + + /** + * @it does not return a Promise for sync execution + */ + public function testDoesNotReturnAPromiseForSyncExecution() + { + $doc = 'query Example { syncField }'; + $result = $this->graphqlSync( + $this->schema, + $doc, + 'rootValue' + ); + $this->assertSync(['data' => ['syncField' => 'rootValue']], $result); + } + + private function graphqlSync($schema, $doc, $rootValue = null) + { + return GraphQL::promiseToExecute($this->promiseAdapter, $schema, $doc, $rootValue); + } + + private function execute($schema, $doc, $rootValue = null) + { + return Executor::promiseToExecute($this->promiseAdapter, $schema, $doc, $rootValue); + } + + private function assertSync($expectedFinalArray, $actualResult) + { + $message = 'Failed assertion that execution was synchronous'; + $this->assertInstanceOf(Promise::class, $actualResult, $message); + $this->assertInstanceOf(SyncPromise::class, $actualResult->adoptedPromise, $message); + $this->assertEquals(SyncPromise::FULFILLED, $actualResult->adoptedPromise->state, $message); + $this->assertInstanceOf(ExecutionResult::class, $actualResult->adoptedPromise->result, $message); + $this->assertArraySubset($expectedFinalArray, $actualResult->adoptedPromise->result->toArray(), $message); + } + + private function assertAsync($expectedFinalArray, $actualResult) + { + $message = 'Failed assertion that execution was asynchronous'; + $this->assertInstanceOf(Promise::class, $actualResult, $message); + $this->assertInstanceOf(SyncPromise::class, $actualResult->adoptedPromise, $message); + $this->assertEquals(SyncPromise::PENDING, $actualResult->adoptedPromise->state, $message); + $resolvedResult = $this->promiseAdapter->wait($actualResult); + $this->assertInstanceOf(ExecutionResult::class, $resolvedResult, $message); + $this->assertArraySubset($expectedFinalArray, $resolvedResult->toArray(), $message); + } +}