Size carpool routing trees from cached OTP leg durations with baseline fallback

Author: hakoncarlsen

application/src/ext-test/java/org/opentripplanner/ext/carpooling/internal/DefaultCarpoolingRepositoryTest.java

@@ -7,9 +7,14 @@
 import java.time.Duration;
 import java.time.ZoneId;
 import java.time.ZonedDateTime;
+import java.util.List;
 import org.junit.jupiter.api.Test;
+import org.opentripplanner.core.model.id.FeedScopedId;
 import org.opentripplanner.ext.carpooling.CarpoolTripTestData;
+import org.opentripplanner.ext.carpooling.model.CarpoolStop;
 import org.opentripplanner.ext.carpooling.model.CarpoolTrip;
+import org.opentripplanner.ext.carpooling.model.CarpoolTripBuilder;
+import org.opentripplanner.street.geometry.WgsCoordinate;
 
 class DefaultCarpoolingRepositoryTest {
 
@@ -92,6 +97,106 @@ void throttlesSweepsToOncePerInterval() {
     assertThat(repository.getCarpoolTrips()).isEmpty();
   }
 
+  @Test
+  void keepsCachedLegDurationsAcrossASameGeometryUpsert() {
+    var repository = new DefaultCarpoolingRepository();
+    var trip = tripWithCoordinates("kept", OSLO_CENTER, OSLO_EAST, NOON.plusHours(1));
+    repository.upsertCarpoolTrip(trip);
+    repository.cacheBaselineLegDurations(trip.getId(), new Duration[] { Duration.ofMinutes(12) });
+
+    // A budget- or time-only update keeps the same route points, so the cache survives.
+    var updated = tripWithCoordinates("kept", OSLO_CENTER, OSLO_EAST, NOON.plusHours(2));
+    repository.upsertCarpoolTrip(updated);
+
+    assertThat(repository.cachedBaselineLegDurations(trip.getId()))
+      .asList()
+      .containsExactly(Duration.ofMinutes(12));
+  }
+
+  @Test
+  void dropsCachedLegDurationsWhenRoutePointsChange() {
+    var repository = new DefaultCarpoolingRepository();
+    var trip = tripWithCoordinates("moved", OSLO_CENTER, OSLO_EAST, NOON.plusHours(1));
+    repository.upsertCarpoolTrip(trip);
+    repository.cacheBaselineLegDurations(trip.getId(), new Duration[] { Duration.ofMinutes(12) });
+
+    var rerouted = tripWithCoordinates(
+      "moved",
+      OSLO_CENTER,
+      new WgsCoordinate(60.0, 11.0),
+      NOON.plusHours(1)
+    );
+    repository.upsertCarpoolTrip(rerouted);
+
+    assertThat(repository.cachedBaselineLegDurations(trip.getId())).isNull();
+  }
+
+  @Test
+  void dropsCachedLegDurationsWhenTripRemoved() {
+    var repository = new DefaultCarpoolingRepository();
+    var trip = tripWithCoordinates("removed", OSLO_CENTER, OSLO_EAST, NOON.plusHours(1));
+    repository.upsertCarpoolTrip(trip);
+    repository.cacheBaselineLegDurations(trip.getId(), new Duration[] { Duration.ofMinutes(12) });
+
+    repository.removeCarpoolTrip(trip.getId());
+
+    assertThat(repository.cachedBaselineLegDurations(trip.getId())).isNull();
+  }
+
+  @Test
+  void dropsCachedLegDurationsWhenTripExpires() {
+    var repository = new DefaultCarpoolingRepository();
+    var trip = tripWithCoordinates("expired", OSLO_CENTER, OSLO_EAST, NOON);
+    repository.upsertCarpoolTrip(trip);
+    repository.cacheBaselineLegDurations(trip.getId(), new Duration[] { Duration.ofMinutes(12) });
+
+    repository.removeExpiredTrips(NOON.plusHours(1).toInstant(), Duration.ZERO);
+
+    assertThat(repository.cachedBaselineLegDurations(trip.getId())).isNull();
+  }
+
+  @Test
+  void returnsADefensiveCopyOfCachedLegDurations() {
+    var repository = new DefaultCarpoolingRepository();
+    var trip = tripWithCoordinates("copy", OSLO_CENTER, OSLO_EAST, NOON.plusHours(1));
+    repository.upsertCarpoolTrip(trip);
+    var stored = new Duration[] { Duration.ofMinutes(12) };
+    repository.cacheBaselineLegDurations(trip.getId(), stored);
+
+    // Neither mutating the stored source array nor the returned array may corrupt the cache.
+    stored[0] = Duration.ofMinutes(99);
+    repository.cachedBaselineLegDurations(trip.getId())[0] = Duration.ofMinutes(7);
+
+    assertThat(repository.cachedBaselineLegDurations(trip.getId()))
+      .asList()
+      .containsExactly(Duration.ofMinutes(12));
+  }
+
+  private static CarpoolTrip tripWithCoordinates(
+    String id,
+    WgsCoordinate origin,
+    WgsCoordinate destination,
+    ZonedDateTime endTime
+  ) {
+    return new CarpoolTripBuilder(FeedScopedId.ofNullable("TEST", id))
+      .withStops(
+        List.of(
+          CarpoolStop.of(FeedScopedId.ofNullable("TEST", id + "-origin"))
+            .withCoordinate(origin)
+            .withOnboardCount(1)
+            .build(),
+          CarpoolStop.of(FeedScopedId.ofNullable("TEST", id + "-destination"))
+            .withCoordinate(destination)
+            .withOnboardCount(1)
+            .build()
+        )
+      )
+      .withTotalCapacity(CarpoolTrip.DEFAULT_TOTAL_CAPACITY)
+      .withStartTime(NOON)
+      .withEndTime(endTime)
+      .build();
+  }
+
   private static CarpoolTrip tripEndingAt(ZonedDateTime endTime) {
     return CarpoolTripTestData.createSimpleTripWithTimes(
       OSLO_CENTER,

application/src/ext-test/java/org/opentripplanner/ext/carpooling/service/DefaultCarpoolingServiceTreeLimitTest.java

@@ -1,8 +1,8 @@
 package org.opentripplanner.ext.carpooling.service;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
-import static org.opentripplanner.ext.carpooling.service.DefaultCarpoolingService.LEG_DURATION_SLACK;
 import static org.opentripplanner.ext.carpooling.service.DefaultCarpoolingService.MAX_SEARCH_DURATION_FOR_NEARBY_STOPS_FOR_ACCESS_EGRESS;
 
 import java.time.Duration;
@@ -16,13 +16,19 @@
 import org.opentripplanner.ext.carpooling.model.CarpoolStop;
 import org.opentripplanner.ext.carpooling.model.CarpoolTrip;
 import org.opentripplanner.ext.carpooling.model.CarpoolTripBuilder;
+import org.opentripplanner.ext.carpooling.updater.CarpoolSiriMapper;
 import org.opentripplanner.street.geometry.WgsCoordinate;
 
 /**
- * Unit tests for {@link DefaultCarpoolingService#driverLegTreeLimits}: each leg gets
- * {@code leg + slack + smallestDownstreamBudget}, multi-leg trips get per-leg limits well below
- * the whole-trip limit, and an incomplete or non-monotonic timeline falls back to whole-trip
- * sizing for every leg.
+ * Unit tests for {@link DefaultCarpoolingService#driverLegTreeLimits} and
+ * {@link DefaultCarpoolingService#scheduledLegDurations}: each leg gets
+ * {@code leg + slack + smallestDownstreamBudget} capped at
+ * {@link org.opentripplanner.ext.carpooling.updater.CarpoolSiriMapper#MAX_TRIP_DURATION}, multi-leg
+ * trips get per-leg limits well below the whole-trip limit, and an incomplete or non-monotonic
+ * schedule yields no scheduled durations (signalling the caller to fall back to whole-trip sizing).
+ * The leg durations here come from the SIRI schedule; in production they are OTP's own routed
+ * durations (see {@link DefaultCarpoolingService#resolveLegDurations}), but the limit arithmetic is
+ * identical.
  */
 class DefaultCarpoolingServiceTreeLimitTest {
 
@@ -33,11 +39,26 @@ class DefaultCarpoolingServiceTreeLimitTest {
 
   private static int idCounter = 0;
 
+  /** Mirrors the small slack {@link DefaultCarpoolingService#driverLegTreeLimits} adds per leg. */
+  private static final Duration SLACK = Duration.ofMinutes(1);
+
   /**
    * Matches the production formula: leg + flat slack + smallest downstream deviation budget.
    */
   private static Duration expectedLimit(Duration leg, Duration allowance) {
-    return leg.plus(LEG_DURATION_SLACK).plus(allowance);
+    return leg.plus(SLACK).plus(allowance);
+  }
+
+  /**
+   * Sizes the trip's legs from its SIRI schedule, exercising the same
+   * {@link DefaultCarpoolingService#driverLegTreeLimits} arithmetic production uses with OTP's
+   * routed durations.
+   */
+  private static Duration[] limitsFromSchedule(CarpoolTrip trip) {
+    return DefaultCarpoolingService.driverLegTreeLimits(
+      trip,
+      DefaultCarpoolingService.scheduledLegDurations(trip)
+    );
   }
 
   @Test
@@ -50,7 +71,7 @@ void sizesEachLegIndependentlyAndWellBelowWholeTrip() {
       destinationStop(BASE.plusMinutes(160), BUDGET)
     );
 
-    var limits = DefaultCarpoolingService.driverLegTreeLimits(trip);
+    var limits = limitsFromSchedule(trip);
 
     var leg0 = expectedLimit(Duration.ofMinutes(60), BUDGET);
     var leg1 = expectedLimit(Duration.ofMinutes(100), BUDGET);
@@ -77,12 +98,12 @@ void usesSmallestDownstreamBudgetAsDetourAllowance() {
       destinationStop(BASE.plusMinutes(120), Duration.ofMinutes(40))
     );
 
-    var limits = DefaultCarpoolingService.driverLegTreeLimits(trip);
+    var limits = limitsFromSchedule(trip);
 
-    // Leg 0: allowance min(5, 40) = 5 → 60 + 15 + 5 = 80; the origin's budget never participates.
-    assertEquals(Duration.ofMinutes(80), limits[0]);
-    // Leg 1 only delays the destination: allowance 40 → 60 + 15 + 40 = 115.
-    assertEquals(Duration.ofMinutes(115), limits[1]);
+    // Leg 0: allowance min(5, 40) = 5; the origin's budget never participates.
+    assertEquals(expectedLimit(Duration.ofMinutes(60), Duration.ofMinutes(5)), limits[0]);
+    // Leg 1 only delays the destination: allowance 40.
+    assertEquals(expectedLimit(Duration.ofMinutes(60), Duration.ofMinutes(40)), limits[1]);
   }
 
   @Test
@@ -96,7 +117,7 @@ void sizesShortLegsWellBelowNearbyStopRadius() {
       destinationStop(BASE.plusMinutes(10), Duration.ofMinutes(2))
     );
 
-    var limits = DefaultCarpoolingService.driverLegTreeLimits(trip);
+    var limits = limitsFromSchedule(trip);
 
     var leg = expectedLimit(Duration.ofMinutes(5), Duration.ofMinutes(2));
     assertEquals(leg, limits[0]);
@@ -116,7 +137,7 @@ void usesAimedArrivalWhenExpectedMissing() {
       destinationStop(BASE.plusMinutes(90), BUDGET)
     );
 
-    var limits = DefaultCarpoolingService.driverLegTreeLimits(trip);
+    var limits = limitsFromSchedule(trip);
 
     assertEquals(expectedLimit(Duration.ofMinutes(50), BUDGET), limits[0]);
     assertEquals(expectedLimit(Duration.ofMinutes(40), BUDGET), limits[1]);
@@ -133,7 +154,7 @@ void destinationLatestArrivalDoesNotInflateLastLeg() {
       destinationStopWithTimes(BASE.plusMinutes(120), BASE.plusMinutes(160), BUDGET)
     );
 
-    var limits = DefaultCarpoolingService.driverLegTreeLimits(trip);
+    var limits = limitsFromSchedule(trip);
 
     // Last leg is the scheduled 60 min (12:00+60 → +120), not the 100 min the latest expected
     // arrival (+160) would give.
@@ -148,62 +169,90 @@ void twoWaypointTripSizesTheSingleLegToTheWholeSpan() {
       destinationStop(BASE.plusMinutes(60), BUDGET)
     );
 
-    var limits = DefaultCarpoolingService.driverLegTreeLimits(trip);
+    var limits = limitsFromSchedule(trip);
 
     var legLimit = expectedLimit(Duration.ofMinutes(60), BUDGET);
     assertEquals(1, limits.length);
     assertEquals(legLimit, limits[0]);
   }
 
   @Test
-  void fallsBackToWholeTripWhenIntermediateTimeMissing() {
+  void capsEachLegLimitAtMaxTripDuration() {
+    // An inconsistent trip whose leg travel time already exceeds the trip cap (e.g. one that
+    // slipped past the mapper) must not size a tree beyond MAX_TRIP_DURATION; otherwise a single
+    // request could expand a multi-hour street tree.
+    var trip = trip(
+      BASE.plusMinutes(200),
+      originStop(BUDGET),
+      destinationStop(BASE.plusMinutes(200), BUDGET)
+    );
+
+    var limits = DefaultCarpoolingService.driverLegTreeLimits(
+      trip,
+      new Duration[] { Duration.ofHours(4) }
+    );
+
+    assertEquals(CarpoolSiriMapper.MAX_TRIP_DURATION, limits[0]);
+  }
+
+  @Test
+  void doesNotCapLegLimitsThatStayWithinMaxTripDuration() {
+    // A leg comfortably under the cap keeps its full duration + slack + allowance — the cap only
+    // bites the extreme case above.
+    var trip = trip(
+      BASE.plusMinutes(60),
+      originStop(BUDGET),
+      destinationStop(BASE.plusMinutes(60), BUDGET)
+    );
+
+    var limits = DefaultCarpoolingService.driverLegTreeLimits(
+      trip,
+      new Duration[] { Duration.ofMinutes(30) }
+    );
+
+    assertEquals(expectedLimit(Duration.ofMinutes(30), BUDGET), limits[0]);
+    assertTrue(limits[0].compareTo(CarpoolSiriMapper.MAX_TRIP_DURATION) < 0);
+  }
+
+  @Test
+  void scheduledLegDurationsIsNullWhenIntermediateTimeMissing() {
     var trip = trip(
       BASE.plusMinutes(120),
       originStop(BUDGET),
       intermediateStop(null, BUDGET),
       destinationStop(BASE.plusMinutes(120), BUDGET)
     );
 
-    var limits = DefaultCarpoolingService.driverLegTreeLimits(trip);
-
-    var wholeTrip = expectedLimit(Duration.ofMinutes(120), BUDGET);
-    assertEquals(wholeTrip, limits[0]);
-    assertEquals(wholeTrip, limits[1]);
+    // No scheduled durations → the caller sizes every leg from the whole-trip span instead.
+    assertNull(DefaultCarpoolingService.scheduledLegDurations(trip));
   }
 
   @Test
-  void fallsBackToWholeTripWhenDestinationTimeMissing() {
-    // The destination is not special-cased: its missing arrival falls back like any other stop's.
+  void scheduledLegDurationsIsNullWhenDestinationTimeMissing() {
+    // The destination is not special-cased: its missing arrival yields no scheduled durations,
+    // like any other stop's.
     var trip = trip(
       BASE.plusMinutes(120),
       originStop(BUDGET),
       intermediateStop(BASE.plusMinutes(60), BUDGET),
       destinationStop(null, BUDGET)
     );
 
-    var limits = DefaultCarpoolingService.driverLegTreeLimits(trip);
-
-    var wholeTrip = expectedLimit(Duration.ofMinutes(120), BUDGET);
-    assertEquals(wholeTrip, limits[0]);
-    assertEquals(wholeTrip, limits[1]);
+    assertNull(DefaultCarpoolingService.scheduledLegDurations(trip));
   }
 
   @Test
-  void fallsBackToWholeTripWhenTimelineNotMonotonic() {
+  void scheduledLegDurationsIsNullWhenTimelineNotMonotonic() {
     // Intermediate arrival is before the trip start — an impossible timeline that must not yield a
-    // negative leg; the whole-trip span sizes every leg instead.
+    // negative leg; the caller sizes every leg from the whole-trip span instead.
     var trip = trip(
       BASE.plusMinutes(120),
       originStop(BUDGET),
       intermediateStop(BASE.minusMinutes(10), BUDGET),
       destinationStop(BASE.plusMinutes(120), BUDGET)
     );
 
-    var limits = DefaultCarpoolingService.driverLegTreeLimits(trip);
-
-    var wholeTrip = expectedLimit(Duration.ofMinutes(120), BUDGET);
-    assertEquals(wholeTrip, limits[0]);
-    assertEquals(wholeTrip, limits[1]);
+    assertNull(DefaultCarpoolingService.scheduledLegDurations(trip));
   }
 
   private static CarpoolTrip trip(ZonedDateTime endTime, CarpoolStop... stops) {

application/src/ext/java/org/opentripplanner/ext/carpooling/CarpoolingRepository.java

@@ -3,6 +3,7 @@
 import java.time.Duration;
 import java.time.Instant;
 import java.util.Collection;
+import javax.annotation.Nullable;
 import org.opentripplanner.core.model.id.FeedScopedId;
 import org.opentripplanner.ext.carpooling.model.CarpoolTrip;
 
@@ -71,4 +72,26 @@ public interface CarpoolingRepository {
    * @return the number of trips removed, or {@code 0} when the call was throttled
    */
   int removeExpiredTrips(Instant now, Duration expiry);
+
+  /**
+   * Returns OTP's routed travel duration for each leg of the trip with the given id, or
+   * {@code null} if none are cached.
+   * <p>
+   * These durations are a routing memoization, not part of the trip's domain data: they depend
+   * only on the trip's waypoint geometry and the static street graph, never on the passenger
+   * request, so they are computed once and reused across requests. The repository keeps them
+   * because it owns the trip lifecycle that decides when they go stale — they are dropped when the
+   * trip's route points change on {@link #upsertCarpoolTrip} and when the trip is removed or
+   * expires. The returned array has one entry per leg, i.e. {@code stops().size() - 1} entries,
+   * and is a copy the caller may not mutate into the cache.
+   */
+  @Nullable
+  Duration[] cachedBaselineLegDurations(FeedScopedId tripId);
+
+  /**
+   * Stores OTP's routed travel duration for each leg of the trip with the given id, replacing any
+   * previously cached value (see {@link #cachedBaselineLegDurations}). The array is copied, so
+   * later mutation by the caller does not affect the cache.
+   */
+  void cacheBaselineLegDurations(FeedScopedId tripId, Duration[] legDurations);
 }