From 36f6c6ed5599fdf83c7e0a15164bc013dabe5d6b Mon Sep 17 00:00:00 2001 From: Christian Date: Sat, 21 Mar 2020 23:33:42 +0100 Subject: [PATCH 01/12] validate field arguments --- src/Support/Field.php | 15 +++ src/Support/RulesInFields.php | 105 ++++++++++++++++++ .../AccountType.php | 58 ++++++++++ .../ProfileType.php | 52 +++++++++ .../ValidationOfFieldArguments/TestQuery.php | 48 ++++++++ .../ValidationOfFieldArgumentsTest.php | 82 ++++++++++++++ 6 files changed, 360 insertions(+) create mode 100644 src/Support/RulesInFields.php create mode 100644 tests/Unit/ValidationOfFieldArguments/AccountType.php create mode 100644 tests/Unit/ValidationOfFieldArguments/ProfileType.php create mode 100644 tests/Unit/ValidationOfFieldArguments/TestQuery.php create mode 100644 tests/Unit/ValidationOfFieldArguments/ValidationOfFieldArgumentsTest.php diff --git a/src/Support/Field.php b/src/Support/Field.php index c4ef60e3..c51b41d5 100644 --- a/src/Support/Field.php +++ b/src/Support/Field.php @@ -86,6 +86,18 @@ public function getRules(array $arguments = []): array return array_merge($argsRules, $rules); } + public function validateFieldArguments(ResolveInfo $resolveInfo): void + { + [$argumentValues, $argsRules] = (new RulesInFields($this->type(), $resolveInfo))->get(); + + if (count($argsRules)) { + $validator = $this->getValidator($argumentValues, $argsRules); + if ($validator->fails()) { + throw new ValidationError('validation', $validator); + } + } + } + public function getValidator(array $args, array $rules): ValidatorContract { // allow our error messages to be customised @@ -111,6 +123,9 @@ protected function getResolver(): ?Closure // 4 (!) - added by this library, encapsulates creating a `SelectFields` instance $arguments = func_get_args(); + // Validate arguments in fields + $this->validateFieldArguments($arguments[3]); + // Validate mutation arguments $args = $arguments[1]; diff --git a/src/Support/RulesInFields.php b/src/Support/RulesInFields.php new file mode 100644 index 00000000..799ad97c --- /dev/null +++ b/src/Support/RulesInFields.php @@ -0,0 +1,105 @@ + + */ + private $fieldsAndArguments; + + /** + * @param Type $parentType + * @param ResolveInfo $resolveInfo + */ + public function __construct(Type $parentType, ResolveInfo $resolveInfo) + { + $this->parentType = $parentType; + $this->resolveInfo = $resolveInfo; + $this->fieldsAndArguments = (new ResolveInfoFieldsAndArguments($this->resolveInfo))->getFieldsAndArgumentsSelection(5); + } + + /** + * @return array + */ + public function get(): array + { + if ($this->parentType instanceof WrappingType) { + $this->parentType = $this->parentType->getWrappedType(true); + } + + return [$this->fieldsAndArguments, $this->getRules($this->fieldsAndArguments, null, $this->parentType)]; + } + + /** + * @param array|string|callable $rules + * @param array $arguments + * @return array|string + */ + protected function resolveRules($rules, array $arguments) + { + if (is_callable($rules)) { + return call_user_func($rules, $arguments, $this->fieldsAndArguments); + } + + return $rules; + } + + /** + * Get rules from fields. + * + * @param array $fields + * @param string|null $prefix + * @return array + */ + protected function getRules(array $fields, ?string $prefix, Type $parentType): array + { + $rules = []; + + foreach ($fields as $name => $field) { + $key = $prefix === null ? $name : "{$prefix}.{$name}"; + + //If field doesn't exist on definition we don't select it + try { + if (method_exists($parentType, 'getField')) { + $fieldObject = $parentType->getField($name); + } else { + continue; + } + } catch (InvariantViolation $e) { + continue; + } + + if (is_array($field['fields'])) { + $rules = $rules + $this->getRules($field['fields'], $key.'.fields', $fieldObject->getType()); + } + + $args = $fieldObject->config['args'] ?? []; + + foreach ($args as $argName => $info) { + if (isset($info['rules'])) { + $rules[$key.'.args.'.$argName] = $this->resolveRules($info['rules'], $field['args']); + } + } + } + + return $rules; + } +} diff --git a/tests/Unit/ValidationOfFieldArguments/AccountType.php b/tests/Unit/ValidationOfFieldArguments/AccountType.php new file mode 100644 index 00000000..71164d78 --- /dev/null +++ b/tests/Unit/ValidationOfFieldArguments/AccountType.php @@ -0,0 +1,58 @@ + + */ + protected $attributes = [ + 'name' => 'AccountType', + 'description' => 'An example', + ]; + + public function fields(): array + { + return [ + 'id' => [ + 'type' => Type::int(), + 'description' => 'A test field', + ], + 'profile' => [ + 'type' => GraphQL::type('ProfileType'), + 'args' => [ + 'profileId' => [ + 'type' => Type::int(), + 'rules' => function ($args, $fullQueryArgs) { + Assert::assertSame([ + 'profileId' => 100, + ], $args); + + return ['required', 'integer', 'max:10']; + }, + ], + ], + ], + 'alias' => [ + 'type' => Type::string(), + 'args' => [ + 'type' => [ + 'type' => Type::string(), + 'rules' => function ($args, $fullQueryArgs) { + return ['regex:/^l33t|normal$/i']; + }, + ], + ], + ], + + ]; + } +} diff --git a/tests/Unit/ValidationOfFieldArguments/ProfileType.php b/tests/Unit/ValidationOfFieldArguments/ProfileType.php new file mode 100644 index 00000000..457a6fa9 --- /dev/null +++ b/tests/Unit/ValidationOfFieldArguments/ProfileType.php @@ -0,0 +1,52 @@ + + */ + protected $attributes = [ + 'name' => 'ProfileType', + 'description' => 'An example', + ]; + + public function fields(): array + { + return [ + 'name' => [ + 'type' => Type::string(), + 'description' => 'A test field', + 'args' => [ + 'includeMiddleNames' => [ + 'type' => Type::string(), + 'rules' => ['regex:/^(yes|no)$/i'], + ], + ], + ], + 'height' => [ + 'type' => Type::string(), + 'description' => 'A test field', + 'args' => [ + 'unit' => [ + 'type' => Type::string(), + 'rules' => function ($args) { + Assert::assertSame([ + 'unit' => 'not_correct', + ], $args); + + return ['regex:/^inch|cm$/i']; + }, + ], + ], + ], + ]; + } +} diff --git a/tests/Unit/ValidationOfFieldArguments/TestQuery.php b/tests/Unit/ValidationOfFieldArguments/TestQuery.php new file mode 100644 index 00000000..173aab48 --- /dev/null +++ b/tests/Unit/ValidationOfFieldArguments/TestQuery.php @@ -0,0 +1,48 @@ + + */ + protected $attributes = [ + 'name' => 'test', + ]; + + public function type(): Type + { + return GraphQL::type('AccountType'); + } + + public function args(): array + { + return [ + 'id' => [ + 'type' => Type::int(), + 'rules' => ['integer', 'max:2'], + ], + ]; + } + + /** + * @param mixed $root + * @param array $args + * @param mixed $context + * @return array + */ + public function resolve($root, array $args, $context): array + { + return [ + 'name' => 'fff', + 'profile' => null, + ]; + } +} diff --git a/tests/Unit/ValidationOfFieldArguments/ValidationOfFieldArgumentsTest.php b/tests/Unit/ValidationOfFieldArguments/ValidationOfFieldArgumentsTest.php new file mode 100644 index 00000000..8b1ecd80 --- /dev/null +++ b/tests/Unit/ValidationOfFieldArguments/ValidationOfFieldArgumentsTest.php @@ -0,0 +1,82 @@ +set('graphql.types', [ + 'AccountType' => AccountType::class, + 'ProfileType' => ProfileType::class, + ]); + + $app['config']->set('graphql.schemas.default', [ + 'query' => [ + TestQuery::class, + ], + ]); + } + + public function testRulesTakesEffect(): void + { + $graphql = <<<'GRAPHQL' +query ($profileId: Int, $height: String) { + test { + id + profile(profileId: $profileId) { + name(includeMiddleNames: "maybe") + height(unit: $height) + } + + } +} +GRAPHQL; + + $result = $this->graphql($graphql, [ + 'expectErrors' => true, + 'variables' => [ + 'profileId' => 100, + 'height' => 'not_correct', + ], + ]); + + /** @var MessageBag $messageBag */ + $messageBag = $result['errors'][0]['extensions']['validation']; + $expectedMessages = [ + 'The profile.fields.name.args.include middle names format is invalid.', + 'The profile.fields.height.args.unit format is invalid.', + 'The profile.args.profile id may not be greater than 10.', + ]; + $this->assertSame($expectedMessages, $messageBag->all()); + } + + public function testOnlyApplicableRulesTakesEffect(): void + { + $graphql = <<<'GRAPHQL' +query { + test { + id + alias(type:"not_it") + } +} +GRAPHQL; + + $result = $this->graphql($graphql, [ + 'expectErrors' => true, + 'variables' => [], + ]); + + /** @var MessageBag $messageBag */ + $messageBag = $result['errors'][0]['extensions']['validation']; + $expectedMessages = [ + 'The alias.args.type format is invalid.', + ]; + $this->assertSame($expectedMessages, $messageBag->all()); + } +} From 2392babc0b2e1a42343d3f38dc3035e26214e11e Mon Sep 17 00:00:00 2001 From: Christian Date: Sun, 22 Mar 2020 02:24:26 +0100 Subject: [PATCH 02/12] fixes --- src/Support/Field.php | 7 ++++--- tests/Unit/FieldTest.php | 10 +++++++++- tests/Unit/GraphQLQueryTest.php | 2 +- tests/Unit/MutationTest.php | 32 +++++++++++++++++++++----------- 4 files changed, 35 insertions(+), 16 deletions(-) diff --git a/src/Support/Field.php b/src/Support/Field.php index c51b41d5..5217ca1b 100644 --- a/src/Support/Field.php +++ b/src/Support/Field.php @@ -123,9 +123,6 @@ protected function getResolver(): ?Closure // 4 (!) - added by this library, encapsulates creating a `SelectFields` instance $arguments = func_get_args(); - // Validate arguments in fields - $this->validateFieldArguments($arguments[3]); - // Validate mutation arguments $args = $arguments[1]; @@ -134,10 +131,14 @@ protected function getResolver(): ?Closure if (count($rules)) { $validator = $this->getValidator($args, $rules); if ($validator->fails()) { + throw new ValidationError('validation', $validator); } } + // Validate arguments in fields + $this->validateFieldArguments($arguments[3]); + $arguments[1] = $this->getArgs($arguments); // Authorize diff --git a/tests/Unit/FieldTest.php b/tests/Unit/FieldTest.php index 10bda9c0..24ae79a7 100644 --- a/tests/Unit/FieldTest.php +++ b/tests/Unit/FieldTest.php @@ -7,6 +7,7 @@ use Closure; use Rebing\GraphQL\Tests\Support\Objects\ExampleField; use Rebing\GraphQL\Tests\TestCase; +use GraphQL\Type\Definition\ResolveInfo; class FieldTest extends TestCase { @@ -15,6 +16,13 @@ protected function getFieldClass() return ExampleField::class; } + protected function resolveInfoMock() + { + return $this->getMockBuilder(ResolveInfo::class) + ->disableOriginalConstructor() + ->getMock(); + } + /** * Test get attributes. */ @@ -47,7 +55,7 @@ public function testResolve(): void ->method('resolve'); $attributes = $field->getAttributes(); - $attributes['resolve'](null, [], [], null); + $attributes['resolve'](null, [], [], $this->resolveInfoMock()); } /** diff --git a/tests/Unit/GraphQLQueryTest.php b/tests/Unit/GraphQLQueryTest.php index b18967b0..4343db1c 100644 --- a/tests/Unit/GraphQLQueryTest.php +++ b/tests/Unit/GraphQLQueryTest.php @@ -182,7 +182,7 @@ public function testQueryWithValidationError(): void $this->assertArrayHasKey('errors', $result); $this->assertArrayHasKey('extensions', $result['errors'][0]); $this->assertArrayHasKey('validation', $result['errors'][0]['extensions']); - $this->assertTrue($result['errors'][0]['extensions']['validation']->has('index')); + $this->assertTrue($result['errors'][0]['extensions']['validation']->has('test_validation.args.index')); } /** diff --git a/tests/Unit/MutationTest.php b/tests/Unit/MutationTest.php index e07b6607..cdab3259 100644 --- a/tests/Unit/MutationTest.php +++ b/tests/Unit/MutationTest.php @@ -12,6 +12,7 @@ use Rebing\GraphQL\Tests\Support\Objects\ExampleValidationInputObject; use Rebing\GraphQL\Tests\Support\Objects\UpdateExampleMutationForRuleTesting; use Rebing\GraphQL\Tests\Support\Objects\UpdateExampleMutationWithInputType; +use GraphQL\Type\Definition\ResolveInfo; class MutationTest extends FieldTest { @@ -32,6 +33,13 @@ protected function getEnvironmentSetUp($app) ]); } + protected function resolveInfoMock() + { + return $this->getMockBuilder(ResolveInfo::class) + ->disableOriginalConstructor() + ->getMock(); + } + /** * Test resolve. */ @@ -66,7 +74,7 @@ public function testResolve(): void ['email' => 'test@test.com'], ], ], - ], [], null); + ], [], $this->resolveInfoMock()); } /** @@ -79,9 +87,11 @@ public function testResolveThrowValidationError(): void $attributes = $field->getAttributes(); $this->expectException(ValidationError::class); - $attributes['resolve'](null, [], [], null); + $attributes['resolve'](null, [], [], $this->resolveInfoMock()); } + + /** * Test validation error. */ @@ -98,7 +108,7 @@ public function testValidationError(): void 'test_with_rules_non_nullable_input_object' => [ 'val' => 4, ], - ], [], null); + ], [], $this->resolveInfoMock()); } catch (ValidationError $exception) { } @@ -131,7 +141,7 @@ public function testWithInput(): void 'test_with_rules_non_nullable_input_object' => [ 'val' => 4, ], - ], [], null); + ], [], $this->resolveInfoMock()); } catch (ValidationError $e) { $validator = $e->getValidator(); @@ -160,7 +170,7 @@ public function testWithEmptyInput(): void $exception = null; try { - $attributes['resolve'](null, [], [], null); + $attributes['resolve'](null, [], [], $this->resolveInfoMock()); } catch (ValidationError $exception) { } @@ -190,7 +200,7 @@ public function testWithInputDepthOne(): void try { $attributes['resolve'](null, [ 'test_with_rules' => 'test', - ], [], null); + ], [], $this->resolveInfoMock()); } catch (ValidationError $exception) { } @@ -220,7 +230,7 @@ public function testWithInputWithEmptyInputObjects(): void $attributes['resolve'](null, [ 'test_with_rules_non_nullable_input_object' => [], 'test_with_rules_nullable_input_object' => [], - ], [], null); + ], [], $this->resolveInfoMock()); } catch (ValidationError $exception) { } @@ -259,7 +269,7 @@ public function testWithEmptyArrayOfInputsObjects(): void try { $attributes['resolve'](null, [ 'test_with_rules_non_nullable_list_of_non_nullable_input_object' => [], - ], [], null); + ], [], $this->resolveInfoMock()); } catch (ValidationError $exception) { } @@ -291,7 +301,7 @@ public function testWithArrayOfInputsObjects(): void 'val' => 1245, ], ], - ], [], null); + ], [], $this->resolveInfoMock()); } catch (ValidationError $exception) { } @@ -330,7 +340,7 @@ public function testCustomValidationErrorMessages(): void 'test_with_rules_non_nullable_input_object' => [ 'nest' => ['email' => 'invalidTestEmail.com'], ], - ], [], null); + ], [], $this->resolveInfoMock()); } catch (ValidationError $exception) { } @@ -352,6 +362,6 @@ public function testRuleCallbackArgumentsMatchesTheInput(): void 'test_with_rules_callback_params' => [ 'otherValue' => 1337, ], - ], [], null); + ], [], $this->resolveInfoMock()); } } From 17f7194bd17de9a9ce4e3d44ab920d4f572ee7e4 Mon Sep 17 00:00:00 2001 From: Christian Date: Sun, 22 Mar 2020 02:40:45 +0100 Subject: [PATCH 03/12] fix --- src/Support/Field.php | 1 - tests/Unit/FieldTest.php | 2 +- tests/Unit/MutationTest.php | 4 +--- 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/Support/Field.php b/src/Support/Field.php index 5217ca1b..90dbc5e9 100644 --- a/src/Support/Field.php +++ b/src/Support/Field.php @@ -131,7 +131,6 @@ protected function getResolver(): ?Closure if (count($rules)) { $validator = $this->getValidator($args, $rules); if ($validator->fails()) { - throw new ValidationError('validation', $validator); } } diff --git a/tests/Unit/FieldTest.php b/tests/Unit/FieldTest.php index 24ae79a7..17d0bb60 100644 --- a/tests/Unit/FieldTest.php +++ b/tests/Unit/FieldTest.php @@ -5,9 +5,9 @@ namespace Rebing\GraphQL\Tests\Unit; use Closure; +use GraphQL\Type\Definition\ResolveInfo; use Rebing\GraphQL\Tests\Support\Objects\ExampleField; use Rebing\GraphQL\Tests\TestCase; -use GraphQL\Type\Definition\ResolveInfo; class FieldTest extends TestCase { diff --git a/tests/Unit/MutationTest.php b/tests/Unit/MutationTest.php index cdab3259..460fe643 100644 --- a/tests/Unit/MutationTest.php +++ b/tests/Unit/MutationTest.php @@ -4,6 +4,7 @@ namespace Rebing\GraphQL\Tests\Unit; +use GraphQL\Type\Definition\ResolveInfo; use Illuminate\Validation\Validator; use Rebing\GraphQL\Error\ValidationError; use Rebing\GraphQL\Tests\Support\Objects\ExampleNestedValidationInputObject; @@ -12,7 +13,6 @@ use Rebing\GraphQL\Tests\Support\Objects\ExampleValidationInputObject; use Rebing\GraphQL\Tests\Support\Objects\UpdateExampleMutationForRuleTesting; use Rebing\GraphQL\Tests\Support\Objects\UpdateExampleMutationWithInputType; -use GraphQL\Type\Definition\ResolveInfo; class MutationTest extends FieldTest { @@ -90,8 +90,6 @@ public function testResolveThrowValidationError(): void $attributes['resolve'](null, [], [], $this->resolveInfoMock()); } - - /** * Test validation error. */ From 9984377487e27fb0acc3410a3fdee067eae5bde3 Mon Sep 17 00:00:00 2001 From: Christian Date: Sun, 22 Mar 2020 11:48:43 +0100 Subject: [PATCH 04/12] fixes --- tests/Unit/FieldTest.php | 3 ++- tests/Unit/MutationTest.php | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/Unit/FieldTest.php b/tests/Unit/FieldTest.php index 17d0bb60..dd78622e 100644 --- a/tests/Unit/FieldTest.php +++ b/tests/Unit/FieldTest.php @@ -8,6 +8,7 @@ use GraphQL\Type\Definition\ResolveInfo; use Rebing\GraphQL\Tests\Support\Objects\ExampleField; use Rebing\GraphQL\Tests\TestCase; +use PHPUnit\Framework\MockObject\MockObject; class FieldTest extends TestCase { @@ -16,7 +17,7 @@ protected function getFieldClass() return ExampleField::class; } - protected function resolveInfoMock() + protected function resolveInfoMock(): MockObject { return $this->getMockBuilder(ResolveInfo::class) ->disableOriginalConstructor() diff --git a/tests/Unit/MutationTest.php b/tests/Unit/MutationTest.php index 460fe643..276fd8ab 100644 --- a/tests/Unit/MutationTest.php +++ b/tests/Unit/MutationTest.php @@ -13,6 +13,7 @@ use Rebing\GraphQL\Tests\Support\Objects\ExampleValidationInputObject; use Rebing\GraphQL\Tests\Support\Objects\UpdateExampleMutationForRuleTesting; use Rebing\GraphQL\Tests\Support\Objects\UpdateExampleMutationWithInputType; +use PHPUnit\Framework\MockObject\MockObject; class MutationTest extends FieldTest { @@ -33,7 +34,7 @@ protected function getEnvironmentSetUp($app) ]); } - protected function resolveInfoMock() + protected function resolveInfoMock(): MockObject { return $this->getMockBuilder(ResolveInfo::class) ->disableOriginalConstructor() From f96922e27cf6a31da502aded9765b018175f6381 Mon Sep 17 00:00:00 2001 From: Christian Date: Sun, 22 Mar 2020 11:52:44 +0100 Subject: [PATCH 05/12] fix --- tests/Unit/FieldTest.php | 2 +- tests/Unit/MutationTest.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Unit/FieldTest.php b/tests/Unit/FieldTest.php index dd78622e..12b8be8e 100644 --- a/tests/Unit/FieldTest.php +++ b/tests/Unit/FieldTest.php @@ -6,9 +6,9 @@ use Closure; use GraphQL\Type\Definition\ResolveInfo; +use PHPUnit\Framework\MockObject\MockObject; use Rebing\GraphQL\Tests\Support\Objects\ExampleField; use Rebing\GraphQL\Tests\TestCase; -use PHPUnit\Framework\MockObject\MockObject; class FieldTest extends TestCase { diff --git a/tests/Unit/MutationTest.php b/tests/Unit/MutationTest.php index 276fd8ab..e8b5830c 100644 --- a/tests/Unit/MutationTest.php +++ b/tests/Unit/MutationTest.php @@ -6,6 +6,7 @@ use GraphQL\Type\Definition\ResolveInfo; use Illuminate\Validation\Validator; +use PHPUnit\Framework\MockObject\MockObject; use Rebing\GraphQL\Error\ValidationError; use Rebing\GraphQL\Tests\Support\Objects\ExampleNestedValidationInputObject; use Rebing\GraphQL\Tests\Support\Objects\ExampleRuleTestingInputObject; @@ -13,7 +14,6 @@ use Rebing\GraphQL\Tests\Support\Objects\ExampleValidationInputObject; use Rebing\GraphQL\Tests\Support\Objects\UpdateExampleMutationForRuleTesting; use Rebing\GraphQL\Tests\Support\Objects\UpdateExampleMutationWithInputType; -use PHPUnit\Framework\MockObject\MockObject; class MutationTest extends FieldTest { From 00b922da782c8e3ea6335ff49bda66cd1d3e3db1 Mon Sep 17 00:00:00 2001 From: Christian Date: Thu, 26 Mar 2020 22:17:19 +0100 Subject: [PATCH 06/12] update readme --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5cb3d535..28a85fc2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ CHANGELOG 2019-04-03, 5.0.0 ----------------- ### Added +- Add support for validation in field arguments [\#608 / crissi](https://github.com/rebing/graphql-laravel/pull/608) - Support Laravel 7 [\#597 / exodusanto](https://github.com/rebing/graphql-laravel/pull/597) - Add support for custom authorization message [\#578 / Sh1d0w](https://github.com/rebing/graphql-laravel/pull/578) - Add support for macros on the GraphQL service/facade [\#592 / stevelacey](https://github.com/rebing/graphql-laravel/pull/592) From 689a1151e56177bc465970c34f2ca1d176aff078 Mon Sep 17 00:00:00 2001 From: Christian Date: Fri, 10 Apr 2020 21:09:35 +0200 Subject: [PATCH 07/12] refactor --- phpstan-baseline.neon | 15 ------- src/Support/Field.php | 45 ++++++++++++++----- src/Support/RulesInFields.php | 38 ++++++---------- src/Support/SelectFields.php | 20 +++++---- .../AccountType.php | 4 +- 5 files changed, 63 insertions(+), 59 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 5f92d2e7..1b7d4ccc 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -206,11 +206,6 @@ parameters: count: 1 path: src/Support/Field.php - - - message: "#^Method Rebing\\\\GraphQL\\\\Support\\\\Field\\:\\:instanciateSelectFields\\(\\) has parameter \\$arguments with no value type specified in iterable type array\\.$#" - count: 1 - path: src/Support/Field.php - - message: "#^Method Rebing\\\\GraphQL\\\\Support\\\\Field\\:\\:aliasArgs\\(\\) has parameter \\$arguments with no value type specified in iterable type array\\.$#" count: 1 @@ -306,16 +301,6 @@ parameters: count: 1 path: src/Support/SelectFields.php - - - message: "#^Method Rebing\\\\GraphQL\\\\Support\\\\SelectFields\\:\\:getFieldSelection\\(\\) has parameter \\$args with no value type specified in iterable type array\\.$#" - count: 1 - path: src/Support/SelectFields.php - - - - message: "#^Method Rebing\\\\GraphQL\\\\Support\\\\SelectFields\\:\\:getFieldSelection\\(\\) return type has no value type specified in iterable type array\\.$#" - count: 1 - path: src/Support/SelectFields.php - - message: "#^Method Rebing\\\\GraphQL\\\\Support\\\\SelectFields\\:\\:getSelectableFieldsAndRelations\\(\\) has parameter \\$queryArgs with no value type specified in iterable type array\\.$#" count: 1 diff --git a/src/Support/Field.php b/src/Support/Field.php index 90dbc5e9..c45ef839 100644 --- a/src/Support/Field.php +++ b/src/Support/Field.php @@ -20,6 +20,13 @@ */ abstract class Field { + /** + * The depth the SelectField and ResolveInfoFieldsAndArguments classes traverse + * + * @var integer + */ + protected $depth = 5; + protected $attributes = []; /** @@ -86,12 +93,16 @@ public function getRules(array $arguments = []): array return array_merge($argsRules, $rules); } - public function validateFieldArguments(ResolveInfo $resolveInfo): void + /** + * @param array $fieldsAndArgumentsSelection + * @return void + */ + public function validateFieldArguments(array $fieldsAndArgumentsSelection): void { - [$argumentValues, $argsRules] = (new RulesInFields($this->type(), $resolveInfo))->get(); + $argsRules = (new RulesInFields($this->type(), $fieldsAndArgumentsSelection))->get(); if (count($argsRules)) { - $validator = $this->getValidator($argumentValues, $argsRules); + $validator = $this->getValidator($fieldsAndArgumentsSelection, $argsRules); if ($validator->fails()) { throw new ValidationError('validation', $validator); } @@ -135,8 +146,10 @@ protected function getResolver(): ?Closure } } + $fieldsAndArguments = (new ResolveInfoFieldsAndArguments($arguments[3]))->getFieldsAndArgumentsSelection($this->depth); + // Validate arguments in fields - $this->validateFieldArguments($arguments[3]); + $this->validateFieldArguments($fieldsAndArguments); $arguments[1] = $this->getArgs($arguments); @@ -149,7 +162,7 @@ protected function getResolver(): ?Closure $additionalParams = array_slice($method->getParameters(), 3); - $additionalArguments = array_map(function ($param) use ($arguments) { + $additionalArguments = array_map(function ($param) use ($arguments, $fieldsAndArguments) { $className = null !== $param->getClass() ? $param->getClass()->getName() : null; if (null === $className) { @@ -157,13 +170,13 @@ protected function getResolver(): ?Closure } if (Closure::class === $param->getType()->getName()) { - return function (int $depth = null) use ($arguments): SelectFields { - return $this->instanciateSelectFields($arguments, $depth); + return function (int $depth = null) use ($arguments, $fieldsAndArguments): SelectFields { + return $this->instanciateSelectFields($arguments, $depth, $fieldsAndArguments); }; } if (SelectFields::class === $className) { - return $this->instanciateSelectFields($arguments); + return $this->instanciateSelectFields($arguments, null, $fieldsAndArguments); } if (ResolveInfo::class === $className) { @@ -180,11 +193,23 @@ protected function getResolver(): ?Closure }; } - private function instanciateSelectFields(array $arguments, int $depth = null): SelectFields + /** + + * @param array $arguments + * @param integer $depth + * @param array $fieldsAndArguments + * @return SelectFields + */ + private function instanciateSelectFields(array $arguments, int $depth = null, array $fieldsAndArguments): SelectFields { $ctx = $arguments[2] ?? null; - return new SelectFields($arguments[3], $this->type(), $arguments[1], $depth ?? 5, $ctx); + if ($depth !== null && $depth !== $this->depth) { + $fieldsAndArguments = (new ResolveInfoFieldsAndArguments($arguments[3])) + ->getFieldsAndArgumentsSelection($depth); + } + + return new SelectFields($this->type(), $arguments[1], $ctx, $fieldsAndArguments); } protected function aliasArgs(array $arguments): array diff --git a/src/Support/RulesInFields.php b/src/Support/RulesInFields.php index 799ad97c..b2da9c5c 100644 --- a/src/Support/RulesInFields.php +++ b/src/Support/RulesInFields.php @@ -14,26 +14,23 @@ class RulesInFields /** * @var Type */ - private $parentType; - /** - * @var ResolveInfo - */ - private $resolveInfo; + protected $parentType; /** * @var array */ - private $fieldsAndArguments; + protected $fieldsAndArguments; /** * @param Type $parentType - * @param ResolveInfo $resolveInfo + * @param array $fieldsAndArgumentsSelection */ - public function __construct(Type $parentType, ResolveInfo $resolveInfo) + public function __construct(Type $parentType, array $fieldsAndArgumentsSelection) { - $this->parentType = $parentType; - $this->resolveInfo = $resolveInfo; - $this->fieldsAndArguments = (new ResolveInfoFieldsAndArguments($this->resolveInfo))->getFieldsAndArgumentsSelection(5); + $this->parentType = $parentType instanceof WrappingType + ? $parentType->getWrappedType(true) + : $parentType; + $this->fieldsAndArguments = $fieldsAndArgumentsSelection; } /** @@ -41,11 +38,7 @@ public function __construct(Type $parentType, ResolveInfo $resolveInfo) */ public function get(): array { - if ($this->parentType instanceof WrappingType) { - $this->parentType = $this->parentType->getWrappedType(true); - } - - return [$this->fieldsAndArguments, $this->getRules($this->fieldsAndArguments, null, $this->parentType)]; + return $this->getRules($this->fieldsAndArguments, null, $this->parentType); } /** @@ -56,7 +49,7 @@ public function get(): array protected function resolveRules($rules, array $arguments) { if (is_callable($rules)) { - return call_user_func($rules, $arguments, $this->fieldsAndArguments); + return call_user_func($rules, $arguments); } return $rules; @@ -67,6 +60,7 @@ protected function resolveRules($rules, array $arguments) * * @param array $fields * @param string|null $prefix + * @param Type $parentType * @return array */ protected function getRules(array $fields, ?string $prefix, Type $parentType): array @@ -77,13 +71,9 @@ protected function getRules(array $fields, ?string $prefix, Type $parentType): a $key = $prefix === null ? $name : "{$prefix}.{$name}"; //If field doesn't exist on definition we don't select it - try { - if (method_exists($parentType, 'getField')) { - $fieldObject = $parentType->getField($name); - } else { - continue; - } - } catch (InvariantViolation $e) { + if (method_exists($parentType, 'getField')) { + $fieldObject = $parentType->getField($name); + } else { continue; } diff --git a/src/Support/SelectFields.php b/src/Support/SelectFields.php index c5e6b61b..52d869b3 100644 --- a/src/Support/SelectFields.php +++ b/src/Support/SelectFields.php @@ -32,32 +32,36 @@ class SelectFields const ALWAYS_RELATION_KEY = 'ALWAYS_RELATION_KEY'; /** - * @param ResolveInfo $info * @param GraphqlType $parentType * @param array $queryArgs Arguments given with the query/mutation - * @param int $depth The depth to walk the AST and introspect for nested relations * @param mixed $ctx The GraphQL context; can be anything and is only passed through * Can be created/overridden by \Rebing\GraphQL\GraphQLController::queryContext + * @param array $fieldsAndArguments Field and argument tree */ - public function __construct(ResolveInfo $info, GraphqlType $parentType, array $queryArgs, int $depth, $ctx) + public function __construct(GraphqlType $parentType, array $queryArgs, $ctx, array $fieldsAndArguments) { if ($parentType instanceof WrappingType) { $parentType = $parentType->getWrappedType(true); } - $requestedFields = $this->getFieldSelection($info, $queryArgs, $depth); + $requestedFields = $this->getFieldSelection($fieldsAndArguments, $queryArgs); $fields = self::getSelectableFieldsAndRelations($queryArgs, $requestedFields, $parentType, null, true, $ctx); $this->select = $fields[0]; $this->relations = $fields[1]; } - private function getFieldSelection(ResolveInfo $resolveInfo, array $args, int $depth): array + /** + * Undocumented function + * + * @param array $fieldsAndArguments + * @param array $args + * @return array + */ + private function getFieldSelection(array $fieldsAndArguments, array $args): array { - $resolveInfoFieldsAndArguments = new ResolveInfoFieldsAndArguments($resolveInfo); - return [ 'args' => $args, - 'fields' => $resolveInfoFieldsAndArguments->getFieldsAndArgumentsSelection($depth), + 'fields' => $fieldsAndArguments, ]; } diff --git a/tests/Unit/ValidationOfFieldArguments/AccountType.php b/tests/Unit/ValidationOfFieldArguments/AccountType.php index 71164d78..ad40078e 100644 --- a/tests/Unit/ValidationOfFieldArguments/AccountType.php +++ b/tests/Unit/ValidationOfFieldArguments/AccountType.php @@ -31,7 +31,7 @@ public function fields(): array 'args' => [ 'profileId' => [ 'type' => Type::int(), - 'rules' => function ($args, $fullQueryArgs) { + 'rules' => function ($args) { Assert::assertSame([ 'profileId' => 100, ], $args); @@ -46,7 +46,7 @@ public function fields(): array 'args' => [ 'type' => [ 'type' => Type::string(), - 'rules' => function ($args, $fullQueryArgs) { + 'rules' => function ($args) { return ['regex:/^l33t|normal$/i']; }, ], From a63ed1790454e04f35cbbd61fe74d5f753b254c0 Mon Sep 17 00:00:00 2001 From: Christian Date: Fri, 10 Apr 2020 21:15:27 +0200 Subject: [PATCH 08/12] fix style --- src/Support/Field.php | 8 +++----- src/Support/RulesInFields.php | 2 -- src/Support/SelectFields.php | 3 +-- 3 files changed, 4 insertions(+), 9 deletions(-) diff --git a/src/Support/Field.php b/src/Support/Field.php index c45ef839..b723139a 100644 --- a/src/Support/Field.php +++ b/src/Support/Field.php @@ -21,9 +21,9 @@ abstract class Field { /** - * The depth the SelectField and ResolveInfoFieldsAndArguments classes traverse + * The depth the SelectField and ResolveInfoFieldsAndArguments classes traverse. * - * @var integer + * @var int */ protected $depth = 5; @@ -99,7 +99,6 @@ public function getRules(array $arguments = []): array */ public function validateFieldArguments(array $fieldsAndArgumentsSelection): void { - $argsRules = (new RulesInFields($this->type(), $fieldsAndArgumentsSelection))->get(); if (count($argsRules)) { $validator = $this->getValidator($fieldsAndArgumentsSelection, $argsRules); @@ -194,9 +193,8 @@ protected function getResolver(): ?Closure } /** - * @param array $arguments - * @param integer $depth + * @param int $depth * @param array $fieldsAndArguments * @return SelectFields */ diff --git a/src/Support/RulesInFields.php b/src/Support/RulesInFields.php index b2da9c5c..b5801b6d 100644 --- a/src/Support/RulesInFields.php +++ b/src/Support/RulesInFields.php @@ -4,8 +4,6 @@ namespace Rebing\GraphQL\Support; -use GraphQL\Error\InvariantViolation; -use GraphQL\Type\Definition\ResolveInfo; use GraphQL\Type\Definition\Type; use GraphQL\Type\Definition\WrappingType; diff --git a/src/Support/SelectFields.php b/src/Support/SelectFields.php index 52d869b3..6258ae02 100644 --- a/src/Support/SelectFields.php +++ b/src/Support/SelectFields.php @@ -7,7 +7,6 @@ use Closure; use GraphQL\Error\InvariantViolation; use GraphQL\Type\Definition\FieldDefinition; -use GraphQL\Type\Definition\ResolveInfo; use GraphQL\Type\Definition\Type as GraphqlType; use GraphQL\Type\Definition\UnionType; use GraphQL\Type\Definition\WrappingType; @@ -51,7 +50,7 @@ public function __construct(GraphqlType $parentType, array $queryArgs, $ctx, arr } /** - * Undocumented function + * Undocumented function. * * @param array $fieldsAndArguments * @param array $args From 427d6d9ead1ceb976b52a9e22bcd80cff96302ac Mon Sep 17 00:00:00 2001 From: Christian Date: Mon, 20 Apr 2020 20:47:42 +0200 Subject: [PATCH 09/12] fixes --- CHANGELOG.md | 3 ++- src/Support/Field.php | 6 +++--- src/Support/RulesInFields.php | 10 +++++----- src/Support/SelectFields.php | 21 +++++---------------- 4 files changed, 15 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 28a85fc2..78e835df 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,11 +3,12 @@ CHANGELOG [Next release](https://github.com/rebing/graphql-laravel/compare/5.0.0...master) -------------- +### Added +- Add support for validation in field arguments [\#608 / crissi](https://github.com/rebing/graphql-laravel/pull/608) 2019-04-03, 5.0.0 ----------------- ### Added -- Add support for validation in field arguments [\#608 / crissi](https://github.com/rebing/graphql-laravel/pull/608) - Support Laravel 7 [\#597 / exodusanto](https://github.com/rebing/graphql-laravel/pull/597) - Add support for custom authorization message [\#578 / Sh1d0w](https://github.com/rebing/graphql-laravel/pull/578) - Add support for macros on the GraphQL service/facade [\#592 / stevelacey](https://github.com/rebing/graphql-laravel/pull/592) diff --git a/src/Support/Field.php b/src/Support/Field.php index b723139a..e904a440 100644 --- a/src/Support/Field.php +++ b/src/Support/Field.php @@ -170,12 +170,12 @@ protected function getResolver(): ?Closure if (Closure::class === $param->getType()->getName()) { return function (int $depth = null) use ($arguments, $fieldsAndArguments): SelectFields { - return $this->instanciateSelectFields($arguments, $depth, $fieldsAndArguments); + return $this->instanciateSelectFields($arguments, $fieldsAndArguments, $depth); }; } if (SelectFields::class === $className) { - return $this->instanciateSelectFields($arguments, null, $fieldsAndArguments); + return $this->instanciateSelectFields($arguments, $fieldsAndArguments, null); } if (ResolveInfo::class === $className) { @@ -198,7 +198,7 @@ protected function getResolver(): ?Closure * @param array $fieldsAndArguments * @return SelectFields */ - private function instanciateSelectFields(array $arguments, int $depth = null, array $fieldsAndArguments): SelectFields + private function instanciateSelectFields(array $arguments, array $fieldsAndArguments, int $depth = null): SelectFields { $ctx = $arguments[2] ?? null; diff --git a/src/Support/RulesInFields.php b/src/Support/RulesInFields.php index b5801b6d..56973bf8 100644 --- a/src/Support/RulesInFields.php +++ b/src/Support/RulesInFields.php @@ -15,7 +15,7 @@ class RulesInFields protected $parentType; /** - * @var array + * @var array */ protected $fieldsAndArguments; @@ -47,7 +47,7 @@ public function get(): array protected function resolveRules($rules, array $arguments) { if (is_callable($rules)) { - return call_user_func($rules, $arguments); + return $rules($arguments); } return $rules; @@ -69,12 +69,12 @@ protected function getRules(array $fields, ?string $prefix, Type $parentType): a $key = $prefix === null ? $name : "{$prefix}.{$name}"; //If field doesn't exist on definition we don't select it - if (method_exists($parentType, 'getField')) { - $fieldObject = $parentType->getField($name); - } else { + if (!method_exists($parentType, 'getField')) { continue; } + $fieldObject = $parentType->getField($name); + if (is_array($field['fields'])) { $rules = $rules + $this->getRules($field['fields'], $key.'.fields', $fieldObject->getType()); } diff --git a/src/Support/SelectFields.php b/src/Support/SelectFields.php index 6258ae02..d6cd0add 100644 --- a/src/Support/SelectFields.php +++ b/src/Support/SelectFields.php @@ -43,27 +43,16 @@ public function __construct(GraphqlType $parentType, array $queryArgs, $ctx, arr $parentType = $parentType->getWrappedType(true); } - $requestedFields = $this->getFieldSelection($fieldsAndArguments, $queryArgs); + $requestedFields = [ + 'args' => $queryArgs, + 'fields' => $fieldsAndArguments + ]; + $fields = self::getSelectableFieldsAndRelations($queryArgs, $requestedFields, $parentType, null, true, $ctx); $this->select = $fields[0]; $this->relations = $fields[1]; } - /** - * Undocumented function. - * - * @param array $fieldsAndArguments - * @param array $args - * @return array - */ - private function getFieldSelection(array $fieldsAndArguments, array $args): array - { - return [ - 'args' => $args, - 'fields' => $fieldsAndArguments, - ]; - } - /** * Retrieve the fields (top level) and relations that * will be selected with the query. From cf8c536682e881b366ca48fa9c0e2ee91e4ddb0d Mon Sep 17 00:00:00 2001 From: Christian Date: Mon, 20 Apr 2020 21:19:35 +0200 Subject: [PATCH 10/12] fix --- src/Support/RulesInFields.php | 2 +- src/Support/SelectFields.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Support/RulesInFields.php b/src/Support/RulesInFields.php index 56973bf8..a917a464 100644 --- a/src/Support/RulesInFields.php +++ b/src/Support/RulesInFields.php @@ -69,7 +69,7 @@ protected function getRules(array $fields, ?string $prefix, Type $parentType): a $key = $prefix === null ? $name : "{$prefix}.{$name}"; //If field doesn't exist on definition we don't select it - if (!method_exists($parentType, 'getField')) { + if (! method_exists($parentType, 'getField')) { continue; } diff --git a/src/Support/SelectFields.php b/src/Support/SelectFields.php index d6cd0add..152d7cf7 100644 --- a/src/Support/SelectFields.php +++ b/src/Support/SelectFields.php @@ -45,7 +45,7 @@ public function __construct(GraphqlType $parentType, array $queryArgs, $ctx, arr $requestedFields = [ 'args' => $queryArgs, - 'fields' => $fieldsAndArguments + 'fields' => $fieldsAndArguments, ]; $fields = self::getSelectableFieldsAndRelations($queryArgs, $requestedFields, $parentType, null, true, $ctx); From b91e7df14542e8a516c5ad0ea66228af0452a602 Mon Sep 17 00:00:00 2001 From: Markus Podar Date: Mon, 20 Apr 2020 23:11:47 +0200 Subject: [PATCH 11/12] travis: update composer --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index be83c8d0..4244b815 100644 --- a/.travis.yml +++ b/.travis.yml @@ -45,6 +45,7 @@ before_script: - phpenv config-rm xdebug.ini || true install: + - composer self-update - | # Due to version incompatiblity with older Laravels we test, we remove it composer remove --dev matt-allan/laravel-code-style --no-interaction --no-update From 82438f7fcf6d172b14a09f899a69658da29f456c Mon Sep 17 00:00:00 2001 From: Markus Podar Date: Mon, 20 Apr 2020 23:19:13 +0200 Subject: [PATCH 12/12] travis: update composer to the latest snapshot The new 2.0 as of now --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 4244b815..fe9a31ee 100644 --- a/.travis.yml +++ b/.travis.yml @@ -45,7 +45,7 @@ before_script: - phpenv config-rm xdebug.ini || true install: - - composer self-update + - composer self-update --snapshot - | # Due to version incompatiblity with older Laravels we test, we remove it composer remove --dev matt-allan/laravel-code-style --no-interaction --no-update