From 8f189d53956b990ab659ced2bce8330cda877b1c Mon Sep 17 00:00:00 2001 From: Markus Podar Date: Wed, 21 Apr 2021 22:15:16 +0200 Subject: [PATCH] Return messages directly and not the bag I don't think the intended output was ever to have the MessageBag being an intermediary for serializing the response, when directly returning the messages yields the same result. The gracious changes in the tests result from not having the MessageBag available anymore and making the shortcut just comparing the whole response. IMHO it also gives a clearer indication how such a thing looks like (the shape) and find the individual probes into the result harder to grasp in the big picture. --- src/GraphQL.php | 2 +- ...ionValidationUniqueWithCustomRulesTest.php | 96 ++++++++++++++----- tests/Unit/GraphQLQueryTest.php | 33 ++++++- tests/Unit/GraphQLTest.php | 19 +++- .../MutationCustomRulesTest.php | 61 +++++++++--- ...utationValidationInWithCustomRulesTest.php | 96 ++++++++++++++----- .../ValidationAuthorizationTest.php | 32 +++++-- .../ValidationOfFieldArgumentsTest.php | 79 +++++++++++---- 8 files changed, 326 insertions(+), 92 deletions(-) diff --git a/src/GraphQL.php b/src/GraphQL.php index 65b93de3..92172dc1 100644 --- a/src/GraphQL.php +++ b/src/GraphQL.php @@ -435,7 +435,7 @@ public static function formatError(Error $e): array } if ($previous instanceof ValidationError) { - $error['extensions']['validation'] = $previous->getValidatorMessages(); + $error['extensions']['validation'] = $previous->getValidatorMessages()->getMessages(); } } diff --git a/tests/Database/MutationValidationUniqueWithCustomRulesTests/MutationValidationUniqueWithCustomRulesTest.php b/tests/Database/MutationValidationUniqueWithCustomRulesTests/MutationValidationUniqueWithCustomRulesTest.php index 7711ffd4..d7a00767 100644 --- a/tests/Database/MutationValidationUniqueWithCustomRulesTests/MutationValidationUniqueWithCustomRulesTest.php +++ b/tests/Database/MutationValidationUniqueWithCustomRulesTests/MutationValidationUniqueWithCustomRulesTest.php @@ -3,7 +3,6 @@ declare(strict_types = 1); namespace Rebing\GraphQL\Tests\Database\MutationValidationUniqueWithCustomRulesTests; -use Illuminate\Contracts\Support\MessageBag; use Rebing\GraphQL\Tests\Support\Models\User; use Rebing\GraphQL\Tests\Support\Traits\SqlAssertionTrait; use Rebing\GraphQL\Tests\TestCaseDatabase; @@ -89,14 +88,31 @@ public function testUniqueFailRulePass(): void SQL ); - self::assertCount(1, $result['errors']); - self::assertSame('validation', $result['errors'][0]['message']); - /** @var MessageBag $messageBag */ - $messageBag = $result['errors'][0]['extensions']['validation']; - $expectedMessages = [ - 'The arg unique rule pass has already been taken.', + $expected = [ + 'errors' => [ + [ + 'message' => 'validation', + 'extensions' => [ + 'category' => 'validation', + 'validation' => [ + 'arg_unique_rule_pass' => [ + 'The arg unique rule pass has already been taken.', + ], + ], + ], + 'locations' => [ + [ + 'line' => 2, + 'column' => 3, + ], + ], + 'path' => [ + 'mutationWithCustomRuleWithRuleObject', + ], + ], + ], ]; - self::assertSame($expectedMessages, $messageBag->all()); + self::assertEquals($expected, $result); } public function testUniquePassRuleFail(): void @@ -122,14 +138,31 @@ public function testUniquePassRuleFail(): void ], ]); - self::assertCount(1, $result['errors']); - self::assertSame('validation', $result['errors'][0]['message']); - /** @var MessageBag $messageBag */ - $messageBag = $result['errors'][0]['extensions']['validation']; - $expectedMessages = [ - 'rule object validation fails', + $expected = [ + 'errors' => [ + [ + 'message' => 'validation', + 'extensions' => [ + 'category' => 'validation', + 'validation' => [ + 'arg_unique_rule_fail' => [ + 'rule object validation fails', + ], + ], + ], + 'locations' => [ + [ + 'line' => 2, + 'column' => 3, + ], + ], + 'path' => [ + 'mutationWithCustomRuleWithRuleObject', + ], + ], + ], ]; - self::assertSame($expectedMessages, $messageBag->all()); + self::assertEquals($expected, $result); } public function testUniqueFailRuleFail(): void @@ -161,15 +194,32 @@ public function testUniqueFailRuleFail(): void SQL ); - self::assertCount(1, $result['errors']); - self::assertSame('validation', $result['errors'][0]['message']); - /** @var MessageBag $messageBag */ - $messageBag = $result['errors'][0]['extensions']['validation']; - $expectedMessages = [ - 'The arg unique rule fail has already been taken.', - 'rule object validation fails', + $expected = [ + 'errors' => [ + [ + 'message' => 'validation', + 'extensions' => [ + 'category' => 'validation', + 'validation' => [ + 'arg_unique_rule_fail' => [ + 'The arg unique rule fail has already been taken.', + 'rule object validation fails', + ], + ], + ], + 'locations' => [ + [ + 'line' => 2, + 'column' => 3, + ], + ], + 'path' => [ + 'mutationWithCustomRuleWithRuleObject', + ], + ], + ], ]; - self::assertSame($expectedMessages, $messageBag->all()); + self::assertEquals($expected, $result); } public function testErrorExtension(): void diff --git a/tests/Unit/GraphQLQueryTest.php b/tests/Unit/GraphQLQueryTest.php index 7d648c7b..31971c78 100644 --- a/tests/Unit/GraphQLQueryTest.php +++ b/tests/Unit/GraphQLQueryTest.php @@ -151,11 +151,34 @@ public function testQueryWithValidationError(): void 'expectErrors' => true, ]); - self::assertArrayHasKey('data', $result); - self::assertArrayHasKey('errors', $result); - self::assertArrayHasKey('extensions', $result['errors'][0]); - self::assertArrayHasKey('validation', $result['errors'][0]['extensions']); - self::assertTrue($result['errors'][0]['extensions']['validation']->has('test_validation.args.index')); + $expected = [ + 'errors' => [ + [ + 'message' => 'validation', + 'extensions' => [ + 'category' => 'validation', + 'validation' => [ + 'test_validation.args.index' => [ + 'The test validation.args.index field is required.', + ], + ], + ], + 'locations' => [ + [ + 'line' => 3, + 'column' => 13, + ], + ], + 'path' => [ + 'examples', + ], + ], + ], + 'data' => [ + 'examples' => null, + ], + ]; + self::assertEquals($expected, $result); } public function testQueryWithValidation(): void diff --git a/tests/Unit/GraphQLTest.php b/tests/Unit/GraphQLTest.php index 75864655..d98cf4ef 100644 --- a/tests/Unit/GraphQLTest.php +++ b/tests/Unit/GraphQLTest.php @@ -349,10 +349,21 @@ public function testFormatValidationError(): void $error = new Error('error', null, null, [], null, $validationError); $error = GraphQL::formatError($error); - self::assertIsArray($error); - self::assertArrayHasKey('extensions', $error); - self::assertArrayHasKey('validation', $error['extensions']); - self::assertTrue($error['extensions']['validation']->has('test')); + self::assertArrayHasKey('trace', $error); + unset($error['trace']); + + $expected = [ + 'message' => 'error', + 'extensions' => [ + 'category' => 'validation', + 'validation' => [ + 'test' => [ + 'The test field is required.', + ], + ], + ], + ]; + self::assertEquals($expected, $error); } public function testAddType(): void diff --git a/tests/Unit/MutationCustomRulesTests/MutationCustomRulesTest.php b/tests/Unit/MutationCustomRulesTests/MutationCustomRulesTest.php index 06480420..1e646010 100644 --- a/tests/Unit/MutationCustomRulesTests/MutationCustomRulesTest.php +++ b/tests/Unit/MutationCustomRulesTests/MutationCustomRulesTest.php @@ -3,7 +3,6 @@ declare(strict_types = 1); namespace Rebing\GraphQL\Tests\Unit\MutationCustomRulesTests; -use Illuminate\Contracts\Support\MessageBag; use Rebing\GraphQL\Tests\TestCase; class MutationCustomRulesTest extends TestCase @@ -35,11 +34,31 @@ public function testMutationWithCustomRuleWithClosure(): void ], ]); - self::assertCount(1, $result['errors']); - self::assertSame('validation', $result['errors'][0]['message']); - /** @var MessageBag $messageBag */ - $messageBag = $result['errors'][0]['extensions']['validation']; - self::assertSame(['arg1 is invalid'], $messageBag->all()); + $expected = [ + 'errors' => [ + [ + 'message' => 'validation', + 'extensions' => [ + 'category' => 'validation', + 'validation' => [ + 'arg1' => [ + 'arg1 is invalid', + ], + ], + ], + 'locations' => [ + [ + 'line' => 2, + 'column' => 3, + ], + ], + 'path' => [ + 'mutationWithCustomRuleWithClosure', + ], + ], + ], + ]; + self::assertEquals($expected, $result); } public function testMutationWithCustomRuleWithRuleObject(): void @@ -57,10 +76,30 @@ public function testMutationWithCustomRuleWithRuleObject(): void ], ]); - self::assertCount(1, $result['errors']); - self::assertSame('validation', $result['errors'][0]['message']); - /** @var MessageBag $messageBag */ - $messageBag = $result['errors'][0]['extensions']['validation']; - self::assertSame(['arg1 is invalid'], $messageBag->all()); + $expected = [ + 'errors' => [ + [ + 'message' => 'validation', + 'extensions' => [ + 'category' => 'validation', + 'validation' => [ + 'arg1' => [ + 'arg1 is invalid', + ], + ], + ], + 'locations' => [ + [ + 'line' => 2, + 'column' => 3, + ], + ], + 'path' => [ + 'mutationWithCustomRuleWithRuleObject', + ], + ], + ], + ]; + self::assertEquals($expected, $result); } } diff --git a/tests/Unit/MutationValidationInWithCustomRulesTests/MutationValidationInWithCustomRulesTest.php b/tests/Unit/MutationValidationInWithCustomRulesTests/MutationValidationInWithCustomRulesTest.php index ac1c9a3d..cd5ec08c 100644 --- a/tests/Unit/MutationValidationInWithCustomRulesTests/MutationValidationInWithCustomRulesTest.php +++ b/tests/Unit/MutationValidationInWithCustomRulesTests/MutationValidationInWithCustomRulesTest.php @@ -3,7 +3,6 @@ declare(strict_types = 1); namespace Rebing\GraphQL\Tests\Unit\MutationValidationInWithCustomRulesTests; -use Illuminate\Contracts\Support\MessageBag; use Rebing\GraphQL\Tests\TestCase; class MutationValidationInWithCustomRulesTest extends TestCase @@ -54,14 +53,31 @@ public function testInPassRuleFail(): void ], ]); - self::assertCount(1, $result['errors']); - self::assertSame('validation', $result['errors'][0]['message']); - /** @var MessageBag $messageBag */ - $messageBag = $result['errors'][0]['extensions']['validation']; - $expectedMessages = [ - 'rule object validation fails', + $expected = [ + 'errors' => [ + [ + 'message' => 'validation', + 'extensions' => [ + 'category' => 'validation', + 'validation' => [ + 'arg_in_rule_fail' => [ + 'rule object validation fails', + ], + ], + ], + 'locations' => [ + [ + 'line' => 2, + 'column' => 3, + ], + ], + 'path' => [ + 'mutationWithCustomRuleWithRuleObject', + ], + ], + ], ]; - self::assertSame($expectedMessages, $messageBag->all()); + self::assertEquals($expected, $result); } public function testInFailRulePass(): void @@ -79,14 +95,31 @@ public function testInFailRulePass(): void ], ]); - self::assertCount(1, $result['errors']); - self::assertSame('validation', $result['errors'][0]['message']); - /** @var MessageBag $messageBag */ - $messageBag = $result['errors'][0]['extensions']['validation']; - $expectedMessages = [ - 'The selected arg in rule pass is invalid.', + $expected = [ + 'errors' => [ + [ + 'message' => 'validation', + 'extensions' => [ + 'category' => 'validation', + 'validation' => [ + 'arg_in_rule_pass' => [ + 'The selected arg in rule pass is invalid.', + ], + ], + ], + 'locations' => [ + [ + 'line' => 2, + 'column' => 3, + ], + ], + 'path' => [ + 'mutationWithCustomRuleWithRuleObject', + ], + ], + ], ]; - self::assertSame($expectedMessages, $messageBag->all()); + self::assertEquals($expected, $result); } public function testInFailRuleFail(): void @@ -104,14 +137,31 @@ public function testInFailRuleFail(): void ], ]); - self::assertCount(1, $result['errors']); - self::assertSame('validation', $result['errors'][0]['message']); - /** @var MessageBag $messageBag */ - $messageBag = $result['errors'][0]['extensions']['validation']; - $expectedMessages = [ - 'The selected arg in rule fail is invalid.', - 'rule object validation fails', + $expected = [ + 'errors' => [ + [ + 'message' => 'validation', + 'extensions' => [ + 'category' => 'validation', + 'validation' => [ + 'arg_in_rule_fail' => [ + 'The selected arg in rule fail is invalid.', + 'rule object validation fails', + ], + ], + ], + 'locations' => [ + [ + 'line' => 2, + 'column' => 3, + ], + ], + 'path' => [ + 'mutationWithCustomRuleWithRuleObject', + ], + ], + ], ]; - self::assertSame($expectedMessages, $messageBag->all()); + self::assertEquals($expected, $result); } } diff --git a/tests/Unit/ValidationAuthorizationTests/ValidationAuthorizationTest.php b/tests/Unit/ValidationAuthorizationTests/ValidationAuthorizationTest.php index 86d2c2c0..38959865 100644 --- a/tests/Unit/ValidationAuthorizationTests/ValidationAuthorizationTest.php +++ b/tests/Unit/ValidationAuthorizationTests/ValidationAuthorizationTest.php @@ -3,7 +3,6 @@ declare(strict_types = 1); namespace Rebing\GraphQL\Tests\Unit\ValidationAuthorizationTests; -use Illuminate\Support\MessageBag; use Rebing\GraphQL\Tests\TestCase; class ValidationAuthorizationTest extends TestCase @@ -34,16 +33,31 @@ public function testAuthorizeArgumentsInvalid(): void ], ]); - self::assertSame('validation', $result['errors'][0]['message']); - - /** @var MessageBag $messageBag */ - $messageBag = $result['errors'][0]['extensions']['validation']; - $expectedErrors = [ - 'arg1' => [ - 'The selected arg1 is invalid.', + $expected = [ + 'errors' => [ + [ + 'message' => 'validation', + 'extensions' => [ + 'category' => 'validation', + 'validation' => [ + 'arg1' => [ + 'The selected arg1 is invalid.', + ], + ], + ], + 'locations' => [ + [ + 'line' => 2, + 'column' => 3, + ], + ], + 'path' => [ + 'validationAndAuthorization', + ], + ], ], ]; - self::assertSame($expectedErrors, $messageBag->messages()); + self::assertEquals($expected, $result); } public function testAuthorizeArgumentsValid(): void diff --git a/tests/Unit/ValidationOfFieldArguments/ValidationOfFieldArgumentsTest.php b/tests/Unit/ValidationOfFieldArguments/ValidationOfFieldArgumentsTest.php index 189d40f5..db0b82bb 100644 --- a/tests/Unit/ValidationOfFieldArguments/ValidationOfFieldArgumentsTest.php +++ b/tests/Unit/ValidationOfFieldArguments/ValidationOfFieldArgumentsTest.php @@ -4,7 +4,6 @@ namespace Rebing\GraphQL\Tests\Unit\ValidationOfFieldArguments; use Composer\InstalledVersions; -use Illuminate\Support\MessageBag; use Rebing\GraphQL\Tests\TestCase; class ValidationOfFieldArgumentsTest extends TestCase @@ -47,21 +46,47 @@ public function testRulesTakesEffect(): void ], ]); - /** @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.', + $expected = [ + 'errors' => [ + [ + 'message' => 'validation', + 'extensions' => [ + 'category' => 'validation', + 'validation' => [ + 'profile.fields.name.args.includeMiddleNames' => [ + 'The profile.fields.name.args.include middle names format is invalid.', + ], + 'profile.fields.height.args.unit' => [ + 'The profile.fields.height.args.unit format is invalid.', + ], + 'profile.args.profileId' => [ + 'The profile.args.profile id must not be greater than 10.', + ], + ], + ], + 'locations' => [ + [ + 'line' => 2, + 'column' => 3, + ], + ], + 'path' => [ + 'test', + ], + ], + ], + 'data' => [ + 'test' => null, + ], ]; - // See https://github.com/orchestral/testbench-core/commit/6c9c77b2e978890cb6a2712251ddab5eb1b79049 if ($this->orchestraTestbenchCoreVersionBelow('6.17.1.0')) { - $expectedMessages[] = 'The profile.args.profile id may not be greater than 10.'; - } else { - $expectedMessages[] = 'The profile.args.profile id must not be greater than 10.'; + $expected['errors'][0]['extensions']['validation']['profile.args.profileId'] = [ + 'The profile.args.profile id may not be greater than 10.', + ]; } - self::assertSame($expectedMessages, $messageBag->all()); + self::assertEquals($expected, $result); } public function testOnlyApplicableRulesTakesEffect(): void @@ -80,12 +105,34 @@ public function testOnlyApplicableRulesTakesEffect(): void 'variables' => [], ]); - /** @var MessageBag $messageBag */ - $messageBag = $result['errors'][0]['extensions']['validation']; - $expectedMessages = [ - 'The alias.args.type format is invalid.', + $expected = [ + 'errors' => [ + [ + 'message' => 'validation', + 'extensions' => [ + 'category' => 'validation', + 'validation' => [ + 'alias.args.type' => [ + 'The alias.args.type format is invalid.', + ], + ], + ], + 'locations' => [ + [ + 'line' => 2, + 'column' => 3, + ], + ], + 'path' => [ + 'test', + ], + ], + ], + 'data' => [ + 'test' => null, + ], ]; - self::assertSame($expectedMessages, $messageBag->all()); + self::assertEquals($expected, $result); } private function orchestraTestbenchCoreVersionBelow(string $versionString): bool