add validation for linear ring being closed (#167)

* updated the validation to use geojson library as backup for existing json validation
* 1.8.0
Co-authored-by: John Klancer <[email protected]>

Author: sofiapag

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 =======
 
+# 1.8.0 (2022-08-25)
+- validates source in `upload-source` command using geojson library.
+- Provides line number for an invalid feature that is detected with the geojson validator.
+
 # 1.7.4 (2022-07-13)
 - validates source id for correct syntax when `upload-source` command to resolve `Connection reset by peer error`
 

mapbox_tilesets/__init__.py

@@ -1,3 +1,3 @@
 """mapbox_tilesets package"""
 
-__version__ = "1.7.4"
+__version__ = "1.8.0"

mapbox_tilesets/scripts/cli.py

@@ -504,8 +504,8 @@ def validate_source(features):
     """
     click.echo("Validating features", err=True)
 
-    for feature in features:
-        utils.validate_geojson(feature)
+    for index, feature in enumerate(features):
+        utils.validate_geojson(index, feature)
 
     click.echo("✔ valid")
 
@@ -582,9 +582,9 @@ def _upload_source(
             )
 
     with tempfile.TemporaryFile() as file:
-        for feature in features:
+        for index, feature in enumerate(features):
             if not no_validation:
-                utils.validate_geojson(feature)
+                utils.validate_geojson(index, feature)
 
             file.write(
                 (json.dumps(feature, separators=(",", ":")) + "\n").encode("utf-8")
@@ -730,8 +730,8 @@ def list_sources(username, token=None):
 
 
 def validate_stream(features):
-    for feature in features:
-        utils.validate_geojson(feature)
+    for index, feature in enumerate(features):
+        utils.validate_geojson(index, feature)
         yield feature
 
 

mapbox_tilesets/utils.py

@@ -8,6 +8,8 @@
 from requests import Session
 
 import mapbox_tilesets
+import geojson
+import json
 
 
 def load_module(modulename):
@@ -71,7 +73,15 @@ def validate_tileset_id(tileset_id):
     return re.match(pattern, tileset_id, flags=re.IGNORECASE)
 
 
-def validate_geojson(feature):
+def geojson_validate(index, feature):
+    geojsonFeature = geojson.loads(json.dumps(feature))
+    if not geojsonFeature.is_valid:
+        raise mapbox_tilesets.errors.TilesetsError(
+            f"Error in feature number {index}: " + "".join(geojsonFeature.errors())
+        )
+
+
+def validate_geojson(index, feature):
     schema = {
         "definitions": {},
         "$schema": "http://json-schema.org/draft-07/schema#",
@@ -116,8 +126,8 @@ def validate_geojson(feature):
             },
         },
     }
-
-    return validate(instance=feature, schema=schema)
+    validate(instance=feature, schema=schema)
+    geojson_validate(index, feature)
 
 
 def _convert_precision_to_zoom(precision):

requirements.txt

@@ -7,4 +7,5 @@ requests-toolbelt==0.9.1
 jsonschema==3.0.1
 jsonseq==1.0.0
 mercantile==1.1.6
-supermercado==0.2.0
+supermercado==0.2.0
+geojson===2.5.0

tests/fixtures/invalid-polygon.ldgeojson

@@ -0,0 +1,16 @@
+{
+  "type": "Feature",
+  "id":"01",
+  "geometry": {
+    "type": "Polygon",
+    "coordinates": [
+      [
+        [-150.957,-40.5948],
+        [-155,-41],
+        [-152,-42],
+        [-152, -40]
+      ]
+    ]
+  },
+  "properties":{"name":"Ducky Loo"}
+}

tests/test_cli_sources.py

@@ -224,6 +224,41 @@ def side_effect(fields):
     )
 
 
+@pytest.mark.usefixtures("token_environ")
+@mock.patch("mapbox_tilesets.scripts.cli.MultipartEncoder")
+@mock.patch("mapbox_tilesets.scripts.cli.MultipartEncoderMonitor")
+@mock.patch("requests.Session.post")
+def test_cli_upload_source_invalid_polygon(
+    mock_request_post,
+    mock_multipart_encoder,
+    MockResponse,
+    MockMultipartEncoding,
+):
+    expected_json = b'{"type":"Feature","id":"01","geometry":{"type":"Polygon","coordinates":[[-150.957,-40.5948],[-155,-41],[-152,-42],[-152,-40]]},"properties":{"name":"Ducky Loo"}}\n'
+
+    def side_effect(fields):
+        assert fields["file"][1].read() == expected_json
+        return MockMultipartEncoding()
+
+    mock_multipart_encoder.side_effect = side_effect
+
+    runner = CliRunner()
+    validated_result = runner.invoke(
+        upload_source,
+        [
+            "test-user",
+            "populated-places-source",
+            "tests/fixtures/invalid-polygon.ldgeojson",
+        ],
+    )
+    assert validated_result.exit_code == 1
+
+    assert (
+        str(validated_result.exception)
+        == "Error in feature number 0: Each linear ring must end where it started"
+    )
+
+
 @pytest.mark.usefixtures("token_environ")
 def validate_source_id(self):
     self.assertRaises(

tests/test_utils.py

@@ -5,6 +5,7 @@
     _get_api,
     _get_session,
     _get_token,
+    geojson_validate,
     validate_tileset_id,
     _convert_precision_to_zoom,
     calculate_tiles_area,
@@ -70,6 +71,49 @@ def test_validate_tileset_id_toolong():
     assert not validate_tileset_id(tileset)
 
 
+def test_geojson_validate():
+    geometry = {"type": "Polygon", "coordinates": [[[1, 2], [3, 4], [5, 6]]]}
+    with pytest.raises(TilesetsError) as excinfo:
+        geojson_validate(2, geometry)
+
+    assert (
+        str(excinfo.value)
+        == "Error in feature number 2: Each linear ring must contain at least 4 positions"
+    )
+
+
+def test_geojson_validate_open_ring():
+    geometry = {"type": "Polygon", "coordinates": [[[1, 2], [3, 4], [5, 6], [7, 8]]]}
+    with pytest.raises(TilesetsError) as excinfo:
+        geojson_validate(2, geometry)
+
+    assert (
+        str(excinfo.value)
+        == "Error in feature number 2: Each linear ring must end where it started"
+    )
+
+
+def test_geojson_validate_closed_ring():
+    geometry = {"type": "Polygon", "coordinates": [[[1, 2], [3, 4], [5, 6], [1, 2]]]}
+    assert geojson_validate(2, geometry) is None
+
+
+def test_geojson_validate_closed_ring_correct_winding_order():
+    geometry = {
+        "type": "Polygon",
+        "coordinates": [[[0, 0], [1, 0], [1, 1], [0, 1], [0, 0]]],
+    }
+    assert geojson_validate(2, geometry) is None
+
+
+def test_geojson_validate_closed_ring_incorrect_winding_order():
+    geometry = {
+        "type": "Polygon",
+        "coordinates": [[[0, 0], [0, 1], [1, 1], [1, 0], [0, 0]]],
+    }
+    assert geojson_validate(2, geometry) is None
+
+
 def test_convert_precision_to_zoom_10m():
     precision = "10m"
     assert _convert_precision_to_zoom(precision) == 6