Merge pull request #6050 from ibi-group/arrive-by-filtering

Fix arrive by filtering for on-street/flex itineraries

src/ext-test/java/org/opentripplanner/ext/emissions/EmissionsTest.java

@@ -2,6 +2,7 @@
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.opentripplanner.model.plan.Itinerary.createScheduledTransitItinerary;
 import static org.opentripplanner.transit.model._data.TransitModelForTest.id;
 
 import java.time.OffsetDateTime;
@@ -63,42 +64,48 @@ static void SetUp() {
 
   @Test
   void testGetEmissionsForItinerary() {
-    Itinerary i = new Itinerary(List.of(createTransitLeg(ROUTE_WITH_EMISSIONS)));
+    Itinerary i = createScheduledTransitItinerary(List.of(createTransitLeg(ROUTE_WITH_EMISSIONS)));
     decorateWithEmission.decorate(i);
     assertEquals(new Grams(2223.902), i.getEmissionsPerPerson().getCo2());
   }
 
   @Test
   void testGetEmissionsForCarRoute() {
-    Itinerary i = new Itinerary(List.of(STREET_LEG));
+    Itinerary i = createScheduledTransitItinerary(List.of(STREET_LEG));
     decorateWithEmission.decorate(i);
     assertEquals(new Grams(28.0864), i.getEmissionsPerPerson().getCo2());
   }
 
   @Test
   void testNoEmissionsForFeedWithoutEmissionsConfigured() {
-    Itinerary i = new Itinerary(List.of(createTransitLeg(ROUTE_WITHOUT_EMISSIONS_CONFIGURED)));
+    Itinerary i = createScheduledTransitItinerary(
+      List.of(createTransitLeg(ROUTE_WITHOUT_EMISSIONS_CONFIGURED))
+    );
     decorateWithEmission.decorate(i);
     assertNull(i.getEmissionsPerPerson());
   }
 
   @Test
   void testZeroEmissionsForItineraryWithZeroEmissions() {
-    Itinerary i = new Itinerary(List.of(createTransitLeg(ROUTE_WITH_ZERO_EMISSIONS)));
+    Itinerary i = createScheduledTransitItinerary(
+      List.of(createTransitLeg(ROUTE_WITH_ZERO_EMISSIONS))
+    );
     decorateWithEmission.decorate(i);
     assertEquals(new Grams(0.0), i.getEmissionsPerPerson().getCo2());
   }
 
   @Test
   void testGetEmissionsForCombinedRoute() {
-    Itinerary i = new Itinerary(List.of(createTransitLeg(ROUTE_WITH_EMISSIONS), STREET_LEG));
+    Itinerary i = createScheduledTransitItinerary(
+      List.of(createTransitLeg(ROUTE_WITH_EMISSIONS), STREET_LEG)
+    );
     decorateWithEmission.decorate(i);
     assertEquals(new Grams(2251.9884), i.getEmissionsPerPerson().getCo2());
   }
 
   @Test
   void testNoEmissionsForCombinedRouteWithoutTransitEmissions() {
-    Itinerary i = new Itinerary(
+    Itinerary i = createScheduledTransitItinerary(
       List.of(createTransitLeg(ROUTE_WITHOUT_EMISSIONS_CONFIGURED), STREET_LEG)
     );
     decorateWithEmission.decorate(i);

src/ext-test/java/org/opentripplanner/ext/fares/impl/OrcaFareServiceTest.java

@@ -79,7 +79,7 @@ public static void setUpClass() {
    * types.
    */
   private static void calculateFare(List<Leg> legs, FareType fareType, Money expectedPrice) {
-    var itinerary = new Itinerary(legs);
+    var itinerary = Itinerary.createScheduledTransitItinerary(legs);
     var itineraryFares = orcaFareService.calculateFares(itinerary);
     assertEquals(
       expectedPrice,

src/main/java/org/opentripplanner/model/plan/Itinerary.java

@@ -64,12 +64,14 @@ public class Itinerary implements ItinerarySortKey {
   /* other properties */
 
   private final List<SystemNotice> systemNotices = new ArrayList<>();
+  private final boolean searchWindowAware;
   private List<Leg> legs;
 
   private ItineraryFares fare = ItineraryFares.empty();
 
-  public Itinerary(List<Leg> legs) {
+  private Itinerary(List<Leg> legs, boolean searchWindowAware) {
     setLegs(legs);
+    this.searchWindowAware = searchWindowAware;
 
     // Set aggregated data
     ItinerariesCalculateLegTotals totals = new ItinerariesCalculateLegTotals(legs);
@@ -87,6 +89,21 @@ public Itinerary(List<Leg> legs) {
     this.setElevationLost(totals.totalElevationLost);
   }
 
+  /**
+   * Creates an itinerary that contains scheduled transit which is aware of the search window.
+   */
+  public static Itinerary createScheduledTransitItinerary(List<Leg> legs) {
+    return new Itinerary(legs, true);
+  }
+
+  /**
+   * Creates an itinerary that creates only street or flex results which are not aware of the
+   * time window.
+   */
+  public static Itinerary createDirectItinerary(List<Leg> legs) {
+    return new Itinerary(legs, false);
+  }
+
   /**
    * Time that the trip departs.
    */
@@ -162,13 +179,30 @@ public boolean isOnStreetAllTheWay() {
     return isStreetOnly();
   }
 
+  /**
+   * Returns true if this itinerary has only flex and walking legs.
+   */
+  public boolean isDirectFlex() {
+    var containsFlex = legs.stream().anyMatch(Leg::isFlexibleTrip);
+    var flexOrWalkOnly = legs.stream().allMatch(l -> l.isFlexibleTrip() || l.isWalkingLeg());
+    return containsFlex && flexOrWalkOnly;
+  }
+
   /** TRUE if at least one leg is a transit leg. */
   public boolean hasTransit() {
     return legs
       .stream()
       .anyMatch(l -> l instanceof ScheduledTransitLeg || l instanceof FlexibleTransitLeg);
   }
 
+  /**
+   * Returns true if this itinerary was produced by an algorithm that is aware of the search window.
+   * As of 2024 only the itineraries produced by RAPTOR that do that.
+   */
+  public boolean isSearchWindowAware() {
+    return searchWindowAware;
+  }
+
   public Leg firstLeg() {
     return getLegs().get(0);
   }
@@ -215,7 +249,7 @@ public Itinerary withTimeShiftToStartAt(ZonedDateTime afterTime) {
       .stream()
       .map(leg -> leg.withTimeShift(duration))
       .collect(Collectors.toList());
-    var newItin = new Itinerary(timeShiftedLegs);
+    var newItin = new Itinerary(timeShiftedLegs, searchWindowAware);
     newItin.setGeneralizedCost(getGeneralizedCost());
     return newItin;
   }

src/main/java/org/opentripplanner/model/plan/paging/cursor/readme.md

@@ -13,7 +13,7 @@ moving on to the next.
 
 ## Terminology
 
-- **search-window (sw)**  The search window is the minutes Raptor iterate over and the time-window
+- **search-window (sw)**  The search window is the minutes Raptor iterates over and the time-window
   the itinerary must start within to be included in the result. The search-window may change from a
   request to the next page.  **sw'** is the search window for the new next/previous page. The search
   window may change between requests, so we need to account for it when computing the next/previous
@@ -28,7 +28,7 @@ moving on to the next.
 - **<< previous page**  The trip search constructed to retrieve itineraries AFTER the original
   search.
 - **crop-search-window** If the `maxNumOfItineraries` limit is reached in the
-  `ItineraryFilterChain`, then one or more itineraries are removed. The filter remove itineraries
+  `ItineraryFilterChain`, then one or more itineraries are removed. The filter removes itineraries
   from the beginning or end of the list depending on the page cursor type (next/previous) and the
   sort order(arrival/departure time).
 
@@ -55,8 +55,8 @@ _previous-page_ must reverse the itinerary-filtering: `crop itineraries at START
 - In this case the `<< Previous page` is the same as in [sort-by-arrival](#sort-by-arrival) and not
   shown.
 - For the `Next page >>` we must adjust the `edt'`.
-- In rare cases we get duplicate itineraries. This happens if the `removed itinerary` depart before,
-  but arrive after the `duplicate`.
+- In rare cases we get duplicate itineraries. This happens if the `removed itinerary` departs before,
+  but arrives after the `duplicate`.
 
 ### sort-by-arrival, crop-search-window & original-prev-page
 
@@ -99,7 +99,7 @@ This is the basic `sort-by-departure` (arrive-by search) without removing itiner
 In this case the itineraries are dropped from the search results in the `Original Search` and the
 `<< Previous page` must be adjusted. We use the first removed itinerary to set both the `edt'` and
 the `lat'`. An `optimal itinarary` in the original search is lost (not found) in the previous page
-if it departs AFTER the `remoed itinerary` and arrive before - hopefully this is a rare case.
+if it departs AFTER the `removed itinerary` and arrives before - hopefully this is a rare case.
 
 The `Next page >>` is the same as the basic case [sort-by-departure](#sort-by-departure).
 

src/main/java/org/opentripplanner/routing/algorithm/filterchain/ItineraryListFilterChainBuilder.java

@@ -25,6 +25,7 @@
 import org.opentripplanner.routing.algorithm.filterchain.filters.street.RemoveNonTransitItinerariesBasedOnGeneralizedCost;
 import org.opentripplanner.routing.algorithm.filterchain.filters.street.RemoveParkAndRideWithMostlyWalkingFilter;
 import org.opentripplanner.routing.algorithm.filterchain.filters.street.RemoveWalkOnlyFilter;
+import org.opentripplanner.routing.algorithm.filterchain.filters.system.FlexSearchWindowFilter;
 import org.opentripplanner.routing.algorithm.filterchain.filters.system.NumItinerariesFilter;
 import org.opentripplanner.routing.algorithm.filterchain.filters.system.OutsideSearchWindowFilter;
 import org.opentripplanner.routing.algorithm.filterchain.filters.system.PagingFilter;
@@ -89,6 +90,7 @@ public class ItineraryListFilterChainBuilder {
   private boolean removeTransitIfWalkingIsBetter = true;
   private ItinerarySortKey itineraryPageCut;
   private boolean transitGroupPriorityUsed = false;
+  private boolean filterDirectFlexBySearchWindow = true;
 
   /**
    * Sandbox filters which decorate the itineraries with extra information.
@@ -470,6 +472,13 @@ public ItineraryListFilterChain build() {
         );
       }
 
+      if (earliestDepartureTime != null && filterDirectFlexBySearchWindow) {
+        addRemoveFilter(
+          filters,
+          new FlexSearchWindowFilter(earliestDepartureTime, searchWindow, sortOrder)
+        );
+      }
+
       // Remove itineraries present in the page retrieved before this page/search.
       if (itineraryPageCut != null) {
         addRemoveFilter(
@@ -533,6 +542,11 @@ public ItineraryListFilterChain build() {
     return new ItineraryListFilterChain(filters, debugHandler);
   }
 
+  public ItineraryListFilterChainBuilder withFilterDirectFlexBySearchWindow(boolean b) {
+    this.filterDirectFlexBySearchWindow = b;
+    return this;
+  }
+
   /**
    * If enabled, this adds the filter to remove itineraries which have the same stops and routes.
    * These are sometimes called "time-shifted duplicates" but since those terms have so many

src/main/java/org/opentripplanner/routing/algorithm/filterchain/filters/system/FlexSearchWindowFilter.java

@@ -0,0 +1,65 @@
+package org.opentripplanner.routing.algorithm.filterchain.filters.system;
+
+import java.time.Duration;
+import java.time.Instant;
+import java.util.function.Predicate;
+import org.opentripplanner.model.plan.Itinerary;
+import org.opentripplanner.model.plan.SortOrder;
+import org.opentripplanner.routing.algorithm.filterchain.framework.spi.RemoveItineraryFlagger;
+
+/**
+ * The flex router doesn't use the transit router's  search-window, but nevertheless using it
+ * for filtering is useful when combining flex with transit.
+ * <p>
+ * The flex router also searches the previous day (arrive by) or the next one (depart after).
+ * If you didn't filter the flex results by something you could get yesterday's or tomorrow's
+ * trips where you would not expect it.
+ */
+public class FlexSearchWindowFilter implements RemoveItineraryFlagger {
+
+  public static final String TAG = "flex-outside-search-window";
+
+  private final Instant earliestDepartureTime;
+  private final Instant latestArrivalTime;
+  private final SortOrder sortOrder;
+
+  public FlexSearchWindowFilter(
+    Instant earliestDepartureTime,
+    Duration searchWindow,
+    SortOrder sortOrder
+  ) {
+    this.earliestDepartureTime = earliestDepartureTime;
+    this.latestArrivalTime = earliestDepartureTime.plus(searchWindow);
+    this.sortOrder = sortOrder;
+  }
+
+  @Override
+  public String name() {
+    return TAG;
+  }
+
+  @Override
+  public Predicate<Itinerary> shouldBeFlaggedForRemoval() {
+    return it -> {
+      if (it.isDirectFlex()) {
+        return switch (sortOrder) {
+          case STREET_AND_DEPARTURE_TIME -> {
+            var time = it.startTime().toInstant();
+            yield time.isBefore(earliestDepartureTime);
+          }
+          case STREET_AND_ARRIVAL_TIME -> {
+            var time = it.startTime().toInstant();
+            yield time.isAfter(latestArrivalTime);
+          }
+        };
+      } else {
+        return false;
+      }
+    };
+  }
+
+  @Override
+  public boolean skipAlreadyFlaggedItineraries() {
+    return false;
+  }
+}

src/main/java/org/opentripplanner/routing/algorithm/filterchain/filters/system/OutsideSearchWindowFilter.java

@@ -7,8 +7,10 @@
 import org.opentripplanner.routing.algorithm.filterchain.framework.spi.RemoveItineraryFlagger;
 
 /**
- * This filter will remove all itineraries that are outside the search-window. In some
- * cases the access is time-shifted after the end of the search-window. These results
+ * This filter will remove all itineraries that are both search-window aware and outside the
+ * search-window. Only those that use transit are search-window aware, street and flex itineraries are not.
+ * <p>
+ * In some cases the access is time-shifted after the end of the search-window. These results
  * should appear again when paging to the next page. Hence, this filter will remove
  * such itineraries. The same is true for when paging to the previous page for arriveBy=true.
  * <p>
@@ -35,8 +37,12 @@ public String name() {
   @Override
   public Predicate<Itinerary> shouldBeFlaggedForRemoval() {
     return it -> {
-      var time = it.startTime().toInstant();
-      return time.isBefore(earliestDepartureTime) || !time.isBefore(latestDepartureTime);
+      if (it.isSearchWindowAware()) {
+        var time = it.startTime().toInstant();
+        return time.isBefore(earliestDepartureTime) || !time.isBefore(latestDepartureTime);
+      } else {
+        return false;
+      }
     };
   }
 

src/main/java/org/opentripplanner/routing/algorithm/mapping/GraphPathToItineraryMapper.java

@@ -134,7 +134,7 @@ public Itinerary generateItinerary(GraphPath<State, Edge, Vertex> path) {
       }
     }
 
-    Itinerary itinerary = new Itinerary(legs);
+    Itinerary itinerary = Itinerary.createDirectItinerary(legs);
 
     calculateElevations(itinerary, path.edges);
 

src/main/java/org/opentripplanner/routing/algorithm/mapping/RaptorPathToItineraryMapper.java

@@ -140,7 +140,7 @@ else if (pathLeg.isTransferLeg()) {
     Itinerary mapped = mapEgressLeg(egressPathLeg);
     legs.addAll(mapped == null ? List.of() : mapped.getLegs());
 
-    Itinerary itinerary = new Itinerary(legs);
+    Itinerary itinerary = Itinerary.createScheduledTransitItinerary(legs);
 
     // Map general itinerary fields
     itinerary.setArrivedAtDestinationWithRentedVehicle(
@@ -390,7 +390,7 @@ private List<Leg> mapNonTransitLeg(
   }
 
   private Itinerary mapDirectPath(RaptorPath<T> path) {
-    return new Itinerary(
+    return Itinerary.createScheduledTransitItinerary(
       List.of(
         new UnknownTransitPathLeg(
           mapPlace(request.from()),

src/main/java/org/opentripplanner/routing/algorithm/mapping/RouteRequestToFilterChainMapper.java

@@ -92,6 +92,7 @@ public static ItineraryListFilterChain createFilterChain(
       .withPageCursorInputSubscriber(pageCursorInputSubscriber)
       .withRemoveWalkAllTheWayResults(removeWalkAllTheWayResults)
       .withRemoveTransitIfWalkingIsBetter(true)
+      .withFilterDirectFlexBySearchWindow(params.filterDirectFlexBySearchWindow())
       .withDebugEnabled(params.debug());
 
     if (!request.preferences().transit().relaxTransitGroupPriority().isNormal()) {

src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/router/FilterTransitWhenDirectModeIsEmpty.java

@@ -5,13 +5,13 @@
 
 /**
  * <p>
- * In OTP the street search and transit search is done as to separate searches. The results is then
- * merged and filtered to remove none optimal itineraries. But, when the client do NOT provide a
- * ´directMode´, OTP do not do the streetSearch. And, the removal of none optimal results is not
- * done, there is not street results to use to prune bad transit results with. In other words OTP is
+ * In OTP, the street search and transit search are done separately. The results are then
+ * merged and filtered to remove non-optimal itineraries. But, when the client does NOT provide a
+ * ´directMode´, OTP does not do the streetSearch. And, the removal of non-optimal results is not
+ * done, there are no street results to use to prune bad transit results with. In other words, OTP is
  * forced to return at least one itinerary with at least one transit leg. So, instead of walking
- * maybe 100 meters, the OTP suggest you need to walk to the closest buss stop, take the bus one
- * stop and walk back, oten with more walking than just those 100 meters.
+ * maybe 100 meters, OTP suggests you need to walk to the closest bus stop, take the bus for one
+ * stop and walk back, often with more walking than just those 100 meters.
  * <p>
  * <b>Let say OTP produces these internal results:</b>
  * <ul>
@@ -31,7 +31,7 @@
  * <p>
  * If no directMode is set, the responsibility of this class it to always filter away itineraries
  * with a generalized-cost that is higher than the WALK-ALL-THE-WAY. We achieve this by setting the
- * directMode before searching. This trigger the direct street search, and later the result is
+ * directMode before searching. This triggers the direct street search, and later the result is
  * passed into the filter chain where none optimal results are removed. Finally the street itinerary
  * is removed and the request street mode rest to the original state.
  * <p>