Make methods in graph simpler

Author: leonardehrenfried

application/src/main/java/org/opentripplanner/routing/graph/Graph.java

@@ -22,6 +22,7 @@
 import org.opentripplanner.routing.services.notes.StreetNotesService;
 import org.opentripplanner.street.model.edge.Edge;
 import org.opentripplanner.street.model.edge.StreetEdge;
+import org.opentripplanner.street.model.vertex.StationCentroidVertex;
 import org.opentripplanner.street.model.vertex.TransitStopVertex;
 import org.opentripplanner.street.model.vertex.Vertex;
 import org.opentripplanner.street.model.vertex.VertexLabel;
@@ -210,7 +211,7 @@ public <T extends Vertex> List<T> getVerticesOfType(Class<T> cls) {
   @Nullable
   public TransitStopVertex getStopVertex(FeedScopedId id) {
     requireIndex();
-    return streetIndex.getStopVertex(id);
+    return streetIndex.findStopVertex(id).orElse(null);
   }
 
   /**
@@ -223,15 +224,12 @@ public Optional<TransitStopVertex> findStopVertex(FeedScopedId stopId) {
   }
 
   /**
-   * Get the vertex for a site id. If the id corresponds to a regular stop it will return the
-   * vertex for the stop.
-   * If the id corresponds to a station and the station is configured to route to centroid
-   * then that vertex will be returned.
-   * Otherwise, an empty optional will be returned.
+   * If the {@code id} is a station id and it is configured to route to its center,
+   * return the corresponding vertex, otherwise return an empty optional.
    */
-  public Optional<Vertex> findStopOrCentroidVertex(FeedScopedId stopId) {
+  public Optional<StationCentroidVertex> findStationCentroidVertex(FeedScopedId stopId) {
     requireIndex();
-    return streetIndex.findStopOrCentroidVertex(stopId);
+    return streetIndex.findStationCentroidVertex(stopId);
   }
 
   /**

application/src/main/java/org/opentripplanner/routing/graph/StreetIndex.java

@@ -6,7 +6,6 @@
 import java.util.Map;
 import java.util.Optional;
 import java.util.stream.Collectors;
-import javax.annotation.Nullable;
 import org.locationtech.jts.geom.Coordinate;
 import org.locationtech.jts.geom.Envelope;
 import org.locationtech.jts.geom.LineString;
@@ -34,7 +33,7 @@ class StreetIndex {
 
   private static final Logger LOG = LoggerFactory.getLogger(StreetIndex.class);
 
-  private final Map<FeedScopedId, TransitStopVertex> stopVerticesById;
+  private final Map<FeedScopedId, TransitStopVertex> stopVertices;
 
   /**
    * This list contains transitStationVertices for the stations that are configured to route to centroid
@@ -50,36 +49,24 @@ class StreetIndex {
   StreetIndex(Graph graph) {
     this.edgeIndex = new EdgeSpatialIndex();
     this.vertexIndex = new HashGridSpatialIndex<>();
-    this.stopVerticesById = indexStopIds(graph);
+    this.stopVertices = indexStopIds(graph);
 
     this.stationCentroidVertices = indexStationCentroids(graph);
     postSetup(graph.getVertices());
   }
 
-  /**
-   * @see Graph#getStopVertex(FeedScopedId)
-   */
-  @Nullable
-  TransitStopVertex getStopVertex(FeedScopedId stopId) {
-    return stopVerticesById.get(stopId);
-  }
-
   /**
    * @see Graph#findStopVertex(FeedScopedId) (FeedScopedId)
    */
   Optional<TransitStopVertex> findStopVertex(FeedScopedId id) {
-    return Optional.ofNullable(stopVerticesById.get(id));
+    return Optional.ofNullable(stopVertices.get(id));
   }
 
   /**
-   * @see Graph#findStopOrCentroidVertex(FeedScopedId)
+   * @see Graph#findStationCentroidVertex(FeedScopedId)
    */
-  Optional<Vertex> findStopOrCentroidVertex(FeedScopedId id) {
-    var stationVertex = stationCentroidVertices.get(id);
-    if (stationVertex != null) {
-      return Optional.of(stationVertex);
-    }
-    return findStopVertex(id).map(Vertex.class::cast);
+  Optional<StationCentroidVertex> findStationCentroidVertex(FeedScopedId id) {
+    return Optional.ofNullable(stationCentroidVertices.get(id));
   }
 
   /**

application/src/main/java/org/opentripplanner/street/search/TemporaryVerticesContainer.java

@@ -174,21 +174,25 @@ private Set<Vertex> getStreetVerticesForLocation(
       }
     } else {
       if (location.stopId != null) {
-        // check if stop or station centroid is found
-        // station centroids are a special vertex to indicate that you don't want to route
-        // to/from the child stops because they are too spread out.
-        var stopVertices = graph.findStopOrCentroidVertex(location.stopId);
-        if (stopVertices.isPresent()) {
-          return Set.of(stopVertices.get());
-        } else {
-          // in the regular case you want to resolve a (multi-modal) station into its child stops
-          var vertices = findStopOrChildStopVertices(location.stopId);
-          if (!vertices.isEmpty()) {
-            return vertices
-              .stream()
-              .map(Vertex.class::cast)
-              .collect(Collectors.toUnmodifiableSet());
-          }
+        // check if there is a stop by the given id
+        var stopVertex = graph.findStopVertex(location.stopId);
+        if (stopVertex.isPresent()) {
+          return Set.of(stopVertex.get());
+        }
+
+        // station centroids may be used instead of child stop vertices for stations
+        var centroidVertex = graph.findStationCentroidVertex(location.stopId);
+        if (centroidVertex.isPresent()) {
+          return Set.of(centroidVertex.get());
+        }
+
+        // in the regular case you want to resolve a (multi-modal) station into its child stops
+        var childVertices = findStopOrChildStopVertices(location.stopId);
+        if (!childVertices.isEmpty()) {
+          return childVertices
+            .stream()
+            .map(Vertex.class::cast)
+            .collect(Collectors.toUnmodifiableSet());
         }
       }
     }

application/src/test/java/org/opentripplanner/routing/graph/StreetIndexTest.java

@@ -26,22 +26,19 @@ class StreetIndexTest {
   @Test
   void stopId() {
     var streetIndex = buildIndex();
-    assertEquals(stopVertex, streetIndex.getStopVertex(stop.getId()));
     assertThat(streetIndex.findStopVertex(stop.getId())).hasValue(stopVertex);
   }
 
   @Test
   void nonExistentId() {
     var streetIndex = buildIndex();
-    assertNull(streetIndex.getStopVertex(id("non-existent-stop-id")));
     assertThat(streetIndex.findStopVertex(id("non-existent-stop-id"))).isEmpty();
   }
 
   @Test
   void stationCentroid() {
     var streetIndex = buildIndex();
-    assertNull(streetIndex.getStopVertex(station.getId()));
-    assertThat(streetIndex.findStopOrCentroidVertex(station.getId())).hasValue(centroidVertex);
+    assertThat(streetIndex.findStationCentroidVertex(station.getId())).hasValue(centroidVertex);
   }
 
   private StreetIndex buildIndex() {

application/src/test/java/org/opentripplanner/street/search/TemporaryVerticesContainerTest.java

@@ -16,20 +16,32 @@
 import org.opentripplanner.model.GenericLocation;
 import org.opentripplanner.routing.graph.Graph;
 import org.opentripplanner.street.model._data.StreetModelForTest;
+import org.opentripplanner.street.model.edge.StreetStationCentroidLink;
+import org.opentripplanner.street.model.vertex.StationCentroidVertex;
 import org.opentripplanner.street.model.vertex.TransitStopVertex;
 import org.opentripplanner.street.model.vertex.Vertex;
 import org.opentripplanner.transit.model._data.TimetableRepositoryForTest;
 import org.opentripplanner.transit.model.framework.FeedScopedId;
 import org.opentripplanner.transit.model.site.RegularStop;
+import org.opentripplanner.transit.model.site.Station;
 
 class TemporaryVerticesContainerTest {
 
   private static final WgsCoordinate CENTER = new WgsCoordinate(0, 0);
   private static final int DISTANCE = 20;
 
+  private static final FeedScopedId ALPHA_ID = id("alpha");
   private static final FeedScopedId OMEGA_ID = id("omega");
 
   private final TimetableRepositoryForTest testModel = TimetableRepositoryForTest.of();
+
+  private final Station stationAlpha = testModel
+    .station("alpha")
+    .withId(ALPHA_ID)
+    .withCoordinate(CENTER)
+    .withShouldRouteToCentroid(true)
+    .build();
+
   private final RegularStop stopA = testModel
     .stop("A")
     .withCoordinate(CENTER.moveEastMeters(DISTANCE))
@@ -46,7 +58,7 @@ class TemporaryVerticesContainerTest {
     .stop("D")
     .withCoordinate(CENTER.moveNorthMeters(DISTANCE))
     .build();
-  private final Graph graph = buildGraph(stopA, stopB, stopC, stopD);
+  private final Graph graph = buildGraph(stationAlpha, stopA, stopB, stopC, stopD);
 
   @Test
   void coordinates() {
@@ -102,10 +114,31 @@ void stationId() {
     assertThat(toStops(container.getFromStopVertices())).containsExactly(stopC, stopD);
   }
 
-  private static Graph buildGraph(RegularStop... stops) {
+  @Test
+  void centroid() {
+    var container = new TemporaryVerticesContainer(
+      graph,
+      TestVertexLinker.of(graph),
+      Set::of,
+      GenericLocation.fromStopId("station", ALPHA_ID.getFeedId(), ALPHA_ID.getId()),
+      stopToLocation(stopB),
+      WALK,
+      WALK
+    );
+    var fromVertices = List.copyOf(container.getFromVertices());
+    assertThat(fromVertices).hasSize(1);
+
+    var station = ((StationCentroidVertex) fromVertices.getFirst()).getStation();
+    assertEquals(station, this.stationAlpha);
+  }
+
+  private static Graph buildGraph(Station station, RegularStop... stops) {
     var graph = new Graph();
     var center = StreetModelForTest.intersectionVertex(CENTER.asJtsCoordinate());
     graph.addVertex(center);
+    var centroidVertex = new StationCentroidVertex(station);
+    graph.addVertex(centroidVertex);
+    StreetStationCentroidLink.createStreetStationLink(centroidVertex, center);
     Arrays.stream(stops).forEach(s -> {
       graph.addVertex(TransitStopVertex.of().withStop(s).build());
       var vertex = StreetModelForTest.intersectionVertex(s.getCoordinate().asJtsCoordinate());