Skip to content

Commit 3948e00

Browse files
vpaturetvpaturet
committed
Add module test for cancelled stop across patterns
1 parent 51ffea1 commit 3948e00

3 files changed

Lines changed: 116 additions & 0 deletions

File tree

‎application/src/main/java/org/opentripplanner/transit/model/network/TripPattern.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -418,6 +418,10 @@ public Timetable getScheduledTimetable() {
418418
* SiriTripPatternCache.
419419
*
420420
* FIXME RT_AB: Revise comments to make it clear how this is used (it is only used rarely).
421+
*
422+
* TODO RT_VP: this method is unreliable due to a design flaw in the current implementation of
423+
* {@link org.opentripplanner.updater.trip.patterncache.TripPatternCache}
424+
* See comment in TripPatternCache.
421425
*/
422426
@Nullable
423427
public TripPattern getOriginalTripPattern() {

‎application/src/main/java/org/opentripplanner/updater/trip/patterncache/TripPatternCache.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,15 @@
1919
* be part of the main model - not a separate cache. It is possible that this class works when it comes to
2020
* the thread-safety, but just by looking at a few lines of code I see problems - a strategy needs to be
2121
* analysed, designed and documented.
22+
* TODO RT_VP TripPatternCache caches RT patterns keyed by StopPattern only, setting
23+
* originalTripPattern from the first trip that created the entry in the cache.
24+
* When a second trip on a different route produces the same modified StopPattern,
25+
* the cache returns a pattern with potentially the wrong originalTripPattern.
26+
* If the updater uses originalTripPattern to identify the original scheduled pattern
27+
* of a modified trip, it will return the wrong result
28+
* (symptom: when looking up for TripTimes in the wrong pattern's timetable,
29+
* it will get null → TRIP_NOT_FOUND_IN_PATTERN).
30+
*
2231
*/
2332
public class TripPatternCache {
2433

‎application/src/test/java/org/opentripplanner/updater/trip/siri/moduletests/cancellation/CancelledStopCrossPatternTest.java‎

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
package org.opentripplanner.updater.trip.siri.moduletests.cancellation;
2+
3+
import static org.opentripplanner.updater.spi.UpdateResultAssertions.assertSuccess;
4+
5+
import org.junit.jupiter.api.Test;
6+
import org.opentripplanner.transit.model._data.TransitTestEnvironment;
7+
import org.opentripplanner.transit.model._data.TransitTestEnvironmentBuilder;
8+
import org.opentripplanner.transit.model._data.TripInput;
9+
import org.opentripplanner.transit.model.site.RegularStop;
10+
import org.opentripplanner.updater.trip.RealtimeTestConstants;
11+
import org.opentripplanner.updater.trip.SiriTestHelper;
12+
13+
/**
14+
* Test that cancelling the same stop on two trips from different routes (but with the same stop
15+
* pattern) does not cause cross-contamination via TripPatternCache.
16+
* <p>
17+
* TripPatternCache caches RT patterns keyed by StopPattern only, setting
18+
* originalTripPattern from the first trip. When a second trip on a different route produces the
19+
* same modified StopPattern, the cache returns a pattern with the wrong originalTripPattern.
20+
* If the updater uses originalTripPattern to identify the original scheduled pattern of a
21+
* modified trip, it will return the wrong result (symptom: when looking up for TripTimes
22+
* in the wrong pattern's timetable, it will get null → TRIP_NOT_FOUND_IN_PATTERN).
23+
* TODO RT_VP this design flaw is documented also in TripPatternCache
24+
* and org.opentripplanner.transit.model.network.TripPattern#getOriginalTripPattern()
25+
*/
26+
class CancelledStopCrossPatternTest implements RealtimeTestConstants {
27+
28+
private final TransitTestEnvironmentBuilder ENV_BUILDER = TransitTestEnvironment.of();
29+
private final RegularStop STOP_A = ENV_BUILDER.stop(STOP_A_ID);
30+
private final RegularStop STOP_B = ENV_BUILDER.stop(STOP_B_ID);
31+
private final RegularStop STOP_D = ENV_BUILDER.stop(STOP_D_ID);
32+
33+
private final TripInput TRIP_1_INPUT = TripInput.of(TRIP_1_ID)
34+
.withRoute(ENV_BUILDER.route("Route1"))
35+
.withWithTripOnServiceDate(TRIP_1_ID)
36+
.addStop(STOP_A, "0:01:00", "0:01:01")
37+
.addStop(STOP_B, "0:01:10", "0:01:11")
38+
.addStop(STOP_D, "0:01:20", "0:01:21");
39+
40+
private final TripInput TRIP_2_INPUT = TripInput.of(TRIP_2_ID)
41+
.withRoute(ENV_BUILDER.route("Route2"))
42+
.withWithTripOnServiceDate(TRIP_2_ID)
43+
.addStop(STOP_A, "0:02:00", "0:02:01")
44+
.addStop(STOP_B, "0:02:10", "0:02:11")
45+
.addStop(STOP_D, "0:02:20", "0:02:21");
46+
47+
@Test
48+
void cancelledStopOnTwoTripsFromDifferentRoutes() {
49+
var env = ENV_BUILDER.addTrip(TRIP_1_INPUT).addTrip(TRIP_2_INPUT).build();
50+
var siri = SiriTestHelper.of(env);
51+
52+
// Batch 1: Cancel stop B on TRIP_1 — creates TripPatternCache entry
53+
var batch1 = siri
54+
.etBuilder()
55+
.withDatedVehicleJourneyRef(TRIP_1_ID)
56+
.withEstimatedCalls(builder ->
57+
builder
58+
.call(STOP_A)
59+
.departAimedExpected("00:01:01", "00:01:01")
60+
.call(STOP_B)
61+
.withIsCancellation(true)
62+
.call(STOP_D)
63+
.arriveAimedExpected("00:01:20", "00:01:20")
64+
)
65+
.buildEstimatedTimetableDeliveries();
66+
assertSuccess(siri.applyEstimatedTimetable(batch1));
67+
68+
// Batch 2: Cancel stop B on TRIP_2 — reuses cached RT pattern (wrong originalTripPattern)
69+
var batch2 = siri
70+
.etBuilder()
71+
.withDatedVehicleJourneyRef(TRIP_2_ID)
72+
.withEstimatedCalls(builder ->
73+
builder
74+
.call(STOP_A)
75+
.departAimedExpected("00:02:01", "00:02:01")
76+
.call(STOP_B)
77+
.withIsCancellation(true)
78+
.call(STOP_D)
79+
.arriveAimedExpected("00:02:20", "00:02:20")
80+
)
81+
.buildEstimatedTimetableDeliveries();
82+
assertSuccess(siri.applyEstimatedTimetable(batch2));
83+
84+
// Batch 3: Cancel stop B on TRIP_2 again (differential update)
85+
// if matching with originalTripPattern, this fails with TRIP_NOT_FOUND_IN_PATTERN because
86+
// findPattern(trip2, serviceDate) returns the contaminated RT pattern
87+
// whose originalTripPattern belongs to TRIP_1's route.
88+
var batch3 = siri
89+
.etBuilder()
90+
.withDatedVehicleJourneyRef(TRIP_2_ID)
91+
.withEstimatedCalls(builder ->
92+
builder
93+
.call(STOP_A)
94+
.departAimedExpected("00:02:01", "00:02:01")
95+
.call(STOP_B)
96+
.withIsCancellation(true)
97+
.call(STOP_D)
98+
.arriveAimedExpected("00:02:20", "00:02:20")
99+
)
100+
.buildEstimatedTimetableDeliveries();
101+
assertSuccess(siri.applyEstimatedTimetable(batch3));
102+
}
103+
}

0 commit comments

Comments
 (0)