[BUGFIX] Keep existing validation errors for recursive domain relations

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/58833
Reviewed-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Tested-by: Anja Leichsenring <aleichsenring@ab-softlab.de>

typo3/sysext/extbase/Classes/Validation/Validator/CollectionValidator.php

@@ -63,8 +63,11 @@ public function validate($value)
             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);
         }

typo3/sysext/extbase/Classes/Validation/Validator/GenericObjectValidator.php

@@ -36,11 +36,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);
             }
         }
@@ -149,11 +154,18 @@ protected function isValidatedAlready($object)
         if ($this->validatedInstancesContainer->contains($object)) {
             return true;
         }
-        $this->validatedInstancesContainer->attach($object);
 
         return false;
     }
 
+    /**
+     * @param $object
+     */
+    protected function markInstanceAsValidated($object)
+    {
+        $this->validatedInstancesContainer->attach($object);
+    }
+
     /**
      * Returns all property validators - or only validators of the specified property
      *

typo3/sysext/extbase/Tests/Unit/Validation/Validator/GenericObjectValidatorTest.php

@@ -21,17 +21,22 @@
  * The TYPO3 project - inspiring people to share!                         *
  *                                                                        */
 
+use TYPO3\CMS\Extbase\Error\Error;
+use TYPO3\CMS\Extbase\Error\Result;
+use TYPO3\CMS\Extbase\Validation\Validator\GenericObjectValidator;
+use TYPO3\CMS\Extbase\Validation\Validator\ValidatorInterface;
+
 /**
  * Testcase for the Generic Object Validator
  *
  * @license http://www.gnu.org/licenses/lgpl.html GNU Lesser General Public License, version 3 or later
  */
-class GenericObjectValidatorTest extends \TYPO3\CMS\Extbase\Tests\Unit\Validation\Validator\AbstractValidatorTestcase
+class GenericObjectValidatorTest extends AbstractValidatorTestcase
 {
     /**
      * @var string
      */
-    protected $validatorClassName = \TYPO3\CMS\Extbase\Validation\Validator\GenericObjectValidator::class;
+    protected $validatorClassName = GenericObjectValidator::class;
 
     protected function setUp()
     {
@@ -88,11 +93,11 @@ public function dataProviderForValidator()
      */
     public function validateChecksAllPropertiesForWhichAPropertyValidatorExists($mockObject, $validationResultForFoo, $validationResultForBar, $errors)
     {
-        $validatorForFoo = $this->getMockBuilder(\TYPO3\CMS\Extbase\Validation\Validator\ValidatorInterface::class)
+        $validatorForFoo = $this->getMockBuilder(ValidatorInterface::class)
             ->setMethods(['validate', 'getOptions'])
             ->getMock();
         $validatorForFoo->expects($this->once())->method('validate')->with('foovalue')->will($this->returnValue($validationResultForFoo));
-        $validatorForBar = $this->getMockBuilder(\TYPO3\CMS\Extbase\Validation\Validator\ValidatorInterface::class)
+        $validatorForBar = $this->getMockBuilder(ValidatorInterface::class)
             ->setMethods(['validate', 'getOptions'])
             ->getMock();
         $validatorForBar->expects($this->once())->method('validate')->with('barvalue')->will($this->returnValue($validationResultForBar));
@@ -115,8 +120,8 @@ public function validateCanHandleRecursiveTargetsWithoutEndlessLooping()
         $A->b = $B;
         $B->a = $A;
 
-        $aValidator = new \TYPO3\CMS\Extbase\Validation\Validator\GenericObjectValidator([]);
-        $bValidator = new \TYPO3\CMS\Extbase\Validation\Validator\GenericObjectValidator([]);
+        $aValidator = new GenericObjectValidator([]);
+        $bValidator = new GenericObjectValidator([]);
 
         $aValidator->addPropertyValidator('b', $bValidator);
         $bValidator->addPropertyValidator('a', $aValidator);
@@ -144,7 +149,7 @@ public function validateDetectsFailuresInRecursiveTargetsI()
         $error = new \TYPO3\CMS\Extbase\Error\Error('error1', 123);
         $result = new \TYPO3\CMS\Extbase\Error\Result();
         $result->addError($error);
-        $mockUuidValidator = $this->getMockBuilder(\TYPO3\CMS\Extbase\Validation\Validator\ValidatorInterface::class)
+        $mockUuidValidator = $this->getMockBuilder(ValidatorInterface::class)
             ->setMethods(['validate', 'getOptions'])
             ->getMock();
         $mockUuidValidator->expects($this->any())->method('validate')->with(15)->will($this->returnValue($result));
@@ -173,12 +178,56 @@ public function validateDetectsFailuresInRecursiveTargetsII()
         $error1 = new \TYPO3\CMS\Extbase\Error\Error('error1', 123);
         $result1 = new \TYPO3\CMS\Extbase\Error\Result();
         $result1->addError($error1);
-        $mockUuidValidator = $this->getMockBuilder(\TYPO3\CMS\Extbase\Validation\Validator\ValidatorInterface::class)
+        $mockUuidValidator = $this->getMockBuilder(ValidatorInterface::class)
             ->setMethods(['validate', 'getOptions'])
             ->getMock();
         $mockUuidValidator->expects($this->any())->method('validate')->with(15)->will($this->returnValue($result1));
         $aValidator->addPropertyValidator('uuid', $mockUuidValidator);
         $bValidator->addPropertyValidator('uuid', $mockUuidValidator);
         $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()
+        );
+    }
 }