Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add validation for missing calls in SIRI update #5403

Merged
merged 2 commits into from
Oct 16, 2023
Merged

Add validation for missing calls in SIRI update #5403

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8b8f020
Add validation for missing calls in SIRI update
vpaturet Oct 6, 2023
5b8fa6b
Add validation for unknown stop in SIRI update
vpaturet Oct 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 90 additions & 6 deletions src/ext-test/java/org/opentripplanner/ext/siri/AddedTripBuilderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
import org.opentripplanner.updater.spi.UpdateError;
import uk.org.siri.siri20.VehicleModesEnumeration;

public class AddedTripBuilderTest {
class AddedTripBuilderTest {

private static final Agency AGENCY = TransitModelForTest.AGENCY;
private static final ZoneId TIME_ZONE = AGENCY.getTimezone();
Expand Down Expand Up @@ -105,7 +105,7 @@ void setUp() {
}

@Test
public void testAddedTrip() {
void testAddedTrip() {
var addedTrip = new AddedTripBuilder(
TRANSIT_MODEL,
ENTITY_RESOLVER,
Expand Down Expand Up @@ -370,7 +370,7 @@ void testAddedTripWithoutReplacedRoute() {
}

@Test
public void testAddedTripFailOnMissingServiceId() {
void testAddedTripFailOnMissingServiceId() {
var addedTrip = new AddedTripBuilder(
TRANSIT_MODEL,
ENTITY_RESOLVER,
Expand All @@ -382,7 +382,7 @@ public void testAddedTripFailOnMissingServiceId() {
null,
TRANSIT_MODE,
SUB_MODE,
List.of(),
getCalls(0),
false,
null,
false,
Expand All @@ -400,7 +400,7 @@ public void testAddedTripFailOnMissingServiceId() {
}

@Test
public void testAddedTripFailOnNonIncreasingDwellTime() {
void testAddedTripFailOnNonIncreasingDwellTime() {
List<CallWrapper> calls = List.of(
TestCall
.of()
Expand Down Expand Up @@ -453,6 +453,90 @@ public void testAddedTripFailOnNonIncreasingDwellTime() {
);
}

@Test
void testAddedTripFailOnTooFewCalls() {
List<CallWrapper> calls = List.of(
TestCall
.of()
.withStopPointRef(STOP_A.getId().getId())
.withAimedDepartureTime(zonedDateTime(10, 20))
.withExpectedDepartureTime(zonedDateTime(10, 20))
.build()
);
var addedTrip = new AddedTripBuilder(
TRANSIT_MODEL,
ENTITY_RESOLVER,
AbstractTransitEntity::getId,
TRIP_ID,
OPERATOR,
LINE_REF,
REPLACED_ROUTE,
SERVICE_DATE,
TRANSIT_MODE,
SUB_MODE,
calls,
false,
null,
false,
SHORT_NAME,
HEADSIGN
)
.build();

assertTrue(addedTrip.isFailure(), "Trip creation should fail");
assertEquals(
UpdateError.UpdateErrorType.TOO_FEW_STOPS,
addedTrip.failureValue().errorType(),
"Trip creation should fail with too few calls"
);
}

@Test
void testAddedTripFailOnUnknownStop() {
List<CallWrapper> calls = List.of(
TestCall
.of()
.withStopPointRef("UNKNOWN_STOP_REF")
.withAimedDepartureTime(zonedDateTime(10, 20))
.withExpectedDepartureTime(zonedDateTime(10, 20))
.build(),
TestCall
.of()
.withStopPointRef(STOP_B.getId().getId())
.withAimedArrivalTime(zonedDateTime(10, 30))
.withExpectedArrivalTime(zonedDateTime(10, 31))
.withAimedDepartureTime(zonedDateTime(10, 30))
.withExpectedDepartureTime(zonedDateTime(10, 29))
.build()
);
var addedTrip = new AddedTripBuilder(
TRANSIT_MODEL,
ENTITY_RESOLVER,
AbstractTransitEntity::getId,
TRIP_ID,
OPERATOR,
LINE_REF,
REPLACED_ROUTE,
SERVICE_DATE,
TRANSIT_MODE,
SUB_MODE,
calls,
false,
null,
false,
SHORT_NAME,
HEADSIGN
)
.build();

assertTrue(addedTrip.isFailure(), "Trip creation should fail");
assertEquals(
UpdateError.UpdateErrorType.NO_VALID_STOPS,
addedTrip.failureValue().errorType(),
"Trip creation should fail with call referring to unknown stop"
);
}

@ParameterizedTest
@CsvSource(
{
Expand All @@ -462,7 +546,7 @@ public void testAddedTripFailOnNonIncreasingDwellTime() {
"ferry,FERRY,RAIL,",
}
)
public void testGetTransportMode(
void testGetTransportMode(
String siriMode,
String internalMode,
String replacedRouteMode,
Expand Down
22 changes: 21 additions & 1 deletion src/ext/java/org/opentripplanner/ext/siri/AddedTripBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
import static java.lang.Boolean.TRUE;
import static org.opentripplanner.ext.siri.mapper.SiriTransportModeMapper.mapTransitMainMode;
import static org.opentripplanner.updater.spi.UpdateError.UpdateErrorType.NO_START_DATE;
import static org.opentripplanner.updater.spi.UpdateError.UpdateErrorType.NO_VALID_STOPS;
import static org.opentripplanner.updater.spi.UpdateError.UpdateErrorType.TOO_FEW_STOPS;

import java.time.LocalDate;
import java.time.ZoneId;
Expand All @@ -23,6 +25,7 @@
import org.opentripplanner.transit.model.network.TripPattern;
import org.opentripplanner.transit.model.organization.Agency;
import org.opentripplanner.transit.model.organization.Operator;
import org.opentripplanner.transit.model.site.RegularStop;
import org.opentripplanner.transit.model.timetable.RealTimeState;
import org.opentripplanner.transit.model.timetable.Trip;
import org.opentripplanner.transit.model.timetable.TripTimes;
Expand Down Expand Up @@ -144,6 +147,10 @@ class AddedTripBuilder {
}

Result<TripUpdate, UpdateError> build() {
if (calls.size() < 2) {
return UpdateError.result(tripId, TOO_FEW_STOPS);
}

if (serviceDate == null) {
return UpdateError.result(tripId, NO_START_DATE);
}
Expand Down Expand Up @@ -176,6 +183,11 @@ Result<TripUpdate, UpdateError> build() {
stopSequence == (calls.size() - 1)
);

// Drop this update if the call refers to an unknown stop (not present in the stop model).
if (stopTime == null) {
return UpdateError.result(tripId, NO_VALID_STOPS);
}

aimedStopTimes.add(stopTime);
}

Expand Down Expand Up @@ -279,6 +291,9 @@ private Trip createTrip(Route route, FeedScopedId calServiceId) {
return tripBuilder.build();
}

/**
* Map the call to a StopTime or return null if the stop cannot be found in the stop model.
*/
private StopTime createStopTime(
Trip trip,
ZonedDateTime departureDate,
Expand All @@ -287,10 +302,15 @@ private StopTime createStopTime(
boolean isFirstStop,
boolean isLastStop
) {
RegularStop stop = entityResolver.resolveQuay(call.getStopPointRef());
if (stop == null) {
return null;
}

StopTime stopTime = new StopTime();
stopTime.setStopSequence(stopSequence);
stopTime.setTrip(trip);
stopTime.setStop(entityResolver.resolveQuay(call.getStopPointRef()));
stopTime.setStop(stop);

// Fallback to other time, if one doesn't exist
var aimedArrivalTime = call.getAimedArrivalTime() != null
Expand Down