Improve readability, add tests

src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/Transfer.java

@@ -1,6 +1,5 @@
 package org.opentripplanner.routing.algorithm.raptoradapter.transit;
 
-import java.time.Duration;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
@@ -18,17 +17,21 @@
 public class Transfer {
 
   /**
-   * Since transfers costs are not computed through a full A* they can incur an absurdly high
-   * cost that overflows the integer cost inside raptor.
+   * Since transfers costs are not computed through a full A* with pruning they can incur an
+   * absurdly high cost that overflows the integer cost inside RAPTOR
+   * (https://github.com/opentripplanner/OpenTripPlanner/issues/5509).
    * <p>
    * An example would be a transfer using lots of stairs being used on a wheelchair when no
    * wheelchair-specific one has been generated.
    * (see https://docs.opentripplanner.org/en/dev-2.x/Accessibility/).
    * <p>
-   * For this reason there is this sanit limit that make sure that the transfer cost stays inside a
-   * bound that is still very high but far away from the integer overflow.
+   * For this reason there is this sanity limit that makes sure that the transfer cost stays below a
+   * limit is still very high (several days of transit-equivalent cost) but far away from the
+   * integer overflow.
+   *
+   * @see EdgeTraverser
    */
-  private final double MAX_TRANSFER_COST = Duration.ofDays(3).toSeconds();
+  private static final int MAX_TRANSFER_COST = Integer.MAX_VALUE / 100 / 30;
 
   private final int toStop;
 
@@ -78,9 +81,7 @@ public Optional<RaptorTransfer> asRaptorTransfer(StreetSearchRequest request) {
     if (edges == null || edges.isEmpty()) {
       double durationSeconds = distanceMeters / walkPreferences.speed();
       final double domainCost = durationSeconds * walkPreferences.reluctance();
-      if (domainCost > MAX_TRANSFER_COST) {
-        return Optional.empty();
-      } else {
+      if (domainCost < MAX_TRANSFER_COST) {
         return Optional.of(
           new DefaultRaptorTransfer(
             this.toStop,
@@ -89,6 +90,8 @@ public Optional<RaptorTransfer> asRaptorTransfer(StreetSearchRequest request) {
             this
           )
         );
+      } else {
+        return Optional.empty();
       }
     }
 
@@ -99,15 +102,14 @@ public Optional<RaptorTransfer> asRaptorTransfer(StreetSearchRequest request) {
 
     return state
       .filter(s -> s.weight < MAX_TRANSFER_COST)
-      .map(s -> {
-        final int raptorCost = RaptorCostConverter.toRaptorCost(s.getWeight());
-        return new DefaultRaptorTransfer(
+      .map(s ->
+        new DefaultRaptorTransfer(
           this.toStop,
           (int) s.getElapsedTimeSeconds(),
-          raptorCost,
+          RaptorCostConverter.toRaptorCost(s.getWeight()),
           this
-        );
-      });
+        )
+      );
   }
 
   @Override

src/test/java/org/opentripplanner/_support/geometry/Coordinates.java

@@ -5,6 +5,7 @@
 public class Coordinates {
 
   public static final Coordinate BERLIN = new Coordinate(52.5212, 13.4105);
+  public static final Coordinate BERLIN_BRANDENBURG_GATE = new Coordinate(52.51627, 13.37770);
   public static final Coordinate HAMBURG = new Coordinate(53.5566, 10.0003);
   public static final Coordinate KONGSBERG_PLATFORM_1 = new Coordinate(59.67216, 9.65107);
 }

src/test/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/TransferTest.java

@@ -0,0 +1,73 @@
+package org.opentripplanner.routing.algorithm.raptoradapter.transit;
+
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.opentripplanner.street.model._data.StreetModelForTest.intersectionVertex;
+
+import java.util.List;
+import java.util.Optional;
+import org.junit.jupiter.api.Nested;
+import org.junit.jupiter.api.Test;
+import org.opentripplanner._support.geometry.Coordinates;
+import org.opentripplanner.raptor.api.model.RaptorTransfer;
+import org.opentripplanner.street.model._data.StreetModelForTest;
+import org.opentripplanner.street.model.vertex.IntersectionVertex;
+import org.opentripplanner.street.search.request.StreetSearchRequest;
+
+class TransferTest {
+
+  private static final IntersectionVertex BERLIN_V = intersectionVertex(Coordinates.BERLIN);
+  private static final IntersectionVertex BRANDENBURG_GATE_V = intersectionVertex(
+    Coordinates.BERLIN_BRANDENBURG_GATE
+  );
+  private static final IntersectionVertex KONGSBERG_V = intersectionVertex(
+    Coordinates.KONGSBERG_PLATFORM_1
+  );
+
+  @Nested
+  class WithEdges {
+
+    @Test
+    void limitMaxCost() {
+      // very long edge from Berlin to Kongsberg, Norway that has of course a huge cost to traverse
+      var edge = StreetModelForTest.streetEdge(BERLIN_V, KONGSBERG_V);
+
+      var veryLongTransfer = new Transfer(0, List.of(edge));
+      assertTrue(veryLongTransfer.getDistanceMeters() > 800_000);
+      // cost would be too high, so it should not be included in RAPTOR search
+      assertTrue(veryLongTransfer.asRaptorTransfer(StreetSearchRequest.of().build()).isEmpty());
+    }
+
+    @Test
+    void allowLowCost() {
+      var edge = StreetModelForTest.streetEdge(BERLIN_V, BRANDENBURG_GATE_V);
+      var transfer = new Transfer(0, List.of(edge));
+      assertTrue(transfer.getDistanceMeters() < 4000);
+      final Optional<RaptorTransfer> raptorTransfer = transfer.asRaptorTransfer(
+        StreetSearchRequest.of().build()
+      );
+      // cost is below max limit and should be included in RAPTOR
+      assertTrue(raptorTransfer.isPresent());
+    }
+  }
+
+  @Nested
+  class WithoutEdges {
+
+    @Test
+    void limitMaxCost() {
+      var veryLongTransfer = new Transfer(0, 800_000);
+      // cost would be too high, so it should not be included in RAPTOR search
+      assertTrue(veryLongTransfer.asRaptorTransfer(StreetSearchRequest.of().build()).isEmpty());
+    }
+
+    @Test
+    void allowLowCost() {
+      var transfer = new Transfer(0, 200);
+      final Optional<RaptorTransfer> raptorTransfer = transfer.asRaptorTransfer(
+        StreetSearchRequest.of().build()
+      );
+      // cost is below max limit and should be included in RAPTOR
+      assertTrue(raptorTransfer.isPresent());
+    }
+  }
+}