Fix double validation (#51)

* Fix double validation

* Test validate separately

* Add PHPDoc

composer.json

@@ -34,7 +34,6 @@
         "yiisoft/form": "^1.0@dev",
         "yiisoft/html": "^3.3",
         "yiisoft/hydrator": "^1.3",
-        "yiisoft/hydrator-validator": "^2.0",
         "yiisoft/strings": "^2.3",
         "yiisoft/validator": "^2.0"
     },

config/di.php

@@ -10,25 +10,19 @@
 use Yiisoft\Hydrator\TypeCaster\HydratorTypeCaster;
 use Yiisoft\Hydrator\TypeCaster\NullTypeCaster;
 use Yiisoft\Hydrator\TypeCaster\PhpNativeTypeCaster;
-use Yiisoft\Hydrator\Validator\ValidatingHydrator;
 
 return [
     FormHydrator::class => [
         '__construct()' => [
             'hydrator' => DynamicReference::to([
-                'class' => ValidatingHydrator::class,
+                'class' => Hydrator::class,
                 '__construct()' => [
-                    'hydrator' => DynamicReference::to([
-                        'class' => Hydrator::class,
-                        '__construct()' => [
-                            'typeCaster' => new CompositeTypeCaster(
-                                new NullTypeCaster(emptyString: true),
-                                new PhpNativeTypeCaster(),
-                                new NonArrayTypeCaster(),
-                                new HydratorTypeCaster(),
-                            ),
-                        ],
-                    ]),
+                    'typeCaster' => new CompositeTypeCaster(
+                        new NullTypeCaster(emptyString: true),
+                        new PhpNativeTypeCaster(),
+                        new NonArrayTypeCaster(),
+                        new HydratorTypeCaster(),
+                    ),
                 ],
             ]),
         ],

src/FormHydrator.php

@@ -8,6 +8,7 @@
 use Yiisoft\Hydrator\ArrayData;
 use Yiisoft\Hydrator\HydratorInterface;
 use Yiisoft\Validator\Helper\ObjectParser;
+use Yiisoft\Validator\Result;
 use Yiisoft\Validator\RulesProviderInterface;
 use Yiisoft\Validator\ValidatorInterface;
 
@@ -78,6 +79,18 @@ public function populate(
         return true;
     }
 
+    /**
+     * Validate form model.
+     *
+     * @param FormModelInterface $model Form model to validate.
+     *
+     * @return Result Validation result.
+     */
+    public function validate(FormModelInterface $model): Result
+    {
+        return $this->validator->validate($model);
+    }
+
     /**
      * Fill the model with the data and validate it.
      *
@@ -106,7 +119,7 @@ public function populateAndValidate(
             return false;
         }
 
-        return $this->validator->validate($model)->isValid();
+        return $this->validate($model)->isValid();
     }
 
     /**
@@ -138,6 +151,37 @@ public function populateFromPost(
         return $this->populate($model, $request->getParsedBody(), $map, $strict, $scope);
     }
 
+    /**
+     * Fill the model with the data parsed from request body and validate it.
+     *
+     * @param FormModelInterface $model Model to fill.
+     * @param ServerRequestInterface $request Request to get parsed data from.
+     * @param ?array $map Map of object property names to keys in the data array to use for hydration.
+     * If not provided, it may be generated automatically based on presence of property validation rules and a `strict`
+     * setting.
+     * @psalm-param MapType $map
+     * @param ?bool $strict If `false`, fills everything that is in the data. If `null`, fills data that is either
+     * defined in a map explicitly or allowed via validation rules. If `false`, fills only data defined explicitly
+     * in a map or only data allowed via validation rules but not both.
+     * @param ?string $scope Key to use in the data array as a source of data. Usually used when there are multiple
+     * forms at the same page. If not set, it equals to {@see FormModelInterface::getFormName()}.
+     *
+     * @return bool Whether model is filled with data and is valid.
+     */
+    public function populateFromPostAndValidate(
+        FormModelInterface $model,
+        ServerRequestInterface $request,
+        ?array $map = null,
+        ?bool $strict = null,
+        ?string $scope = null
+    ): bool {
+        if ($request->getMethod() !== 'POST') {
+            return false;
+        }
+
+        return $this->populateAndValidate($model, $request->getParsedBody(), $map, $strict, $scope);
+    }
+
     /**
      * Get a map of object property names mapped to keys in the data array.
      *
@@ -171,37 +215,6 @@ private function createMap(FormModelInterface $model, ?array $userMap, ?bool $st
         return array_merge($generatedMap, $userMap);
     }
 
-    /**
-     * Fill the model with the data parsed from request body and validate it.
-     *
-     * @param FormModelInterface $model Model to fill.
-     * @param ServerRequestInterface $request Request to get parsed data from.
-     * @param ?array $map Map of object property names to keys in the data array to use for hydration.
-     * If not provided, it may be generated automatically based on presence of property validation rules and a `strict`
-     * setting.
-     * @psalm-param MapType $map
-     * @param ?bool $strict If `false`, fills everything that is in the data. If `null`, fills data that is either
-     * defined in a map explicitly or allowed via validation rules. If `false`, fills only data defined explicitly
-     * in a map or only data allowed via validation rules but not both.
-     * @param ?string $scope Key to use in the data array as a source of data. Usually used when there are multiple
-     * forms at the same page. If not set, it equals to {@see FormModelInterface::getFormName()}.
-     *
-     * @return bool Whether model is filled with data and is valid.
-     */
-    public function populateFromPostAndValidate(
-        FormModelInterface $model,
-        ServerRequestInterface $request,
-        ?array $map = null,
-        ?bool $strict = null,
-        ?string $scope = null
-    ): bool {
-        if ($request->getMethod() !== 'POST') {
-            return false;
-        }
-
-        return $this->populateAndValidate($model, $request->getParsedBody(), $map, $strict, $scope);
-    }
-
     /**
      * Extract object property names mapped to keys in the data array based on model validation rules.
      *

src/FormModel.php

@@ -4,16 +4,18 @@
 
 namespace Yiisoft\FormModel;
 
+use LogicException;
 use ReflectionClass;
 use ReflectionException;
 use Yiisoft\FormModel\Exception\PropertyNotSupportNestedValuesException;
 use Yiisoft\FormModel\Exception\StaticObjectPropertyException;
 use Yiisoft\FormModel\Exception\UndefinedArrayElementException;
 use Yiisoft\FormModel\Exception\UndefinedObjectPropertyException;
 use Yiisoft\FormModel\Exception\ValueNotFoundException;
-use Yiisoft\Hydrator\Validator\ValidatedInputTrait;
+use Yiisoft\Hydrator\Attribute\SkipHydration;
 use Yiisoft\Strings\Inflector;
 use Yiisoft\Strings\StringHelper;
+use Yiisoft\Validator\Result;
 
 use function array_key_exists;
 use function array_slice;
@@ -28,8 +30,6 @@
  */
 abstract class FormModel implements FormModelInterface
 {
-    use ValidatedInputTrait;
-
     /**
      * @psalm-suppress MissingClassConstType Remove after fix https://github.com/vimeo/psalm/issues/11026
      */
@@ -45,6 +45,12 @@ abstract class FormModel implements FormModelInterface
      */
     private const META_PLACEHOLDER = 3;
 
+    /**
+     * @var Result|null Validation result.
+     */
+    #[SkipHydration]
+    private ?Result $validationResult = null;
+
     private static ?Inflector $inflector = null;
 
     public function getPropertyHint(string $property): string
@@ -130,6 +136,20 @@ public function addError(string $message, array $valuePath = []): static
         return $this;
     }
 
+    public function processValidationResult(Result $result): void
+    {
+        $this->validationResult = $result;
+    }
+
+    public function getValidationResult(): Result
+    {
+        if (empty($this->validationResult)) {
+            throw new LogicException('Validation result is not set.');
+        }
+
+        return $this->validationResult;
+    }
+
     /**
      * Returns model property value given a path.
      *

src/FormModelInterface.php

@@ -9,7 +9,6 @@
 use Yiisoft\FormModel\Exception\StaticObjectPropertyException;
 use Yiisoft\FormModel\Exception\UndefinedObjectPropertyException;
 use Yiisoft\FormModel\Exception\ValueNotFoundException;
-use Yiisoft\Hydrator\Validator\ValidatedInputInterface;
 
 /**
  * Form model represents an HTML form: its data, validation and presentation.

src/ValidatedInputInterface.php

@@ -0,0 +1,23 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Yiisoft\FormModel;
+
+use LogicException;
+use Yiisoft\Validator\PostValidationHookInterface;
+use Yiisoft\Validator\Result;
+
+/**
+ * Used for objects that can be validated. It provides a method to get validation result.
+ */
+interface ValidatedInputInterface extends PostValidationHookInterface
+{
+    /**
+     * Returns validation result.
+     *
+     * @throws LogicException When validation result is not set.
+     * @return Result Validation result.
+     */
+    public function getValidationResult(): Result;
+}

tests/FormHydratorTest.php

@@ -11,6 +11,7 @@
 use Yiisoft\FormModel\FormModel;
 use Yiisoft\FormModel\Tests\Support\Form\CarForm;
 use Yiisoft\FormModel\Tests\Support\TestHelper;
+use Yiisoft\Validator\Result;
 
 final class FormHydratorTest extends TestCase
 {
@@ -29,6 +30,31 @@ public function testPopulateWithStrictMap(): void
         $this->assertSame(2, $form->b);
     }
 
+    public static function dataValidate(): array
+    {
+        return [
+            'empty data' => [[], ['name' => ['Name must contain at least 3 characters.']]],
+            'invalid data' => [
+                ['CarForm' => ['name' => 'A']],
+                ['name' => ['Name must contain at least 3 characters.']],
+            ],
+            'valid data' => [['CarForm' => ['name' => 'Test']], []],
+        ];
+    }
+
+    #[DataProvider('dataValidate')]
+    public function testPopulateAndValidateSeparately(array $data, array $expectedErrorMessages): void
+    {
+        $form = new CarForm();
+        $formHydrator = TestHelper::createFormHydrator();
+
+        $formHydrator->populate($form, $data);
+        $result = $formHydrator->validate($form);
+
+        $this->assertInstanceOf(Result::class, $result);
+        $this->assertEquals($expectedErrorMessages, $result->getErrorMessagesIndexedByPath());
+    }
+
     public static function dataPopulateAndValidate(): array
     {
         return [

tests/FormModelTest.php

@@ -446,8 +446,7 @@ public function testFormWithNestedStructures(): void
     public function testNestedRuleWithFormModels(): void
     {
         $form = new MainForm();
-
-        TestHelper::createFormHydrator()->populate(
+        $isValid = TestHelper::createFormHydrator()->populateAndValidate(
             $form,
             [
                 'value' => 'main-form',
@@ -456,14 +455,12 @@ public function testNestedRuleWithFormModels(): void
             scope: ''
         );
 
-        $result = $form->getValidationResult();
-
-        $this->assertFalse($result->isValid());
+        $this->assertFalse($isValid);
         $this->assertSame(
             [
                 'firstLevelForm.secondLevelForm.float' => ['Float must be no less than 0.'],
             ],
-            $result->getErrorMessagesIndexedByPath()
+            $form->getValidationResult()->getErrorMessagesIndexedByPath()
         );
     }
 
@@ -473,8 +470,7 @@ public function testNestedRuleWithFormModels(): void
     public function testNestedRuleInForm(): void
     {
         $form = new NestedMixedForm();
-
-        TestHelper::createFormHydrator()->populate(
+        $isValid = TestHelper::createFormHydrator()->populateAndValidate(
             $form,
             [
                 'body' => [
@@ -486,14 +482,12 @@ public function testNestedRuleInForm(): void
             scope: ''
         );
 
-        $result = $form->getValidationResult();
-
-        $this->assertFalse($result->isValid());
+        $this->assertFalse($isValid);
         $this->assertSame(
             [
                 'body.shipping.phone' => ['Invalid phone.'],
             ],
-            $result->getErrorMessagesIndexedByPath()
+            $form->getValidationResult()->getErrorMessagesIndexedByPath(),
         );
     }
 

tests/Support/TestHelper.php

@@ -10,8 +10,6 @@
 use Yiisoft\FormModel\FormHydrator;
 use Yiisoft\Hydrator\Hydrator;
 use Yiisoft\Hydrator\TypeCaster\TypeCastContext;
-use Yiisoft\Hydrator\Validator\Attribute\ValidateResolver;
-use Yiisoft\Hydrator\Validator\ValidatingHydrator;
 use Yiisoft\Validator\Validator;
 
 final class TestHelper
@@ -20,11 +18,7 @@ public static function createFormHydrator(): FormHydrator
     {
         $validator = new Validator();
         return new FormHydrator(
-            new ValidatingHydrator(
-                new Hydrator(),
-                $validator,
-                new ValidateResolver($validator),
-            ),
+            new Hydrator(),
             $validator,
         );
     }