From 3926dd600476b5fd4989484d367a355ccede1eab Mon Sep 17 00:00:00 2001 From: Jeremiah VALERIE Date: Tue, 22 Nov 2016 20:06:19 +0100 Subject: [PATCH 1/4] Relay/Connection First and last arguments must be positive integers --- Relay/Connection/Output/ConnectionBuilder.php | 8 +++++++ .../Output/ConnectionBuilderTest.php | 24 +++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/Relay/Connection/Output/ConnectionBuilder.php b/Relay/Connection/Output/ConnectionBuilder.php index 22c2df727..a651c9460 100644 --- a/Relay/Connection/Output/ConnectionBuilder.php +++ b/Relay/Connection/Output/ConnectionBuilder.php @@ -93,9 +93,17 @@ public static function connectionFromArraySlice($arraySlice, $args, array $meta) $endOffset = min($sliceEnd, $beforeOffset, $arrayLength); if (is_numeric($first)) { + if ($first < 0) { + throw new \InvalidArgumentException('Argument "first" must be a non-negative integer'); + } $endOffset = min($endOffset, $startOffset + $first); } + if (is_numeric($last)) { + if ($last < 0) { + throw new \InvalidArgumentException('Argument "last" must be a non-negative integer'); + } + $startOffset = max($startOffset, $endOffset - $last); } diff --git a/Tests/Relay/Connection/Output/ConnectionBuilderTest.php b/Tests/Relay/Connection/Output/ConnectionBuilderTest.php index 3ee186dfe..3a6cb7785 100644 --- a/Tests/Relay/Connection/Output/ConnectionBuilderTest.php +++ b/Tests/Relay/Connection/Output/ConnectionBuilderTest.php @@ -178,6 +178,30 @@ public function testRespectsLastAndAfterAndBeforeExactlyRight() $this->assertEquals($expected, $actual); } + /** + * @expectedException \InvalidArgumentException + * @expectedExceptionMessage Argument "first" must be a non-negative integer + */ + public function testThrowsAnErrorIfFirstLessThan0() + { + ConnectionBuilder::connectionFromArray( + $this->letters, + ['first' => -1] + ); + } + + /** + * @expectedException \InvalidArgumentException + * @expectedExceptionMessage Argument "last" must be a non-negative integer + */ + public function testThrowsAnErrorIfLastLessThan0() + { + ConnectionBuilder::connectionFromArray( + $this->letters, + ['last' => -1] + ); + } + public function testReturnsNoElementsIfFirstIs0() { $actual = ConnectionBuilder::connectionFromArray( From 39416807dcb91e01eb185aca9fb4bd2aa92ad8fd Mon Sep 17 00:00:00 2001 From: Jeremiah VALERIE Date: Tue, 22 Nov 2016 20:34:20 +0100 Subject: [PATCH 2/4] Remove non-null restriction on clientMutationId field definitions. --- Relay/Mutation/InputDefinition.php | 2 +- Relay/Mutation/PayloadDefinition.php | 2 +- .../Relay/Mutation/MutationTest.php | 25 +++---------------- 3 files changed, 6 insertions(+), 23 deletions(-) diff --git a/Relay/Mutation/InputDefinition.php b/Relay/Mutation/InputDefinition.php index e449acefe..2056a7fec 100644 --- a/Relay/Mutation/InputDefinition.php +++ b/Relay/Mutation/InputDefinition.php @@ -30,7 +30,7 @@ public function toMappingDefinition(array $config) 'fields' => array_merge( $inputFields, [ - 'clientMutationId' => ['type' => 'String!'], + 'clientMutationId' => ['type' => 'String'], ] ), ], diff --git a/Relay/Mutation/PayloadDefinition.php b/Relay/Mutation/PayloadDefinition.php index 7aaab1238..8d88a8e78 100644 --- a/Relay/Mutation/PayloadDefinition.php +++ b/Relay/Mutation/PayloadDefinition.php @@ -29,7 +29,7 @@ public function toMappingDefinition(array $config) 'fields' => array_merge( $outputFields, [ - 'clientMutationId' => ['type' => 'String!'], + 'clientMutationId' => ['type' => 'String'], ] ), ], diff --git a/Tests/Functional/Relay/Mutation/MutationTest.php b/Tests/Functional/Relay/Mutation/MutationTest.php index bbaae92b6..f1909f694 100644 --- a/Tests/Functional/Relay/Mutation/MutationTest.php +++ b/Tests/Functional/Relay/Mutation/MutationTest.php @@ -98,10 +98,6 @@ public function testContainsCorrectInput() type { name kind - ofType { - name - kind - } } } } @@ -115,12 +111,8 @@ public function testContainsCorrectInput() [ 'name' => 'clientMutationId', 'type' => [ - 'name' => null, - 'kind' => 'NON_NULL', - 'ofType' => [ - 'name' => 'String', - 'kind' => 'SCALAR', - ], + 'name' => 'String', + 'kind' => 'SCALAR', ], ], ], @@ -142,10 +134,6 @@ public function testContainsCorrectPayload() type { name kind - ofType { - name - kind - } } } } @@ -162,18 +150,13 @@ public function testContainsCorrectPayload() 'type' => [ 'name' => 'Int', 'kind' => 'SCALAR', - 'ofType' => null, ], ], [ 'name' => 'clientMutationId', 'type' => [ - 'name' => null, - 'kind' => 'NON_NULL', - 'ofType' => [ - 'name' => 'String', - 'kind' => 'SCALAR', - ], + 'name' => 'String', + 'kind' => 'SCALAR', ], ], From 6331f777dc01edefb92e52df21464998bdf26ddd Mon Sep 17 00:00:00 2001 From: Jeremiah VALERIE Date: Tue, 22 Nov 2016 20:42:22 +0100 Subject: [PATCH 3/4] Fix cs --- Command/GraphQLDumpSchemaCommand.php | 2 +- Error/ErrorHandler.php | 2 +- Relay/Connection/Paginator.php | 9 +++++++ Request/Executor.php | 6 ++--- .../Compiler/FakeCompilerPass.php | 12 ++++++++-- .../Compiler/FakeInjectedService.php | 11 ++++++++- .../ResolverTaggedServiceMappingPassTest.php | 24 ++++++++++++------- .../Compiler/ResolverTestService.php | 12 ++++++++-- .../Controller/GraphControllerTest.php | 1 - Tests/Relay/Connection/PaginatorTest.php | 12 +++++++++- Tests/Request/ExecutorTest.php | 8 +++++-- 11 files changed, 76 insertions(+), 23 deletions(-) diff --git a/Command/GraphQLDumpSchemaCommand.php b/Command/GraphQLDumpSchemaCommand.php index 6fd4b9180..0d4408287 100644 --- a/Command/GraphQLDumpSchemaCommand.php +++ b/Command/GraphQLDumpSchemaCommand.php @@ -57,7 +57,7 @@ protected function execute(InputInterface $input, OutputInterface $output) ->execute($request, [], $schemaName) ->toArray(); - $file = $input->getOption('file') ?: $container->getParameter('kernel.root_dir').sprintf('/../var/schema%s.json', $schemaName? '.'.$schemaName:''); + $file = $input->getOption('file') ?: $container->getParameter('kernel.root_dir').sprintf('/../var/schema%s.json', $schemaName ? '.'.$schemaName : ''); $schema = json_encode($result['data']); diff --git a/Error/ErrorHandler.php b/Error/ErrorHandler.php index 4dd178ed5..b145ea973 100644 --- a/Error/ErrorHandler.php +++ b/Error/ErrorHandler.php @@ -14,8 +14,8 @@ use GraphQL\Error; use GraphQL\Executor\ExecutionResult; use Psr\Log\LoggerInterface; -use Psr\Log\NullLogger; use Psr\Log\LogLevel; +use Psr\Log\NullLogger; class ErrorHandler { diff --git a/Relay/Connection/Paginator.php b/Relay/Connection/Paginator.php index d2f38da99..7468fad26 100644 --- a/Relay/Connection/Paginator.php +++ b/Relay/Connection/Paginator.php @@ -1,5 +1,14 @@ + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + namespace Overblog\GraphQLBundle\Relay\Connection; use Overblog\GraphQLBundle\Definition\Argument; diff --git a/Request/Executor.php b/Request/Executor.php index b77de397b..49c6f6531 100644 --- a/Request/Executor.php +++ b/Request/Executor.php @@ -57,8 +57,7 @@ public function __construct( ErrorHandler $errorHandler = null, $hasDebugInfo = false, callable $executor = null - ) - { + ) { $this->dispatcher = $dispatcher; $this->throwException = (bool) $throwException; $this->errorHandler = $errorHandler; @@ -67,7 +66,6 @@ public function __construct( if (null === $this->executor) { $this->executor = self::DEFAULT_EXECUTOR; } - } public function setExecutor(callable $executor) @@ -152,7 +150,7 @@ public function execute(array $data, array $context = [], $schemaName = null) isset($data[ParserInterface::PARAM_OPERATION_NAME]) ? $data[ParserInterface::PARAM_OPERATION_NAME] : null ); - if (!is_object($executionResult) || !$executionResult instanceof ExecutionResult) { + if (!is_object($executionResult) || !$executionResult instanceof ExecutionResult) { throw new \RuntimeException(sprintf('Execution result should be an object instantiating "%s".', 'GraphQL\\Executor\\ExecutionResult')); } diff --git a/Tests/DependencyInjection/Compiler/FakeCompilerPass.php b/Tests/DependencyInjection/Compiler/FakeCompilerPass.php index 92502c4af..b607d67a0 100644 --- a/Tests/DependencyInjection/Compiler/FakeCompilerPass.php +++ b/Tests/DependencyInjection/Compiler/FakeCompilerPass.php @@ -1,5 +1,14 @@ + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + namespace Overblog\GraphQLBundle\Tests\DependencyInjection\Compiler; use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; @@ -7,8 +16,7 @@ use Symfony\Component\DependencyInjection\Reference; /** - * Class FakeCompilerPass - * @package DependencyInjection\Compiler + * Class FakeCompilerPass. */ class FakeCompilerPass implements CompilerPassInterface { diff --git a/Tests/DependencyInjection/Compiler/FakeInjectedService.php b/Tests/DependencyInjection/Compiler/FakeInjectedService.php index 1631fee23..f3497df06 100644 --- a/Tests/DependencyInjection/Compiler/FakeInjectedService.php +++ b/Tests/DependencyInjection/Compiler/FakeInjectedService.php @@ -1,9 +1,18 @@ + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + namespace Overblog\GraphQLBundle\Tests\DependencyInjection\Compiler; /** - * Class FakeInjectedService + * Class FakeInjectedService. */ class FakeInjectedService { diff --git a/Tests/DependencyInjection/Compiler/ResolverTaggedServiceMappingPassTest.php b/Tests/DependencyInjection/Compiler/ResolverTaggedServiceMappingPassTest.php index c062b49b8..477c07e62 100644 --- a/Tests/DependencyInjection/Compiler/ResolverTaggedServiceMappingPassTest.php +++ b/Tests/DependencyInjection/Compiler/ResolverTaggedServiceMappingPassTest.php @@ -1,5 +1,14 @@ + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + namespace Overblog\GraphQLBundle\Tests\DependencyInjection\Compiler; use Overblog\GraphQLBundle\DependencyInjection\Compiler\ResolverTaggedServiceMappingPass; @@ -7,8 +16,7 @@ use Symfony\Component\DependencyInjection\Definition; /** - * Class ResolverTaggedServiceMappingPassTest - * @package DependencyInjection + * Class ResolverTaggedServiceMappingPassTest. */ class ResolverTaggedServiceMappingPassTest extends \PHPUnit_Framework_TestCase { @@ -27,13 +35,13 @@ public function setUp() $container->register( 'overblog_graphql.resolver_resolver', - "Overblog\\GraphQLBundle\\Resolver\\ResolverResolver" + 'Overblog\\GraphQLBundle\\Resolver\\ResolverResolver' ); $testResolver = new Definition('Overblog\GraphQLBundle\Tests\DependencyInjection\Compiler\ResolverTestService'); $testResolver - ->addTag( 'overblog_graphql.resolver', [ - 'alias' => 'test_resolver', 'method' => 'doSomethingWithContainer' + ->addTag('overblog_graphql.resolver', [ + 'alias' => 'test_resolver', 'method' => 'doSomethingWithContainer', ]); $container->setDefinition('test_resolver', $testResolver); @@ -43,9 +51,9 @@ public function setUp() private function addCompilerPassesAndCompile() { - $this->container->addCompilerPass(new ResolverTaggedServiceMappingPass()); - $this->container->addCompilerPass(new FakeCompilerPass()); - $this->container->compile(); + $this->container->addCompilerPass(new ResolverTaggedServiceMappingPass()); + $this->container->addCompilerPass(new FakeCompilerPass()); + $this->container->compile(); } public function testCompilationWorksPassConfigDirective() diff --git a/Tests/DependencyInjection/Compiler/ResolverTestService.php b/Tests/DependencyInjection/Compiler/ResolverTestService.php index 5149e5bce..0016ed42c 100644 --- a/Tests/DependencyInjection/Compiler/ResolverTestService.php +++ b/Tests/DependencyInjection/Compiler/ResolverTestService.php @@ -1,13 +1,21 @@ + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + namespace DependencyInjection\Compiler; use Symfony\Component\DependencyInjection\ContainerAwareInterface; use Symfony\Component\DependencyInjection\ContainerAwareTrait; /** - * Class ResolverTestService - * @package DependencyInjection\Compiler + * Class ResolverTestService. */ class ResolverTestService implements ContainerAwareInterface { diff --git a/Tests/Functional/Controller/GraphControllerTest.php b/Tests/Functional/Controller/GraphControllerTest.php index 18c0189c8..c0fbae247 100644 --- a/Tests/Functional/Controller/GraphControllerTest.php +++ b/Tests/Functional/Controller/GraphControllerTest.php @@ -106,7 +106,6 @@ public function testEndpointWithEmptyJsonBodyQuery() $client->getResponse()->getContent(); } - /** * @expectedException \Symfony\Component\HttpKernel\Exception\BadRequestHttpException * @expectedExceptionMessage POST body sent invalid JSON diff --git a/Tests/Relay/Connection/PaginatorTest.php b/Tests/Relay/Connection/PaginatorTest.php index c147b50f1..604adc491 100644 --- a/Tests/Relay/Connection/PaginatorTest.php +++ b/Tests/Relay/Connection/PaginatorTest.php @@ -1,5 +1,14 @@ + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + namespace Overblog\GraphQLBundle\Tests\Relay\Connection; use Overblog\GraphQLBundle\Definition\Argument; @@ -28,7 +37,7 @@ public function testForwardAfter() return array_fill(0, 6, 'item'); }); - $this->assertCount(5, $paginator->forward(new Argument(['first' => 5, 'after' => base64_encode('arrayconnection:5') ]))->edges); + $this->assertCount(5, $paginator->forward(new Argument(['first' => 5, 'after' => base64_encode('arrayconnection:5')]))->edges); } public function testBackward() @@ -88,6 +97,7 @@ public function testAuto() $countCalled = false; $result = $paginator->auto(new Argument(['last' => 5]), function () use (&$countCalled) { $countCalled = true; + return 10; }); diff --git a/Tests/Request/ExecutorTest.php b/Tests/Request/ExecutorTest.php index b30fb17fd..8bbd9e52c 100644 --- a/Tests/Request/ExecutorTest.php +++ b/Tests/Request/ExecutorTest.php @@ -46,7 +46,9 @@ public function setUp() */ public function testInvalidExecutorReturnNotObject() { - $this->executor->setExecutor(function() { return false; }); + $this->executor->setExecutor(function () { + return false; + }); $this->executor->execute($this->request); } @@ -56,7 +58,9 @@ public function testInvalidExecutorReturnNotObject() */ public function testInvalidExecutorReturnInvalidObject() { - $this->executor->setExecutor(function() { return new \stdClass(); }); + $this->executor->setExecutor(function () { + return new \stdClass(); + }); $this->executor->execute($this->request); } From 478ea36918862bc1cececc3815673d9c2088fbfa Mon Sep 17 00:00:00 2001 From: Jeremiah VALERIE Date: Wed, 23 Nov 2016 08:52:47 +0100 Subject: [PATCH 4/4] Add context to doc :memo: --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 4330c6c08..927214a91 100644 --- a/README.md +++ b/README.md @@ -891,6 +891,7 @@ Expression | Description | Scope **value** | Resolver value | only available in resolve context **args** | Resolver args array | only available in resolve context **info** | Resolver GraphQL\Type\Definition\ResolveInfo Object | only available in resolve context +**context** | context is defined by your application on the top level of query execution (useful for storing current user, environment details, etc) | only available in resolve context **childrenComplexity** | Selection field children complexity | only available in complexity context [For more details on expression syntax](http://symfony.com/doc/current/components/expression_language/syntax.html)