Merge pull request cakephp#17348 from cakephp/5.x-merge

merge 4.next => 5.x

src/Collection/CollectionTrait.php

@@ -604,14 +604,37 @@ public function combine(
             $rowVal = $options['valuePath'];
 
             if (!$options['groupPath']) {
-                $mapReduce->emit($rowVal($value, $key), $rowKey($value, $key));
+                $mapKey = $rowKey($value, $key);
+                if ($mapKey === null) {
+                    throw new InvalidArgumentException(
+                        'Cannot index by path that does not exist or contains a null value. ' .
+                        'Use a callback to return a default value for that path.'
+                    );
+                }
+
+                $mapReduce->emit($rowVal($value, $key), $mapKey);
 
                 return null;
             }
 
             $key = $options['groupPath']($value, $key);
+            if ($key === null) {
+                throw new InvalidArgumentException(
+                    'Cannot group by path that does not exist or contains a null value. ' .
+                    'Use a callback to return a default value for that path.'
+                );
+            }
+
+            $mapKey = $rowKey($value, $key);
+            if ($mapKey === null) {
+                throw new InvalidArgumentException(
+                    'Cannot index by path that does not exist or contains a null value. ' .
+                    'Use a callback to return a default value for that path.'
+                );
+            }
+
             $mapReduce->emitIntermediate(
-                [$rowKey($value, $key) => $rowVal($value, $key)],
+                [$mapKey => $rowVal($value, $key)],
                 $key
             );
         };

src/Datasource/EntityTrait.php

@@ -126,6 +126,13 @@ trait EntityTrait
      */
     protected string $_registryAlias = '';
 
+    /**
+     * Storing the current visitation status while recursing through entities getting errors.
+     *
+     * @var bool
+     */
+    protected bool $_hasBeenVisited = false;
+
     /**
      * Whether the presence of a field is checked when accessing a property.
      *
@@ -951,6 +958,11 @@ public function isNew(): bool
      */
     public function hasErrors(bool $includeNested = true): bool
     {
+        if ($this->_hasBeenVisited) {
+            // While recursing through entities, each entity should only be visited once. See https://github.com/cakephp/cakephp/issues/17318
+            return false;
+        }
+
         if (Hash::filter($this->_errors)) {
             return true;
         }
@@ -959,10 +971,15 @@ public function hasErrors(bool $includeNested = true): bool
             return false;
         }
 
-        foreach ($this->_fields as $field) {
-            if ($this->_readHasErrors($field)) {
-                return true;
+        $this->_hasBeenVisited = true;
+        try {
+            foreach ($this->_fields as $field) {
+                if ($this->_readHasErrors($field)) {
+                    return true;
+                }
             }
+        } finally {
+            $this->_hasBeenVisited = false;
         }
 
         return false;
@@ -975,17 +992,29 @@ public function hasErrors(bool $includeNested = true): bool
      */
     public function getErrors(): array
     {
+        if ($this->_hasBeenVisited) {
+            // While recursing through entities, each entity should only be visited once. See https://github.com/cakephp/cakephp/issues/17318
+            return [];
+        }
+
         $diff = array_diff_key($this->_fields, $this->_errors);
 
-        return $this->_errors + (new Collection($diff))
-            ->filter(function ($value) {
-                return is_array($value) || $value instanceof EntityInterface;
-            })
-            ->map(function ($value) {
-                return $this->_readError($value);
-            })
-            ->filter()
-            ->toArray();
+        $this->_hasBeenVisited = true;
+        try {
+            $errors = $this->_errors + (new Collection($diff))
+                ->filter(function ($value) {
+                    return is_array($value) || $value instanceof EntityInterface;
+                })
+                ->map(function ($value) {
+                    return $this->_readError($value);
+                })
+                ->filter()
+                ->toArray();
+        } finally {
+            $this->_hasBeenVisited = false;
+        }
+
+        return $errors;
     }
 
     /**

tests/TestCase/Collection/CollectionTest.php

@@ -1269,6 +1269,45 @@ function ($value, $key) {
         $this->assertEquals([1 => null, 2 => null, 3 => null], $collection->toArray());
     }
 
+    public function testCombineNullKey(): void
+    {
+        $items = [
+            ['id' => 1, 'name' => 'foo', 'parent' => 'a'],
+            ['id' => null, 'name' => 'bar', 'parent' => 'b'],
+            ['id' => 3, 'name' => 'baz', 'parent' => 'a'],
+        ];
+
+        $this->expectException(InvalidArgumentException::class);
+        $this->expectExceptionMessage('Cannot index by path that does not exist or contains a null value');
+        (new Collection($items))->combine('id', 'name');
+    }
+
+    public function testCombineNullGroup(): void
+    {
+        $items = [
+            ['id' => 1, 'name' => 'foo', 'parent' => 'a'],
+            ['id' => 2, 'name' => 'bar', 'parent' => 'b'],
+            ['id' => 3, 'name' => 'baz', 'parent' => null],
+        ];
+
+        $this->expectException(InvalidArgumentException::class);
+        $this->expectExceptionMessage('Cannot group by path that does not exist or contains a null value');
+        (new Collection($items))->combine('id', 'name', 'parent');
+    }
+
+    public function testCombineGroupNullKey(): void
+    {
+        $items = [
+            ['id' => 1, 'name' => 'foo', 'parent' => 'a'],
+            ['id' => 2, 'name' => 'bar', 'parent' => 'b'],
+            ['id' => null, 'name' => 'baz', 'parent' => 'a'],
+        ];
+
+        $this->expectException(InvalidArgumentException::class);
+        $this->expectExceptionMessage('Cannot index by path that does not exist or contains a null value');
+        (new Collection($items))->combine('id', 'name', 'parent');
+    }
+
     /**
      * Tests the nest method with only one level
      */

tests/TestCase/ORM/EntityTest.php

@@ -1718,4 +1718,37 @@ public function testSetOriginalFieldInClean(): void
         $return = $entity->isOriginalField('foo');
         $this->assertSame(true, $return);
     }
+
+    /**
+     * Test infinite recursion in getErrors and hasErrors
+     * See https://github.com/cakephp/cakephp/issues/17318
+     */
+    public function testGetErrorsRecursionError()
+    {
+        $entity = new Entity();
+        $secondEntity = new Entity();
+
+        $entity->set('child', $secondEntity);
+        $secondEntity->set('parent', $entity);
+
+        $expectedErrors = ['name' => ['_required' => 'Must be present.']];
+        $secondEntity->setErrors($expectedErrors);
+
+        $this->assertEquals(['child' => $expectedErrors], $entity->getErrors());
+    }
+
+    /**
+     * Test infinite recursion in getErrors and hasErrors
+     * See https://github.com/cakephp/cakephp/issues/17318
+     */
+    public function testHasErrorsRecursionError()
+    {
+        $entity = new Entity();
+        $secondEntity = new Entity();
+
+        $entity->set('child', $secondEntity);
+        $secondEntity->set('parent', $entity);
+
+        $this->assertFalse($entity->hasErrors());
+    }
 }

tests/phpstan-baseline.neon

@@ -1,3 +1,2 @@
 parameters:
 	ignoreErrors:
-

tests/test_app/TestApp/Controller/UnionDependenciesController.php

@@ -0,0 +1,36 @@
+<?php
+declare(strict_types=1);
+
+namespace TestApp\Controller;
+
+use Cake\Controller\ComponentRegistry;
+use Cake\Controller\Controller;
+use Cake\Event\EventManagerInterface;
+use Cake\Http\Response;
+use Cake\Http\ServerRequest;
+use stdClass;
+
+/**
+ * UnionDependenciesController class
+ *
+ * Separate from Dependencies Controller because unions are not supported in PHP 7.4
+ */
+class UnionDependenciesController extends Controller
+{
+    public function __construct(
+        ?ServerRequest $request = null,
+        ?Response $response = null,
+        ?string $name = null,
+        ?EventManagerInterface $eventManager = null,
+        ?ComponentRegistry $components = null,
+        ?stdClass $inject = null
+    ) {
+        parent::__construct($request, $response, $name, $eventManager, $components);
+        $this->inject = $inject;
+    }
+
+    public function typedUnion(string|int $one)
+    {
+        return $this->response->withStringBody(json_encode(compact('one')));
+    }
+}