fix: update validation rules to use ValidationRule interface and improve error handling (#32)

yanis-git · web-flow · commit 1f4da7a717fd · 2025-10-16T14:37:20.000+02:00
diff --git a/.github/workflows/php.yml b/.github/workflows/php.yml
@@ -17,7 +17,7 @@ jobs:
     strategy:
       matrix:
         php_version: ['8.1', '8.2', '8.3', '8.4']
-        laravel_version: ['^9.0', '^10.0', '^11.0', '^12.0']
+        laravel_version: ['^10.0', '^11.0', '^12.0']
         exclude:
           - php_version: '8.1'
             laravel_version: '^11.0'
diff --git a/.gitignore b/.gitignore
@@ -1,4 +1,6 @@
 .idea
 .phpunit.cache
+.phpunit.result.cache
+coverage.xml
 /vendor/
 composer.lock
diff --git a/composer.json b/composer.json
@@ -11,9 +11,9 @@
   "type": "library",
   "require": {
     "php": "^8.1",
-    "illuminate/collections": "^9.0|^10.0|^11.0|^12.0",
-    "illuminate/http": "^9.0|^10.0|^11.0|^12.0",
-    "illuminate/support": "^9.0|^10.0|^11.0|^12.0"
+    "illuminate/collections": "^10.0|^11.0|^12.0",
+    "illuminate/http": "^10.0|^11.0|^12.0",
+    "illuminate/support": "^10.0|^11.0|^12.0"
   },
   "require-dev": {
     "phpunit/phpunit": "^9.0|^10.0|^11.0|^12.0",
diff --git a/src/Requests/Rules/Fields.php b/src/Requests/Rules/Fields.php
@@ -5,15 +5,17 @@
 use Ark4ne\JsonApi\Requests\Rules\Traits\UseTrans;
 use Ark4ne\JsonApi\Resources\Skeleton;
 use Ark4ne\JsonApi\Support\Fields as SupportFields;
-use Illuminate\Contracts\Validation\Rule;
+use Illuminate\Contracts\Validation\ValidationRule;
 
 /**
  * @template T as \Ark4ne\JsonApi\Resources\JsonApiResource
  */
-class Fields implements Rule
+class Fields implements ValidationRule
 {
     use UseTrans;
 
+    private const BASE = 'validation.custom.jsonapi.fields';
+
     /**
      * @var array<int, array{":resource": string, ":fields": ?string}>>
      */
@@ -27,38 +29,46 @@ public function __construct(
     ) {
     }
 
-    public function passes($attribute, $value): bool
+    public function validate(string $attribute, mixed $value, \Closure $fail): void
     {
         if (!is_array($value)) {
-            return false;
+            $fail($this->trans(
+                sprintf('%s.invalid', self::BASE),
+                'The selected :attribute is invalid.'
+            ));
+            return;
         }
 
         $desired = SupportFields::parse($value);
         $schema = $this->resource::schema();
 
-        return $this->assert($schema, $desired);
+        if (!$this->assert($schema, $desired)) {
+
+            foreach ($this->message() as $message) {
+                $fail($message);
+            }
+        }
     }
 
     /**
      * @return array<string>
      */
-    public function message(): array
+    private function message(): array
     {
-        $base = 'validation.custom.jsonapi.fields';
         $message = $this->trans(
-            "$base.invalid",
+            sprintf('%s.invalid', self::BASE),
             'The selected :attribute is invalid.'
         );
 
-        return array_merge($message, ...array_map(
-            fn($failure) => isset($failure[':fields'])
+        return array_merge([$message], array_map(
+            fn($failure) =>  isset($failure[':fields'])
                 ? $this->trans(
-                    "$base.invalid_fields",
+                    sprintf('%s.invalid_fields', self::BASE),
                     '":resource" doesn\'t have fields ":fields".',
                     $failure
                 )
                 : $this->trans(
-                    "$base.invalid_resource",
+                    sprintf('%s.invalid_resource', self::BASE),
                     '":resource" doesn\'t exists.',
                     $failure
                 ),
diff --git a/src/Requests/Rules/Includes.php b/src/Requests/Rules/Includes.php
@@ -6,14 +6,17 @@
 use Ark4ne\JsonApi\Resources\Skeleton;
 use Ark4ne\JsonApi\Support\Includes as SupportIncludes;
 use Illuminate\Contracts\Validation\Rule;
+use Illuminate\Contracts\Validation\ValidationRule;
 
 /**
  * @template T as \Ark4ne\JsonApi\Resources\JsonApiResource
  */
-class Includes implements Rule
+class Includes implements ValidationRule
 {
     use UseTrans;
 
+    private const BASE = 'validation.custom.jsonapi.fields';
+
     /**
      * @var array<int, array{":include": string, ":relation": string}>>
      */
@@ -27,32 +30,39 @@ public function __construct(
     ) {
     }
 
-    public function passes($attribute, $value): bool
+    public function validate(string $attribute, mixed $value, \Closure $fail): void
     {
         if (!is_string($value)) {
-            return false;
+            $fail($this->trans(
+                sprintf('%s.invalid', self::BASE),
+                'The selected :attribute is invalid.'
+            ));
+            return;
         }
 
         $desired = SupportIncludes::parse($value);
         $schema = $this->resource::schema();
 
-        return $this->assert($schema, $desired);
+        if (!$this->assert($schema, $desired)) {
+            foreach ($this->message() as $message) {
+                $fail($message);
+            }
+        }
     }
 
     /**
      * @return array<string>
      */
-    public function message(): array
+    private function message(): array
     {
-        $base = 'validation.custom.jsonapi.includes';
         $message = $this->trans(
-            "$base.invalid",
+            sprintf('%s.invalid', self::BASE),
             'The selected :attribute is invalid.'
         );
 
-        return array_merge($message, ...array_map(
+        return array_merge([$message], array_map(
             fn($failure) => $this->trans(
-                "$base.invalid_includes",
+                sprintf('%s.invalid_includes', self::BASE),
                 '":include" doesn\'t have relationship ":relation".',
                 $failure
             ),
diff --git a/src/Requests/Rules/Traits/UseTrans.php b/src/Requests/Rules/Traits/UseTrans.php
@@ -13,15 +13,12 @@ trait UseTrans
      * @param string                $default
      * @param array<string, string> $replace
      *
-     * @return array<string>
+     * @return string
      */
-    protected function trans(?string $key, string $default, array $replace = []): array
+    protected function trans(?string $key, string $default, array $replace = []): string
     {
         $message = trans($key);
 
-        return array_map(
-            static fn($msg) => Str::replace(array_keys($replace), array_values($replace), $msg),
-            $message === $key ? [$default] : (array)$message
-        );
+        return Str::replace(array_keys($replace), array_values($replace), $message === $key ? $default : $message);
     }
 }
diff --git a/tests/Unit/Requests/Rules/FieldsTest.php b/tests/Unit/Requests/Rules/FieldsTest.php
@@ -3,35 +3,52 @@
 namespace Test\Unit\Requests\Rules;
 
 use Ark4ne\JsonApi\Requests\Rules\Fields;
+use PHPUnit\Framework\Attributes\DataProvider;
 use Test\app\Http\Resources\UserResource;
 use Test\TestCase;
 
 class FieldsTest extends TestCase
 {
-    public function testPasses()
+    /**
+     * @dataProvider validationDataProvider
+     */
+    #[DataProvider('validationDataProvider')]
+    public function testPasses(mixed $values, array $expected)
     {
         $rule = new Fields(UserResource::class);
+        $errors = [];
 
-        $this->assertFalse($rule->passes(null, 'user,post'));
-
-        $this->assertTrue($rule->passes(null, [
-            'user' => 'name,email',
-            'post' => 'content',
-        ]));
-
-        $this->assertFalse($rule->passes(null, [
-            'user' => 'name,email',
-            'unknown' => 'content',
-        ]));
-
-        $this->assertFalse($rule->passes(null, [
-            'user' => 'name,unknown',
-            'post' => 'content',
-        ]));
+        $rule->validate('fields', $values, function($message) use (&$errors) {
+            $errors[] = $message;
+        });
+        $this->assertEquals($expected, $errors);
+    }
 
-        $this->assertFalse($rule->passes(null, [
-            'user' => 'name,email',
-            'post' => 'unknown',
-        ]));
+    public static function validationDataProvider()
+    {
+        return [
+            // not expected type
+            ['string', ['The selected :attribute is invalid.']],
+            [123, ['The selected :attribute is invalid.']],
+            [null, ['The selected :attribute is invalid.']],
+
+            // valid cases
+            [['user' => 'name,email'], []],
+            [['user' => 'name,email', 'post' => 'content'], []],
+
+            // invalid cases
+            [['user' => 'name,email', 'unknown' => 'content'], [
+                'The selected :attribute is invalid.',
+                '"unknown" doesn\'t exists.',
+            ]],
+            [['user' => 'name,unknown', 'post' => 'content'], [
+                'The selected :attribute is invalid.',
+                '"user" doesn\'t have fields "unknown".'
+            ]],
+            [['user' => 'name,email', 'post' => 'unknown'], [
+                'The selected :attribute is invalid.',
+                '"post" doesn\'t have fields "unknown".'
+            ]],
+        ];
     }
 }
diff --git a/tests/Unit/Requests/Rules/IncludesTest.php b/tests/Unit/Requests/Rules/IncludesTest.php
@@ -3,36 +3,56 @@
 namespace Test\Unit\Requests\Rules;
 
 use Ark4ne\JsonApi\Requests\Rules\Includes;
+use PHPUnit\Framework\Attributes\DataProvider;
 use Test\app\Http\Resources\UserResource;
 use Test\TestCase;
 
 class IncludesTest extends TestCase
 {
-    public function testPasses()
+    /**
+     * @dataProvider validationDataProvider
+     */
+    #[DataProvider('validationDataProvider')]
+    public function testPasses(mixed $values, array $expected)
     {
         $rule = new Includes(UserResource::class);
+        $errors = [];
 
-        $this->assertFalse($rule->passes(null, [
-            'posts',
-            'posts.user',
-            'posts.user.comments',
-            'posts.user.posts'
-        ]));
+        $rule->validate('fields', $values, function($message) use (&$errors) {
+            $errors[] = $message;
+        });
+        $this->assertEquals($expected, $errors);
+    }
 
-        $this->assertTrue($rule->passes(null, implode(',', [
-            'posts',
-            'posts.user',
-            'posts.user.comments',
-            'posts.user.posts'
-        ])));
+    public static function validationDataProvider()
+    {
+        return [
+            // not expected type
+            [123, ['The selected :attribute is invalid.']],
+            [null, ['The selected :attribute is invalid.']],
 
-        $this->assertFalse($rule->passes(null, implode(',', [
-            'posts',
-            'unknown',
-        ])));
+            // valid cases
+            [implode(',', [
+                'posts',
+                'posts.user',
+                'posts.user.comments',
+                'posts.user.posts'
+            ]), []],
 
-        $this->assertFalse($rule->passes(null, implode(',', [
-            'posts.unknown',
-        ])));
+            // invalid cases
+            [implode(',', [
+                'posts',
+                'unknown',
+            ]), [
+                'The selected :attribute is invalid.',
+                '"user" doesn\'t have relationship "unknown".',
+            ]],
+            [implode(',', [
+                'posts.unknown',
+            ]), [
+                'The selected :attribute is invalid.',
+                '"posts" doesn\'t have relationship "unknown".'
+            ]],
+        ];
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -13,15 +13,12 @@ trait UseTrans`
`13`	`13`	`* @param string $default`
`14`	`14`	`* @param array<string, string> $replace`
`15`	`15`	`*`
`16`		`- * @return array<string>`
	`16`	`+ * @return string`
`17`	`17`	`*/`
`18`		`- protected function trans(?string $key, string $default, array $replace = []): array`
	`18`	`+ protected function trans(?string $key, string $default, array $replace = []): string`
`19`	`19`	`{`
`20`	`20`	`$message = trans($key);`
`21`	`21`
`22`		`- return array_map(`
`23`		`- static fn($msg) => Str::replace(array_keys($replace), array_values($replace), $msg),`
`24`		`- $message === $key ? [$default] : (array)$message`
`25`		`- );`
	`22`	`+ return Str::replace(array_keys($replace), array_values($replace), $message === $key ? $default : $message);`
`26`	`23`	`}`
`27`	`24`	`}`