Pass firstItineraryResultDeparture as parameter to withOriginalSearch instead of itinerary list.

Author: VillePihlava

application/src/main/java/org/opentripplanner/model/plan/paging/cursor/PageCursorFactory.java

@@ -5,9 +5,7 @@
 
 import java.time.Duration;
 import java.time.Instant;
-import java.util.List;
 import javax.annotation.Nullable;
-import org.opentripplanner.model.plan.Itinerary;
 import org.opentripplanner.model.plan.ItinerarySortKey;
 import org.opentripplanner.model.plan.SortOrder;
 import org.opentripplanner.utils.tostring.ToStringBuilder;
@@ -46,27 +44,28 @@ public PageCursorFactory(SortOrder sortOrder, Duration newSearchWindow) {
 
   /**
    * Set the original search earliest-departure-time({@code edt}), latest-arrival-time ({@code lat},
-   * optional) and the search-window used.
+   * optional) and the search-window used. Also resolve the page-type and
+   * first-search-latest-itinerary-departure.
    */
   public PageCursorFactory withOriginalSearch(
     @Nullable PageType pageType,
+    @Nullable Instant firstItineraryResultDeparture,
     Instant edt,
     Instant lat,
-    Duration searchWindow,
-    List<Itinerary> itineraries
+    Duration searchWindow
   ) {
     this.currentPageType = pageType == null
       ? resolvePageTypeForTheFirstSearch(sortOrder)
       : pageType;
-
-    this.currentEdt = edt;
-    this.currentLat = lat;
-    this.currentSearchWindow = searchWindow;
     this.firstSearchLatestItineraryDeparture = resolveFirstSearchLatestItineraryDeparture(
       pageType,
-      itineraries,
+      firstItineraryResultDeparture,
       edt
     );
+
+    this.currentEdt = edt;
+    this.currentLat = lat;
+    this.currentSearchWindow = searchWindow;
     return this;
   }
 
@@ -86,20 +85,6 @@ public PageCursorFactory withRemovedItineraries(PageCursorInput pageCursorFactor
     return this;
   }
 
-  /**
-   * If the initial request is of the PREVIOUS_PAGE type, the current search window is misleading.
-   * Instead of using the current search window to set the page cursor of the next page,
-   * the departure time of the latest itinerary result is used.
-   *
-   * @param firstSearchLatestItineraryDeparture the time of the latest departure of the itinerary results from the first search
-   */
-  public PageCursorFactory withFirstSearchLatestItineraryDeparture(
-    Instant firstSearchLatestItineraryDeparture
-  ) {
-    this.firstSearchLatestItineraryDeparture = firstSearchLatestItineraryDeparture;
-    return this;
-  }
-
   @Nullable
   public PageCursor previousPageCursor() {
     createPageCursors();
@@ -138,17 +123,18 @@ private static PageType resolvePageTypeForTheFirstSearch(SortOrder sortOrder) {
   }
 
   /**
-   * If the first search is an arrive by search, store the latest itinerary's start time
-   * to allow setting cursor information correctly for the page cursor of the next page.
+   * If the first search is an arrive by search (PREVIOUS_PAGE type), the current search window is
+   * misleading. Instead of using the current search window to set the page cursor of the next
+   * page, the departure time of the latest itinerary result is used to avoid missing itineraries.
    */
   private Instant resolveFirstSearchLatestItineraryDeparture(
     @Nullable PageType pageType,
-    List<Itinerary> itineraries,
+    @Nullable Instant firstItineraryResultDeparture,
     Instant edt
   ) {
     if (pageType == null && resolvePageTypeForTheFirstSearch(sortOrder) == PREVIOUS_PAGE) {
-      if (itineraries.size() > 0) {
-        return itineraries.get(0).startTimeAsInstant();
+      if (firstItineraryResultDeparture != null) {
+        return firstItineraryResultDeparture;
       } else {
         return edt;
       }

application/src/main/java/org/opentripplanner/service/paging/PagingService.java

@@ -179,10 +179,10 @@ private PageCursorFactory mapIntoPageCursorFactory(@Nullable PageType currentPag
 
     factory = factory.withOriginalSearch(
       currentPageType,
+      itineraries.size() > 0 ? itineraries.get(0).startTimeAsInstant() : null,
       earliestDepartureTime,
       latestArrivalTime,
-      searchWindowUsed,
-      itineraries
+      searchWindowUsed
     );
 
     if (pageCursorInput != null) {

application/src/test/java/org/opentripplanner/model/plan/paging/cursor/PageCursorFactoryTest.java

@@ -36,11 +36,11 @@ class PageCursorFactoryTest implements PlanTestConstants {
   @Test
   public void sortArrivalAscending() {
     var factory = new PageCursorFactory(STREET_AND_ARRIVAL_TIME, D90M).withOriginalSearch(
+      null,
       null,
       T12_00,
       null,
-      D1H,
-      List.of()
+      D1H
     );
 
     var nextPage = factory.nextPageCursor();
@@ -53,7 +53,7 @@ public void sortArrivalAscending() {
   @Test
   public void sortArrivalAscendingCropSearchWindow() {
     var factory = new PageCursorFactory(STREET_AND_ARRIVAL_TIME, D90M)
-      .withOriginalSearch(NEXT_PAGE, T12_00, null, D1H, List.of())
+      .withOriginalSearch(NEXT_PAGE, null, T12_00, null, D1H)
       .withRemovedItineraries(
         new TestPageCursorInput(
           newItinerary(A).bus(55, timeAsSeconds(T12_00), timeAsSeconds(T12_10), B).build(),
@@ -72,10 +72,10 @@ public void sortArrivalAscendingCropSearchWindow() {
   public void sortArrivalAscendingPreviousPage() {
     var factory = new PageCursorFactory(STREET_AND_ARRIVAL_TIME, D90M).withOriginalSearch(
       PREVIOUS_PAGE,
+      null,
       T12_00,
       null,
-      D1H,
-      List.of()
+      D1H
     );
 
     var nextPage = factory.nextPageCursor();
@@ -88,7 +88,7 @@ public void sortArrivalAscendingPreviousPage() {
   @Test
   public void sortArrivalAscendingCropSearchWindowPreviousPage() {
     var factory = new PageCursorFactory(STREET_AND_ARRIVAL_TIME, D90M)
-      .withOriginalSearch(PREVIOUS_PAGE, T12_00, null, D1H, List.of())
+      .withOriginalSearch(PREVIOUS_PAGE, null, T12_00, null, D1H)
       .withRemovedItineraries(
         new TestPageCursorInput(
           newItinerary(A).bus(55, timeAsSeconds(T12_00), timeAsSeconds(T12_10), B).build(),
@@ -106,11 +106,11 @@ public void sortArrivalAscendingCropSearchWindowPreviousPage() {
   @Test
   public void sortDepartureDescending() {
     var factory = new PageCursorFactory(STREET_AND_DEPARTURE_TIME, D90M).withOriginalSearch(
+      null,
       null,
       T12_00,
       T13_30,
-      D1H,
-      List.of()
+      D1H
     );
 
     var nextPage = factory.nextPageCursor();
@@ -123,7 +123,7 @@ public void sortDepartureDescending() {
   @Test
   public void sortDepartureDescendingCropSearchWindow() {
     var factory = new PageCursorFactory(STREET_AND_DEPARTURE_TIME, D90M)
-      .withOriginalSearch(PREVIOUS_PAGE, T12_00, T13_30, D1H, List.of())
+      .withOriginalSearch(PREVIOUS_PAGE, null, T12_00, T13_30, D1H)
       .withRemovedItineraries(
         new TestPageCursorInput(
           newItinerary(A).bus(55, timeAsSeconds(T12_00), timeAsSeconds(T12_30), B).build(),
@@ -142,10 +142,10 @@ public void sortDepartureDescendingCropSearchWindow() {
   public void sortDepartureDescendingNextPage() {
     var factory = new PageCursorFactory(STREET_AND_DEPARTURE_TIME, D90M).withOriginalSearch(
       NEXT_PAGE,
+      null,
       T12_00,
       T13_30,
-      D1H,
-      List.of()
+      D1H
     );
 
     var nextPage = factory.nextPageCursor();
@@ -158,7 +158,7 @@ public void sortDepartureDescendingNextPage() {
   @Test
   public void sortDepartureDescendingCropSearchWindowNextPage() {
     var factory = new PageCursorFactory(STREET_AND_DEPARTURE_TIME, D90M)
-      .withOriginalSearch(NEXT_PAGE, T12_00, T13_30, D1H, List.of())
+      .withOriginalSearch(NEXT_PAGE, null, T12_00, T13_30, D1H)
       .withRemovedItineraries(
         new TestPageCursorInput(
           newItinerary(A).bus(55, timeAsSeconds(T12_00), timeAsSeconds(T12_30), B).build(),