Skip to content

Commit 0646397

Browse files
vpaturetvpaturet
authored
Merge pull request opentripplanner#7324 from entur/validate-siri-call-order
Reject SIRI messages with missing Order attribute on calls
2 parents 36f8767 + cefbd4b commit 0646397

5 files changed

Lines changed: 269 additions & 6 deletions

File tree

‎application/src/main/java/org/opentripplanner/updater/spi/UpdateError.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@ public enum UpdateErrorType {
4444
NO_FUZZY_TRIP_MATCH,
4545
MULTIPLE_FUZZY_TRIP_MATCHES,
4646
EMPTY_STOP_POINT_REF,
47+
MISSING_CALL_ORDER,
48+
MIXED_CALL_ORDER_AND_VISIT_NUMBER,
4749
NO_TRIP_FOR_CANCELLATION_FOUND,
4850
TRIP_ALREADY_EXISTS,
4951
NO_START_DATE,

‎application/src/main/java/org/opentripplanner/updater/trip/siri/CallWrapper.java‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@
33
import java.time.ZonedDateTime;
44
import java.util.ArrayList;
55
import java.util.List;
6+
import java.util.Optional;
7+
import org.opentripplanner.updater.spi.UpdateError.UpdateErrorType;
8+
import org.opentripplanner.utils.lang.StringUtils;
69
import uk.org.siri.siri21.ArrivalBoardingActivityEnumeration;
710
import uk.org.siri.siri21.CallStatusEnumeration;
811
import uk.org.siri.siri21.DepartureBoardingActivityEnumeration;
@@ -43,7 +46,56 @@ static List<CallWrapper> of(EstimatedVehicleJourney estimatedVehicleJourney) {
4346
return List.copyOf(result);
4447
}
4548

49+
/**
50+
* Validate that all calls have a non-empty stop point ref and either order or visit number (but
51+
* not both). Also checks cross-call consistency: all calls must use the same strategy (all Order
52+
* or all VisitNumber). A mix of Order and VisitNumber-only calls is rejected.
53+
*/
54+
static Optional<UpdateErrorType> validateAll(List<CallWrapper> calls) {
55+
var perCallError = calls
56+
.stream()
57+
.map(CallWrapper::validate)
58+
.flatMap(Optional::stream)
59+
.findFirst();
60+
if (perCallError.isPresent()) {
61+
return perCallError;
62+
}
63+
boolean anyHasOrder = calls.stream().anyMatch(CallWrapper::hasOrder);
64+
boolean anyMissingOrder = calls.stream().anyMatch(c -> !c.hasOrder());
65+
if (anyHasOrder && anyMissingOrder) {
66+
return Optional.of(UpdateErrorType.MIXED_CALL_ORDER_AND_VISIT_NUMBER);
67+
}
68+
return Optional.empty();
69+
}
70+
71+
/**
72+
* Validate that the call has a non-empty stop point ref and either order or visit number (but not
73+
* both).
74+
*/
75+
default Optional<UpdateErrorType> validate() {
76+
if (StringUtils.hasNoValueOrNullAsString(getStopPointRef())) {
77+
return Optional.of(UpdateErrorType.EMPTY_STOP_POINT_REF);
78+
}
79+
if (!hasOrder() && !hasVisitNumber()) {
80+
return Optional.of(UpdateErrorType.MISSING_CALL_ORDER);
81+
}
82+
if (hasOrder() && hasVisitNumber()) {
83+
return Optional.of(UpdateErrorType.MIXED_CALL_ORDER_AND_VISIT_NUMBER);
84+
}
85+
return Optional.empty();
86+
}
87+
4688
String getStopPointRef();
89+
90+
boolean hasOrder();
91+
92+
boolean hasVisitNumber();
93+
94+
/**
95+
* Return the sort order of this call. Prefers Order if present, falls back to VisitNumber.
96+
*/
97+
int getSortOrder();
98+
4799
Boolean isCancellation();
48100
Boolean isPredictionInaccurate();
49101
boolean isExtraCall();
@@ -86,6 +138,29 @@ public String getStopPointRef() {
86138
return call.getStopPointRef() != null ? call.getStopPointRef().getValue() : null;
87139
}
88140

141+
@Override
142+
public boolean hasOrder() {
143+
return call.getOrder() != null;
144+
}
145+
146+
@Override
147+
public boolean hasVisitNumber() {
148+
return call.getVisitNumber() != null;
149+
}
150+
151+
/**
152+
* Return the call order, either from the Order field or the VisitNumber field.
153+
* Validation ensures that one of them is set.
154+
* See {@link #validate()}
155+
* @return
156+
*/
157+
@Override
158+
public int getSortOrder() {
159+
return call.getOrder() != null
160+
? call.getOrder().intValueExact()
161+
: call.getVisitNumber().intValueExact();
162+
}
163+
89164
@Override
90165
public Boolean isCancellation() {
91166
return call.isCancellation();
@@ -193,6 +268,23 @@ public String getStopPointRef() {
193268
return call.getStopPointRef() != null ? call.getStopPointRef().getValue() : null;
194269
}
195270

271+
@Override
272+
public boolean hasOrder() {
273+
return call.getOrder() != null;
274+
}
275+
276+
@Override
277+
public boolean hasVisitNumber() {
278+
return call.getVisitNumber() != null;
279+
}
280+
281+
@Override
282+
public int getSortOrder() {
283+
return call.getOrder() != null
284+
? call.getOrder().intValueExact()
285+
: call.getVisitNumber().intValueExact();
286+
}
287+
196288
@Override
197289
public Boolean isCancellation() {
198290
return call.isCancellation();

‎application/src/main/java/org/opentripplanner/updater/trip/siri/SiriRealTimeTripUpdateAdapter.java‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package org.opentripplanner.updater.trip.siri;
22

33
import static java.lang.Boolean.TRUE;
4-
import static org.opentripplanner.updater.spi.UpdateError.UpdateErrorType.EMPTY_STOP_POINT_REF;
54
import static org.opentripplanner.updater.spi.UpdateError.UpdateErrorType.NOT_MONITORED;
65
import static org.opentripplanner.updater.spi.UpdateError.UpdateErrorType.NO_START_DATE;
76
import static org.opentripplanner.updater.spi.UpdateError.UpdateErrorType.TRIP_NOT_FOUND;
@@ -33,7 +32,6 @@
3332
import org.opentripplanner.updater.trip.UpdateIncrementality;
3433
import org.opentripplanner.updater.trip.patterncache.TripPatternCache;
3534
import org.opentripplanner.updater.trip.patterncache.TripPatternIdGenerator;
36-
import org.opentripplanner.utils.lang.StringUtils;
3735
import org.slf4j.Logger;
3836
import org.slf4j.LoggerFactory;
3937
import uk.org.siri.siri21.EstimatedTimetableDeliveryStructure;
@@ -133,10 +131,9 @@ private Result<UpdateSuccess, UpdateError> apply(
133131
EntityResolver entityResolver
134132
) {
135133
List<CallWrapper> callWrappers = CallWrapper.of(journey);
136-
for (var call : callWrappers) {
137-
if (StringUtils.hasNoValueOrNullAsString(call.getStopPointRef())) {
138-
return UpdateError.result(null, EMPTY_STOP_POINT_REF, journey.getDataSource());
139-
}
134+
var error = CallWrapper.validateAll(callWrappers);
135+
if (error.isPresent()) {
136+
return UpdateError.result(null, error.get(), journey.getDataSource());
140137
}
141138
SiriUpdateType siriUpdateType = null;
142139
try {

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,21 @@ public String getStopPointRef() {
7575
return stopPointRef;
7676
}
7777

78+
@Override
79+
public boolean hasOrder() {
80+
return false;
81+
}
82+
83+
@Override
84+
public boolean hasVisitNumber() {
85+
return false;
86+
}
87+
88+
@Override
89+
public int getSortOrder() {
90+
return 0;
91+
}
92+
7893
@Override
7994
public Boolean isCancellation() {
8095
return cancellation;

‎application/src/test/java/org/opentripplanner/updater/trip/siri/moduletests/rejection/MissingCallOrderTest.java‎

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
package org.opentripplanner.updater.trip.siri.moduletests.rejection;
2+
3+
import static org.opentripplanner.updater.spi.UpdateError.UpdateErrorType.MISSING_CALL_ORDER;
4+
import static org.opentripplanner.updater.spi.UpdateError.UpdateErrorType.MIXED_CALL_ORDER_AND_VISIT_NUMBER;
5+
import static org.opentripplanner.updater.spi.UpdateResultAssertions.assertFailure;
6+
import static org.opentripplanner.updater.spi.UpdateResultAssertions.assertSuccess;
7+
8+
import org.junit.jupiter.api.Test;
9+
import org.opentripplanner.transit.model._data.TransitTestEnvironment;
10+
import org.opentripplanner.transit.model._data.TransitTestEnvironmentBuilder;
11+
import org.opentripplanner.transit.model._data.TripInput;
12+
import org.opentripplanner.transit.model.site.RegularStop;
13+
import org.opentripplanner.updater.trip.RealtimeTestConstants;
14+
import org.opentripplanner.updater.trip.SiriTestHelper;
15+
16+
class MissingCallOrderTest implements RealtimeTestConstants {
17+
18+
private final TransitTestEnvironmentBuilder ENV_BUILDER = TransitTestEnvironment.of();
19+
private final RegularStop STOP_A = ENV_BUILDER.stop(STOP_A_ID);
20+
private final RegularStop STOP_B = ENV_BUILDER.stop(STOP_B_ID);
21+
private final RegularStop STOP_C = ENV_BUILDER.stop(STOP_C_ID);
22+
23+
private final TripInput TRIP_INPUT = TripInput.of(TRIP_1_ID)
24+
.withWithTripOnServiceDate(TRIP_1_ID)
25+
.addStop(STOP_A, "0:00:10", "0:00:11")
26+
.addStop(STOP_B, "0:00:20", "0:00:21")
27+
.addStop(STOP_C, "0:00:40", "0:00:41");
28+
29+
@Test
30+
void rejectMissingOrderOnEstimatedCall() {
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+
.withEstimatedCalls(builder ->
38+
builder
39+
.call(STOP_A)
40+
.departAimedExpected("00:00:11", "00:00:15")
41+
.call(STOP_B)
42+
.departAimedExpected("00:00:21", "00:00:25")
43+
.call(STOP_C)
44+
.arriveAimedExpected("00:00:40", "00:00:45")
45+
.clearOrder()
46+
)
47+
.buildEstimatedTimetableDeliveries();
48+
49+
var result = siri.applyEstimatedTimetable(updates);
50+
assertFailure(MISSING_CALL_ORDER, result);
51+
}
52+
53+
@Test
54+
void rejectMissingOrderOnRecordedCall() {
55+
var env = ENV_BUILDER.addTrip(TRIP_INPUT).build();
56+
var siri = SiriTestHelper.of(env);
57+
58+
var updates = siri
59+
.etBuilder()
60+
.withDatedVehicleJourneyRef(TRIP_1_ID)
61+
.withRecordedCalls(builder ->
62+
builder.call(STOP_A).departAimedActual("00:00:11", "00:00:15").clearOrder()
63+
)
64+
.withEstimatedCalls(builder ->
65+
builder
66+
.call(STOP_B)
67+
.departAimedExpected("00:00:21", "00:00:25")
68+
.call(STOP_C)
69+
.arriveAimedExpected("00:00:40", "00:00:45")
70+
)
71+
.buildEstimatedTimetableDeliveries();
72+
73+
var result = siri.applyEstimatedTimetable(updates);
74+
assertFailure(MISSING_CALL_ORDER, result);
75+
}
76+
77+
@Test
78+
void acceptVisitNumberAsFallback() {
79+
var env = ENV_BUILDER.addTrip(TRIP_INPUT).build();
80+
var siri = SiriTestHelper.of(env);
81+
82+
var updates = siri
83+
.etBuilder()
84+
.withDatedVehicleJourneyRef(TRIP_1_ID)
85+
.withRecordedCalls(builder ->
86+
builder
87+
.call(STOP_A)
88+
.clearOrder()
89+
.withVisitNumber(1)
90+
.departAimedActual("00:00:11", "00:00:15")
91+
)
92+
.withEstimatedCalls(builder ->
93+
builder
94+
.call(STOP_B)
95+
.clearOrder()
96+
.withVisitNumber(2)
97+
.departAimedExpected("00:00:21", "00:00:25")
98+
.call(STOP_C)
99+
.clearOrder()
100+
.withVisitNumber(3)
101+
.arriveAimedExpected("00:00:40", "00:00:45")
102+
)
103+
.buildEstimatedTimetableDeliveries();
104+
105+
var result = siri.applyEstimatedTimetable(updates);
106+
assertSuccess(result);
107+
}
108+
109+
@Test
110+
void rejectMixedOrderAndVisitNumber() {
111+
var env = ENV_BUILDER.addTrip(TRIP_INPUT).build();
112+
var siri = SiriTestHelper.of(env);
113+
114+
var updates = siri
115+
.etBuilder()
116+
.withDatedVehicleJourneyRef(TRIP_1_ID)
117+
.withEstimatedCalls(builder ->
118+
builder
119+
.call(STOP_A)
120+
.withVisitNumber(1)
121+
.departAimedExpected("00:00:11", "00:00:15")
122+
.call(STOP_B)
123+
.departAimedExpected("00:00:21", "00:00:25")
124+
.call(STOP_C)
125+
.arriveAimedExpected("00:00:40", "00:00:45")
126+
)
127+
.buildEstimatedTimetableDeliveries();
128+
129+
var result = siri.applyEstimatedTimetable(updates);
130+
assertFailure(MIXED_CALL_ORDER_AND_VISIT_NUMBER, result);
131+
}
132+
133+
@Test
134+
void rejectInconsistentOrderAndVisitNumberAcrossCalls() {
135+
var env = ENV_BUILDER.addTrip(TRIP_INPUT).build();
136+
var siri = SiriTestHelper.of(env);
137+
138+
var updates = siri
139+
.etBuilder()
140+
.withDatedVehicleJourneyRef(TRIP_1_ID)
141+
.withEstimatedCalls(builder ->
142+
builder
143+
.call(STOP_A)
144+
.departAimedExpected("00:00:11", "00:00:15")
145+
.call(STOP_B)
146+
.departAimedExpected("00:00:21", "00:00:25")
147+
.call(STOP_C)
148+
.clearOrder()
149+
.withVisitNumber(3)
150+
.arriveAimedExpected("00:00:40", "00:00:45")
151+
)
152+
.buildEstimatedTimetableDeliveries();
153+
154+
var result = siri.applyEstimatedTimetable(updates);
155+
assertFailure(MIXED_CALL_ORDER_AND_VISIT_NUMBER, result);
156+
}
157+
}

0 commit comments

Comments
 (0)