Browse Source

Throw exception on invalid path for groupBy and indexBy

Corey Taylor 5 years ago
parent
commit
b227fb6628
2 changed files with 55 additions and 2 deletions
  1. 16 2
      src/Collection/CollectionTrait.php
  2. 39 0
      tests/TestCase/Collection/CollectionTest.php

+ 16 - 2
src/Collection/CollectionTrait.php

@@ -267,7 +267,14 @@ trait CollectionTrait
         $callback = $this->_propertyExtractor($path);
         $group = [];
         foreach ($this->optimizeUnwrap() as $value) {
-            $group[$callback($value)][] = $value;
+            $pathValue = $callback($value);
+            if ($pathValue === null) {
+                throw new InvalidArgumentException(
+                    'Cannot use a nonexistent path or null value. ' .
+                    'Use a callback to provide default values if necessary.'
+                );
+            }
+            $group[$pathValue][] = $value;
         }
 
         return $this->newCollection($group);
@@ -281,7 +288,14 @@ trait CollectionTrait
         $callback = $this->_propertyExtractor($path);
         $group = [];
         foreach ($this->optimizeUnwrap() as $value) {
-            $group[$callback($value)] = $value;
+            $pathValue = $callback($value);
+            if ($pathValue === null) {
+                throw new InvalidArgumentException(
+                    'Cannot use a nonexistent path or null value. ' .
+                    'Use a callback to provide default values if necessary.'
+                );
+            }
+            $group[$pathValue] = $value;
         }
 
         return $this->newCollection($group);

+ 39 - 0
tests/TestCase/Collection/CollectionTest.php

@@ -21,6 +21,7 @@ use ArrayObject;
 use Cake\Collection\Collection;
 use Cake\ORM\Entity;
 use Cake\TestSuite\TestCase;
+use InvalidArgumentException;
 use NoRewindIterator;
 use stdClass;
 use TestApp\Collection\CountableIterator;
@@ -710,6 +711,25 @@ class CollectionTest extends TestCase
     }
 
     /**
+     * Tests passing an invalid path to groupBy.
+     *
+     * @return void
+     */
+    public function testGroupByInvalidPath()
+    {
+        $items = [
+            ['id' => 1, 'name' => 'foo'],
+            ['id' => 2, 'name' => 'bar'],
+            ['id' => 3, 'name' => 'baz'],
+        ];
+        $collection = new Collection($items);
+
+        $this->expectException(InvalidArgumentException::class);
+        $this->expectExceptionMessage('Cannot use a nonexistent path or null value.');
+        $collection->groupBy('missing');
+    }
+
+    /**
      * Provider for some indexBy tests
      *
      * @return array
@@ -789,6 +809,25 @@ class CollectionTest extends TestCase
     }
 
     /**
+     * Tests passing an invalid path to indexBy.
+     *
+     * @return void
+     */
+    public function testIndexByInvalidPath()
+    {
+        $items = [
+            ['id' => 1, 'name' => 'foo'],
+            ['id' => 2, 'name' => 'bar'],
+            ['id' => 3, 'name' => 'baz'],
+        ];
+        $collection = new Collection($items);
+
+        $this->expectException(InvalidArgumentException::class);
+        $this->expectExceptionMessage('Cannot use a nonexistent path or null value.');
+        $collection->indexBy('missing');
+    }
+
+    /**
      * Tests countBy
      *
      * @dataProvider groupByProvider