Skip to content

Commit f39a18b

Browse files
deyihufuzhenn
andauthored
Provide a prompt when GeometryCollection is nested within itself (#2142)
* Provide a prompt when GeometryCollection is nested within itself * spec --------- Co-authored-by: Fu Zhen <fuzhen@maptalks.org>
1 parent 52355ee commit f39a18b

File tree

2 files changed

+60
-13
lines changed

2 files changed

+60
-13
lines changed

src/geometry/GeometryCollection.js

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import Coordinate from '../geo/Coordinate';
55
import PointExtent from '../geo/PointExtent';
66
import Extent from '../geo/Extent';
77
import Geometry from './Geometry';
8+
import Browser from '../core/Browser';
89

910
const TEMP_EXTENT = new PointExtent();
1011

@@ -277,20 +278,26 @@ class GeometryCollection extends Geometry {
277278
*/
278279
_checkGeometries(geometries) {
279280
const invalidGeoError = 'The geometry added to collection is invalid.';
280-
if (geometries && !Array.isArray(geometries)) {
281-
if (geometries instanceof Geometry) {
282-
return [geometries];
283-
} else {
284-
throw new Error(invalidGeoError);
281+
geometries = Array.isArray(geometries) ? geometries : [geometries];
282+
const filterGeometries = [];
283+
for (let i = 0, l = geometries.length; i < l; i++) {
284+
const geometry = geometries[i];
285+
if (!geometry) {
286+
continue;
285287
}
286-
} else {
287-
for (let i = 0, l = geometries.length; i < l; i++) {
288-
if (!this._checkGeo(geometries[i])) {
289-
throw new Error(invalidGeoError + ' Index: ' + i);
288+
if (!this._checkGeo(geometry)) {
289+
console.error(invalidGeoError + ' Index: ' + i);
290+
continue;
291+
}
292+
if (isSelf(geometry)) {
293+
if (!Browser.isTest) {
294+
console.error(geometry, ' is GeometryCollection sub class,it Cannot be placed in GeometryCollection');
290295
}
296+
continue;
291297
}
292-
return geometries;
298+
filterGeometries.push(geometry);
293299
}
300+
return filterGeometries;
294301
}
295302

296303
_checkGeo(geo) {
@@ -589,3 +596,8 @@ function computeExtent(projection, fn) {
589596

590597
return extent;
591598
}
599+
600+
function isSelf(geom) {
601+
return (geom instanceof GeometryCollection);
602+
}
603+

test/geometry/GeometryCollectionSpec.js

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,9 @@ describe('#GeometryCollection', function () {
5858

5959
expect(collection.getGeometries()).to.be.empty();
6060

61-
var geometries = GEN_GEOMETRIES_OF_ALL_TYPES();
61+
var geometries = GEN_GEOMETRIES_OF_ALL_TYPES().filter(geo => {
62+
return !(geo instanceof maptalks.GeometryCollection);
63+
})
6264
collection.setGeometries(geometries);
6365

6466
expect(collection.getGeometries()).to.eql(geometries);
@@ -102,7 +104,7 @@ describe('#GeometryCollection', function () {
102104
it('normal constructor', function () {
103105
var geometries = GEN_GEOMETRIES_OF_ALL_TYPES();
104106
var collection = new maptalks.GeometryCollection(geometries);
105-
expect(collection.getGeometries()).to.have.length(geometries.length);
107+
expect(collection.getGeometries().length).to.be.eql(geometries.length - 2);
106108
});
107109

108110
it('can be empty.', function () {
@@ -350,6 +352,40 @@ describe('#GeometryCollection', function () {
350352
});
351353
});
352354

355+
356+
it('#2141 Provide a prompt when GeometryCollection is nested within itself', function () {
357+
const pointSymbol = {
358+
markerType: 'ellipse',
359+
markerWidth: 20,
360+
markerHeight: 20
361+
};
362+
const lineSymbol = {
363+
lineColor: 'black',
364+
lineWidth: 4
365+
};
366+
367+
const fillSymbol = {
368+
polygonFill: "black",
369+
polygonOpacity: 1
370+
};
371+
const lefttop = [-0.01, 0.01, 1], righttop = [0.01, 0.01, 1], rightbottom = [0.01, -0.01, 1], leftbottom = [-0.01, -0.01, 1];
372+
const point = new maptalks.Marker(lefttop, { symbol: pointSymbol });
373+
const multipoint = new maptalks.MultiPoint([lefttop, lefttop], { symbol: pointSymbol });
374+
const line = new maptalks.LineString([lefttop, righttop], { symbol: lineSymbol });
375+
const multiline = new maptalks.MultiLineString([[lefttop, righttop], [lefttop, righttop]], { symbol: lineSymbol });
376+
const polygon = new maptalks.Polygon([[lefttop, righttop, rightbottom, leftbottom]], { symbol: fillSymbol });
377+
const multipolygon = new maptalks.MultiPolygon([[[lefttop, righttop, rightbottom, leftbottom]], [[lefttop, righttop, rightbottom, leftbottom]]], { symbol: fillSymbol });
378+
const rectange = new maptalks.Rectangle(lefttop, 2000, 1000, { symbol: fillSymbol });
379+
const ellispe = new maptalks.Ellipse(lefttop, 2000, 1000, { symbol: fillSymbol });
380+
const sector = new maptalks.Sector(lefttop, 1000, 0, 90, { symbol: fillSymbol });
381+
const circle = new maptalks.Circle(lefttop, 1000, { symbol: fillSymbol });
382+
const collectionTest = new maptalks.GeometryCollection([]);
383+
const geos = [point, multipoint, line, multiline, polygon, multipolygon, circle, rectange, ellispe, sector, collectionTest];
384+
385+
const collection = new maptalks.GeometryCollection(geos);
386+
expect(collection.getGeometries().length).to.be.eql(7);
387+
});
388+
353389
it('#2146 _toJSON(null) from feature-filter', function () {
354390
const geojson = {
355391
"type": "FeatureCollection",
@@ -447,7 +483,6 @@ describe('#GeometryCollection', function () {
447483
})
448484
layer.addGeometry(polygons)
449485
});
450-
451486
});
452487

453488
function genPoints() {

0 commit comments

Comments
 (0)