Always record edges in pairs in the custom namer

opentripplanner · Apr 17, 2024 · 1d749c3 · 1d749c3
1 parent 17279ba
commit 1d749c3
Show file tree

Hide file tree

Showing 8 changed files with 85 additions and 47 deletions.
diff --git a/src/main/java/org/opentripplanner/graph_builder/module/osm/OsmModule.java b/src/main/java/org/opentripplanner/graph_builder/module/osm/OsmModule.java
@@ -113,8 +113,6 @@ public Map<Vertex, Double> elevationDataOutput() {
     return elevationData;
   }
 
-  private record StreetEdgePair(StreetEdge main, StreetEdge back) {}
-
   private void build() {
     var parkingProcessor = new ParkingProcessor(
       graph,
@@ -390,8 +388,10 @@ private void buildBasicGraph() {
             geometry
           );
 
-          StreetEdge street = streets.main;
-          StreetEdge backStreet = streets.back;
+          params.edgeNamer().recordEdges(way, streets);
+
+          StreetEdge street = streets.main();
+          StreetEdge backStreet = streets.back();
           normalizer.applyWayProperties(street, backStreet, wayData, way);
 
           applyEdgesToTurnRestrictions(way, startNode, endNode, street, backStreet);
@@ -545,7 +545,6 @@ private StreetEdge getEdgeForStreet(
       .withBogusName(way.needsFallbackName());
 
     StreetEdge street = seb.buildAndConnect();
-    params.edgeNamer().recordEdge(way, street);
 
     return street;
   }

diff --git a/src/main/java/org/opentripplanner/graph_builder/module/osm/StreetEdgePair.java b/src/main/java/org/opentripplanner/graph_builder/module/osm/StreetEdgePair.java
@@ -0,0 +1,35 @@
+package org.opentripplanner.graph_builder.module.osm;
+
+import java.util.ArrayList;
+import org.opentripplanner.street.model.edge.StreetEdge;
+
+public record StreetEdgePair(StreetEdge main, StreetEdge back) {
+  /**
+   * Return the non-null elements of this pair as an Iterable.
+   */
+  public Iterable<StreetEdge> asIterable() {
+    var ret = new ArrayList<StreetEdge>(2);
+    if (main != null) {
+      ret.add(main);
+    }
+    if (back != null) {
+      ret.add(back);
+    }
+    return ret;
+  }
+
+  /**
+   * Select one of the edges contained in this pair that is not null. No particular algorithm is
+   * guaranteed, and it may change in the future.
+   */
+  public StreetEdge pickAny() {
+    if (main != null) {
+      return main;
+    } else if (back != null) {
+      return back;
+    }
+    throw new IllegalStateException(
+      "%s must not contain two null elements".formatted(getClass().getSimpleName())
+    );
+  }
+}
diff --git a/src/main/java/org/opentripplanner/graph_builder/module/osm/naming/DefaultNamer.java b/src/main/java/org/opentripplanner/graph_builder/module/osm/naming/DefaultNamer.java
@@ -1,9 +1,9 @@
 package org.opentripplanner.graph_builder.module.osm.naming;
 
 import org.opentripplanner.framework.i18n.I18NString;
+import org.opentripplanner.graph_builder.module.osm.StreetEdgePair;
 import org.opentripplanner.graph_builder.services.osm.EdgeNamer;
 import org.opentripplanner.openstreetmap.model.OSMWithTags;
-import org.opentripplanner.street.model.edge.StreetEdge;
 
 public class DefaultNamer implements EdgeNamer {
 
@@ -13,7 +13,7 @@ public I18NString name(OSMWithTags way) {
   }
 
   @Override
-  public void recordEdge(OSMWithTags way, StreetEdge edge) {}
+  public void recordEdges(OSMWithTags way, StreetEdgePair edge) {}
 
   @Override
   public void postprocess() {}

diff --git a/src/main/java/org/opentripplanner/graph_builder/module/osm/naming/PortlandCustomNamer.java b/src/main/java/org/opentripplanner/graph_builder/module/osm/naming/PortlandCustomNamer.java
@@ -3,6 +3,7 @@
 import java.util.HashSet;
 import org.opentripplanner.framework.i18n.I18NString;
 import org.opentripplanner.framework.i18n.NonLocalizedString;
+import org.opentripplanner.graph_builder.module.osm.StreetEdgePair;
 import org.opentripplanner.graph_builder.services.osm.EdgeNamer;
 import org.opentripplanner.openstreetmap.model.OSMWithTags;
 import org.opentripplanner.street.model.edge.StreetEdge;
@@ -69,28 +70,32 @@ public I18NString name(OSMWithTags way) {
   }
 
   @Override
-  public void recordEdge(OSMWithTags way, StreetEdge edge) {
-    if (!edge.hasBogusName()) {
-      return; // this edge already has a real name so there is nothing to do
-    }
-    String highway = way.getTag("highway");
-    if ("motorway_link".equals(highway) || "trunk_link".equals(highway)) {
-      if (edge.isBack()) {
-        nameByDestination.add(edge);
-      } else {
-        nameByOrigin.add(edge);
-      }
-    } else if (
-      "secondary_link".equals(highway) ||
-      "primary_link".equals(highway) ||
-      "tertiary_link".equals(highway)
-    ) {
-      if (edge.isBack()) {
-        nameByOrigin.add(edge);
-      } else {
-        nameByDestination.add(edge);
-      }
-    }
+  public void recordEdges(OSMWithTags way, StreetEdgePair edgePair) {
+    edgePair
+      .asIterable()
+      .forEach(edge -> {
+        if (!edge.hasBogusName()) {
+          return; // this edge already has a real name so there is nothing to do
+        }
+        String highway = way.getTag("highway");
+        if ("motorway_link".equals(highway) || "trunk_link".equals(highway)) {
+          if (edge.isBack()) {
+            nameByDestination.add(edge);
+          } else {
+            nameByOrigin.add(edge);
+          }
+        } else if (
+          "secondary_link".equals(highway) ||
+          "primary_link".equals(highway) ||
+          "tertiary_link".equals(highway)
+        ) {
+          if (edge.isBack()) {
+            nameByOrigin.add(edge);
+          } else {
+            nameByDestination.add(edge);
+          }
+        }
+      });
   }
 
   @Override

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
@@ -30,6 +30,7 @@
 import org.opentripplanner.framework.i18n.I18NString;
 import org.opentripplanner.framework.lang.DoubleUtils;
 import org.opentripplanner.framework.logging.ProgressTracker;
+import org.opentripplanner.graph_builder.module.osm.StreetEdgePair;
 import org.opentripplanner.graph_builder.services.osm.EdgeNamer;
 import org.opentripplanner.openstreetmap.model.OSMWithTags;
 import org.opentripplanner.street.model.edge.StreetEdge;
@@ -71,23 +72,20 @@ public I18NString name(OSMWithTags way) {
   }
 
   @Override
-  public void recordEdge(OSMWithTags way, StreetEdge edge) {
+  public void recordEdges(OSMWithTags way, StreetEdgePair pair) {
     if (way.isSidewalk() && way.needsFallbackName() && !way.isExplicitlyUnnamed()) {
-      unnamedSidewalks.add(new EdgeOnLevel(edge, way.getLevels()));
+      pair
+        .asIterable()
+        .forEach(edge -> unnamedSidewalks.add(new EdgeOnLevel(edge, way.getLevels())));
     }
     if (way.isNamed() && !way.isLink()) {
       // we generate two edges for each osm way: one there and one back. since we don't do any routing
       // in this class we don't need the two directions and keep only one of them.
-      var containsReverse = streetEdges
-        .query(edge.getGeometry().getEnvelopeInternal())
-        .stream()
-        .anyMatch(candidate -> candidate.edge.isReverseOf(edge));
-      if (!containsReverse) {
-        streetEdges.insert(
-          edge.getGeometry().getEnvelopeInternal(),
-          new EdgeOnLevel(edge, way.getLevels())
-        );
-      }
+      var edge = pair.pickAny();
+      streetEdges.insert(
+        edge.getGeometry().getEnvelopeInternal(),
+        new EdgeOnLevel(edge, way.getLevels())
+      );
     }
   }
 

diff --git a/src/main/java/org/opentripplanner/graph_builder/services/osm/EdgeNamer.java b/src/main/java/org/opentripplanner/graph_builder/services/osm/EdgeNamer.java
@@ -3,13 +3,13 @@
 import javax.annotation.Nonnull;
 import org.opentripplanner.framework.i18n.I18NString;
 import org.opentripplanner.framework.i18n.NonLocalizedString;
+import org.opentripplanner.graph_builder.module.osm.StreetEdgePair;
 import org.opentripplanner.graph_builder.module.osm.naming.DefaultNamer;
 import org.opentripplanner.graph_builder.module.osm.naming.PortlandCustomNamer;
 import org.opentripplanner.graph_builder.module.osm.naming.SidewalkNamer;
 import org.opentripplanner.openstreetmap.model.OSMWithTags;
 import org.opentripplanner.standalone.config.framework.json.NodeAdapter;
 import org.opentripplanner.standalone.config.framework.json.OtpVersion;
-import org.opentripplanner.street.model.edge.StreetEdge;
 
 /**
  * Interface responsible for naming edges of the street graph. It allows you to write your own
@@ -25,11 +25,11 @@ public interface EdgeNamer {
    * Callback function for each way/edge combination so that more complicated names can be built
    * in the post-processing step.
    */
-  void recordEdge(OSMWithTags way, StreetEdge edge);
+  void recordEdges(OSMWithTags way, StreetEdgePair edge);
 
   /**
    * Called after each edge has been named to build a more complex name out of the relationships
-   * tracked in {@link EdgeNamer#recordEdge(OSMWithTags, StreetEdge)}.
+   * tracked in {@link EdgeNamer#recordEdges(OSMWithTags, StreetEdgePair)}.
    */
   void postprocess();
 

diff --git a/src/test/java/org/opentripplanner/graph_builder/module/islandpruning/TestNamer.java b/src/test/java/org/opentripplanner/graph_builder/module/islandpruning/TestNamer.java
@@ -2,9 +2,9 @@
 
 import org.opentripplanner.framework.i18n.I18NString;
 import org.opentripplanner.framework.i18n.NonLocalizedString;
+import org.opentripplanner.graph_builder.module.osm.StreetEdgePair;
 import org.opentripplanner.graph_builder.services.osm.EdgeNamer;
 import org.opentripplanner.openstreetmap.model.OSMWithTags;
-import org.opentripplanner.street.model.edge.StreetEdge;
 
 class TestNamer implements EdgeNamer {
 
@@ -14,7 +14,7 @@ public I18NString name(OSMWithTags way) {
   }
 
   @Override
-  public void recordEdge(OSMWithTags way, StreetEdge edge) {}
+  public void recordEdges(OSMWithTags way, StreetEdgePair edge) {}
 
   @Override
   public void postprocess() {}

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
@@ -12,6 +12,7 @@
 import org.opentripplanner.framework.geometry.GeometryUtils;
 import org.opentripplanner.framework.geometry.WgsCoordinate;
 import org.opentripplanner.framework.i18n.I18NString;
+import org.opentripplanner.graph_builder.module.osm.StreetEdgePair;
 import org.opentripplanner.graph_builder.services.osm.EdgeNamer;
 import org.opentripplanner.openstreetmap.model.OSMWay;
 import org.opentripplanner.openstreetmap.wayproperty.specifier.WayTestData;
@@ -92,7 +93,7 @@ EdgePair addStreetEdge(String name, WgsCoordinate... coordinates) {
     }
 
     void postProcess(EdgeNamer namer) {
-      pairs.forEach(p -> namer.recordEdge(p.way, p.edge));
+      pairs.forEach(p -> namer.recordEdges(p.way, new StreetEdgePair(p.edge, null)));
       namer.postprocess();
     }