Fix default properties generating validation rules when not provided

spatie · Jul 5, 2023 · 60fbafc · 60fbafc
1 parent ede0f9e
commit 60fbafc
Show file tree

Hide file tree

Showing 2 changed files with 75 additions and 13 deletions.
diff --git a/src/Resolvers/DataValidationRulesResolver.php b/src/Resolvers/DataValidationRulesResolver.php
@@ -34,10 +34,14 @@ public function execute(
     ): array {
         $dataClass = $this->dataConfig->getDataClass($class);
 
+        $withoutValidationProperties = [];
+
         foreach ($dataClass->properties as $dataProperty) {
             $propertyPath = $path->property($dataProperty->inputMappedName ?? $dataProperty->name);
 
-            if ($dataProperty->validate === false) {
+            if ($this->shouldSkipPropertyValidation($dataProperty, $fullPayload, $propertyPath)) {
+                $withoutValidationProperties[] = $dataProperty->name;
+
                 continue;
             }
 
@@ -63,11 +67,33 @@ public function execute(
             $dataRules->add($propertyPath, $rules);
         }
 
-        $this->resolveOverwrittenRules($dataClass, $fullPayload, $path, $dataRules);
+        $this->resolveOverwrittenRules(
+            $dataClass,
+            $fullPayload,
+            $path,
+            $dataRules,
+            $withoutValidationProperties
+        );
 
         return $dataRules->rules;
     }
 
+    protected function shouldSkipPropertyValidation(
+        DataProperty $dataProperty,
+        array $fullPayload,
+        ValidationPath $propertyPath,
+    ): bool {
+        if ($dataProperty->validate === false) {
+            return true;
+        }
+
+        if ($dataProperty->hasDefaultValue && Arr::has($fullPayload, $propertyPath->get()) === false) {
+            return true;
+        }
+
+        return false;
+    }
+
     protected function resolveDataSpecificRules(
         DataProperty $dataProperty,
         array $fullPayload,
@@ -169,6 +195,7 @@ protected function resolveOverwrittenRules(
         array $fullPayload,
         ValidationPath $path,
         DataRules $dataRules,
+        array $withoutValidationProperties
     ): void {
         if (! method_exists($class->name, 'rules')) {
             return;
@@ -183,6 +210,10 @@ protected function resolveOverwrittenRules(
         $overwrittenRules = app()->call([$class->name, 'rules'], ['context' => $validationContext]);
 
         foreach ($overwrittenRules as $key => $rules) {
+            if (in_array($key, $withoutValidationProperties)) {
+                continue;
+            }
+
             $dataRules->add(
                 $path->property($key),
                 collect(Arr::wrap($rules))

diff --git a/tests/ValidationTest.php b/tests/ValidationTest.php
@@ -9,25 +9,15 @@
 use Illuminate\Support\Facades\Validator as ValidatorFacade;
 use Illuminate\Validation\Rules\Enum;
 use Illuminate\Validation\Rules\Exists as LaravelExists;
-
 use Illuminate\Validation\ValidationException;
 use Illuminate\Validation\Validator;
-
-use function Pest\Laravel\mock;
-use function PHPUnit\Framework\assertFalse;
-
 use Spatie\LaravelData\Attributes\DataCollectionOf;
-
 use Spatie\LaravelData\Attributes\MapInputName;
-
 use Spatie\LaravelData\Attributes\MapName;
 use Spatie\LaravelData\Attributes\Validation\ArrayType;
-
 use Spatie\LaravelData\Attributes\Validation\Bail;
-
 use Spatie\LaravelData\Attributes\Validation\Exists;
 use Spatie\LaravelData\Attributes\Validation\In;
-
 use Spatie\LaravelData\Attributes\Validation\IntegerType;
 use Spatie\LaravelData\Attributes\Validation\Max;
 use Spatie\LaravelData\Attributes\Validation\Min;
@@ -41,7 +31,6 @@
 use Spatie\LaravelData\Data;
 use Spatie\LaravelData\DataCollection;
 use Spatie\LaravelData\DataPipeline;
-
 use Spatie\LaravelData\DataPipes\AuthorizedDataPipe;
 use Spatie\LaravelData\DataPipes\CastPropertiesDataPipe;
 use Spatie\LaravelData\DataPipes\DefaultValuesDataPipe;
@@ -71,6 +60,8 @@
 use Spatie\LaravelData\Tests\Fakes\SimpleDataWithOverwrittenRules;
 use Spatie\LaravelData\Tests\Fakes\Support\FakeInjectable;
 use Spatie\LaravelData\Tests\TestSupport\DataValidationAsserter;
+use function Pest\Laravel\mock;
+use function PHPUnit\Framework\assertFalse;
 
 it('can validate a string', function () {
     $dataClass = new class () extends Data {
@@ -2169,3 +2160,43 @@ public static function rules(): array
     expect($validatorToPass->passes())->toBeTrue()
         ->and($validatorToFail->passes())->toBeFalse();
 });
+
+it('wont validate default values when they are not provided', function () {
+    $dataClass = new class () extends Data {
+        #[Min(10)]
+        public string $default = 'Hello World';
+    };
+
+    DataValidationAsserter::for($dataClass)
+        ->assertOk([])
+        ->assertOk(['default' => 'Hi there in this world'])
+        ->assertErrors(['default' => 'minimal'])
+        ->assertErrors(['default' => null])
+        ->assertRules([], payload: [])
+        ->assertRules([
+            'default' => ['required', 'string', 'min:10'],
+        ], ['default' => 'something']);
+});
+
+it('wont validate default values when they are not provided and rules are overwritten', function () {
+    $dataClass = new class () extends Data {
+        public string $default = 'Hello World';
+
+        public static function rules(ValidationContext $context): array
+        {
+            return [
+                'default' => ['required', 'string', 'min:10'],
+            ];
+        }
+    };
+
+    DataValidationAsserter::for($dataClass)
+        ->assertOk([])
+        ->assertOk(['default' => 'Hi there in this world'])
+        ->assertErrors(['default' => 'minimal'])
+        ->assertErrors(['default' => null])
+        ->assertRules([], payload: [])
+        ->assertRules([
+            'default' => ['required', 'string', 'min:10'],
+        ], ['default' => 'something']);
+});