From 2959623d5b5c4075e680f894995695ccde9f4359 Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Thu, 28 Mar 2024 12:27:22 +0100 Subject: [PATCH] Add test, docs --- docs/BuildConfiguration.md | 6 +- .../apis/gtfs/GraphQLScalars.java | 7 +- .../framework/json/ObjectMappers.java | 9 ++ .../module/osm/naming/SidewalkNamer.java | 30 +++++ .../module/osm/naming/SidewalkNamerTest.java | 106 ++++++++++++++++++ .../wayproperty/specifier/WayTestData.java | 9 +- .../test/support/GeoJsonIo.java | 25 +++++ 7 files changed, 181 insertions(+), 11 deletions(-) create mode 100644 src/test/java/org/opentripplanner/graph_builder/module/osm/naming/SidewalkNamerTest.java create mode 100644 src/test/java/org/opentripplanner/test/support/GeoJsonIo.java diff --git a/docs/BuildConfiguration.md b/docs/BuildConfiguration.md index a95421e47f7..fe3a2109ba4 100644 --- a/docs/BuildConfiguration.md +++ b/docs/BuildConfiguration.md @@ -35,7 +35,7 @@ Sections follow that describe particular settings in more depth. | maxTransferDuration | `duration` | Transfers up to this duration with the default walk speed value will be pre-calculated and included in the Graph. | *Optional* | `"PT30M"` | 2.1 | | [multiThreadElevationCalculations](#multiThreadElevationCalculations) | `boolean` | Configuring multi-threading during elevation calculations. | *Optional* | `false` | 2.0 | | [osmCacheDataInMem](#osmCacheDataInMem) | `boolean` | If OSM data should be cached in memory during processing. | *Optional* | `false` | 2.0 | -| [osmNaming](#osmNaming) | `enum` | A custom OSM namer to use. | *Optional* | `"none"` | 1.5 | +| [osmNaming](#osmNaming) | `enum` | A custom OSM namer to use. | *Optional* | `"default"` | 1.5 | | platformEntriesLinking | `boolean` | Link unconnected entries to public transport platforms. | *Optional* | `false` | 2.0 | | [readCachedElevations](#readCachedElevations) | `boolean` | Whether to read cached elevation data. | *Optional* | `true` | 2.0 | | staticBikeParkAndRide | `boolean` | Whether we should create bike P+R stations from OSM data. | *Optional* | `false` | 1.5 | @@ -538,9 +538,9 @@ data, and to `false` to read the stream from the source each time.

osmNaming

-**Since version:** `1.5` ∙ **Type:** `enum` ∙ **Cardinality:** `Optional` ∙ **Default value:** `"none"` +**Since version:** `1.5` ∙ **Type:** `enum` ∙ **Cardinality:** `Optional` ∙ **Default value:** `"default"` **Path:** / -**Enum values:** `none` | `portland` | `sidewalks` +**Enum values:** `default` | `portland` | `sidewalks` A custom OSM namer to use. diff --git a/src/main/java/org/opentripplanner/apis/gtfs/GraphQLScalars.java b/src/main/java/org/opentripplanner/apis/gtfs/GraphQLScalars.java index 42ffe992539..5ab24c89543 100644 --- a/src/main/java/org/opentripplanner/apis/gtfs/GraphQLScalars.java +++ b/src/main/java/org/opentripplanner/apis/gtfs/GraphQLScalars.java @@ -1,6 +1,5 @@ package org.opentripplanner.apis.gtfs; -import com.bedatadriven.jackson.datatype.jts.JtsModule; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import graphql.language.StringValue; @@ -16,15 +15,15 @@ import java.time.format.DateTimeFormatter; import javax.annotation.Nonnull; import org.locationtech.jts.geom.Geometry; -import org.opentripplanner.framework.geometry.GeometryUtils; import org.opentripplanner.framework.graphql.scalar.DurationScalarFactory; +import org.opentripplanner.framework.json.ObjectMappers; import org.opentripplanner.framework.model.Grams; import org.opentripplanner.framework.time.OffsetDateTimeParser; public class GraphQLScalars { - private static final ObjectMapper geoJsonMapper = new ObjectMapper() - .registerModule(new JtsModule(GeometryUtils.getGeometryFactory())); + private static final ObjectMapper geoJsonMapper = ObjectMappers.geoJson(); + public static GraphQLScalarType DURATION_SCALAR = DurationScalarFactory.createDurationScalar(); public static final GraphQLScalarType POLYLINE_SCALAR = GraphQLScalarType diff --git a/src/main/java/org/opentripplanner/framework/json/ObjectMappers.java b/src/main/java/org/opentripplanner/framework/json/ObjectMappers.java index 8802db0fc41..1670ae94fb3 100644 --- a/src/main/java/org/opentripplanner/framework/json/ObjectMappers.java +++ b/src/main/java/org/opentripplanner/framework/json/ObjectMappers.java @@ -1,7 +1,9 @@ package org.opentripplanner.framework.json; +import com.bedatadriven.jackson.datatype.jts.JtsModule; import com.fasterxml.jackson.databind.DeserializationFeature; import com.fasterxml.jackson.databind.ObjectMapper; +import org.opentripplanner.framework.geometry.GeometryUtils; public class ObjectMappers { @@ -13,4 +15,11 @@ public static ObjectMapper ignoringExtraFields() { mapper.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false); return mapper; } + + /** + * Returns a mapper that can serialize JTS geometries into GeoJSON. + */ + public static ObjectMapper geoJson() { + return new ObjectMapper().registerModule(new JtsModule(GeometryUtils.getGeometryFactory())); + } } diff --git a/src/main/java/org/opentripplanner/graph_builder/module/osm/naming/SidewalkNamer.java b/src/main/java/org/opentripplanner/graph_builder/module/osm/naming/SidewalkNamer.java index a44b4551919..fdf2870c1ec 100644 --- a/src/main/java/org/opentripplanner/graph_builder/module/osm/naming/SidewalkNamer.java +++ b/src/main/java/org/opentripplanner/graph_builder/module/osm/naming/SidewalkNamer.java @@ -34,6 +34,12 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +/** + * A namer that assigns names of nearby streets to sidewalks if they meet certain + * geometric similarity criteria. + * + * The algorithm is as follows: + */ public class SidewalkNamer implements EdgeNamer { private static final Logger LOG = LoggerFactory.getLogger(SidewalkNamer.class); @@ -102,6 +108,9 @@ public void postprocess() { unnamedSidewalks = null; } + /** + * The actual worker method that runs the business logic on an individual sidewalk edge. + */ private void assignNameToSidewalk(EdgeOnLevel sidewalkOnLevel, AtomicInteger namesApplied) { var sidewalk = sidewalkOnLevel.edge; var buffer = preciseBuffer(sidewalk.getGeometry(), 25); @@ -112,7 +121,9 @@ private void assignNameToSidewalk(EdgeOnLevel sidewalkOnLevel, AtomicInteger nam var candidates = streetEdges.query(envelope); groupEdgesByName(candidates) + // remove edges that are far away .filter(g -> g.nearestDistanceTo(sidewalk.getGeometry()) < MAX_DISTANCE_TO_SIDEWALK) + // make sure we only compare sidewalks and streets that are on the same level .filter(g -> g.levels.equals(sidewalkOnLevel.levels)) .map(g -> computePercentInsideBuffer(g, buffer, sidewalkLength)) // remove those groups where less than a certain percentage is inside the buffer around @@ -126,6 +137,10 @@ private void assignNameToSidewalk(EdgeOnLevel sidewalkOnLevel, AtomicInteger nam }); } + /** + * Compute the length of the group that is inside the buffer and return it as a percentage + * of the lenght of the sidewalk. + */ private static NamedEdgeGroup computePercentInsideBuffer( CandidateGroup g, Geometry buffer, @@ -136,6 +151,11 @@ private static NamedEdgeGroup computePercentInsideBuffer( return new NamedEdgeGroup(percentInBuffer, g.name); } + /** + * If a single street is split into several eges each individual part of the street would potentially + * have a low similarity with the (longer) sidewalk. For that reason we combined them into a group + * and have a better basis for comparison. + */ private static Stream groupEdgesByName(List candidates) { return candidates .stream() @@ -157,6 +177,13 @@ private static Stream groupEdgesByName(List candida } /** + * Add a buffer around a geometry that uses both meters as the input and makes sure + * that the buffer is the same distance (in meters) anywhere on earth. + *

+ * Background: If you call the regular buffer() method on a JTS geometry that uses WGS84 as the + * coordinate reference system, the buffer will be accurate at the equator but will become more + * and more elongated the furter north/south you go. + *

* Taken from https://stackoverflow.com/questions/36455020 */ private Geometry preciseBuffer(Geometry geometry, double distanceInMeters) { @@ -188,6 +215,9 @@ private record NamedEdgeGroup(double percentInBuffer, I18NString name) { * to figure out if the name of the group can be applied to a nearby sidewalk. */ private record CandidateGroup(I18NString name, List edges, Set levels) { + /** + * How much of this group intersects with the give geometry, in meters. + */ double intersectionLength(Geometry polygon) { return edges .stream() diff --git a/src/test/java/org/opentripplanner/graph_builder/module/osm/naming/SidewalkNamerTest.java b/src/test/java/org/opentripplanner/graph_builder/module/osm/naming/SidewalkNamerTest.java new file mode 100644 index 00000000000..7de5fcd7fa8 --- /dev/null +++ b/src/test/java/org/opentripplanner/graph_builder/module/osm/naming/SidewalkNamerTest.java @@ -0,0 +1,106 @@ +package org.opentripplanner.graph_builder.module.osm.naming; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import org.junit.jupiter.api.Test; +import org.opentripplanner.framework.geometry.GeometryUtils; +import org.opentripplanner.framework.geometry.WgsCoordinate; +import org.opentripplanner.framework.i18n.I18NString; +import org.opentripplanner.graph_builder.services.osm.EdgeNamer; +import org.opentripplanner.openstreetmap.model.OSMWay; +import org.opentripplanner.openstreetmap.wayproperty.specifier.WayTestData; +import org.opentripplanner.street.model.StreetTraversalPermission; +import org.opentripplanner.street.model._data.StreetModelForTest; +import org.opentripplanner.street.model.edge.StreetEdge; +import org.opentripplanner.street.model.edge.StreetEdgeBuilder; +import org.opentripplanner.test.support.GeoJsonIo; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +class SidewalkNamerTest { + + private static final I18NString SIDEWALK = I18NString.of("sidewalk"); + private static final Logger LOG = LoggerFactory.getLogger(SidewalkNamerTest.class); + + @Test + void postprocess() { + var builder = new ModelBuilder(); + var sidewalk = builder.addUnamedSidewalk( + new WgsCoordinate(33.75029, -84.39198), + new WgsCoordinate(33.74932, -84.39275) + ); + + LOG.info("Geometry of {}: {}", sidewalk.edge.getName(), GeoJsonIo.toUrl(sidewalk.edge.getGeometry())); + + var pryorStreet = builder.addStreetEdge( + "Pryor Street", + new WgsCoordinate(33.75032, -84.39190), + new WgsCoordinate(33.74924, -84.39275) + ); + + LOG.info("Geometry of {}: {}", pryorStreet.edge.getName(), GeoJsonIo.toUrl(pryorStreet.edge.getGeometry())); + + assertNotEquals(sidewalk.edge.getName(), pryorStreet.edge.getName()); + builder.postProcess(new SidewalkNamer()); + assertEquals(sidewalk.edge.getName(), pryorStreet.edge.getName()); + } + + private static class ModelBuilder { + + private final List pairs = new ArrayList<>(); + + EdgePair addUnamedSidewalk(WgsCoordinate... coordinates) { + var edge = edgeBuilder(coordinates) + .withName(SIDEWALK) + .withPermission(StreetTraversalPermission.PEDESTRIAN) + .withBogusName(true) + .buildAndConnect(); + + var way = WayTestData.footwaySidewalk(); + assertTrue(way.isSidewalk()); + var p = new EdgePair(way, edge); + pairs.add(p); + return p; + } + + EdgePair addStreetEdge(String name, WgsCoordinate... coordinates) { + var edge = edgeBuilder(coordinates) + .withName(I18NString.of(name)) + .withPermission(StreetTraversalPermission.ALL) + .buildAndConnect(); + var way = WayTestData.highwayTertiary(); + way.addTag("name", name); + assertFalse(way.isSidewalk()); + assertTrue(way.isNamed()); + var p = new EdgePair(way, edge); + pairs.add(p); + return p; + } + + void postProcess(EdgeNamer namer) { + pairs.forEach(p -> namer.recordEdge(p.way, p.edge)); + namer.postprocess(); + } + + private static StreetEdgeBuilder edgeBuilder(WgsCoordinate... c) { + var coordinates = Arrays.stream(c).toList(); + var ls = GeometryUtils.makeLineString(c); + return new StreetEdgeBuilder<>() + .withFromVertex( + StreetModelForTest.intersectionVertex(coordinates.getFirst().asJtsCoordinate()) + ) + .withToVertex( + StreetModelForTest.intersectionVertex(coordinates.getLast().asJtsCoordinate()) + ) + .withGeometry(ls); + } + } + + private record EdgePair(OSMWay way, StreetEdge edge) {} +} diff --git a/src/test/java/org/opentripplanner/openstreetmap/wayproperty/specifier/WayTestData.java b/src/test/java/org/opentripplanner/openstreetmap/wayproperty/specifier/WayTestData.java index 20dbcbb5a78..b09f690f794 100644 --- a/src/test/java/org/opentripplanner/openstreetmap/wayproperty/specifier/WayTestData.java +++ b/src/test/java/org/opentripplanner/openstreetmap/wayproperty/specifier/WayTestData.java @@ -121,9 +121,10 @@ public static OSMWithTags cyclewayBoth() { return way; } - public static OSMWithTags footwaySidewalk() { - var way = new OSMWithTags(); + public static OSMWay footwaySidewalk() { + var way = new OSMWay(); way.addTag("footway", "sidewalk"); + way.addTag("highway", "footway"); return way; } @@ -155,8 +156,8 @@ public static OSMWithTags highwayTrunk() { return way; } - public static OSMWithTags highwayTertiary() { - var way = new OSMWithTags(); + public static OSMWay highwayTertiary() { + var way = new OSMWay(); way.addTag("highway", "tertiary"); return way; } diff --git a/src/test/java/org/opentripplanner/test/support/GeoJsonIo.java b/src/test/java/org/opentripplanner/test/support/GeoJsonIo.java new file mode 100644 index 00000000000..d16c9f11d0c --- /dev/null +++ b/src/test/java/org/opentripplanner/test/support/GeoJsonIo.java @@ -0,0 +1,25 @@ +package org.opentripplanner.test.support; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import java.net.URLEncoder; +import java.nio.charset.StandardCharsets; +import org.locationtech.jts.geom.Geometry; +import org.opentripplanner.framework.json.ObjectMappers; + +/** + * Helper class for generating URLs to geojson.io. + */ +public class GeoJsonIo { + private static final ObjectMapper MAPPER = ObjectMappers.geoJson(); + + public static String toUrl(Geometry geometry) { + try { + var geoJson = MAPPER.writeValueAsString(geometry); + var encoded = URLEncoder.encode(geoJson, StandardCharsets.UTF_8); + return "http://geojson.io/#data=data:application/json,%s".formatted(encoded); + } catch (JsonProcessingException e) { + throw new RuntimeException(e); + } + } +}