Merge pull request #120 from grimzy/issue-95-point-collection-min-items

Fix validation when creating GeometryCollections
grimzy · Mar 8, 2020 · e5e4c89 · e5e4c89
2 parents 2960628 + acd15ea
commit e5e4c89
Show file tree

Hide file tree

Showing 15 changed files with 215 additions and 83 deletions.
diff --git a/composer.json b/composer.json
@@ -15,15 +15,15 @@
         }
     ],
     "require": {
-        "php": ">=5.5",
+        "php": ">=5.5.9",
         "ext-pdo": "*",
         "ext-json": "*",
         "illuminate/database": "^5.2|^6.0|^7.0",
         "geo-io/wkb-parser": "^1.0",
         "jmikola/geojson": "^1.0"
     },
     "require-dev": {
-        "phpunit/phpunit": "~4.8||~5.7",
+        "phpunit/phpunit": "~4.8|~5.7",
         "mockery/mockery": "^0.9.9",
         "laravel/laravel": "^5.2|^6.0|^7.0",
         "doctrine/dbal": "^2.5",

diff --git a/src/Types/GeometryCollection.php b/src/Types/GeometryCollection.php
@@ -14,6 +14,20 @@
 
 class GeometryCollection extends Geometry implements IteratorAggregate, ArrayAccess, Arrayable, Countable
 {
+    /**
+     * The minimum number of items required to create this collection.
+     *
+     * @var int
+     */
+    protected $minimumCollectionItems = 0;
+
+    /**
+     * The class of the items in the collection.
+     *
+     * @var string
+     */
+    protected $collectionItemType = GeometryInterface::class;
+
     /**
      * The items contained in the spatial collection.
      *
@@ -28,13 +42,7 @@ class GeometryCollection extends Geometry implements IteratorAggregate, ArrayAcc
      */
     public function __construct(array $geometries)
     {
-        $validated = array_filter($geometries, function ($value) {
-            return $value instanceof GeometryInterface;
-        });
-
-        if (count($geometries) !== count($validated)) {
-            throw new InvalidArgumentException('$geometries must be an array of Geometry objects');
-        }
+        $this->validateItems($geometries);
 
         $this->items = $geometries;
     }
@@ -58,6 +66,10 @@ public function __toString()
 
     public static function fromString($wktArgument)
     {
+        if (empty($wktArgument)) {
+            return new static([]);
+        }
+
         $geometry_strings = preg_split('/,\s*(?=[A-Za-z])/', $wktArgument);
 
         return new static(array_map(function ($geometry_string) {
@@ -89,9 +101,7 @@ public function offsetGet($offset)
 
     public function offsetSet($offset, $value)
     {
-        if (!($value instanceof GeometryInterface)) {
-            throw new InvalidArgumentException('$value must be an instance of GeometryInterface');
-        }
+        $this->validateItemType($value);
 
         if (is_null($offset)) {
             $this->items[] = $value;
@@ -142,4 +152,57 @@ public function jsonSerialize()
 
         return new \GeoJson\Geometry\GeometryCollection($geometries);
     }
+
+    /**
+     * Checks whether the items are valid to create this collection.
+     *
+     * @param array $items
+     */
+    protected function validateItems(array $items)
+    {
+        $this->validateItemCount($items);
+
+        foreach ($items as $item) {
+            $this->validateItemType($item);
+        }
+    }
+
+    /**
+     * Checks whether the array has enough items to generate a valid WKT.
+     *
+     * @param array $items
+     *
+     * @see $minimumCollectionItems
+     */
+    protected function validateItemCount(array $items)
+    {
+        if (count($items) < $this->minimumCollectionItems) {
+            $entries = $this->minimumCollectionItems === 1 ? 'entry' : 'entries';
+
+            throw new InvalidArgumentException(sprintf(
+                '%s must contain at least %d %s',
+                get_class($this),
+                $this->minimumCollectionItems,
+                $entries
+            ));
+        }
+    }
+
+    /**
+     * Checks the type of the items in the array.
+     *
+     * @param $item
+     *
+     * @see $collectionItemType
+     */
+    protected function validateItemType($item)
+    {
+        if (!$item instanceof $this->collectionItemType) {
+            throw new InvalidArgumentException(sprintf(
+                '%s must be a collection of %s',
+                get_class($this),
+                $this->collectionItemType
+            ));
+        }
+    }
 }
diff --git a/src/Types/LineString.php b/src/Types/LineString.php
@@ -8,6 +8,13 @@
 
 class LineString extends PointCollection
 {
+    /**
+     * The minimum number of items required to create this collection.
+     *
+     * @var int
+     */
+    protected $minimumCollectionItems = 2;
+
     public function toWKT()
     {
         return sprintf('LINESTRING(%s)', $this->toPairList());

diff --git a/src/Types/MultiLineString.php b/src/Types/MultiLineString.php
@@ -5,29 +5,22 @@
 use GeoJson\GeoJson;
 use GeoJson\Geometry\MultiLineString as GeoJsonMultiLineString;
 use Grimzy\LaravelMysqlSpatial\Exceptions\InvalidGeoJsonException;
-use InvalidArgumentException;
 
 class MultiLineString extends GeometryCollection
 {
     /**
-     * @param LineString[] $lineStrings
+     * The minimum number of items required to create this collection.
+     *
+     * @var int
      */
-    public function __construct(array $lineStrings)
-    {
-        if (count($lineStrings) < 1) {
-            throw new InvalidArgumentException('$lineStrings must contain at least one entry');
-        }
-
-        $validated = array_filter($lineStrings, function ($value) {
-            return $value instanceof LineString;
-        });
-
-        if (count($lineStrings) !== count($validated)) {
-            throw new InvalidArgumentException('$lineStrings must be an array of LineString');
-        }
+    protected $minimumCollectionItems = 1;
 
-        parent::__construct($lineStrings);
-    }
+    /**
+     * The class of the items in the collection.
+     *
+     * @var string
+     */
+    protected $collectionItemType = LineString::class;
 
     public function getLineStrings()
     {
@@ -58,9 +51,7 @@ public function __toString()
 
     public function offsetSet($offset, $value)
     {
-        if (!($value instanceof LineString)) {
-            throw new InvalidArgumentException('$value must be an instance of LineString');
-        }
+        $this->validateItemType($value);
 
         parent::offsetSet($offset, $value);
     }

diff --git a/src/Types/MultiPoint.php b/src/Types/MultiPoint.php
@@ -8,6 +8,13 @@
 
 class MultiPoint extends PointCollection
 {
+    /**
+     * The minimum number of items required to create this collection.
+     *
+     * @var int
+     */
+    protected $minimumCollectionItems = 1;
+
     public function toWKT()
     {
         return sprintf('MULTIPOINT(%s)', (string) $this);

diff --git a/src/Types/MultiPolygon.php b/src/Types/MultiPolygon.php
@@ -5,24 +5,22 @@
 use GeoJson\GeoJson;
 use GeoJson\Geometry\MultiPolygon as GeoJsonMultiPolygon;
 use Grimzy\LaravelMysqlSpatial\Exceptions\InvalidGeoJsonException;
-use InvalidArgumentException;
 
 class MultiPolygon extends GeometryCollection
 {
     /**
-     * @param Polygon[] $polygons
+     * The minimum number of items required to create this collection.
+     *
+     * @var int
      */
-    public function __construct(array $polygons)
-    {
-        $validated = array_filter($polygons, function ($value) {
-            return $value instanceof Polygon;
-        });
+    protected $minimumCollectionItems = 1;
 
-        if (count($polygons) !== count($validated)) {
-            throw new InvalidArgumentException('$polygons must be an array of Polygon');
-        }
-        parent::__construct($polygons);
-    }
+    /**
+     * The class of the items in the collection.
+     *
+     * @var string
+     */
+    protected $collectionItemType = Polygon::class;
 
     public function toWKT()
     {
@@ -93,9 +91,7 @@ protected static function assembleParts(array $parts)
 
     public function offsetSet($offset, $value)
     {
-        if (!($value instanceof Polygon)) {
-            throw new InvalidArgumentException('$value must be an instance of Polygon');
-        }
+        $this->validateItemType($value);
 
         parent::offsetSet($offset, $value);
     }

diff --git a/src/Types/PointCollection.php b/src/Types/PointCollection.php
@@ -8,24 +8,11 @@
 abstract class PointCollection extends GeometryCollection
 {
     /**
-     * @param Point[] $points
+     * The class of the items in the collection.
+     *
+     * @var string
      */
-    public function __construct(array $points)
-    {
-        if (count($points) < 2) {
-            throw new InvalidArgumentException('$points must contain at least two entries');
-        }
-
-        $validated = array_filter($points, function ($value) {
-            return $value instanceof Point;
-        });
-
-        if (count($points) !== count($validated)) {
-            throw new InvalidArgumentException('$points must be an array of Points');
-        }
-
-        parent::__construct($points);
-    }
+    protected $collectionItemType = Point::class;
 
     public function toPairList()
     {
@@ -36,9 +23,7 @@ public function toPairList()
 
     public function offsetSet($offset, $value)
     {
-        if (!($value instanceof Point)) {
-            throw new InvalidArgumentException('$value must be an instance of Point');
-        }
+        $this->validateItemType($value);
 
         parent::offsetSet($offset, $value);
     }

diff --git a/tests/Integration/SpatialTest.php b/tests/Integration/SpatialTest.php
@@ -242,6 +242,17 @@ public function testInsertGeometryCollection()
         $this->assertDatabaseHas('geometry', ['id' => $geo->id]);
     }
 
+    public function testInsertEmptyGeometryCollection()
+    {
+        $geo = new GeometryModel();
+
+        $geo->location = new Point(1, 2);
+
+        $geo->multi_geometries = new GeometryCollection([]);
+        $geo->save();
+        $this->assertDatabaseHas('geometry', ['id' => $geo->id]);
+    }
+
     public function testUpdate()
     {
         $geo = new GeometryModel();

diff --git a/tests/Unit/BaseTestCase.php b/tests/Unit/BaseTestCase.php
@@ -7,12 +7,14 @@ public function tearDown()
         Mockery::close();
     }
 
-    protected function assertException($exceptionName)
+    protected function assertException($exceptionName, $exceptionMessage = '', $exceptionCode = 0)
     {
         if (method_exists(parent::class, 'expectException')) {
             parent::expectException($exceptionName);
+            parent::expectExceptionMessage($exceptionMessage);
+            parent::expectExceptionCode($exceptionCode);
         } else {
-            $this->setExpectedException($exceptionName);
+            $this->setExpectedException($exceptionName, $exceptionMessage, $exceptionCode);
         }
     }
 }
diff --git a/tests/Unit/Eloquent/SpatialTraitTest.php b/tests/Unit/Eloquent/SpatialTraitTest.php
@@ -484,7 +484,10 @@ public function testScopeDoesTouch()
     public function testScopeOrderBySpatialThrowsExceptionWhenFunctionNotRegistered()
     {
         $point = new Point(1, 2);
-        $this->assertException(\Grimzy\LaravelMysqlSpatial\Exceptions\UnknownSpatialFunctionException::class);
+        $this->assertException(
+            \Grimzy\LaravelMysqlSpatial\Exceptions\UnknownSpatialFunctionException::class,
+            'does-not-exist'
+        );
         TestModel::orderBySpatial('point', $point, 'does-not-exist');
     }
 

diff --git a/tests/Unit/Types/GeometryCollectionTest.php b/tests/Unit/Types/GeometryCollectionTest.php
@@ -33,9 +33,34 @@ public function testJsonSerialize()
         $this->assertSame('{"type":"GeometryCollection","geometries":[{"type":"LineString","coordinates":[[0,0],[1,0],[1,1],[0,1],[0,0]]},{"type":"Point","coordinates":[200,100]}]}', json_encode($this->getGeometryCollection()->jsonSerialize()));
     }
 
+    public function testCanCreateEmptyGeometryCollection()
+    {
+        $geometryCollection = new GeometryCollection([]);
+        $this->assertInstanceOf(GeometryCollection::class, $geometryCollection);
+    }
+
+    public function testFromWKTWithEmptyGeometryCollection()
+    {
+        /**
+         * @var GeometryCollection
+         */
+        $geometryCollection = GeometryCollection::fromWKT('GEOMETRYCOLLECTION()');
+        $this->assertInstanceOf(GeometryCollection::class, $geometryCollection);
+
+        $this->assertEquals(0, $geometryCollection->count());
+    }
+
+    public function testToWKTWithEmptyGeometryCollection()
+    {
+        $this->assertEquals('GEOMETRYCOLLECTION()', (new GeometryCollection([]))->toWKT());
+    }
+
     public function testInvalidArgumentExceptionNotArrayGeometries()
     {
-        $this->assertException(InvalidArgumentException::class);
+        $this->assertException(
+            InvalidArgumentException::class,
+            'Grimzy\LaravelMysqlSpatial\Types\GeometryCollection must be a collection of Grimzy\LaravelMysqlSpatial\Types\GeometryInterface'
+        );
         $geometrycollection = new GeometryCollection([
             $this->getPoint(),
             1,
@@ -85,7 +110,10 @@ public function testArrayAccess()
         $this->assertEquals($point100, $geometryCollection[100]);
 
         // assert invalid
-        $this->assertException(InvalidArgumentException::class);
+        $this->assertException(
+            InvalidArgumentException::class,
+            'Grimzy\LaravelMysqlSpatial\Types\GeometryCollection must be a collection of Grimzy\LaravelMysqlSpatial\Types\GeometryInterface'
+        );
         $geometryCollection[] = 1;
     }