Skip to content

Commit e5e4c89

Browse files
authored
Merge pull request #120 from grimzy/issue-95-point-collection-min-items
Fix validation when creating GeometryCollections
2 parents 2960628 + acd15ea commit e5e4c89

15 files changed

+215
-83
lines changed

composer.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,15 +15,15 @@
1515
}
1616
],
1717
"require": {
18-
"php": ">=5.5",
18+
"php": ">=5.5.9",
1919
"ext-pdo": "*",
2020
"ext-json": "*",
2121
"illuminate/database": "^5.2|^6.0|^7.0",
2222
"geo-io/wkb-parser": "^1.0",
2323
"jmikola/geojson": "^1.0"
2424
},
2525
"require-dev": {
26-
"phpunit/phpunit": "~4.8||~5.7",
26+
"phpunit/phpunit": "~4.8|~5.7",
2727
"mockery/mockery": "^0.9.9",
2828
"laravel/laravel": "^5.2|^6.0|^7.0",
2929
"doctrine/dbal": "^2.5",

src/Types/GeometryCollection.php

Lines changed: 73 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,20 @@
1414

1515
class GeometryCollection extends Geometry implements IteratorAggregate, ArrayAccess, Arrayable, Countable
1616
{
17+
/**
18+
* The minimum number of items required to create this collection.
19+
*
20+
* @var int
21+
*/
22+
protected $minimumCollectionItems = 0;
23+
24+
/**
25+
* The class of the items in the collection.
26+
*
27+
* @var string
28+
*/
29+
protected $collectionItemType = GeometryInterface::class;
30+
1731
/**
1832
* The items contained in the spatial collection.
1933
*
@@ -28,13 +42,7 @@ class GeometryCollection extends Geometry implements IteratorAggregate, ArrayAcc
2842
*/
2943
public function __construct(array $geometries)
3044
{
31-
$validated = array_filter($geometries, function ($value) {
32-
return $value instanceof GeometryInterface;
33-
});
34-
35-
if (count($geometries) !== count($validated)) {
36-
throw new InvalidArgumentException('$geometries must be an array of Geometry objects');
37-
}
45+
$this->validateItems($geometries);
3846

3947
$this->items = $geometries;
4048
}
@@ -58,6 +66,10 @@ public function __toString()
5866

5967
public static function fromString($wktArgument)
6068
{
69+
if (empty($wktArgument)) {
70+
return new static([]);
71+
}
72+
6173
$geometry_strings = preg_split('/,\s*(?=[A-Za-z])/', $wktArgument);
6274

6375
return new static(array_map(function ($geometry_string) {
@@ -89,9 +101,7 @@ public function offsetGet($offset)
89101

90102
public function offsetSet($offset, $value)
91103
{
92-
if (!($value instanceof GeometryInterface)) {
93-
throw new InvalidArgumentException('$value must be an instance of GeometryInterface');
94-
}
104+
$this->validateItemType($value);
95105

96106
if (is_null($offset)) {
97107
$this->items[] = $value;
@@ -142,4 +152,57 @@ public function jsonSerialize()
142152

143153
return new \GeoJson\Geometry\GeometryCollection($geometries);
144154
}
155+
156+
/**
157+
* Checks whether the items are valid to create this collection.
158+
*
159+
* @param array $items
160+
*/
161+
protected function validateItems(array $items)
162+
{
163+
$this->validateItemCount($items);
164+
165+
foreach ($items as $item) {
166+
$this->validateItemType($item);
167+
}
168+
}
169+
170+
/**
171+
* Checks whether the array has enough items to generate a valid WKT.
172+
*
173+
* @param array $items
174+
*
175+
* @see $minimumCollectionItems
176+
*/
177+
protected function validateItemCount(array $items)
178+
{
179+
if (count($items) < $this->minimumCollectionItems) {
180+
$entries = $this->minimumCollectionItems === 1 ? 'entry' : 'entries';
181+
182+
throw new InvalidArgumentException(sprintf(
183+
'%s must contain at least %d %s',
184+
get_class($this),
185+
$this->minimumCollectionItems,
186+
$entries
187+
));
188+
}
189+
}
190+
191+
/**
192+
* Checks the type of the items in the array.
193+
*
194+
* @param $item
195+
*
196+
* @see $collectionItemType
197+
*/
198+
protected function validateItemType($item)
199+
{
200+
if (!$item instanceof $this->collectionItemType) {
201+
throw new InvalidArgumentException(sprintf(
202+
'%s must be a collection of %s',
203+
get_class($this),
204+
$this->collectionItemType
205+
));
206+
}
207+
}
145208
}

src/Types/LineString.php

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,13 @@
88

99
class LineString extends PointCollection
1010
{
11+
/**
12+
* The minimum number of items required to create this collection.
13+
*
14+
* @var int
15+
*/
16+
protected $minimumCollectionItems = 2;
17+
1118
public function toWKT()
1219
{
1320
return sprintf('LINESTRING(%s)', $this->toPairList());

src/Types/MultiLineString.php

Lines changed: 11 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -5,29 +5,22 @@
55
use GeoJson\GeoJson;
66
use GeoJson\Geometry\MultiLineString as GeoJsonMultiLineString;
77
use Grimzy\LaravelMysqlSpatial\Exceptions\InvalidGeoJsonException;
8-
use InvalidArgumentException;
98

109
class MultiLineString extends GeometryCollection
1110
{
1211
/**
13-
* @param LineString[] $lineStrings
12+
* The minimum number of items required to create this collection.
13+
*
14+
* @var int
1415
*/
15-
public function __construct(array $lineStrings)
16-
{
17-
if (count($lineStrings) < 1) {
18-
throw new InvalidArgumentException('$lineStrings must contain at least one entry');
19-
}
20-
21-
$validated = array_filter($lineStrings, function ($value) {
22-
return $value instanceof LineString;
23-
});
24-
25-
if (count($lineStrings) !== count($validated)) {
26-
throw new InvalidArgumentException('$lineStrings must be an array of LineString');
27-
}
16+
protected $minimumCollectionItems = 1;
2817

29-
parent::__construct($lineStrings);
30-
}
18+
/**
19+
* The class of the items in the collection.
20+
*
21+
* @var string
22+
*/
23+
protected $collectionItemType = LineString::class;
3124

3225
public function getLineStrings()
3326
{
@@ -58,9 +51,7 @@ public function __toString()
5851

5952
public function offsetSet($offset, $value)
6053
{
61-
if (!($value instanceof LineString)) {
62-
throw new InvalidArgumentException('$value must be an instance of LineString');
63-
}
54+
$this->validateItemType($value);
6455

6556
parent::offsetSet($offset, $value);
6657
}

src/Types/MultiPoint.php

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,13 @@
88

99
class MultiPoint extends PointCollection
1010
{
11+
/**
12+
* The minimum number of items required to create this collection.
13+
*
14+
* @var int
15+
*/
16+
protected $minimumCollectionItems = 1;
17+
1118
public function toWKT()
1219
{
1320
return sprintf('MULTIPOINT(%s)', (string) $this);

src/Types/MultiPolygon.php

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,24 +5,22 @@
55
use GeoJson\GeoJson;
66
use GeoJson\Geometry\MultiPolygon as GeoJsonMultiPolygon;
77
use Grimzy\LaravelMysqlSpatial\Exceptions\InvalidGeoJsonException;
8-
use InvalidArgumentException;
98

109
class MultiPolygon extends GeometryCollection
1110
{
1211
/**
13-
* @param Polygon[] $polygons
12+
* The minimum number of items required to create this collection.
13+
*
14+
* @var int
1415
*/
15-
public function __construct(array $polygons)
16-
{
17-
$validated = array_filter($polygons, function ($value) {
18-
return $value instanceof Polygon;
19-
});
16+
protected $minimumCollectionItems = 1;
2017

21-
if (count($polygons) !== count($validated)) {
22-
throw new InvalidArgumentException('$polygons must be an array of Polygon');
23-
}
24-
parent::__construct($polygons);
25-
}
18+
/**
19+
* The class of the items in the collection.
20+
*
21+
* @var string
22+
*/
23+
protected $collectionItemType = Polygon::class;
2624

2725
public function toWKT()
2826
{
@@ -93,9 +91,7 @@ protected static function assembleParts(array $parts)
9391

9492
public function offsetSet($offset, $value)
9593
{
96-
if (!($value instanceof Polygon)) {
97-
throw new InvalidArgumentException('$value must be an instance of Polygon');
98-
}
94+
$this->validateItemType($value);
9995

10096
parent::offsetSet($offset, $value);
10197
}

src/Types/PointCollection.php

Lines changed: 5 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -8,24 +8,11 @@
88
abstract class PointCollection extends GeometryCollection
99
{
1010
/**
11-
* @param Point[] $points
11+
* The class of the items in the collection.
12+
*
13+
* @var string
1214
*/
13-
public function __construct(array $points)
14-
{
15-
if (count($points) < 2) {
16-
throw new InvalidArgumentException('$points must contain at least two entries');
17-
}
18-
19-
$validated = array_filter($points, function ($value) {
20-
return $value instanceof Point;
21-
});
22-
23-
if (count($points) !== count($validated)) {
24-
throw new InvalidArgumentException('$points must be an array of Points');
25-
}
26-
27-
parent::__construct($points);
28-
}
15+
protected $collectionItemType = Point::class;
2916

3017
public function toPairList()
3118
{
@@ -36,9 +23,7 @@ public function toPairList()
3623

3724
public function offsetSet($offset, $value)
3825
{
39-
if (!($value instanceof Point)) {
40-
throw new InvalidArgumentException('$value must be an instance of Point');
41-
}
26+
$this->validateItemType($value);
4227

4328
parent::offsetSet($offset, $value);
4429
}

tests/Integration/SpatialTest.php

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,17 @@ public function testInsertGeometryCollection()
242242
$this->assertDatabaseHas('geometry', ['id' => $geo->id]);
243243
}
244244

245+
public function testInsertEmptyGeometryCollection()
246+
{
247+
$geo = new GeometryModel();
248+
249+
$geo->location = new Point(1, 2);
250+
251+
$geo->multi_geometries = new GeometryCollection([]);
252+
$geo->save();
253+
$this->assertDatabaseHas('geometry', ['id' => $geo->id]);
254+
}
255+
245256
public function testUpdate()
246257
{
247258
$geo = new GeometryModel();

tests/Unit/BaseTestCase.php

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,14 @@ public function tearDown()
77
Mockery::close();
88
}
99

10-
protected function assertException($exceptionName)
10+
protected function assertException($exceptionName, $exceptionMessage = '', $exceptionCode = 0)
1111
{
1212
if (method_exists(parent::class, 'expectException')) {
1313
parent::expectException($exceptionName);
14+
parent::expectExceptionMessage($exceptionMessage);
15+
parent::expectExceptionCode($exceptionCode);
1416
} else {
15-
$this->setExpectedException($exceptionName);
17+
$this->setExpectedException($exceptionName, $exceptionMessage, $exceptionCode);
1618
}
1719
}
1820
}

tests/Unit/Eloquent/SpatialTraitTest.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -484,7 +484,10 @@ public function testScopeDoesTouch()
484484
public function testScopeOrderBySpatialThrowsExceptionWhenFunctionNotRegistered()
485485
{
486486
$point = new Point(1, 2);
487-
$this->assertException(\Grimzy\LaravelMysqlSpatial\Exceptions\UnknownSpatialFunctionException::class);
487+
$this->assertException(
488+
\Grimzy\LaravelMysqlSpatial\Exceptions\UnknownSpatialFunctionException::class,
489+
'does-not-exist'
490+
);
488491
TestModel::orderBySpatial('point', $point, 'does-not-exist');
489492
}
490493

tests/Unit/Types/GeometryCollectionTest.php

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,34 @@ public function testJsonSerialize()
3333
$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()));
3434
}
3535

36+
public function testCanCreateEmptyGeometryCollection()
37+
{
38+
$geometryCollection = new GeometryCollection([]);
39+
$this->assertInstanceOf(GeometryCollection::class, $geometryCollection);
40+
}
41+
42+
public function testFromWKTWithEmptyGeometryCollection()
43+
{
44+
/**
45+
* @var GeometryCollection
46+
*/
47+
$geometryCollection = GeometryCollection::fromWKT('GEOMETRYCOLLECTION()');
48+
$this->assertInstanceOf(GeometryCollection::class, $geometryCollection);
49+
50+
$this->assertEquals(0, $geometryCollection->count());
51+
}
52+
53+
public function testToWKTWithEmptyGeometryCollection()
54+
{
55+
$this->assertEquals('GEOMETRYCOLLECTION()', (new GeometryCollection([]))->toWKT());
56+
}
57+
3658
public function testInvalidArgumentExceptionNotArrayGeometries()
3759
{
38-
$this->assertException(InvalidArgumentException::class);
60+
$this->assertException(
61+
InvalidArgumentException::class,
62+
'Grimzy\LaravelMysqlSpatial\Types\GeometryCollection must be a collection of Grimzy\LaravelMysqlSpatial\Types\GeometryInterface'
63+
);
3964
$geometrycollection = new GeometryCollection([
4065
$this->getPoint(),
4166
1,
@@ -85,7 +110,10 @@ public function testArrayAccess()
85110
$this->assertEquals($point100, $geometryCollection[100]);
86111

87112
// assert invalid
88-
$this->assertException(InvalidArgumentException::class);
113+
$this->assertException(
114+
InvalidArgumentException::class,
115+
'Grimzy\LaravelMysqlSpatial\Types\GeometryCollection must be a collection of Grimzy\LaravelMysqlSpatial\Types\GeometryInterface'
116+
);
89117
$geometryCollection[] = 1;
90118
}
91119

0 commit comments

Comments
 (0)