Merge pull request #5782 from HBTGmbH/island-pruning-for-stops-withou…

…t-modes

Prune islands with mode-less stop vertices

src/main/java/org/opentripplanner/graph_builder/module/islandpruning/PruneIslands.java

@@ -10,7 +10,6 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Queue;
-import java.util.Set;
 import java.util.stream.Collectors;
 import org.opentripplanner.graph_builder.issue.api.DataImportIssueStore;
 import org.opentripplanner.graph_builder.issues.GraphConnectivity;
@@ -33,7 +32,6 @@
 import org.opentripplanner.street.search.TraverseMode;
 import org.opentripplanner.street.search.request.StreetSearchRequest;
 import org.opentripplanner.street.search.state.State;
-import org.opentripplanner.transit.model.basic.TransitMode;
 import org.opentripplanner.transit.service.TransitModel;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -252,16 +250,7 @@ private int processIslands(
       if (island.stopSize() > 0) {
         //for islands with stops
         islandsWithStops++;
-        boolean onlyFerry = true;
-        for (Iterator<Vertex> vIter = island.stopIterator(); vIter.hasNext();) {
-          TransitStopVertex v = (TransitStopVertex) vIter.next();
-          Set<TransitMode> modes = v.getModes();
-          // test if stop has other transit modes than FERRY
-          if (!modes.isEmpty() && !modes.contains(TransitMode.FERRY)) {
-            onlyFerry = false;
-            break;
-          }
-        }
+        boolean onlyFerry = island.hasOnlyFerryStops();
         // do not remove real islands which have only ferry stops
         if (!onlyFerry && island.streetSize() < pruningThresholdWithStops * adaptivePruningFactor) {
           double sizeCoeff = (adaptivePruningFactor > 1.0)
@@ -487,8 +476,8 @@ private boolean restrictOrRemove(
       // note: do not unlink stop if only CAR mode is pruned
       // maybe this needs more logic for flex routing cases
       List<VertexLabel> stopLabels = new ArrayList<>();
-      for (Iterator<Vertex> vIter = island.stopIterator(); vIter.hasNext();) {
-        Vertex v = vIter.next();
+      for (Iterator<TransitStopVertex> vIter = island.stopIterator(); vIter.hasNext();) {
+        TransitStopVertex v = vIter.next();
         stopLabels.add(v.getLabel());
         Collection<Edge> edges = new ArrayList<>(v.getOutgoing());
         edges.addAll(v.getIncoming());

src/main/java/org/opentripplanner/graph_builder/module/islandpruning/Subgraph.java

@@ -17,20 +17,21 @@
 import org.opentripplanner.street.model.vertex.OsmVertex;
 import org.opentripplanner.street.model.vertex.TransitStopVertex;
 import org.opentripplanner.street.model.vertex.Vertex;
+import org.opentripplanner.transit.model.basic.TransitMode;
 
 class Subgraph {
 
   private final Set<Vertex> streetVertexSet;
-  private final Set<Vertex> stopsVertexSet;
+  private final Set<TransitStopVertex> stopsVertexSet;
 
   Subgraph() {
     streetVertexSet = new HashSet<>();
     stopsVertexSet = new HashSet<>();
   }
 
   void addVertex(Vertex vertex) {
-    if (vertex instanceof TransitStopVertex) {
-      stopsVertexSet.add(vertex);
+    if (vertex instanceof TransitStopVertex transitStopVertex) {
+      stopsVertexSet.add(transitStopVertex);
     } else {
       streetVertexSet.add(vertex);
     }
@@ -64,7 +65,7 @@ Iterator<Vertex> streetIterator() {
     return streetVertexSet.iterator();
   }
 
-  Iterator<Vertex> stopIterator() {
+  Iterator<TransitStopVertex> stopIterator() {
     return stopsVertexSet.iterator();
   }
 
@@ -98,8 +99,7 @@ Iterator<Vertex> stopIterator() {
       Vertex vx = vIter.next();
       envelope.expandToInclude(vx.getCoordinate());
     }
-    for (Iterator<Vertex> vIter = stopIterator(); vIter.hasNext();) {
-      Vertex vx = vIter.next();
+    for (TransitStopVertex vx : stopsVertexSet) {
       envelope.expandToInclude(vx.getCoordinate());
     }
     envelope.expandBy(searchRadiusDegrees / xscale, searchRadiusDegrees);
@@ -127,4 +127,21 @@ Geometry getGeometry() {
 
     return new MultiPoint(points.toArray(new Point[0]), geometryFactory);
   }
+
+  /**
+   * Checks whether the subgraph has only transit-stops for ferries
+   *
+   * @return true if only ferries stop at the subgraph and false if other or no modes are
+   * stopping at the subgraph
+   */
+  boolean hasOnlyFerryStops() {
+    for (TransitStopVertex v : stopsVertexSet) {
+      Set<TransitMode> modes = v.getModes();
+      // test if stop has other transit modes than FERRY
+      if (!modes.contains(TransitMode.FERRY)) {
+        return false;
+      }
+    }
+    return true;
+  }
 }

src/test/java/org/opentripplanner/graph_builder/module/islandpruning/SubgraphOnlyFerryTest.java

@@ -0,0 +1,112 @@
+package org.opentripplanner.graph_builder.module.islandpruning;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.util.Set;
+import org.junit.jupiter.api.Test;
+import org.opentripplanner.street.model.vertex.TransitStopVertex;
+import org.opentripplanner.street.model.vertex.TransitStopVertexBuilder;
+import org.opentripplanner.transit.model._data.TransitModelForTest;
+import org.opentripplanner.transit.model.basic.TransitMode;
+import org.opentripplanner.transit.model.site.RegularStop;
+
+class SubgraphOnlyFerryTest {
+
+  private static final TransitModelForTest TEST_MODEL = TransitModelForTest.of();
+  private static final RegularStop regularStop1 = TEST_MODEL.stop("TEST-1").build();
+  private static final RegularStop regularStop2 = TEST_MODEL.stop("TEST-2").build();
+
+  @Test
+  void subgraphHasOnlyFerry() {
+    TransitStopVertex transitStopVertex = new TransitStopVertexBuilder()
+      .withStop(regularStop1)
+      .withModes(Set.of(TransitMode.FERRY))
+      .build();
+
+    Subgraph subgraph = new Subgraph();
+    subgraph.addVertex(transitStopVertex);
+
+    assertTrue(subgraph.hasOnlyFerryStops());
+  }
+
+  @Test
+  void subgraphHasOnlyNoFerry() {
+    TransitStopVertex transitStopVertex1 = new TransitStopVertexBuilder()
+      .withStop(regularStop1)
+      .withModes(Set.of(TransitMode.BUS))
+      .build();
+
+    Subgraph subgraph = new Subgraph();
+    subgraph.addVertex(transitStopVertex1);
+
+    assertFalse(subgraph.hasOnlyFerryStops());
+  }
+
+  @Test
+  void subgraphHasOnlyNoMode() {
+    TransitStopVertex transitStopVertex1 = new TransitStopVertexBuilder()
+      .withStop(regularStop1)
+      .withModes(Set.of())
+      .build();
+
+    Subgraph subgraph = new Subgraph();
+    subgraph.addVertex(transitStopVertex1);
+
+    assertFalse(subgraph.hasOnlyFerryStops());
+  }
+
+  @Test
+  void subgraphHasOnlyFerryMoreStops() {
+    TransitStopVertex transitStopVertex1 = new TransitStopVertexBuilder()
+      .withStop(regularStop1)
+      .withModes(Set.of(TransitMode.FERRY))
+      .build();
+    TransitStopVertex transitStopVertex2 = new TransitStopVertexBuilder()
+      .withStop(regularStop2)
+      .withModes(Set.of(TransitMode.FERRY))
+      .build();
+
+    Subgraph subgraph = new Subgraph();
+    subgraph.addVertex(transitStopVertex1);
+    subgraph.addVertex(transitStopVertex2);
+
+    assertTrue(subgraph.hasOnlyFerryStops());
+  }
+
+  @Test
+  void subgraphHasNotOnlyFerryMoreStops() {
+    TransitStopVertex transitStopVertex1 = new TransitStopVertexBuilder()
+      .withStop(regularStop1)
+      .withModes(Set.of(TransitMode.FERRY))
+      .build();
+    TransitStopVertex transitStopVertex2 = new TransitStopVertexBuilder()
+      .withStop(regularStop2)
+      .withModes(Set.of(TransitMode.BUS))
+      .build();
+
+    Subgraph subgraph = new Subgraph();
+    subgraph.addVertex(transitStopVertex1);
+    subgraph.addVertex(transitStopVertex2);
+
+    assertFalse(subgraph.hasOnlyFerryStops());
+  }
+
+  @Test
+  void subgraphHasNoModeMoreStops() {
+    TransitStopVertex transitStopVertex1 = new TransitStopVertexBuilder()
+      .withStop(regularStop1)
+      .withModes(Set.of(TransitMode.FERRY))
+      .build();
+    TransitStopVertex transitStopVertex2 = new TransitStopVertexBuilder()
+      .withStop(regularStop2)
+      .withModes(Set.of())
+      .build();
+
+    Subgraph subgraph = new Subgraph();
+    subgraph.addVertex(transitStopVertex1);
+    subgraph.addVertex(transitStopVertex2);
+
+    assertFalse(subgraph.hasOnlyFerryStops());
+  }
+}