Merge pull request #6321 from leonardehrenfried/bogus-name

Improve "bogus name" handling when setting StreetEdge's name during postprocessing

application/src/main/java/org/opentripplanner/street/model/edge/Edge.java

@@ -145,15 +145,17 @@ public String getDefaultName() {
    */
   public abstract I18NString getName();
 
-  // TODO Add comments about what a "bogus name" is.
+  /**
+   * Returns true if this edge has a generated name that is derived from its properties,
+   * like "path", "stairs" or "tunnel".
+   * <p>
+   * Returns false if the field reflects the real world name, like "Fifth Avenue",
+   * "Hauptstraße" or "Øvre Holmegate".
+   */
   public boolean hasBogusName() {
     return false;
   }
 
-  // The next few functions used to live in EdgeNarrative, which has now been
-  // removed
-  // @author mattwigway
-
   public LineString getGeometry() {
     return null;
   }

application/src/main/java/org/opentripplanner/street/model/edge/StreetEdge.java

@@ -9,9 +9,6 @@
 import java.util.List;
 import java.util.Objects;
 import java.util.Optional;
-import java.util.Set;
-import java.util.stream.Stream;
-import javax.annotation.Nullable;
 import org.locationtech.jts.geom.LineString;
 import org.locationtech.jts.geom.impl.PackedCoordinateSequence;
 import org.opentripplanner.framework.geometry.CompactLineStringUtils;
@@ -82,14 +79,14 @@ public class StreetEdge
 
   /**
    * bicycleSafetyWeight = length * bicycleSafetyFactor. For example, a 100m street with a safety
-   * factor of 2.0 will be considered in term of safety cost as the same as a 200m street with a
+   * factor of 2.0 will be considered in terms of safety cost as the same as a 200m street with a
    * safety factor of 1.0.
    */
   private float bicycleSafetyFactor;
 
   /**
    * walkSafetyFactor = length * walkSafetyFactor. For example, a 100m street with a safety
-   * factor of 2.0 will be considered in term of safety cost as the same as a 200m street with a
+   * factor of 2.0 will be considered in terms of safety cost as the same as a 200m street with a
    * safety factor of 1.0.
    */
   private float walkSafetyFactor;
@@ -446,8 +443,15 @@ public I18NString getName() {
     return this.name;
   }
 
+  /**
+   * Update the name of the edge after it has been constructed. This method also sets the bogusName
+   * property to false, indicating to the code that maps from edges to steps that this is a real
+   * street name.
+   * @see Edge#hasBogusName()
+   */
   public void setName(I18NString name) {
     this.name = name;
+    this.flags = BitSetUtils.set(flags, HASBOGUSNAME_FLAG_INDEX, false);
   }
 
   public boolean hasBogusName() {

application/src/test/java/org/opentripplanner/graph_builder/module/osm/naming/SidewalkNamerTest.java

@@ -58,6 +58,7 @@ void postprocess() {
     assertNotEquals(sidewalk.edge.getName(), pryorStreet.edge.getName());
     builder.postProcess(new SidewalkNamer());
     assertEquals(sidewalk.edge.getName(), pryorStreet.edge.getName());
+    assertFalse(sidewalk.edge.hasBogusName());
   }
 
   private static class ModelBuilder {

application/src/test/java/org/opentripplanner/routing/algorithm/mapping/StatesToWalkStepsMapperTest.java

@@ -42,7 +42,7 @@ void enterStation() {
     var walkSteps = buildWalkSteps(builder);
     assertEquals(2, walkSteps.size());
     var enter = walkSteps.get(1);
-    assertEquals(enter.getRelativeDirection(), ENTER_STATION);
+    assertEquals(ENTER_STATION, enter.getRelativeDirection());
   }
 
   @Test
@@ -54,7 +54,7 @@ void exitStation() {
     var walkSteps = buildWalkSteps(builder);
     assertEquals(3, walkSteps.size());
     var enter = walkSteps.get(2);
-    assertEquals(enter.getRelativeDirection(), EXIT_STATION);
+    assertEquals(EXIT_STATION, enter.getRelativeDirection());
   }
 
   @Test
@@ -64,7 +64,7 @@ void signpostedPathway() {
     var walkSteps = buildWalkSteps(builder);
     assertEquals(2, walkSteps.size());
     var step = walkSteps.get(1);
-    assertEquals(step.getRelativeDirection(), FOLLOW_SIGNS);
+    assertEquals(FOLLOW_SIGNS, step.getRelativeDirection());
     assertEquals(sign, step.getDirectionText().toString());
   }
 

application/src/test/java/org/opentripplanner/street/model/edge/StreetEdgeTest.java

@@ -6,6 +6,7 @@
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.opentripplanner.street.model.StreetTraversalPermission.ALL;
 import static org.opentripplanner.street.model._data.StreetModelForTest.intersectionVertex;
 import static org.opentripplanner.street.model._data.StreetModelForTest.streetEdge;
 import static org.opentripplanner.street.model._data.StreetModelForTest.streetEdgeBuilder;
@@ -17,6 +18,8 @@
 import org.locationtech.jts.geom.GeometryFactory;
 import org.locationtech.jts.geom.LineString;
 import org.locationtech.jts.geom.impl.PackedCoordinateSequence;
+import org.opentripplanner.framework.geometry.GeometryUtils;
+import org.opentripplanner.framework.i18n.I18NString;
 import org.opentripplanner.routing.api.request.StreetMode;
 import org.opentripplanner.routing.core.VehicleRoutingOptimizeType;
 import org.opentripplanner.routing.util.ElevationUtils;
@@ -43,7 +46,7 @@ public class StreetEdgeTest {
   private StreetSearchRequest proto;
 
   @BeforeEach
-  public void before() {
+  void before() {
     v0 = intersectionVertex("maple_0th", 0.0, 0.0); // label, X, Y
     v1 = intersectionVertex("maple_1st", 2.0, 2.0);
     v2 = intersectionVertex("maple_2nd", 2.0, 1.0);
@@ -64,9 +67,9 @@ public void before() {
   }
 
   @Test
-  public void testInAndOutAngles() {
+  void testInAndOutAngles() {
     // An edge heading straight West
-    StreetEdge e1 = streetEdge(v1, v2, 1.0, StreetTraversalPermission.ALL);
+    StreetEdge e1 = streetEdge(v1, v2, 1.0, ALL);
 
     // Edge has same first and last angle.
     assertEquals(90, e1.getInAngle());
@@ -77,18 +80,16 @@ public void testInAndOutAngles() {
     StreetVertex v = intersectionVertex("test2", 2.0, 2.0);
 
     // Second edge, heading straight North
-    StreetEdge e2 = streetEdge(u, v, 1.0, StreetTraversalPermission.ALL);
+    StreetEdge e2 = streetEdge(u, v, 1.0, ALL);
 
     // 180 degrees could be expressed as 180 or -180. Our implementation happens to use -180.
     assertEquals(180, Math.abs(e2.getInAngle()));
     assertEquals(180, Math.abs(e2.getOutAngle()));
   }
 
   @Test
-  public void testTraverseAsPedestrian() {
-    StreetEdge e1 = streetEdgeBuilder(v1, v2, 100.0, StreetTraversalPermission.ALL)
-      .withCarSpeed(10.0f)
-      .buildAndConnect();
+  void testTraverseAsPedestrian() {
+    StreetEdge e1 = streetEdgeBuilder(v1, v2, 100.0, ALL).withCarSpeed(10.0f).buildAndConnect();
 
     StreetSearchRequest options = StreetSearchRequest
       .copyOf(proto)
@@ -106,10 +107,8 @@ public void testTraverseAsPedestrian() {
   }
 
   @Test
-  public void testTraverseAsCar() {
-    StreetEdge e1 = streetEdgeBuilder(v1, v2, 100.0, StreetTraversalPermission.ALL)
-      .withCarSpeed(10.0f)
-      .buildAndConnect();
+  void testTraverseAsCar() {
+    StreetEdge e1 = streetEdgeBuilder(v1, v2, 100.0, ALL).withCarSpeed(10.0f).buildAndConnect();
 
     State s0 = new State(v1, StreetSearchRequest.copyOf(proto).withMode(StreetMode.CAR).build());
     State s1 = e1.traverse(s0)[0];
@@ -122,8 +121,8 @@ public void testTraverseAsCar() {
   }
 
   @Test
-  public void testModeSetCanTraverse() {
-    StreetEdge e = streetEdge(v1, v2, 1.0, StreetTraversalPermission.ALL);
+  void testModeSetCanTraverse() {
+    StreetEdge e = streetEdge(v1, v2, 1.0, ALL);
 
     TraverseModeSet modes = TraverseModeSet.allModes();
     assertTrue(e.canTraverse(modes));
@@ -146,7 +145,7 @@ public void testModeSetCanTraverse() {
    * correctly during turn cost computation. 4. Traffic light wait time is taken into account.
    */
   @Test
-  public void testTraverseModeSwitchBike() {
+  void testTraverseModeSwitchBike() {
     var vWithTrafficLight = new LabelledIntersectionVertex("maple_1st", 2.0, 2.0, false, true);
     StreetEdge e0 = streetEdge(v0, vWithTrafficLight, 50.0, StreetTraversalPermission.PEDESTRIAN);
     StreetEdge e1 = streetEdge(
@@ -183,7 +182,7 @@ public void testTraverseModeSwitchBike() {
    * the bike walking speed on the walking speed. 4. Traffic light wait time is taken into account.
    */
   @Test
-  public void testTraverseModeSwitchWalk() {
+  void testTraverseModeSwitchWalk() {
     var vWithTrafficLight = new LabelledIntersectionVertex("maple_1st", 2.0, 2.0, false, true);
     StreetEdge e0 = streetEdge(
       v0,
@@ -214,7 +213,7 @@ public void testTraverseModeSwitchWalk() {
    * Test the bike switching penalty feature, both its cost penalty and its separate time penalty.
    */
   @Test
-  public void testBikeSwitch() {
+  void testBikeSwitch() {
     StreetEdge e0 = streetEdge(v0, v1, 0.0, StreetTraversalPermission.PEDESTRIAN);
     StreetEdge e1 = streetEdge(v1, v2, 0.0, StreetTraversalPermission.BICYCLE);
     StreetEdge e2 = streetEdge(v2, v0, 0.0, StreetTraversalPermission.PEDESTRIAN_AND_BICYCLE);
@@ -271,9 +270,9 @@ public void testBikeSwitch() {
   }
 
   @Test
-  public void testTurnRestriction() {
-    StreetEdge e0 = streetEdge(v0, v1, 50.0, StreetTraversalPermission.ALL);
-    StreetEdge e1 = streetEdge(v1, v2, 18.4, StreetTraversalPermission.ALL);
+  void testTurnRestriction() {
+    StreetEdge e0 = streetEdge(v0, v1, 50.0, ALL);
+    StreetEdge e1 = streetEdge(v1, v2, 18.4, ALL);
     StreetSearchRequestBuilder streetSearchRequestBuilder = StreetSearchRequest.copyOf(proto);
     streetSearchRequestBuilder.withArriveBy(true);
     StreetSearchRequest request = streetSearchRequestBuilder.withMode(StreetMode.WALK).build();
@@ -285,13 +284,13 @@ public void testTurnRestriction() {
   }
 
   @Test
-  public void testElevationProfile() {
+  void testElevationProfile() {
     var elevationProfile = new PackedCoordinateSequence.Double(
       new double[] { 0, 10, 50, 12 },
       2,
       0
     );
-    var edge = streetEdge(v0, v1, 50.0, StreetTraversalPermission.ALL);
+    var edge = streetEdge(v0, v1, 50.0, ALL);
     StreetElevationExtensionBuilder
       .of(edge)
       .withElevationProfile(elevationProfile)
@@ -306,7 +305,7 @@ public void testElevationProfile() {
   }
 
   @Test
-  public void testBikeOptimizeTriangle() {
+  void testBikeOptimizeTriangle() {
     // This test does not depend on the setup method - and can probably be simplified
 
     Coordinate c1 = new Coordinate(-122.575033, 45.456773);
@@ -326,7 +325,7 @@ public void testBikeOptimizeTriangle() {
       .withGeometry(geometry)
       .withName("Test Lane")
       .withMeterLength(length)
-      .withPermission(StreetTraversalPermission.ALL)
+      .withPermission(ALL)
       .withBack(false)
       // a safe street
       .withBicycleSafetyFactor(0.74f)
@@ -400,4 +399,25 @@ public void testBikeOptimizeTriangle() {
     double expectedWeight = timeWeight * 0.33 + slopeWeight * 0.33 + safetyWeight * 0.34;
     assertEquals(expectedWeight, result.getWeight(), DELTA);
   }
+
+  @Test
+  void setName() {
+    var path = I18NString.of("path");
+    var edge = new StreetEdgeBuilder<>()
+      .withFromVertex(v0)
+      .withToVertex(v1)
+      .withPermission(ALL)
+      .withGeometry(GeometryUtils.makeLineString(v0.getCoordinate(), v1.getCoordinate()))
+      .withName(path)
+      .withBogusName(true)
+      .buildAndConnect();
+
+    assertEquals(path, edge.getName());
+    assertTrue(edge.hasBogusName());
+
+    var mainStreet = I18NString.of("Main Street");
+    edge.setName(mainStreet);
+    assertEquals(mainStreet, edge.getName());
+    assertFalse(edge.hasBogusName());
+  }
 }