Skip to content

Commit

Permalink
relax geometry validation in GeoJsonLayer
Browse files Browse the repository at this point in the history
  • Loading branch information
Xiaoji Chen committed Jul 9, 2019
1 parent 42fd04e commit e3e1b9c
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 43 deletions.
31 changes: 14 additions & 17 deletions modules/layers/src/geojson-layer/geojson.js
Expand Up @@ -17,15 +17,7 @@
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
// THE SOFTWARE.

// Replacement for the external assert method to reduce bundle size
// Since GeoJSON format issues are common to users we do show messages in
// this case
export default function assert(condition, message) {
if (!condition) {
throw new Error(`deck.gl: ${message}`);
}
}
import {log} from '@deck.gl/core';

/**
* "Normalizes" complete or partial GeoJSON data into iterable list of features
Expand All @@ -45,15 +37,15 @@ export function getGeojsonFeatures(geojson) {
return geojson;
}

assert(geojson.type, 'GeoJSON does not have type');
log.assert(geojson.type, 'GeoJSON does not have type');

switch (geojson.type) {
case 'Feature':
// Wrap the feature in a 'Features' array
return [geojson];
case 'FeatureCollection':
// Just return the 'Features' array from the collection
assert(Array.isArray(geojson.features), 'GeoJSON does not have features array');
log.assert(Array.isArray(geojson.features), 'GeoJSON does not have features array');
return geojson.features;
default:
// Assume it's a geometry, we'll check type in separateGeojsonFeatures
Expand All @@ -75,12 +67,12 @@ export function separateGeojsonFeatures(features, wrapFeature, dataRange = {}) {
for (let featureIndex = startRow; featureIndex < endRow; featureIndex++) {
const feature = features[featureIndex];

assert(feature && feature.geometry, 'GeoJSON does not have geometry');
log.assert(feature && feature.geometry, 'GeoJSON does not have geometry');

const {geometry} = feature;

if (geometry.type === 'GeometryCollection') {
assert(Array.isArray(geometry.geometries), 'GeoJSON does not have geometries array');
log.assert(Array.isArray(geometry.geometries), 'GeoJSON does not have geometries array');
const {geometries} = geometry;
for (let i = 0; i < geometries.length; i++) {
const subGeometry = geometries[i];
Expand All @@ -98,7 +90,11 @@ function separateGeometry(geometry, separated, wrapFeature, sourceFeature, sourc
const {type, coordinates} = geometry;
const {pointFeatures, lineFeatures, polygonFeatures, polygonOutlineFeatures} = separated;

checkCoordinates(type, coordinates);
if (!validateGeometry(type, coordinates)) {
// Avoid hard failure if some features are malformed
log.warn(`${type} coordinates are malformed`);
return;
}

// Split each feature, but keep track of the source feature and index (for Multi* geometries)
switch (type) {
Expand Down Expand Up @@ -219,13 +215,14 @@ const COORDINATE_NEST_LEVEL = {
MultiPolygon: 4
};

function checkCoordinates(type, coordinates) {
export function validateGeometry(type, coordinates) {
let nestLevel = COORDINATE_NEST_LEVEL[type];

assert(nestLevel, `Unknown GeoJSON type ${type}`);
log.assert(nestLevel, `Unknown GeoJSON type ${type}`);

while (coordinates && --nestLevel > 0) {
coordinates = coordinates[0];
}
assert(coordinates && Number.isFinite(coordinates[0]), `${type} coordinates are malformed`);

return coordinates && Number.isFinite(coordinates[0]);
}
100 changes: 74 additions & 26 deletions test/modules/layers/geojson.spec.js
Expand Up @@ -19,7 +19,11 @@
// THE SOFTWARE.

import test from 'tape-catch';
import {getGeojsonFeatures, separateGeojsonFeatures} from '@deck.gl/layers/geojson-layer/geojson';
import {
getGeojsonFeatures,
separateGeojsonFeatures,
validateGeometry
} from '@deck.gl/layers/geojson-layer/geojson';

const TEST_DATA = {
POINT: {
Expand Down Expand Up @@ -62,6 +66,16 @@ const TEST_DATA = {
{
type: 'LineString',
coordinates: [[101.0, 0.0], [102.0, 1.0]]
},
{
// empty coordinates, should warn but not throw
type: 'LineString',
coordinates: []
},
{
// empty coordinates, should warn but not throw
type: 'MultiPolygon',
coordinates: []
}
]
}
Expand Down Expand Up @@ -277,31 +291,6 @@ const TEST_CASES = [
title: 'unknown geojson type',
argument: {type: 'Feature', geometry: {type: 'Something', coordinates: [0, 0]}},
error: /unknown geojson type/i
},
{
title: 'malformed geojson: Point',
argument: {type: 'Point'},
error: /coordinates are malformed/i
},
{
title: 'malformed geojson: Point',
argument: {type: 'Point', coordinates: 1},
error: /coordinates are malformed/i
},
{
title: 'malformed geojson: Point',
argument: {type: 'Point', coordinates: [[0, 0]]},
error: /coordinates are malformed/i
},
{
title: 'malformed geojson: Polygon',
argument: {type: 'Polygon', coordinates: [[0, 0]]},
error: /coordinates are malformed/i
},
{
title: 'malformed geojson: MultiPolygon',
argument: {type: 'MultiPolygon', coordinates: [[[0, 0]]]},
error: /coordinates are malformed/i
}
];

Expand Down Expand Up @@ -368,3 +357,62 @@ test('geojson#getGeojsonFeatures, separateGeojsonFeatures', t => {
}
t.end();
});

const TEST_GEOMETRIES = [
{
argument: TEST_DATA.POINT,
isValid: true
},
{
argument: TEST_DATA.LINESTRING,
isValid: true
},
{
argument: TEST_DATA.POLYGON,
isValid: true
},
{
argument: TEST_DATA.MULTI_POINT,
isValid: true
},
{
argument: TEST_DATA.MULTI_LINESTRING,
isValid: true
},
{
argument: TEST_DATA.MULTI_POLYGON,
isValid: true
},
{
argument: {type: 'Point'},
isValid: false
},
{
argument: {type: 'Point', coordinates: 1},
isValid: false
},
{
argument: {type: 'Point', coordinates: [[0, 0]]},
isValid: false
},
{
argument: {type: 'Polygon', coordinates: [[0, 0]]},
isValid: false
},
{
argument: {type: 'MultiPolygon', coordinates: [[[0, 0]]]},
isValid: false
}
];

test('validateGeometry', t => {
for (const testCase of TEST_GEOMETRIES) {
t.is(
Boolean(validateGeometry(testCase.argument.type, testCase.argument.coordinates)),
testCase.isValid,
'validateGeometry returns correct result'
);
}

t.end();
});

0 comments on commit e3e1b9c

Please sign in to comment.