Skip to content

Port: Fix validation when creating GeometryCollections (MySQL 5.6) #123

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Mar 8, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,16 @@
],
"require": {
"php": ">=5.5.9",
"illuminate/database": "^5.2||^6.0||^7.0",
"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",
"laravel/laravel": "^5.2|^6.0|^7.0",
"codeclimate/php-test-reporter": "dev-master",
"doctrine/dbal": "^2.5",
"laravel/browser-kit-testing": "^2.0"
Expand Down
83 changes: 73 additions & 10 deletions src/Types/GeometryCollection.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand All @@ -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;
}
Expand All @@ -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) {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
));
}
}
}
7 changes: 7 additions & 0 deletions src/Types/LineString.php
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
31 changes: 11 additions & 20 deletions src/Types/MultiLineString.php
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand Down Expand Up @@ -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);
}
Expand Down
7 changes: 7 additions & 0 deletions src/Types/MultiPoint.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
26 changes: 11 additions & 15 deletions src/Types/MultiPolygon.php
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand Down Expand Up @@ -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);
}
Expand Down
25 changes: 5 additions & 20 deletions src/Types/PointCollection.php
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand All @@ -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);
}
Expand Down
11 changes: 11 additions & 0 deletions tests/Integration/SpatialTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,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();
Expand Down
6 changes: 4 additions & 2 deletions tests/Unit/BaseTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
3 changes: 2 additions & 1 deletion tests/Unit/Eloquent/SpatialTraitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ public function testInsertUpdatePointHasCorrectSql()

$this->assertStringStartsWith('update', $this->queries[1]);
$this->assertContains('update `test_models` set `point` = ST_GeomFromText(?) where `id` = ?', $this->queries[1]);
// TODO: assert bindings in query
}

public function testInsertUpdateLineStringHasCorrectSql()
Expand Down Expand Up @@ -421,7 +422,7 @@ class TestModel extends Model
{
use \Grimzy\LaravelMysqlSpatial\Eloquent\SpatialTrait;

protected $spatialFields = ['point']; // only required when fetching, not saving
protected $spatialFields = ['point']; // TODO: only required when fetching, not saving

public $timestamps = false;

Expand Down
Loading