Skip to content

Commit e4c3d32

Browse files
committed
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.
1 parent 72629d7 commit e4c3d32

File tree

5 files changed

+171
-13
lines changed

5 files changed

+171
-13
lines changed

application/src/ext-test/java/org/opentripplanner/ext/flex/template/ClosestTripTest.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import static org.opentripplanner.transit.model._data.TimetableRepositoryForTest.id;
77

88
import gnu.trove.set.hash.TIntHashSet;
9+
import java.time.Instant;
910
import java.time.LocalDate;
1011
import java.util.Collection;
1112
import java.util.List;
@@ -34,10 +35,11 @@ class ClosestTripTest {
3435
.build();
3536

3637
private static final LocalDate DATE = LocalDate.of(2025, 2, 28);
37-
private static final FlexServiceDate FSD = new FlexServiceDate(
38+
private static final FlexServiceDate FSD = FlexServiceDate.of(
3839
DATE,
3940
ServiceDateUtils.secondsSinceStartOfTime(DATE.atStartOfDay(ZoneIds.BERLIN), DATE),
40-
10,
41+
Instant.ofEpochSecond(10),
42+
ZoneIds.BERLIN,
4143
new TIntHashSet()
4244
);
4345
private static final StopLocation STOP = FLEX_TRIP.getStop(0);
Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
package org.opentripplanner.ext.flex.template;
2+
3+
import static org.junit.jupiter.api.Assertions.assertEquals;
4+
5+
import java.time.Instant;
6+
import java.time.LocalDate;
7+
import java.time.LocalTime;
8+
import java.time.ZoneId;
9+
import java.time.ZonedDateTime;
10+
import org.junit.jupiter.api.Test;
11+
import org.opentripplanner.transit.model.timetable.booking.RoutingBookingInfo;
12+
13+
/**
14+
* Tests for {@link FlexServiceDate#requestedBookingTime()} to verify that booking time
15+
* is correctly calculated relative to each service date's start-of-service.
16+
*/
17+
class FlexServiceDateBookingTimeTest {
18+
19+
private static final ZoneId ZONE = ZoneId.of("Europe/Oslo");
20+
21+
@Test
22+
void testRequestedBookingTimeSameDay() {
23+
// Booking at 14:00 on Jan 13, service date is Jan 13
24+
LocalDate serviceDate = LocalDate.of(2026, 1, 13);
25+
Instant bookingTime = ZonedDateTime.of(serviceDate, LocalTime.of(14, 0), ZONE).toInstant();
26+
27+
FlexServiceDate flexDate = FlexServiceDate.of(serviceDate, 0, bookingTime, ZONE, null);
28+
29+
// Expected: 14:00 = 14 * 3600 = 50400 seconds from start of service
30+
int expected = 14 * 3600;
31+
assertEquals(expected, flexDate.requestedBookingTime());
32+
}
33+
34+
@Test
35+
void testRequestedBookingTimePreviousDay() {
36+
// Booking at 14:40 on Jan 12, service date is Jan 13
37+
// This is the bug scenario: booking time should be negative relative to Jan 13
38+
LocalDate bookingDate = LocalDate.of(2026, 1, 12);
39+
LocalDate serviceDate = LocalDate.of(2026, 1, 13);
40+
41+
Instant bookingTime = ZonedDateTime.of(bookingDate, LocalTime.of(14, 40), ZONE).toInstant();
42+
43+
FlexServiceDate flexDate = FlexServiceDate.of(serviceDate, 0, bookingTime, ZONE, null);
44+
45+
// Booking is at 14:40 on Jan 12
46+
// Service date start is midnight Jan 13 (NOON - 12h)
47+
// 14:40 on Jan 12 is 9 hours 20 minutes before midnight Jan 13
48+
// = -(9*3600 + 20*60) = -33600 seconds
49+
int expected = -(9 * 3600 + 20 * 60);
50+
assertEquals(expected, flexDate.requestedBookingTime());
51+
}
52+
53+
@Test
54+
void testRequestedBookingTimeMultipleDaysAhead() {
55+
// Booking at 10:00 on Jan 10, service date is Jan 13
56+
LocalDate bookingDate = LocalDate.of(2026, 1, 10);
57+
LocalDate serviceDate = LocalDate.of(2026, 1, 13);
58+
59+
Instant bookingTime = ZonedDateTime.of(bookingDate, LocalTime.of(10, 0), ZONE).toInstant();
60+
61+
FlexServiceDate flexDate = FlexServiceDate.of(serviceDate, 0, bookingTime, ZONE, null);
62+
63+
// 10:00 on Jan 10 is 2 days + 14 hours before midnight Jan 13
64+
// = -(2*24 + 14) hours = -62 hours = -62 * 3600 seconds = -223200
65+
int expected = -((2 * 24 + 14) * 3600);
66+
assertEquals(expected, flexDate.requestedBookingTime());
67+
}
68+
69+
@Test
70+
void testRequestedBookingTimeNull() {
71+
LocalDate serviceDate = LocalDate.of(2026, 1, 13);
72+
73+
FlexServiceDate flexDate = FlexServiceDate.of(serviceDate, 0, null, ZONE, null);
74+
75+
assertEquals(RoutingBookingInfo.NOT_SET, flexDate.requestedBookingTime());
76+
}
77+
78+
@Test
79+
void testBookingTimeOnDifferentDatesProducesDifferentResults() {
80+
// Same booking instant should produce different requestedBookingTime values
81+
// for different service dates - this is the core fix
82+
Instant bookingTime = ZonedDateTime.of(
83+
LocalDate.of(2026, 1, 12),
84+
LocalTime.of(14, 40),
85+
ZONE
86+
).toInstant();
87+
88+
FlexServiceDate jan12 = FlexServiceDate.of(
89+
LocalDate.of(2026, 1, 12),
90+
0,
91+
bookingTime,
92+
ZONE,
93+
null
94+
);
95+
96+
FlexServiceDate jan13 = FlexServiceDate.of(
97+
LocalDate.of(2026, 1, 13),
98+
0,
99+
bookingTime,
100+
ZONE,
101+
null
102+
);
103+
104+
// On Jan 12: booking at 14:40 = 14*3600 + 40*60 = 52800 seconds
105+
assertEquals(14 * 3600 + 40 * 60, jan12.requestedBookingTime());
106+
107+
// On Jan 13: booking at 14:40 on Jan 12 = -9*3600 - 20*60 = -33600 seconds
108+
assertEquals(-(9 * 3600 + 20 * 60), jan13.requestedBookingTime());
109+
110+
// They must be different!
111+
assertEquals(
112+
86400,
113+
jan12.requestedBookingTime() - jan13.requestedBookingTime(),
114+
"Booking time difference should equal one day in seconds"
115+
);
116+
}
117+
}

application/src/ext-test/java/org/opentripplanner/ext/flex/template/FlexTemplateFactoryTest.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import java.time.Duration;
1313
import java.time.LocalDate;
1414
import java.time.Month;
15+
import java.time.ZoneId;
1516
import java.util.Arrays;
1617
import java.util.List;
1718
import java.util.Set;
@@ -34,7 +35,6 @@
3435
import org.opentripplanner.transit.model.site.RegularStop;
3536
import org.opentripplanner.transit.model.site.StopLocation;
3637
import org.opentripplanner.transit.model.timetable.Trip;
37-
import org.opentripplanner.transit.model.timetable.booking.RoutingBookingInfo;
3838

3939
class FlexTemplateFactoryTest {
4040

@@ -59,10 +59,11 @@ class FlexTemplateFactoryTest {
5959
/**
6060
* The date is pass-through information in this test, so one date is enough.
6161
*/
62-
private static final FlexServiceDate DATE = new FlexServiceDate(
62+
private static final FlexServiceDate DATE = FlexServiceDate.of(
6363
LocalDate.of(2024, Month.MAY, 17),
6464
SERVICE_TIME_OFFSET,
65-
RoutingBookingInfo.NOT_SET,
65+
null,
66+
ZoneId.of("Europe/Oslo"),
6667
new TIntHashSet()
6768
);
6869

application/src/ext/java/org/opentripplanner/ext/flex/FlexRouter.java

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@
3636
import org.opentripplanner.transit.model.filter.transit.TripMatcherFactory;
3737
import org.opentripplanner.transit.model.site.StopLocation;
3838
import org.opentripplanner.transit.model.timetable.Trip;
39-
import org.opentripplanner.transit.model.timetable.booking.RoutingBookingInfo;
4039
import org.opentripplanner.transit.service.TransitService;
4140
import org.opentripplanner.utils.time.ServiceDateUtils;
4241

@@ -59,7 +58,8 @@ public class FlexRouter {
5958
/* Request data */
6059
private final ZonedDateTime startOfTime;
6160
private final int requestedTime;
62-
private final int requestedBookingTime;
61+
private final Instant requestedBookingTimeInstant;
62+
private final ZoneId timeZone;
6363
private final List<FlexServiceDate> dates;
6464
private final Matcher<Trip> matcher;
6565

@@ -117,9 +117,8 @@ public FlexRouter(
117117
LocalDate searchDate = LocalDate.ofInstant(requestedTime, tz);
118118
this.startOfTime = ServiceDateUtils.asStartOfService(searchDate, tz);
119119
this.requestedTime = ServiceDateUtils.secondsSinceStartOfTime(startOfTime, requestedTime);
120-
this.requestedBookingTime = requestedBookingTime == null
121-
? RoutingBookingInfo.NOT_SET
122-
: ServiceDateUtils.secondsSinceStartOfTime(startOfTime, requestedBookingTime);
120+
this.requestedBookingTimeInstant = requestedBookingTime;
121+
this.timeZone = tz;
123122
this.dates = createFlexServiceDates(
124123
transitService,
125124
additionalPastSearchDays,
@@ -187,10 +186,11 @@ private List<FlexServiceDate> createFlexServiceDates(
187186
for (int d = -additionalPastSearchDays; d <= additionalFutureSearchDays; ++d) {
188187
LocalDate date = searchDate.plusDays(d);
189188
dates.add(
190-
new FlexServiceDate(
189+
FlexServiceDate.of(
191190
date,
192191
ServiceDateUtils.secondsSinceStartOfTime(startOfTime, date),
193-
requestedBookingTime,
192+
requestedBookingTimeInstant,
193+
timeZone,
194194
transitService.getServiceCodesRunningForDate(date)
195195
)
196196
);

application/src/ext/java/org/opentripplanner/ext/flex/template/FlexServiceDate.java

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,13 @@
11
package org.opentripplanner.ext.flex.template;
22

33
import gnu.trove.set.TIntSet;
4+
import java.time.Instant;
45
import java.time.LocalDate;
6+
import java.time.ZoneId;
7+
import java.time.ZonedDateTime;
8+
import javax.annotation.Nullable;
9+
import org.opentripplanner.transit.model.timetable.booking.RoutingBookingInfo;
10+
import org.opentripplanner.utils.time.ServiceDateUtils;
511

612
/**
713
* This class contains information used in a flex router, and depends on the date the search was
@@ -21,9 +27,13 @@ public class FlexServiceDate {
2127
/** Which services are running on the date. */
2228
private final TIntSet servicesRunning;
2329

30+
/**
31+
* The requested booking time as seconds since the start of service for this date.
32+
* Calculated relative to this specific service date's start-of-service.
33+
*/
2434
private final int requestedBookingTime;
2535

26-
public FlexServiceDate(
36+
private FlexServiceDate(
2737
LocalDate serviceDate,
2838
int secondsFromStartOfTime,
2939
int requestedBookingTime,
@@ -35,6 +45,31 @@ public FlexServiceDate(
3545
this.servicesRunning = servicesRunning;
3646
}
3747

48+
public static FlexServiceDate of(
49+
LocalDate serviceDate,
50+
int secondsFromStartOfTime,
51+
@Nullable Instant requestedBookingTimeInstant,
52+
ZoneId timeZone,
53+
TIntSet servicesRunning
54+
) {
55+
int requestedBookingTime;
56+
if (requestedBookingTimeInstant == null) {
57+
requestedBookingTime = RoutingBookingInfo.NOT_SET;
58+
} else {
59+
ZonedDateTime startOfService = ServiceDateUtils.asStartOfService(serviceDate, timeZone);
60+
requestedBookingTime = ServiceDateUtils.secondsSinceStartOfTime(
61+
startOfService,
62+
requestedBookingTimeInstant
63+
);
64+
}
65+
return new FlexServiceDate(
66+
serviceDate,
67+
secondsFromStartOfTime,
68+
requestedBookingTime,
69+
servicesRunning
70+
);
71+
}
72+
3873
LocalDate serviceDate() {
3974
return serviceDate;
4075
}
@@ -43,6 +78,9 @@ int secondsFromStartOfTime() {
4378
return secondsFromStartOfTime;
4479
}
4580

81+
/**
82+
* Get the requested booking time as seconds since the start of service for this date.
83+
*/
4684
int requestedBookingTime() {
4785
return requestedBookingTime;
4886
}

0 commit comments

Comments
 (0)