Merge branch 'escalator-duration-parse' into dev-2.x

Author: optionsome

application/src/main/java/org/opentripplanner/graph_builder/module/osm/ElevatorProcessor.java

@@ -1,16 +1,16 @@
 package org.opentripplanner.graph_builder.module.osm;
 
 import gnu.trove.list.TLongList;
+import java.time.Duration;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
-import java.util.OptionalInt;
+import java.util.function.Consumer;
 import org.opentripplanner.framework.i18n.NonLocalizedString;
 import org.opentripplanner.graph_builder.issue.api.DataImportIssueStore;
 import org.opentripplanner.graph_builder.issue.api.Issue;
-import org.opentripplanner.osm.model.OsmEntity;
 import org.opentripplanner.osm.model.OsmLevel;
 import org.opentripplanner.osm.model.OsmNode;
 import org.opentripplanner.osm.model.OsmWay;
@@ -40,18 +40,25 @@ class ElevatorProcessor {
 
   private static final Logger LOG = LoggerFactory.getLogger(ElevatorProcessor.class);
 
-  private final DataImportIssueStore issueStore;
   private final OsmDatabase osmdb;
   private final VertexGenerator vertexGenerator;
+  private final Consumer<String> osmEntityDurationIssueConsumer;
 
   public ElevatorProcessor(
     DataImportIssueStore issueStore,
     OsmDatabase osmdb,
     VertexGenerator vertexGenerator
   ) {
-    this.issueStore = issueStore;
     this.osmdb = osmdb;
     this.vertexGenerator = vertexGenerator;
+    this.osmEntityDurationIssueConsumer = v ->
+      issueStore.add(
+        Issue.issue(
+          "InvalidDuration",
+          "Duration for osm node {} is not a valid duration: '{}'; the value is ignored.",
+          v
+        )
+      );
   }
 
   public void buildElevatorEdges(Graph graph) {
@@ -93,7 +100,10 @@ public void buildElevatorEdges(Graph graph) {
           levelName
         );
       }
-      int travelTime = parseDuration(node).orElse(-1);
+      long travelTime = node
+        .getDuration(osmEntityDurationIssueConsumer)
+        .map(Duration::toSeconds)
+        .orElse(-1L);
 
       var wheelchair = node.wheelchairAccessibility();
 
@@ -102,7 +112,7 @@ public void buildElevatorEdges(Graph graph) {
         wheelchair,
         !node.isBicycleExplicitlyDenied(),
         levels.length,
-        travelTime
+        (int) travelTime
       );
     } // END elevator edge loop
 
@@ -136,7 +146,10 @@ public void buildElevatorEdges(Graph graph) {
         );
       }
 
-      int travelTime = parseDuration(elevatorWay).orElse(-1);
+      long travelTime = elevatorWay
+        .getDuration(osmEntityDurationIssueConsumer)
+        .map(Duration::toSeconds)
+        .orElse(-1L);
       int levels = nodes.size();
       var wheelchair = elevatorWay.wheelchairAccessibility();
 
@@ -145,7 +158,7 @@ public void buildElevatorEdges(Graph graph) {
         wheelchair,
         !elevatorWay.isBicycleExplicitlyDenied(),
         levels,
-        travelTime
+        (int) travelTime
       );
       LOG.debug("Created elevatorHopEdges for way {}", elevatorWay.getId());
     }
@@ -221,17 +234,4 @@ private boolean isElevatorWay(OsmWay way) {
     // https://www.openstreetmap.org/way/187719215
     return nodeRefs.get(0) != nodeRefs.get(nodeRefs.size() - 1);
   }
-
-  private OptionalInt parseDuration(OsmEntity element) {
-    return element.getTagAsInt("duration", v ->
-      issueStore.add(
-        Issue.issue(
-          "InvalidDuration",
-          "Duration for osm node %d is not a number: '%s'; it's replaced with '-1' (unknown).",
-          element.getId(),
-          v
-        )
-      )
-    );
-  }
 }

application/src/main/java/org/opentripplanner/osm/model/OsmEntity.java

@@ -318,6 +318,10 @@ public Optional<Duration> getTagValueAsDuration(String key, Consumer<String> err
     return Optional.empty();
   }
 
+  public Optional<Duration> getDuration(Consumer<String> errorHandler) {
+    return getTagValueAsDuration("duration", errorHandler);
+  }
+
   /**
    * Some tags are allowed to have values like 55, "true" or "false".
    * <p>

application/src/main/java/org/opentripplanner/osm/model/OsmWay.java

@@ -133,10 +133,6 @@ public boolean isEscalator() {
     return (isTag("highway", "steps") && isOneOfTags("conveying", ESCALATOR_CONVEYING_TAGS));
   }
 
-  public Optional<Duration> getDuration(Consumer<String> errorHandler) {
-    return getTagValueAsDuration("duration", errorHandler);
-  }
-
   public boolean isForwardEscalator() {
     return isEscalator() && "forward".equals(this.getTag("conveying"));
   }

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

@@ -98,6 +98,10 @@ public StreetTraversalPermission getPermission() {
     return permission;
   }
 
+  public int getTravelTime() {
+    return travelTime;
+  }
+
   @Override
   public State[] traverse(State s0) {
     RoutingPreferences preferences = s0.getPreferences();

application/src/test/java/org/opentripplanner/graph_builder/module/osm/moduletests/BoardingLocationTest.java

@@ -16,7 +16,7 @@
 import org.opentripplanner.service.vehicleparking.internal.DefaultVehicleParkingRepository;
 import org.opentripplanner.transit.model.framework.Deduplicator;
 
-public class BoardingLocationTest {
+class BoardingLocationTest {
 
   /**
    * There is a one-way road which is also marked as a platform in Sky Campus which crashed OSM.

application/src/test/java/org/opentripplanner/graph_builder/module/osm/moduletests/ElevatorTest.java

@@ -0,0 +1,67 @@
+package org.opentripplanner.graph_builder.module.osm.moduletests;
+
+import static com.google.common.truth.Truth.assertThat;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.opentripplanner.graph_builder.module.osm.moduletests._support.NodeBuilder.node;
+
+import org.junit.jupiter.api.Test;
+import org.opentripplanner.framework.geometry.WgsCoordinate;
+import org.opentripplanner.graph_builder.module.osm.OsmModule;
+import org.opentripplanner.graph_builder.module.osm.moduletests._support.TestOsmProvider;
+import org.opentripplanner.osm.model.OsmWay;
+import org.opentripplanner.routing.graph.Graph;
+import org.opentripplanner.service.osminfo.internal.DefaultOsmInfoGraphBuildRepository;
+import org.opentripplanner.service.vehicleparking.internal.DefaultVehicleParkingRepository;
+import org.opentripplanner.street.model.edge.ElevatorHopEdge;
+import org.opentripplanner.transit.model.framework.Deduplicator;
+
+class ElevatorTest {
+
+  @Test
+  void testDuration() {
+    var way = new OsmWay();
+    way.addTag("duration", "00:01:02");
+    way.addTag("highway", "elevator");
+    var provider = TestOsmProvider.of().addWay(way).build();
+    var graph = new Graph(new Deduplicator());
+    var osmModule = OsmModule.of(
+      provider,
+      graph,
+      new DefaultOsmInfoGraphBuildRepository(),
+      new DefaultVehicleParkingRepository()
+    ).build();
+    osmModule.buildGraph();
+    var edges = graph.getEdgesOfType(ElevatorHopEdge.class);
+    assertThat(edges).hasSize(2);
+    for (var edge : edges) {
+      assertEquals(62, edge.getTravelTime());
+    }
+  }
+
+  @Test
+  void testMultilevelNodeDuration() {
+    var node0 = node(0, new WgsCoordinate(0, 0));
+    var node1 = node(1, new WgsCoordinate(2, 0));
+    var node = node(2, new WgsCoordinate(1, 0));
+    node.addTag("duration", "00:01:02");
+    node.addTag("highway", "elevator");
+    node.addTag("level", "1;2");
+    var provider = TestOsmProvider.of()
+      .addWayFromNodes(way -> way.addTag("level", "1"), node0, node)
+      .addWayFromNodes(way -> way.addTag("level", "2"), node1, node)
+      .build();
+    var graph = new Graph(new Deduplicator());
+    var osmModule = OsmModule.of(
+      provider,
+      graph,
+      new DefaultOsmInfoGraphBuildRepository(),
+      new DefaultVehicleParkingRepository()
+    ).build();
+    osmModule.buildGraph();
+    var edges = graph.getEdgesOfType(ElevatorHopEdge.class);
+    assertThat(edges).hasSize(2);
+    for (var edge : edges) {
+      assertEquals(62, edge.getTravelTime());
+    }
+  }
+}

application/src/test/java/org/opentripplanner/graph_builder/module/osm/moduletests/_support/TestOsmProvider.java

@@ -5,6 +5,7 @@
 import java.util.Arrays;
 import java.util.List;
 import java.util.concurrent.atomic.AtomicLong;
+import java.util.function.Consumer;
 import org.opentripplanner._support.time.ZoneIds;
 import org.opentripplanner.graph_builder.module.osm.OsmDatabase;
 import org.opentripplanner.osm.OsmProvider;
@@ -119,21 +120,28 @@ public Builder addWayFromNodes(OsmNode... nodes) {
     }
 
     public Builder addWayFromNodes(long id, List<OsmNode> nodes) {
+      return addWayFromNodes(way -> {}, id, nodes);
+    }
+
+    public Builder addWayFromNodes(Consumer<OsmWay> wayConsumer, OsmNode... nodes) {
+      return addWayFromNodes(wayConsumer, counter.incrementAndGet(), List.of(nodes));
+    }
+
+    public Builder addRelation(OsmRelation relation) {
+      this.relations.add(relation);
+      return this;
+    }
+
+    private Builder addWayFromNodes(Consumer<OsmWay> wayConsumer, long id, List<OsmNode> nodes) {
       this.nodes.addAll(nodes);
       var nodeIds = nodes.stream().map(OsmEntity::getId).toList();
-
       var way = new OsmWay();
       way.setId(id);
       way.addTag("highway", "pedestrian");
+      wayConsumer.accept(way);
       way.getNodeRefs().addAll(nodeIds);
-
       this.ways.add(way);
       return this;
     }
-
-    public Builder addRelation(OsmRelation relation) {
-      this.relations.add(relation);
-      return this;
-    }
   }
 }

application/src/test/java/org/opentripplanner/graph_builder/module/osm/moduletests/walkablearea/SimpleAreaTest.java

@@ -16,7 +16,7 @@
 import org.opentripplanner.test.support.GeoJsonIo;
 import org.opentripplanner.transit.model.framework.Deduplicator;
 
-public class SimpleAreaTest {
+class SimpleAreaTest {
 
   @Test
   void walkableArea() {

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

@@ -6,9 +6,11 @@
 import static org.opentripplanner.street.model._data.StreetModelForTest.intersectionVertex;
 
 import java.util.stream.Stream;
+import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
 import org.junit.jupiter.params.provider.MethodSource;
+import org.opentripplanner.routing.api.request.StreetMode;
 import org.opentripplanner.routing.api.request.preference.AccessibilityPreferences;
 import org.opentripplanner.routing.api.request.preference.WheelchairPreferences;
 import org.opentripplanner.street.model.StreetTraversalPermission;
@@ -76,6 +78,21 @@ void allowByDefault(Accessibility wheelchair, double expectedCost) {
     assertEquals(expectedCost, wheelchairResult.weight);
   }
 
+  @Test
+  void testTraversal() {
+    var edge = ElevatorHopEdge.createElevatorHopEdge(
+      from,
+      to,
+      StreetTraversalPermission.ALL,
+      null,
+      2,
+      62
+    );
+    var req = StreetSearchRequest.of().withMode(StreetMode.WALK);
+    var res = edge.traverse(new State(from, req.build()))[0];
+    assertEquals(62_000, res.getTimeDeltaMilliseconds());
+  }
+
   private State[] traverse(Accessibility wheelchair, StreetSearchRequest req) {
     var edge = ElevatorHopEdge.createElevatorHopEdge(
       from,