[BUGFIX] Keep existing validation errors for recursive domain relations 77/56677/6
authorMarkus Klösges <mkloesges@gmx.de>
Mon, 16 Apr 2018 08:43:24 +0000 (10:43 +0200)
committerAnja Leichsenring <aleichsenring@ab-softlab.de>
Fri, 2 Nov 2018 18:57:06 +0000 (19:57 +0100)
Do not overwrite already generated validation errors when a property
validator leads to a recursive validate() call to the currently
validated GenericObjectValidator.

Resolves: #84475
Releases: master, 8.7
Change-Id: Ifbdb28ddcf6a8e7f1517801ebcd6634149b2bd5d
Reviewed-on: https://review.typo3.org/56677
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Benni Mack <benni@typo3.org>
Tested-by: Benni Mack <benni@typo3.org>
Reviewed-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Tested-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
typo3/sysext/extbase/Classes/Validation/Validator/CollectionValidator.php
typo3/sysext/extbase/Classes/Validation/Validator/GenericObjectValidator.php
typo3/sysext/extbase/Tests/Unit/Validation/Validator/GenericObjectValidatorTest.php

index 4e3df94..5d5570c 100644 (file)
@@ -60,8 +60,11 @@ class CollectionValidator extends GenericObjectValidator
             if ($value instanceof \TYPO3\CMS\Extbase\Persistence\Generic\LazyObjectStorage && !$value->isInitialized()) {
                 return $this->result;
             }
-            if (is_object($value) && $this->isValidatedAlready($value)) {
-                return $this->result;
+            if (is_object($value)) {
+                if ($this->isValidatedAlready($value)) {
+                    return $this->result;
+                }
+                $this->markInstanceAsValidated($value);
             }
             $this->isValid($value);
         }
index 2a85018..def5f15 100644 (file)
@@ -35,11 +35,16 @@ class GenericObjectValidator extends AbstractValidator implements ObjectValidato
      */
     public function validate($value)
     {
+        if (is_object($value) && $this->isValidatedAlready($value)) {
+            return $this->result;
+        }
+
         $this->result = new \TYPO3\CMS\Extbase\Error\Result();
         if ($this->acceptsEmptyValues === false || $this->isEmpty($value) === false) {
             if (!is_object($value)) {
                 $this->addError('Object expected, %1$s given.', 1241099149, [gettype($value)]);
             } elseif ($this->isValidatedAlready($value) === false) {
+                $this->markInstanceAsValidated($value);
                 $this->isValid($value);
             }
         }
@@ -145,12 +150,19 @@ class GenericObjectValidator extends AbstractValidator implements ObjectValidato
         if ($this->validatedInstancesContainer->contains($object)) {
             return true;
         }
-        $this->validatedInstancesContainer->attach($object);
 
         return false;
     }
 
     /**
+     * @param $object
+     */
+    protected function markInstanceAsValidated($object): void
+    {
+        $this->validatedInstancesContainer->attach($object);
+    }
+
+    /**
      * Returns all property validators - or only validators of the specified property
      *
      * @param string $propertyName Name of the property to return validators for
index 6261e10..6b39c92 100644 (file)
@@ -195,4 +195,48 @@ class GenericObjectValidatorTest extends UnitTestCase
 
         $this->assertSame(['b.uuid' => [$error1], 'uuid' => [$error1]], $aValidator->validate($A)->getFlattenedErrors());
     }
+
+    /**
+     * @test
+     */
+    public function validateDetectsFailuresInRecursiveTargetsIII()
+    {
+        // Create to test-entities. Use the same uuid to make the same validator trigger on both objects
+        $A = new class() {
+            public $b;
+            public $uuid = 0xF;
+        };
+
+        $B = new class() {
+            public $a;
+            public $uuid = 0xF;
+        };
+
+        $A->b = $B;
+        $B->a = $A;
+        $aValidator = new GenericObjectValidator();
+        $bValidator = new GenericObjectValidator();
+
+        $error1 = new Error('error1', 123);
+        $result1 = new Result();
+        $result1->addError($error1);
+
+        /** @var ValidatorInterface|\PHPUnit_Framework_MockObject_MockObject $mockValidatorUuidNot0xF */
+        $mockValidatorUuidNot0xF = $this->getMockBuilder(ValidatorInterface::class)
+            ->setMethods(['validate', 'getOptions'])
+            ->getMock();
+        $mockValidatorUuidNot0xF->expects($this->any())
+            ->method('validate')->with(0xF)->will($this->returnValue($result1));
+
+        $aValidator->addPropertyValidator('uuid', $mockValidatorUuidNot0xF);
+        $bValidator->addPropertyValidator('uuid', $mockValidatorUuidNot0xF);
+        $aValidator->addPropertyValidator('b', $bValidator);
+        $bValidator->addPropertyValidator('a', $aValidator);
+
+        // assert that the validation error is being reported for both objects
+        $this->assertSame(
+            ['uuid' => [$error1], 'b.uuid' => [$error1], 'b.a.uuid' => [$error1]],
+            $aValidator->validate($A)->getFlattenedErrors()
+        );
+    }
 }