Skip to content

Commit

Permalink
more degenerate cases
Browse files Browse the repository at this point in the history
  • Loading branch information
@msbarry
msbarry committed Jan 24, 2025
1 parent fa498b7 commit a7ba114
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 18 deletions.
40 changes: 26 additions & 14 deletions ...r-core/src/main/java/com/onthegomap/planetiler/reader/geojson/GeoJsonFeatureIterator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import com.fasterxml.jackson.databind.ObjectMapper;
import com.onthegomap.planetiler.geo.GeoUtils;
import com.onthegomap.planetiler.geo.MutableCoordinateSequence;
import com.onthegomap.planetiler.reader.FileFormatException;
import com.onthegomap.planetiler.util.CloseableIterator;
import java.io.IOException;
import java.io.InputStream;
Expand Down Expand Up @@ -46,9 +47,10 @@ class GeoJsonFeatureIterator implements CloseableIterator<GeoJsonFeature> {
private int warnings = 0;
private static final int WARN_LIMIT = 100;
private final String name;
private static final GeoJsonGeometry EMPTY_GEOJSON_GEOMETRY = new GeoJsonGeometry("", null);

@JsonIgnoreProperties(ignoreUnknown = true)
private record GeoJsonGeometry(String type, Object coordinates) {}
private record GeoJsonGeometry(Object type, Object coordinates) {}

GeoJsonFeatureIterator(InputStream in, String name) throws IOException {
this.name = name;
Expand Down Expand Up @@ -101,10 +103,12 @@ private void advance() {
case "properties" -> consumeProperties();
case "type" -> consume(JsonToken.VALUE_STRING);
case "features" -> {
consume(JsonToken.START_ARRAY);
nestingLevel++;
consume(JsonToken.START_OBJECT);
nestingLevel++;
if (consume(JsonToken.START_ARRAY)) {
nestingLevel++;
if (consume(JsonToken.START_OBJECT)) {
nestingLevel++;
}
}

Check warning on line 111 in planetiler-core/src/main/java/com/onthegomap/planetiler/reader/geojson/GeoJsonFeatureIterator.java

View workflow job for this annotation

GitHub Actions / Analyze with Sonar

MAJOR CODE_SMELL 

Replace this "if" statement with a pattern match guard.

rule: java:S6916 (https://sonarcloud.io/organizations/onthegomap/rules?open=java%3AS6916&rule_key=java%3AS6916)
issue url: https://sonarcloud.io/project/issues?pullRequest=1154&open=AZSYgjf50iLbP_W7jHlN&id=onthegomap_planetiler
}
case null, default -> {
parser.nextToken();
Expand All @@ -126,9 +130,8 @@ private void advance() {
}
}
} catch (IOException e) {
throw new UncheckedIOException(e);
throw new FileFormatException("JSON syntax error in " + name, e);
}

}

private boolean consume(JsonToken tokenType) throws IOException {
Expand All @@ -141,14 +144,17 @@ private boolean consume(JsonToken tokenType) throws IOException {
}

private void consumeProperties() throws IOException {
consume(JsonToken.START_OBJECT);
properties = mapper.readValue(parser, Map.class);
if (consume(JsonToken.START_OBJECT)) {
properties = mapper.readValue(parser, Map.class);
}
}

private void consumeGeometry() throws IOException {
if (consume(JsonToken.START_OBJECT)) {
geometryStart = parser.currentTokenLocation();
geometry = mapper.readValue(parser, GeoJsonGeometry.class);
} else {
geometry = EMPTY_GEOJSON_GEOMETRY;
}
}

Expand All @@ -164,21 +170,27 @@ private void findNextStruct() throws IOException {
}

private Geometry getGeometry() {
if (geometry == null) {
if (geometry == EMPTY_GEOJSON_GEOMETRY) {
return GeoUtils.EMPTY_GEOMETRY;
} else if (geometry == null) {
return null;
}
var factory = JTS_FACTORY;
if (geometry.coordinates instanceof List<?> coords) {
return switch (geometry.type) {
if (!(geometry.coordinates instanceof List<?> coords)) {
warnGeometry("Expected coordinate list but got: " + geometry.coordinates);
} else if (!(geometry.type instanceof String type)) {
warnGeometry("Expected geometry type string but got: " + geometry.coordinates);
} else {
return switch (type) {
case "Point" -> point(coords);
case "LineString" -> lineString(coords);
case "Polygon" -> polygon(coords);
case "MultiPoint" -> factory.createMultiPoint(map(coords, this::point).toArray(Point[]::new));
case "MultiLineString" ->
factory.createMultiLineString(map(coords, this::lineString).toArray(LineString[]::new));
case "MultiPolygon" -> factory.createMultiPolygon(map(coords, this::polygon).toArray(Polygon[]::new));
case null, default -> {
warnGeometry("Unexpected geometry type: " + geometry.type);
default -> {
warnGeometry("Unexpected geometry type: " + type);
yield GeoUtils.EMPTY_GEOMETRY;
}
};
Expand Down
24 changes: 20 additions & 4 deletions planetiler-core/src/test/java/com/onthegomap/planetiler/reader/geojson/GeoJsonTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,16 +147,23 @@ void testEmptyMultiLineString() {
));
}

@Test
void testPointWithNullProperties() {
@ParameterizedTest
@CsvSource(textBlock = """
""
null
true
1
[]
""")
void testPointWithBadProperties(String properties) {
testParse(
"""
{
"type": "Feature",
"geometry": {"type": "Point", "coordinates": [1, 2]},
"properties": null
"properties": %s
}
""",
""".formatted(properties),
List.of(
new GeoJsonFeature(newPoint(1, 2), Map.of())
));
Expand Down Expand Up @@ -230,6 +237,7 @@ void testParseBadFeatures() {
{"type": "Garbage", "geometry": {"type": "Point", "coordinates": [1, 2]}}
{"type": "Feature", "geometry": {"type": "Point", "coordinates": [3, 4], "other2": "value"}, "other": "value"}
{"type": "Feature", "geometry": {"type": "Garbage", "coordinates": [5, 6]}}
{"type": "FeatureCollection", "features": {}}
""", List.of(
new GeoJsonFeature(newPoint(1, 2), Map.of()),
new GeoJsonFeature(newPoint(3, 4), Map.of()),
Expand All @@ -243,6 +251,14 @@ void testParseBadFeatures() {
{"type": "Feature", "geometry": {"type": "Point", "coordinates": [[1, 2]]}}; POINT EMPTY
{"type": "Feature", "geometry": {"type": "Point", "coordinates": [[1]]}}; POINT EMPTY
{"type": "Feature", "geometry": {"type": "Point", "coordinates": {}}}; GEOMETRYCOLLECTION EMPTY
{"type": "Feature", "geometry": {"type": "Point", "coordinates": 1}}; GEOMETRYCOLLECTION EMPTY
{"type": "Feature", "geometry": {"type": "Point", "coordinates": "1"}}; GEOMETRYCOLLECTION EMPTY
{"type": "Feature", "geometry": {"type": 1, "coordinates": [1,1]}}; GEOMETRYCOLLECTION EMPTY
{"type": "Feature", "geometry": {"type": [], "coordinates": [1,1]}}; GEOMETRYCOLLECTION EMPTY
{"type": "Feature", "geometry": {"type": {}, "coordinates": [1,1]}}; GEOMETRYCOLLECTION EMPTY
{"type": "Feature", "geometry": []}; GEOMETRYCOLLECTION EMPTY
{"type": "Feature", "geometry": null}; GEOMETRYCOLLECTION EMPTY
{"type": "Feature", "geometry": "1"}; GEOMETRYCOLLECTION EMPTY
{"type": "Feature", "geometry": {"type": "Point", "coordinates": [1, {}]}}; POINT EMPTY
{"type": "Feature", "geometry": {"type": "MultiPoint", "coordinates": [1, 2]}}; MULTIPOINT EMPTY
{"type": "Feature", "geometry": {"type": "MultiPoint", "coordinates": [[1, 2], {}]}}; MULTIPOINT ((1 2))
Expand Down

0 comments on commit a7ba114

Please sign in to comment.