Simplify implementation by removing changes to StreetNearbyStopFinder.

VillePihlava · VillePihlava · commit 30cc9251a31b · 2024-12-17T09:47:50.000+02:00
diff --git a/application/src/main/java/org/opentripplanner/graph_builder/module/DirectTransferGenerator.java b/application/src/main/java/org/opentripplanner/graph_builder/module/DirectTransferGenerator.java
@@ -93,23 +93,13 @@ public void buildGraph() {
     timetableRepository.index();
 
     /* The linker will use streets if they are available, or straight-line distance otherwise. */
-    NearbyStopFinder nearbyStopFinder = createNearbyStopFinder(
-      defaultMaxTransferDuration,
-      Set.of()
-    );
+    NearbyStopFinder nearbyStopFinder = createNearbyStopFinder(defaultMaxTransferDuration);
     HashMap<StreetMode, NearbyStopFinder> defaultNearbyStopFinders = new HashMap<>();
     /* These are used for calculating transfers only between carsAllowedStops. */
     HashMap<StreetMode, NearbyStopFinder> carsAllowedStopNearbyStopFinders = new HashMap<>();
 
     List<TransitStopVertex> stops = graph.getVerticesOfType(TransitStopVertex.class);
-    Set<TransitStopVertex> carsAllowedStops = timetableRepository
-      .getStopLocationsUsedForCarsAllowedTrips()
-      .stream()
-      .map(StopLocation::getId)
-      .map(graph::getStopVertexForStopId)
-      // filter out null values if no TransitStopVertex is found for ID
-      .filter(TransitStopVertex.class::isInstance)
-      .collect(Collectors.toSet());
+    Set<StopLocation> carsAllowedStops = timetableRepository.getStopLocationsUsedForCarsAllowedTrips();
 
     LOG.info("Creating transfers based on requests:");
     transferRequests.forEach(transferProfile -> LOG.info(transferProfile.toString()));
@@ -147,8 +137,7 @@ public void buildGraph() {
       flexTransferRequests,
       defaultNearbyStopFinders,
       carsAllowedStopNearbyStopFinders,
-      nearbyStopFinder,
-      carsAllowedStops
+      nearbyStopFinder
     );
 
     stops
@@ -169,14 +158,29 @@ public void buildGraph() {
         // Calculate default transfers.
         for (RouteRequest transferProfile : defaultTransferRequests) {
           StreetMode mode = transferProfile.journey().transfer().mode();
-          findNearbyStops(
-            defaultNearbyStopFinders.get(mode),
-            ts0,
-            transferProfile,
-            stop,
-            distinctTransfers,
-            mode
-          );
+          for (NearbyStop sd : defaultNearbyStopFinders
+            .get(mode)
+            .findNearbyStops(ts0, transferProfile, transferProfile.journey().transfer(), false)) {
+            // Skip the origin stop, loop transfers are not needed.
+            if (sd.stop == stop) {
+              continue;
+            }
+            if (sd.stop.transfersNotAllowed()) {
+              continue;
+            }
+            TransferKey transferKey = new TransferKey(stop, sd.stop, sd.edges);
+            PathTransfer pathTransfer = distinctTransfers.get(transferKey);
+            if (pathTransfer == null) {
+              // If the PathTransfer can't be found, it is created.
+              distinctTransfers.put(
+                transferKey,
+                new PathTransfer(stop, sd.stop, sd.distance, sd.edges, EnumSet.of(mode))
+              );
+            } else {
+              // If the PathTransfer is found, a new PathTransfer with the added mode is created.
+              distinctTransfers.put(transferKey, pathTransfer.withAddedMode(mode));
+            }
+          }
         }
         // Calculate flex transfers if flex routing is enabled.
         for (RouteRequest transferProfile : flexTransferRequests) {
@@ -210,17 +214,36 @@ public void buildGraph() {
           }
         }
         // Calculate transfers between stops that can use trips with cars if configured.
-        if (carsAllowedStops.contains(ts0)) {
+        if (carsAllowedStops.contains(stop)) {
           for (RouteRequest transferProfile : carsAllowedStopTransferRequests) {
             StreetMode mode = transferProfile.journey().transfer().mode();
-            findNearbyStops(
-              carsAllowedStopNearbyStopFinders.get(mode),
-              ts0,
-              transferProfile,
-              stop,
-              distinctTransfers,
-              mode
-            );
+            for (NearbyStop sd : carsAllowedStopNearbyStopFinders
+              .get(mode)
+              .findNearbyStops(ts0, transferProfile, transferProfile.journey().transfer(), false)) {
+              // Skip the origin stop, loop transfers are not needed.
+              if (sd.stop == stop) {
+                continue;
+              }
+              if (sd.stop.transfersNotAllowed()) {
+                continue;
+              }
+              // Only calculate transfers between carsAllowedStops.
+              if (!carsAllowedStops.contains(sd.stop)) {
+                continue;
+              }
+              TransferKey transferKey = new TransferKey(stop, sd.stop, sd.edges);
+              PathTransfer pathTransfer = distinctTransfers.get(transferKey);
+              if (pathTransfer == null) {
+                // If the PathTransfer can't be found, it is created.
+                distinctTransfers.put(
+                  transferKey,
+                  new PathTransfer(stop, sd.stop, sd.distance, sd.edges, EnumSet.of(mode))
+                );
+              } else {
+                // If the PathTransfer is found, a new PathTransfer with the added mode is created.
+                distinctTransfers.put(transferKey, pathTransfer.withAddedMode(mode));
+              }
+            }
           }
         }
 
@@ -270,10 +293,7 @@ public void buildGraph() {
    * whether the graph has a street network and if ConsiderPatternsForDirectTransfers feature is
    * enabled.
    */
-  private NearbyStopFinder createNearbyStopFinder(
-    Duration radiusByDuration,
-    Set<Vertex> findOnlyVertices
-  ) {
+  private NearbyStopFinder createNearbyStopFinder(Duration radiusByDuration) {
     var transitService = new DefaultTransitService(timetableRepository);
     NearbyStopFinder finder;
     if (!graph.hasStreets) {
@@ -283,7 +303,7 @@ private NearbyStopFinder createNearbyStopFinder(
       finder = new StraightLineNearbyStopFinder(transitService, radiusByDuration);
     } else {
       LOG.info("Creating direct transfer edges between stops using the street network from OSM...");
-      finder = new StreetNearbyStopFinder(radiusByDuration, 0, null, Set.of(), findOnlyVertices);
+      finder = new StreetNearbyStopFinder(radiusByDuration, 0, null, Set.of());
     }
 
     if (OTPFeature.ConsiderPatternsForDirectTransfers.isOn()) {
@@ -299,8 +319,7 @@ private void parseTransferParameters(
     List<RouteRequest> flexTransferRequests,
     HashMap<StreetMode, NearbyStopFinder> defaultNearbyStopFinders,
     HashMap<StreetMode, NearbyStopFinder> carsAllowedStopNearbyStopFinders,
-    NearbyStopFinder nearbyStopFinder,
-    Set<TransitStopVertex> carsAllowedStops
+    NearbyStopFinder nearbyStopFinder
   ) {
     for (RouteRequest transferProfile : transferRequests) {
       StreetMode mode = transferProfile.journey().transfer().mode();
@@ -313,10 +332,7 @@ private void parseTransferParameters(
           // Set mode-specific maxTransferDuration, if it is set in the build config.
           Duration maxTransferDuration = transferParameters.maxTransferDuration();
           if (maxTransferDuration != Duration.ZERO) {
-            defaultNearbyStopFinders.put(
-              mode,
-              createNearbyStopFinder(maxTransferDuration, Set.of())
-            );
+            defaultNearbyStopFinders.put(mode, createNearbyStopFinder(maxTransferDuration));
           } else {
             defaultNearbyStopFinders.put(mode, nearbyStopFinder);
           }
@@ -327,10 +343,7 @@ private void parseTransferParameters(
           carsAllowedStopTransferRequests.add(transferProfile);
           carsAllowedStopNearbyStopFinders.put(
             mode,
-            createNearbyStopFinder(
-              carsAllowedStopMaxTransferDuration,
-              Collections.<Vertex>unmodifiableSet(carsAllowedStops)
-            )
+            createNearbyStopFinder(carsAllowedStopMaxTransferDuration)
           );
         }
       } else {
@@ -350,41 +363,5 @@ private void parseTransferParameters(
     }
   }
 
-  private void findNearbyStops(
-    NearbyStopFinder nearbyStopFinder,
-    TransitStopVertex ts0,
-    RouteRequest transferProfile,
-    RegularStop stop,
-    Map<TransferKey, PathTransfer> distinctTransfers,
-    StreetMode mode
-  ) {
-    for (NearbyStop sd : nearbyStopFinder.findNearbyStops(
-      ts0,
-      transferProfile,
-      transferProfile.journey().transfer(),
-      false
-    )) {
-      // Skip the origin stop, loop transfers are not needed.
-      if (sd.stop == stop) {
-        continue;
-      }
-      if (sd.stop.transfersNotAllowed()) {
-        continue;
-      }
-      TransferKey transferKey = new TransferKey(stop, sd.stop, sd.edges);
-      PathTransfer pathTransfer = distinctTransfers.get(transferKey);
-      if (pathTransfer == null) {
-        // If the PathTransfer can't be found, it is created.
-        distinctTransfers.put(
-          transferKey,
-          new PathTransfer(stop, sd.stop, sd.distance, sd.edges, EnumSet.of(mode))
-        );
-      } else {
-        // If the PathTransfer is found, a new PathTransfer with the added mode is created.
-        distinctTransfers.put(transferKey, pathTransfer.withAddedMode(mode));
-      }
-    }
-  }
-
   private record TransferKey(StopLocation source, StopLocation target, List<Edge> edges) {}
 }
diff --git a/application/src/main/java/org/opentripplanner/graph_builder/module/nearbystops/StreetNearbyStopFinder.java b/application/src/main/java/org/opentripplanner/graph_builder/module/nearbystops/StreetNearbyStopFinder.java
@@ -38,7 +38,6 @@ public class StreetNearbyStopFinder implements NearbyStopFinder {
   private final int maxStopCount;
   private final DataOverlayContext dataOverlayContext;
   private final Set<Vertex> ignoreVertices;
-  private final Set<Vertex> findOnlyVertices;
 
   /**
    * Construct a NearbyStopFinder for the given graph and search radius.
@@ -51,7 +50,7 @@ public StreetNearbyStopFinder(
     int maxStopCount,
     DataOverlayContext dataOverlayContext
   ) {
-    this(durationLimit, maxStopCount, dataOverlayContext, Set.of(), Set.of());
+    this(durationLimit, maxStopCount, dataOverlayContext, Set.of());
   }
 
   /**
@@ -66,31 +65,11 @@ public StreetNearbyStopFinder(
     int maxStopCount,
     DataOverlayContext dataOverlayContext,
     Set<Vertex> ignoreVertices
-  ) {
-    this(durationLimit, maxStopCount, dataOverlayContext, ignoreVertices, Set.of());
-  }
-
-  /**
-   * Construct a NearbyStopFinder for the given graph and search radius.
-   *
-   * @param maxStopCount The maximum stops to return. 0 means no limit. Regardless of the maxStopCount
-   *                     we will always return all the directly connected stops.
-   * @param ignoreVertices   A set of stop vertices to ignore and not return NearbyStops for.
-   *
-   * @param findOnlyVertices   Only return NearbyStops that are in this set. If this is empty, no filtering is performed.
-   */
-  public StreetNearbyStopFinder(
-    Duration durationLimit,
-    int maxStopCount,
-    DataOverlayContext dataOverlayContext,
-    Set<Vertex> ignoreVertices,
-    Set<Vertex> findOnlyVertices
   ) {
     this.dataOverlayContext = dataOverlayContext;
     this.durationLimit = durationLimit;
     this.maxStopCount = maxStopCount;
     this.ignoreVertices = ignoreVertices;
-    this.findOnlyVertices = findOnlyVertices;
   }
 
   /**
@@ -170,10 +149,7 @@ public Collection<NearbyStop> findNearbyStops(
           continue;
         }
         if (targetVertex instanceof TransitStopVertex tsv && state.isFinal()) {
-          // If a set of findOnlyVertices is provided, only add stops that are in the set.
-          if (findOnlyVertices.isEmpty() || findOnlyVertices.contains(targetVertex)) {
-            stopsFound.add(NearbyStop.nearbyStopForState(state, tsv.getStop()));
-          }
+          stopsFound.add(NearbyStop.nearbyStopForState(state, tsv.getStop()));
         }
         if (
           OTPFeature.FlexRouting.isOn() &&
diff --git a/application/src/test/java/org/opentripplanner/graph_builder/module/nearbystops/StreetNearbyStopFinderTest.java b/application/src/test/java/org/opentripplanner/graph_builder/module/nearbystops/StreetNearbyStopFinderTest.java
@@ -163,78 +163,6 @@ void testIgnoreStopsWithMaxStops() {
     assertStopAtDistance(stopC, 200, sortedNearbyStops.get(0));
   }
 
-  @Test
-  void testFindOnlyVerticesStops() {
-    var durationLimit = Duration.ofMinutes(10);
-    var maxStopCount = 0;
-    Set<Vertex> findOnlyStops = Set.of(stopB, stopC);
-    var finder = new StreetNearbyStopFinder(
-      durationLimit,
-      maxStopCount,
-      null,
-      Set.of(),
-      findOnlyStops
-    );
-
-    var sortedNearbyStops = sort(
-      finder.findNearbyStops(stopA, new RouteRequest(), new StreetRequest(), false)
-    );
-
-    assertThat(sortedNearbyStops).hasSize(3);
-    assertZeroDistanceStop(stopA, sortedNearbyStops.get(0));
-    assertStopAtDistance(stopB, 100, sortedNearbyStops.get(1));
-    assertStopAtDistance(stopC, 200, sortedNearbyStops.get(2));
-  }
-
-  @Test
-  void testFindOnlyVerticesStopsWithIgnore() {
-    var durationLimit = Duration.ofMinutes(10);
-    var maxStopCount = 0;
-    Set<Vertex> findOnlyStops = Set.of(stopB, stopC);
-    Set<Vertex> ignore = Set.of(stopB);
-    var finder = new StreetNearbyStopFinder(
-      durationLimit,
-      maxStopCount,
-      null,
-      ignore,
-      findOnlyStops
-    );
-
-    var sortedNearbyStops = sort(
-      finder.findNearbyStops(stopA, new RouteRequest(), new StreetRequest(), false)
-    );
-
-    assertThat(sortedNearbyStops).hasSize(2);
-    assertZeroDistanceStop(stopA, sortedNearbyStops.get(0));
-    assertStopAtDistance(stopC, 200, sortedNearbyStops.get(1));
-  }
-
-  @Test
-  void testFindOnlyVerticesStopsWithDurationLimit() {
-    // If we only allow walk for 101 seconds and speed is 1 m/s we should only be able to reach
-    // one extra stop.
-    var durationLimit = Duration.ofSeconds(101);
-    var maxStopCount = 0;
-    Set<Vertex> findOnlyStops = Set.of(stopB, stopC);
-    var routeRequest = new RouteRequest()
-      .withPreferences(b -> b.withWalk(walkPreferences -> walkPreferences.withSpeed(1.0)));
-
-    var finder = new StreetNearbyStopFinder(
-      durationLimit,
-      maxStopCount,
-      null,
-      Set.of(),
-      findOnlyStops
-    );
-    var sortedNearbyStops = sort(
-      finder.findNearbyStops(stopA, routeRequest, new StreetRequest(), false)
-    );
-
-    assertThat(sortedNearbyStops).hasSize(2);
-    assertZeroDistanceStop(stopA, sortedNearbyStops.get(0));
-    assertStopAtDistance(stopB, 100, sortedNearbyStops.get(1));
-  }
-
   static List<NearbyStop> sort(Collection<NearbyStop> stops) {
     return stops.stream().sorted(Comparator.comparing(x -> x.distance)).toList();
   }
