Merge pull request #5735 from entur/fix_time_penalty_filter_bug

Fix error in access filter when access with time-penalty exits
opentripplanner · Mar 8, 2024 · d51048f · d51048f
2 parents 806a13c + 87b2108
commit d51048f
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 3 deletions.
diff --git a/src/main/java/org/opentripplanner/raptor/rangeraptor/transit/AccessPaths.java b/src/main/java/org/opentripplanner/raptor/rangeraptor/transit/AccessPaths.java
@@ -194,7 +194,7 @@ private List<RaptorAccessEgress> filterOnTimePenaltyLimitIfExist(List<RaptorAcce
     if (list == null) {
       return List.of();
     }
-    if (hasTimePenalty()) {
+    if (iterationTimePenaltyLimit != RaptorConstants.TIME_NOT_SET) {
       return list.stream().filter(e -> e.timePenalty() > iterationTimePenaltyLimit).toList();
     }
     return list;

diff --git a/src/test/java/org/opentripplanner/raptor/rangeraptor/transit/AccessPathsTest.java b/src/test/java/org/opentripplanner/raptor/rangeraptor/transit/AccessPathsTest.java
@@ -83,7 +83,7 @@ void calculateMaxNumberOfRides() {
   }
 
   @Test
-  void iterateOverPathsWithPenalty() {
+  void iterateOverPathsWithTimePenalty() {
     // Expected at departure 540
     var flexFastWithPenalty = FLEX_FAST.withTimePenalty(60);
 
@@ -113,6 +113,25 @@ void iterateOverPathsWithPenalty() {
       FORWARD
     );
 
+    // Make sure standard iterator works
+    expect(
+      accessPaths.arrivedOnStreetByNumOfRides(0),
+      WALK_B,
+      walkFastWithPenalty,
+      walkCostWithPenalty
+    );
+    expect(accessPaths.arrivedOnBoardByNumOfRides(1));
+    expect(accessPaths.arrivedOnStreetByNumOfRides(1));
+    expect(accessPaths.arrivedOnBoardByNumOfRides(2), flexTxWithPenalty);
+    expect(accessPaths.arrivedOnStreetByNumOfRides(2), FLEX_WALK_B);
+    expect(
+      accessPaths.arrivedOnBoardByNumOfRides(3),
+      FLEX_B,
+      flexFastWithPenalty,
+      flexCostWithPenalty
+    );
+    expect(accessPaths.arrivedOnStreetByNumOfRides(3));
+
     var iterator = accessPaths.iterateOverPathsWithPenalty(600);
 
     // First iteration
@@ -146,7 +165,7 @@ void iterateOverPathsWithPenalty() {
   }
 
   @Test
-  void iterateOverPathsWithPenaltyInReversDirection() {
+  void iterateOverPathsWithTimePenaltyInReversDirection() {
     // Expected at departure 540
     var flexFastWithPenalty = FLEX_FAST.withTimePenalty(60);
 
@@ -164,6 +183,10 @@ void iterateOverPathsWithPenaltyInReversDirection() {
       REVERSE
     );
 
+    // Make sure standard iterator works
+    expect(accessPaths.arrivedOnStreetByNumOfRides(0), WALK_B, walkFastWithPenalty);
+    expect(accessPaths.arrivedOnBoardByNumOfRides(3), FLEX_B, flexFastWithPenalty);
+
     var iterator = accessPaths.iterateOverPathsWithPenalty(600);
 
     // First iteration
@@ -196,6 +219,34 @@ void iterateOverPathsWithPenaltyInReversDirection() {
     assertFalse(iterator.hasNext());
   }
 
+  @Test
+  void testRegularIteratorsAndIteratorWithPenaltyWorksTogether() {
+    var walkFastWithPenalty = WALK_FAST.withTimePenalty(60);
+
+    // Without time-penalty, the iterator should be empty
+    var accessPaths = AccessPaths.create(
+      60,
+      List.of(walkFastWithPenalty, WALK_COST),
+      MULTI_CRITERIA,
+      FORWARD
+    );
+
+    // Both accesses are expected before with enter the "time-penalty" iteration
+    expect(accessPaths.arrivedOnStreetByNumOfRides(0), WALK_COST, walkFastWithPenalty);
+    expect(accessPaths.arrivedOnBoardByNumOfRides(0));
+
+    var iterator = accessPaths.iterateOverPathsWithPenalty(600);
+
+    // First iteration - only access with time-penalty is expected
+    assertTrue(iterator.hasNext());
+    assertEquals(540, iterator.next());
+    expect(accessPaths.arrivedOnStreetByNumOfRides(0), walkFastWithPenalty);
+    expect(accessPaths.arrivedOnBoardByNumOfRides(0));
+
+    // Second iteration - Done
+    assertFalse(iterator.hasNext());
+  }
+
   @Test
   void hasTimeDependentAccess() {
     var accessPaths = AccessPaths.create(