Skip to content

Commit 8eae4aa

Browse files
vpaturetvpaturet
authored
Merge pull request opentripplanner#7330 from entur/add_non_regression_module_tests
Add non regression module tests for SIRI
2 parents 5015873 + 8d29cac commit 8eae4aa

13 files changed

Lines changed: 710 additions & 12 deletions

File tree

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -418,7 +418,12 @@ 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
*/
426+
@Deprecated
422427
@Nullable
423428
public TripPattern getOriginalTripPattern() {
424429
return originalTripPattern;

‎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/transit/model/_data/TimetableRepositoryTestBuilder.java‎

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
import java.util.stream.Collectors;
1414
import org.opentripplanner.core.model.id.FeedScopedId;
1515
import org.opentripplanner.ext.flex.trip.UnscheduledTrip;
16-
import org.opentripplanner.model.PickDrop;
16+
import org.opentripplanner.model.StopTime;
1717
import org.opentripplanner.model.calendar.CalendarServiceData;
1818
import org.opentripplanner.transit.model.basic.TransitMode;
1919
import org.opentripplanner.transit.model.network.Route;
@@ -25,7 +25,6 @@
2525
import org.opentripplanner.transit.model.organization.Operator;
2626
import org.opentripplanner.transit.model.organization.OperatorBuilder;
2727
import org.opentripplanner.transit.model.site.RegularStop;
28-
import org.opentripplanner.transit.model.site.StopLocation;
2928
import org.opentripplanner.transit.model.timetable.Trip;
3029
import org.opentripplanner.transit.model.timetable.TripBuilder;
3130
import org.opentripplanner.transit.model.timetable.TripOnServiceDate;
@@ -184,16 +183,17 @@ public Trip trip(TripInput tripInput, Consumer<TripBuilder> customizer) {
184183
.withHeadsign(tripInput.headsign())
185184
.withServiceId(serviceId)
186185
.withMode(tripInput.mode())
187-
.withNetexSubmode(tripInput.netexSubmode());
186+
.withNetexSubmode(tripInput.netexSubmode())
187+
.withNetexInternalPlanningCode(tripInput.netexInternalPlanningCode());
188188
if (customizer != null) {
189189
customizer.accept(tripBuilder);
190190
}
191191
var trip = tripBuilder.build();
192192

193-
var stopPattern = stopPattern(tripInput.stopLocations());
193+
var stopTimes = tripInput.stopTimes(trip);
194+
var stopPattern = stopPattern(stopTimes);
194195
var tripPattern = getOrCreateTripPattern(stopPattern, route);
195196

196-
var stopTimes = tripInput.stopTimes(trip);
197197
var tripTimes = TripTimesFactory.tripTimes(trip, stopTimes, null);
198198

199199
addTripTimesToPattern(tripPattern, tripTimes);
@@ -271,12 +271,13 @@ private void addTripOnServiceDate(Trip trip, LocalDate serviceDate, String id) {
271271
tripOnServiceDates.add(tripOnServiceDate);
272272
}
273273

274-
private static StopPattern stopPattern(List<StopLocation> stops) {
275-
var builder = StopPattern.create(stops.size());
276-
for (int i = 0; i < stops.size(); i++) {
277-
builder.stops.with(i, stops.get(i));
278-
builder.pickups.with(i, PickDrop.SCHEDULED);
279-
builder.dropoffs.with(i, PickDrop.SCHEDULED);
274+
private static StopPattern stopPattern(List<StopTime> stopTimes) {
275+
var builder = StopPattern.create(stopTimes.size());
276+
for (int i = 0; i < stopTimes.size(); i++) {
277+
var st = stopTimes.get(i);
278+
builder.stops.with(i, st.getStop());
279+
builder.pickups.with(i, st.getPickupType());
280+
builder.dropoffs.with(i, st.getDropOffType());
280281
}
281282
return builder.build();
282283
}

‎application/src/test/java/org/opentripplanner/transit/model/_data/TripInput.java‎

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import javax.annotation.Nullable;
99
import org.opentripplanner.core.model.i18n.I18NString;
1010
import org.opentripplanner.core.model.i18n.NonLocalizedString;
11+
import org.opentripplanner.model.PickDrop;
1112
import org.opentripplanner.model.StopTime;
1213
import org.opentripplanner.transit.model.basic.TransitMode;
1314
import org.opentripplanner.transit.model.network.Route;
@@ -47,6 +48,9 @@ public class TripInput {
4748
@Nullable
4849
private String netexSubmode;
4950

51+
@Nullable
52+
private String netexInternalPlanningCode;
53+
5054
private final boolean isFlex;
5155

5256
private TripInput(String id, boolean isFlex) {
@@ -106,6 +110,11 @@ public String netexSubmode() {
106110
return netexSubmode;
107111
}
108112

113+
@Nullable
114+
public String netexInternalPlanningCode() {
115+
return netexInternalPlanningCode;
116+
}
117+
109118
public boolean isFlex() {
110119
return isFlex;
111120
}
@@ -114,6 +123,26 @@ public TripInput addStop(RegularStop stop, String arrivalTime, String departureT
114123
return addStopWithHeadsign(stop, arrivalTime, departureTime, null);
115124
}
116125

126+
public TripInput addStop(
127+
RegularStop stop,
128+
String arrivalTime,
129+
String departureTime,
130+
PickDrop pickup,
131+
PickDrop dropoff
132+
) {
133+
stops.add(
134+
new RegularStopCallInput(
135+
stop,
136+
TimeUtils.time(arrivalTime),
137+
TimeUtils.time(departureTime),
138+
null,
139+
pickup,
140+
dropoff
141+
)
142+
);
143+
return this;
144+
}
145+
117146
public TripInput addStop(RegularStop stopId, String arrivalAndDeparture) {
118147
return addStop(stopId, arrivalAndDeparture, arrivalAndDeparture);
119148
}
@@ -172,6 +201,11 @@ public TripInput withNetexSubmode(String netexSubmode) {
172201
return this;
173202
}
174203

204+
public TripInput withNetexInternalPlanningCode(String netexInternalPlanningCode) {
205+
this.netexInternalPlanningCode = netexInternalPlanningCode;
206+
return this;
207+
}
208+
175209
private interface StopCallInput {
176210
StopTime toStopTime(Trip trip, int stopSequence);
177211
StopLocation stopLocation();
@@ -181,8 +215,19 @@ private record RegularStopCallInput(
181215
RegularStop stop,
182216
int arrivalTime,
183217
int departureTime,
184-
@Nullable String headsign
218+
@Nullable String headsign,
219+
@Nullable PickDrop pickupType,
220+
@Nullable PickDrop dropoffType
185221
) implements StopCallInput {
222+
RegularStopCallInput(
223+
RegularStop stop,
224+
int arrivalTime,
225+
int departureTime,
226+
@Nullable String headsign
227+
) {
228+
this(stop, arrivalTime, departureTime, headsign, null, null);
229+
}
230+
186231
public StopTime toStopTime(Trip trip, int stopSequence) {
187232
var st = new StopTime();
188233
st.setTrip(trip);
@@ -193,6 +238,12 @@ public StopTime toStopTime(Trip trip, int stopSequence) {
193238
if (headsign != null) {
194239
st.setStopHeadsign(new NonLocalizedString(headsign));
195240
}
241+
if (pickupType != null) {
242+
st.setPickupType(pickupType);
243+
}
244+
if (dropoffType != null) {
245+
st.setDropOffType(dropoffType);
246+
}
196247
return st;
197248
}
198249

‎application/src/test/java/org/opentripplanner/updater/trip/siri/SiriEtBuilder.java‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,11 @@
1010
import org.opentripplanner.LocalTimeParser;
1111
import org.opentripplanner.transit.model.site.RegularStop;
1212
import org.opentripplanner.transit.model.site.StopLocation;
13+
import uk.org.siri.siri21.ArrivalBoardingActivityEnumeration;
1314
import uk.org.siri.siri21.CallStatusEnumeration;
1415
import uk.org.siri.siri21.DataFrameRefStructure;
1516
import uk.org.siri.siri21.DatedVehicleJourneyRef;
17+
import uk.org.siri.siri21.DepartureBoardingActivityEnumeration;
1618
import uk.org.siri.siri21.EstimatedCall;
1719
import uk.org.siri.siri21.EstimatedTimetableDeliveryStructure;
1820
import uk.org.siri.siri21.EstimatedVehicleJourney;
@@ -28,6 +30,7 @@
2830
import uk.org.siri.siri21.StopPointRefStructure;
2931
import uk.org.siri.siri21.VehicleJourneyRef;
3032
import uk.org.siri.siri21.VehicleModesEnumeration;
33+
import uk.org.siri.siri21.VehicleRef;
3134

3235
/**
3336
* This is a helper class for constucting Siri ET messages to use in tests.
@@ -131,6 +134,13 @@ public SiriEtBuilder withPublishedLineName(String lineName) {
131134
return this;
132135
}
133136

137+
public SiriEtBuilder withVehicleRef(String vehicleRef) {
138+
var ref = new VehicleRef();
139+
ref.setValue(vehicleRef);
140+
evj.setVehicleRef(ref);
141+
return this;
142+
}
143+
134144
public SiriEtBuilder withVehicleMode(VehicleModesEnumeration mode) {
135145
evj.getVehicleModes().add(mode);
136146
return this;
@@ -402,6 +412,73 @@ public EstimatedCallsBuilder withArrivalStatus(CallStatusEnumeration callStatus)
402412
return this;
403413
}
404414

415+
public EstimatedCallsBuilder withAimedArrivalTime(String aimedTime) {
416+
var call = calls.getLast();
417+
call.setAimedArrivalTime(localTimeParser.zonedDateTime(aimedTime));
418+
return this;
419+
}
420+
421+
public EstimatedCallsBuilder withExpectedArrivalTime(String expectedTime) {
422+
var call = calls.getLast();
423+
call.setExpectedArrivalTime(localTimeParser.zonedDateTime(expectedTime));
424+
return this;
425+
}
426+
427+
public EstimatedCallsBuilder withAimedDepartureTime(String aimedTime) {
428+
var call = calls.getLast();
429+
call.setAimedDepartureTime(localTimeParser.zonedDateTime(aimedTime));
430+
return this;
431+
}
432+
433+
public EstimatedCallsBuilder withExpectedDepartureTime(String expectedTime) {
434+
var call = calls.getLast();
435+
call.setExpectedDepartureTime(localTimeParser.zonedDateTime(expectedTime));
436+
return this;
437+
}
438+
439+
public EstimatedCallsBuilder withCancellation(boolean cancel) {
440+
var call = calls.getLast();
441+
call.setCancellation(cancel);
442+
return this;
443+
}
444+
445+
public EstimatedCallsBuilder withPredictionInaccurate(boolean inaccurate) {
446+
var call = calls.getLast();
447+
call.setPredictionInaccurate(inaccurate);
448+
return this;
449+
}
450+
451+
public EstimatedCallsBuilder withArrivalBoardingActivity(
452+
ArrivalBoardingActivityEnumeration activity
453+
) {
454+
calls.getLast().setArrivalBoardingActivity(activity);
455+
return this;
456+
}
457+
458+
public EstimatedCallsBuilder withDepartureBoardingActivity(
459+
DepartureBoardingActivityEnumeration activity
460+
) {
461+
calls.getLast().setDepartureBoardingActivity(activity);
462+
return this;
463+
}
464+
465+
public EstimatedCallsBuilder withDestinationDisplay(String destinationDisplay) {
466+
var dd = new NaturalLanguageStringStructure();
467+
dd.setValue(destinationDisplay);
468+
calls.getLast().getDestinationDisplaies().add(dd);
469+
return this;
470+
}
471+
472+
public EstimatedCallsBuilder withOccupancy(uk.org.siri.siri21.OccupancyEnumeration occupancy) {
473+
var call = calls.getLast();
474+
call.setOccupancy(occupancy);
475+
return this;
476+
}
477+
478+
public EstimatedCallsBuilder next() {
479+
return this;
480+
}
481+
405482
public List<EstimatedCall> build() {
406483
return calls;
407484
}

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

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
package org.opentripplanner.updater.trip.siri.moduletests.cancellation;
2+
3+
import static org.junit.jupiter.api.Assertions.assertEquals;
4+
import static org.opentripplanner.updater.spi.UpdateResultAssertions.assertSuccess;
5+
6+
import org.junit.jupiter.api.Test;
7+
import org.opentripplanner.transit.model._data.TransitTestEnvironment;
8+
import org.opentripplanner.transit.model._data.TransitTestEnvironmentBuilder;
9+
import org.opentripplanner.transit.model._data.TripInput;
10+
import org.opentripplanner.transit.model.site.RegularStop;
11+
import org.opentripplanner.updater.trip.RealtimeTestConstants;
12+
import org.opentripplanner.updater.trip.SiriTestHelper;
13+
14+
class CancelledRecordedCallTest implements RealtimeTestConstants {
15+
16+
private final TransitTestEnvironmentBuilder ENV_BUILDER = TransitTestEnvironment.of();
17+
private final RegularStop STOP_A = ENV_BUILDER.stop(STOP_A_ID);
18+
private final RegularStop STOP_B = ENV_BUILDER.stop(STOP_B_ID);
19+
20+
private final TripInput TRIP_INPUT = TripInput.of(TRIP_1_ID)
21+
.withWithTripOnServiceDate(TRIP_1_ID)
22+
.addStop(STOP_A, "0:01:00", "0:01:01")
23+
.addStop(STOP_B, "0:01:10", "0:01:11");
24+
25+
/**
26+
* A RecordedCall with both cancellation=true and an actual departure time should have both status
27+
* [C,R]
28+
*/
29+
@Test
30+
void cancelledRecordedCall() {
31+
var env = ENV_BUILDER.addTrip(TRIP_INPUT).build();
32+
var siri = SiriTestHelper.of(env);
33+
34+
var updates = siri
35+
.etBuilder()
36+
.withDatedVehicleJourneyRef(TRIP_1_ID)
37+
.withRecordedCalls(builder ->
38+
builder.call(STOP_A).withIsCancellation(true).departAimedActual("00:01:01", "00:01:01")
39+
)
40+
.withEstimatedCalls(builder ->
41+
builder.call(STOP_B).arriveAimedExpected("00:01:10", "00:01:10")
42+
)
43+
.buildEstimatedTimetableDeliveries();
44+
45+
var result = siri.applyEstimatedTimetable(updates);
46+
47+
assertSuccess(result);
48+
assertEquals(
49+
"MODIFIED | A [C,R] 0:01:01 0:01:01 | B 0:01:10 0:01:10",
50+
env.tripData(TRIP_1_ID).showTimetable()
51+
);
52+
}
53+
}

0 commit comments

Comments
 (0)