From e4c3d325cdbd5761350319e85b9870cb4f358238 Mon Sep 17 00:00:00 2001 From: Vincent Paturet Date: Mon, 2 Feb 2026 05:31:50 +0100 Subject: [PATCH] Fix booking time calculation for multi-day flex trip searches When a bookingTime parameter is provided in flex trip queries, the requestedBookingTime was calculated once relative to the search date's start-of-service, then reused for all FlexServiceDate objects across multiple days. This caused valid trips to be incorrectly filtered out when the booking time and trip departure were on different days. Changed FlexServiceDate to store the original Instant and ZoneId, then calculate seconds-since-start-of-service on-demand relative to each date's own start-of-service. This ensures booking time comparisons use the correct reference point for each service date. --- .../ext/flex/template/ClosestTripTest.java | 6 +- .../FlexServiceDateBookingTimeTest.java | 117 ++++++++++++++++++ .../template/FlexTemplateFactoryTest.java | 7 +- .../opentripplanner/ext/flex/FlexRouter.java | 14 +-- .../ext/flex/template/FlexServiceDate.java | 40 +++++- 5 files changed, 171 insertions(+), 13 deletions(-) create mode 100644 application/src/ext-test/java/org/opentripplanner/ext/flex/template/FlexServiceDateBookingTimeTest.java diff --git a/application/src/ext-test/java/org/opentripplanner/ext/flex/template/ClosestTripTest.java b/application/src/ext-test/java/org/opentripplanner/ext/flex/template/ClosestTripTest.java index d9885a7844f..6d8f7f94230 100644 --- a/application/src/ext-test/java/org/opentripplanner/ext/flex/template/ClosestTripTest.java +++ b/application/src/ext-test/java/org/opentripplanner/ext/flex/template/ClosestTripTest.java @@ -6,6 +6,7 @@ import static org.opentripplanner.transit.model._data.TimetableRepositoryForTest.id; import gnu.trove.set.hash.TIntHashSet; +import java.time.Instant; import java.time.LocalDate; import java.util.Collection; import java.util.List; @@ -34,10 +35,11 @@ class ClosestTripTest { .build(); private static final LocalDate DATE = LocalDate.of(2025, 2, 28); - private static final FlexServiceDate FSD = new FlexServiceDate( + private static final FlexServiceDate FSD = FlexServiceDate.of( DATE, ServiceDateUtils.secondsSinceStartOfTime(DATE.atStartOfDay(ZoneIds.BERLIN), DATE), - 10, + Instant.ofEpochSecond(10), + ZoneIds.BERLIN, new TIntHashSet() ); private static final StopLocation STOP = FLEX_TRIP.getStop(0); diff --git a/application/src/ext-test/java/org/opentripplanner/ext/flex/template/FlexServiceDateBookingTimeTest.java b/application/src/ext-test/java/org/opentripplanner/ext/flex/template/FlexServiceDateBookingTimeTest.java new file mode 100644 index 00000000000..a6a7e184ddd --- /dev/null +++ b/application/src/ext-test/java/org/opentripplanner/ext/flex/template/FlexServiceDateBookingTimeTest.java @@ -0,0 +1,117 @@ +package org.opentripplanner.ext.flex.template; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.time.Instant; +import java.time.LocalDate; +import java.time.LocalTime; +import java.time.ZoneId; +import java.time.ZonedDateTime; +import org.junit.jupiter.api.Test; +import org.opentripplanner.transit.model.timetable.booking.RoutingBookingInfo; + +/** + * Tests for {@link FlexServiceDate#requestedBookingTime()} to verify that booking time + * is correctly calculated relative to each service date's start-of-service. + */ +class FlexServiceDateBookingTimeTest { + + private static final ZoneId ZONE = ZoneId.of("Europe/Oslo"); + + @Test + void testRequestedBookingTimeSameDay() { + // Booking at 14:00 on Jan 13, service date is Jan 13 + LocalDate serviceDate = LocalDate.of(2026, 1, 13); + Instant bookingTime = ZonedDateTime.of(serviceDate, LocalTime.of(14, 0), ZONE).toInstant(); + + FlexServiceDate flexDate = FlexServiceDate.of(serviceDate, 0, bookingTime, ZONE, null); + + // Expected: 14:00 = 14 * 3600 = 50400 seconds from start of service + int expected = 14 * 3600; + assertEquals(expected, flexDate.requestedBookingTime()); + } + + @Test + void testRequestedBookingTimePreviousDay() { + // Booking at 14:40 on Jan 12, service date is Jan 13 + // This is the bug scenario: booking time should be negative relative to Jan 13 + LocalDate bookingDate = LocalDate.of(2026, 1, 12); + LocalDate serviceDate = LocalDate.of(2026, 1, 13); + + Instant bookingTime = ZonedDateTime.of(bookingDate, LocalTime.of(14, 40), ZONE).toInstant(); + + FlexServiceDate flexDate = FlexServiceDate.of(serviceDate, 0, bookingTime, ZONE, null); + + // Booking is at 14:40 on Jan 12 + // Service date start is midnight Jan 13 (NOON - 12h) + // 14:40 on Jan 12 is 9 hours 20 minutes before midnight Jan 13 + // = -(9*3600 + 20*60) = -33600 seconds + int expected = -(9 * 3600 + 20 * 60); + assertEquals(expected, flexDate.requestedBookingTime()); + } + + @Test + void testRequestedBookingTimeMultipleDaysAhead() { + // Booking at 10:00 on Jan 10, service date is Jan 13 + LocalDate bookingDate = LocalDate.of(2026, 1, 10); + LocalDate serviceDate = LocalDate.of(2026, 1, 13); + + Instant bookingTime = ZonedDateTime.of(bookingDate, LocalTime.of(10, 0), ZONE).toInstant(); + + FlexServiceDate flexDate = FlexServiceDate.of(serviceDate, 0, bookingTime, ZONE, null); + + // 10:00 on Jan 10 is 2 days + 14 hours before midnight Jan 13 + // = -(2*24 + 14) hours = -62 hours = -62 * 3600 seconds = -223200 + int expected = -((2 * 24 + 14) * 3600); + assertEquals(expected, flexDate.requestedBookingTime()); + } + + @Test + void testRequestedBookingTimeNull() { + LocalDate serviceDate = LocalDate.of(2026, 1, 13); + + FlexServiceDate flexDate = FlexServiceDate.of(serviceDate, 0, null, ZONE, null); + + assertEquals(RoutingBookingInfo.NOT_SET, flexDate.requestedBookingTime()); + } + + @Test + void testBookingTimeOnDifferentDatesProducesDifferentResults() { + // Same booking instant should produce different requestedBookingTime values + // for different service dates - this is the core fix + Instant bookingTime = ZonedDateTime.of( + LocalDate.of(2026, 1, 12), + LocalTime.of(14, 40), + ZONE + ).toInstant(); + + FlexServiceDate jan12 = FlexServiceDate.of( + LocalDate.of(2026, 1, 12), + 0, + bookingTime, + ZONE, + null + ); + + FlexServiceDate jan13 = FlexServiceDate.of( + LocalDate.of(2026, 1, 13), + 0, + bookingTime, + ZONE, + null + ); + + // On Jan 12: booking at 14:40 = 14*3600 + 40*60 = 52800 seconds + assertEquals(14 * 3600 + 40 * 60, jan12.requestedBookingTime()); + + // On Jan 13: booking at 14:40 on Jan 12 = -9*3600 - 20*60 = -33600 seconds + assertEquals(-(9 * 3600 + 20 * 60), jan13.requestedBookingTime()); + + // They must be different! + assertEquals( + 86400, + jan12.requestedBookingTime() - jan13.requestedBookingTime(), + "Booking time difference should equal one day in seconds" + ); + } +} diff --git a/application/src/ext-test/java/org/opentripplanner/ext/flex/template/FlexTemplateFactoryTest.java b/application/src/ext-test/java/org/opentripplanner/ext/flex/template/FlexTemplateFactoryTest.java index 43563ce8bce..2ee3a765fda 100644 --- a/application/src/ext-test/java/org/opentripplanner/ext/flex/template/FlexTemplateFactoryTest.java +++ b/application/src/ext-test/java/org/opentripplanner/ext/flex/template/FlexTemplateFactoryTest.java @@ -12,6 +12,7 @@ import java.time.Duration; import java.time.LocalDate; import java.time.Month; +import java.time.ZoneId; import java.util.Arrays; import java.util.List; import java.util.Set; @@ -34,7 +35,6 @@ import org.opentripplanner.transit.model.site.RegularStop; import org.opentripplanner.transit.model.site.StopLocation; import org.opentripplanner.transit.model.timetable.Trip; -import org.opentripplanner.transit.model.timetable.booking.RoutingBookingInfo; class FlexTemplateFactoryTest { @@ -59,10 +59,11 @@ class FlexTemplateFactoryTest { /** * The date is pass-through information in this test, so one date is enough. */ - private static final FlexServiceDate DATE = new FlexServiceDate( + private static final FlexServiceDate DATE = FlexServiceDate.of( LocalDate.of(2024, Month.MAY, 17), SERVICE_TIME_OFFSET, - RoutingBookingInfo.NOT_SET, + null, + ZoneId.of("Europe/Oslo"), new TIntHashSet() ); diff --git a/application/src/ext/java/org/opentripplanner/ext/flex/FlexRouter.java b/application/src/ext/java/org/opentripplanner/ext/flex/FlexRouter.java index df00e855143..0488bdf861e 100644 --- a/application/src/ext/java/org/opentripplanner/ext/flex/FlexRouter.java +++ b/application/src/ext/java/org/opentripplanner/ext/flex/FlexRouter.java @@ -36,7 +36,6 @@ import org.opentripplanner.transit.model.filter.transit.TripMatcherFactory; import org.opentripplanner.transit.model.site.StopLocation; import org.opentripplanner.transit.model.timetable.Trip; -import org.opentripplanner.transit.model.timetable.booking.RoutingBookingInfo; import org.opentripplanner.transit.service.TransitService; import org.opentripplanner.utils.time.ServiceDateUtils; @@ -59,7 +58,8 @@ public class FlexRouter { /* Request data */ private final ZonedDateTime startOfTime; private final int requestedTime; - private final int requestedBookingTime; + private final Instant requestedBookingTimeInstant; + private final ZoneId timeZone; private final List dates; private final Matcher matcher; @@ -117,9 +117,8 @@ public FlexRouter( LocalDate searchDate = LocalDate.ofInstant(requestedTime, tz); this.startOfTime = ServiceDateUtils.asStartOfService(searchDate, tz); this.requestedTime = ServiceDateUtils.secondsSinceStartOfTime(startOfTime, requestedTime); - this.requestedBookingTime = requestedBookingTime == null - ? RoutingBookingInfo.NOT_SET - : ServiceDateUtils.secondsSinceStartOfTime(startOfTime, requestedBookingTime); + this.requestedBookingTimeInstant = requestedBookingTime; + this.timeZone = tz; this.dates = createFlexServiceDates( transitService, additionalPastSearchDays, @@ -187,10 +186,11 @@ private List createFlexServiceDates( for (int d = -additionalPastSearchDays; d <= additionalFutureSearchDays; ++d) { LocalDate date = searchDate.plusDays(d); dates.add( - new FlexServiceDate( + FlexServiceDate.of( date, ServiceDateUtils.secondsSinceStartOfTime(startOfTime, date), - requestedBookingTime, + requestedBookingTimeInstant, + timeZone, transitService.getServiceCodesRunningForDate(date) ) ); diff --git a/application/src/ext/java/org/opentripplanner/ext/flex/template/FlexServiceDate.java b/application/src/ext/java/org/opentripplanner/ext/flex/template/FlexServiceDate.java index ee85cf77b03..69c5946d827 100644 --- a/application/src/ext/java/org/opentripplanner/ext/flex/template/FlexServiceDate.java +++ b/application/src/ext/java/org/opentripplanner/ext/flex/template/FlexServiceDate.java @@ -1,7 +1,13 @@ package org.opentripplanner.ext.flex.template; import gnu.trove.set.TIntSet; +import java.time.Instant; import java.time.LocalDate; +import java.time.ZoneId; +import java.time.ZonedDateTime; +import javax.annotation.Nullable; +import org.opentripplanner.transit.model.timetable.booking.RoutingBookingInfo; +import org.opentripplanner.utils.time.ServiceDateUtils; /** * This class contains information used in a flex router, and depends on the date the search was @@ -21,9 +27,13 @@ public class FlexServiceDate { /** Which services are running on the date. */ private final TIntSet servicesRunning; + /** + * The requested booking time as seconds since the start of service for this date. + * Calculated relative to this specific service date's start-of-service. + */ private final int requestedBookingTime; - public FlexServiceDate( + private FlexServiceDate( LocalDate serviceDate, int secondsFromStartOfTime, int requestedBookingTime, @@ -35,6 +45,31 @@ public FlexServiceDate( this.servicesRunning = servicesRunning; } + public static FlexServiceDate of( + LocalDate serviceDate, + int secondsFromStartOfTime, + @Nullable Instant requestedBookingTimeInstant, + ZoneId timeZone, + TIntSet servicesRunning + ) { + int requestedBookingTime; + if (requestedBookingTimeInstant == null) { + requestedBookingTime = RoutingBookingInfo.NOT_SET; + } else { + ZonedDateTime startOfService = ServiceDateUtils.asStartOfService(serviceDate, timeZone); + requestedBookingTime = ServiceDateUtils.secondsSinceStartOfTime( + startOfService, + requestedBookingTimeInstant + ); + } + return new FlexServiceDate( + serviceDate, + secondsFromStartOfTime, + requestedBookingTime, + servicesRunning + ); + } + LocalDate serviceDate() { return serviceDate; } @@ -43,6 +78,9 @@ int secondsFromStartOfTime() { return secondsFromStartOfTime; } + /** + * Get the requested booking time as seconds since the start of service for this date. + */ int requestedBookingTime() { return requestedBookingTime; }