From e77791e884f60351d7e8501065d2b0e66f87a0df Mon Sep 17 00:00:00 2001 From: Vincent Paturet Date: Fri, 24 May 2024 08:23:24 +0200 Subject: [PATCH] Fix handling of missing aimed departure time --- .../ext/siri/SiriEtBuilder.java | 52 ++++++- .../siri/SiriTimetableSnapshotSourceTest.java | 147 +++++++++++++----- .../ext/siri/SiriFuzzyTripMatcher.java | 25 ++- 3 files changed, 170 insertions(+), 54 deletions(-) diff --git a/src/ext-test/java/org/opentripplanner/ext/siri/SiriEtBuilder.java b/src/ext-test/java/org/opentripplanner/ext/siri/SiriEtBuilder.java index 11f9d137642..a82283f0b48 100644 --- a/src/ext-test/java/org/opentripplanner/ext/siri/SiriEtBuilder.java +++ b/src/ext-test/java/org/opentripplanner/ext/siri/SiriEtBuilder.java @@ -1,6 +1,8 @@ package org.opentripplanner.ext.siri; import java.math.BigInteger; +import java.time.LocalDate; +import java.time.format.DateTimeFormatter; import java.util.ArrayList; import java.util.List; import java.util.function.Function; @@ -8,11 +10,13 @@ import org.opentripplanner.DateTimeHelper; import org.opentripplanner.transit.model.site.RegularStop; import org.opentripplanner.transit.model.site.StopLocation; +import uk.org.siri.siri20.DataFrameRefStructure; import uk.org.siri.siri20.DatedVehicleJourneyRef; import uk.org.siri.siri20.EstimatedCall; import uk.org.siri.siri20.EstimatedTimetableDeliveryStructure; import uk.org.siri.siri20.EstimatedVehicleJourney; import uk.org.siri.siri20.EstimatedVersionFrameStructure; +import uk.org.siri.siri20.FramedVehicleJourneyRefStructure; import uk.org.siri.siri20.LineRef; import uk.org.siri.siri20.OperatorRefStructure; import uk.org.siri.siri20.QuayRefStructure; @@ -129,6 +133,40 @@ public SiriEtBuilder withVehicleJourneyRef(String id) { return this; } + public SiriEtBuilder withFramedVehicleJourneyRef( + Function producer + ) { + var builder = new FramedVehicleRefBuilder(); + builder = producer.apply(builder); + evj.setFramedVehicleJourneyRef(builder.build()); + return this; + } + + public static class FramedVehicleRefBuilder { + + private LocalDate serviceDate; + private String vehicleJourneyRef; + + public SiriEtBuilder.FramedVehicleRefBuilder withServiceDate(LocalDate serviceDate) { + this.serviceDate = serviceDate; + return this; + } + + public SiriEtBuilder.FramedVehicleRefBuilder withVehicleJourneyRef(String vehicleJourneyRef) { + this.vehicleJourneyRef = vehicleJourneyRef; + return this; + } + + public FramedVehicleJourneyRefStructure build() { + DataFrameRefStructure dataFrameRefStructure = new DataFrameRefStructure(); + dataFrameRefStructure.setValue(DateTimeFormatter.ISO_LOCAL_DATE.format(serviceDate)); + FramedVehicleJourneyRefStructure framedVehicleJourneyRefStructure = new FramedVehicleJourneyRefStructure(); + framedVehicleJourneyRefStructure.setDataFrameRef(dataFrameRefStructure); + framedVehicleJourneyRefStructure.setDatedVehicleJourneyRef(vehicleJourneyRef); + return framedVehicleJourneyRefStructure; + } + } + public static class RecordedCallsBuilder { private final ArrayList calls; @@ -210,15 +248,21 @@ public EstimatedCallsBuilder call(StopLocation stop) { public EstimatedCallsBuilder arriveAimedExpected(String aimedTime, String expectedTime) { var call = calls.getLast(); - call.setAimedArrivalTime(dateTimeHelper.zonedDateTime(aimedTime)); - call.setExpectedArrivalTime(dateTimeHelper.zonedDateTime(expectedTime)); + call.setAimedArrivalTime(aimedTime == null ? null : dateTimeHelper.zonedDateTime(aimedTime)); + call.setExpectedArrivalTime( + expectedTime == null ? null : dateTimeHelper.zonedDateTime(expectedTime) + ); return this; } public EstimatedCallsBuilder departAimedExpected(String aimedTime, String expectedTime) { var call = calls.getLast(); - call.setAimedDepartureTime(dateTimeHelper.zonedDateTime(aimedTime)); - call.setExpectedDepartureTime(dateTimeHelper.zonedDateTime(expectedTime)); + call.setAimedDepartureTime( + aimedTime == null ? null : dateTimeHelper.zonedDateTime(aimedTime) + ); + call.setExpectedDepartureTime( + expectedTime == null ? null : dateTimeHelper.zonedDateTime(expectedTime) + ); return this; } diff --git a/src/ext-test/java/org/opentripplanner/ext/siri/SiriTimetableSnapshotSourceTest.java b/src/ext-test/java/org/opentripplanner/ext/siri/SiriTimetableSnapshotSourceTest.java index 19fdfb8636f..feb29cdab58 100644 --- a/src/ext-test/java/org/opentripplanner/ext/siri/SiriTimetableSnapshotSourceTest.java +++ b/src/ext-test/java/org/opentripplanner/ext/siri/SiriTimetableSnapshotSourceTest.java @@ -9,6 +9,7 @@ import java.util.Set; import java.util.stream.Collectors; import java.util.stream.IntStream; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.opentripplanner.DateTimeHelper; @@ -40,10 +41,15 @@ class SiriTimetableSnapshotSourceTest { + private RealtimeTestEnvironment env; + + @BeforeEach + void setUp() { + env = new RealtimeTestEnvironment(); + } + @Test void testCancelTrip() { - var env = new RealtimeTestEnvironment(); - assertEquals(RealTimeState.SCHEDULED, env.getTripTimesForTrip(env.trip1).getRealTimeState()); var updates = new SiriEtBuilder(env.getDateTimeHelper()) @@ -59,8 +65,6 @@ void testCancelTrip() { @Test void testAddJourney() { - var env = new RealtimeTestEnvironment(); - var updates = new SiriEtBuilder(env.getDateTimeHelper()) .withEstimatedVehicleJourneyCode("newJourney") .withIsExtraJourney(true) @@ -81,8 +85,6 @@ void testAddJourney() { @Test void testReplaceJourney() { - var env = new RealtimeTestEnvironment(); - var updates = new SiriEtBuilder(env.getDateTimeHelper()) .withEstimatedVehicleJourneyCode("newJourney") .withIsExtraJourney(true) @@ -111,32 +113,75 @@ void testReplaceJourney() { } /** - * Update calls without changing the pattern + * Update calls without changing the pattern. Match trip by dated vehicle journey. */ @Test - void testUpdateJourney() { - var env = new RealtimeTestEnvironment(); - - var updates = new SiriEtBuilder(env.getDateTimeHelper()) + void testUpdateJourneyWithDatedVehicleJourneyRef() { + var updates = updatedJourneyBuilder(env) .withDatedVehicleJourneyRef(env.trip1.getId().getId()) - .withRecordedCalls(builder -> - builder.call(env.stopA1).departAimedActual("00:00:11", "00:00:15") - ) - .withEstimatedCalls(builder -> - builder.call(env.stopB1).arriveAimedExpected("00:00:20", "00:00:25") + .buildEstimatedTimetableDeliveries(); + var result = env.applyEstimatedTimetable(updates); + assertEquals(1, result.successful()); + assertTripUpdated(env); + } + + /** + * Update calls without changing the pattern. Match trip by framed vehicle journey. + */ + @Test + void testUpdateJourneyWithFramedVehicleJourneyRef() { + var updates = updatedJourneyBuilder(env) + .withFramedVehicleJourneyRef(builder -> + builder.withServiceDate(env.serviceDate).withVehicleJourneyRef(env.trip1.getId().getId()) ) .buildEstimatedTimetableDeliveries(); + var result = env.applyEstimatedTimetable(updates); + assertEquals(1, result.successful()); + assertTripUpdated(env); + } + /** + * Update calls without changing the pattern. Missing reference to vehicle journey. + */ + @Test + void testUpdateJourneyWithoutJourneyRef() { + var updates = updatedJourneyBuilder(env).buildEstimatedTimetableDeliveries(); var result = env.applyEstimatedTimetable(updates); + assertEquals(0, result.successful()); + } + /** + * Update calls without changing the pattern. Fuzzy matching. + */ + @Test + void testUpdateJourneyWithFuzzyMatching() { + var updates = updatedJourneyBuilder(env).buildEstimatedTimetableDeliveries(); + var result = env.applyEstimatedTimetableWithFuzzyMatcher(updates); assertEquals(1, result.successful()); + assertTripUpdated(env); + } - var tripTimes = env.getTripTimesForTrip(env.trip1); - assertEquals(RealTimeState.UPDATED, tripTimes.getRealTimeState()); - assertEquals(11, tripTimes.getScheduledDepartureTime(0)); - assertEquals(15, tripTimes.getDepartureTime(0)); - assertEquals(20, tripTimes.getScheduledArrivalTime(1)); - assertEquals(25, tripTimes.getArrivalTime(1)); + /** + * Update calls without changing the pattern. Fuzzy matching. + * Edge case: invalid reference to vehicle journey and missing aimed departure time. + */ + @Test + void testUpdateJourneyWithFuzzyMatchingAndMissingAimedDepartureTime() { + var updates = new SiriEtBuilder(env.getDateTimeHelper()) + .withFramedVehicleJourneyRef(builder -> + builder.withServiceDate(env.serviceDate).withVehicleJourneyRef("XXX") + ) + .withEstimatedCalls(builder -> + builder + .call(env.stopA1) + .departAimedExpected(null, "00:00:12") + .call(env.stopB1) + .arriveAimedExpected("00:00:20", "00:00:22") + ) + .buildEstimatedTimetableDeliveries(); + + var result = env.applyEstimatedTimetableWithFuzzyMatcher(updates); + assertEquals(0, result.successful(), "Should fail gracefully"); } /** @@ -144,8 +189,6 @@ void testUpdateJourney() { */ @Test void testChangeQuay() { - var env = new RealtimeTestEnvironment(); - var updates = new SiriEtBuilder(env.getDateTimeHelper()) .withDatedVehicleJourneyRef(env.trip1.getId().getId()) .withRecordedCalls(builder -> @@ -172,8 +215,6 @@ void testChangeQuay() { @Test void testCancelStop() { - var env = new RealtimeTestEnvironment(); - var updates = new SiriEtBuilder(env.getDateTimeHelper()) .withDatedVehicleJourneyRef(env.trip2.getId().getId()) .withEstimatedCalls(builder -> @@ -202,8 +243,6 @@ void testCancelStop() { @Test @Disabled("Not supported yet") void testAddStop() { - var env = new RealtimeTestEnvironment(); - var updates = new SiriEtBuilder(env.getDateTimeHelper()) .withDatedVehicleJourneyRef(env.trip1.getId().getId()) .withRecordedCalls(builder -> @@ -248,8 +287,6 @@ void testAddStop() { @Test void testNotMonitored() { - var env = new RealtimeTestEnvironment(); - var updates = new SiriEtBuilder(env.getDateTimeHelper()) .withMonitored(false) .buildEstimatedTimetableDeliveries(); @@ -261,8 +298,6 @@ void testNotMonitored() { @Test void testReplaceJourneyWithoutEstimatedVehicleJourneyCode() { - var env = new RealtimeTestEnvironment(); - var updates = new SiriEtBuilder(env.getDateTimeHelper()) .withDatedVehicleJourneyRef("newJourney") .withIsExtraJourney(true) @@ -286,8 +321,6 @@ void testReplaceJourneyWithoutEstimatedVehicleJourneyCode() { @Test void testNegativeHopTime() { - var env = new RealtimeTestEnvironment(); - var updates = new SiriEtBuilder(env.getDateTimeHelper()) .withDatedVehicleJourneyRef(env.trip1.getId().getId()) .withRecordedCalls(builder -> @@ -306,8 +339,6 @@ void testNegativeHopTime() { @Test void testNegativeDwellTime() { - var env = new RealtimeTestEnvironment(); - var updates = new SiriEtBuilder(env.getDateTimeHelper()) .withDatedVehicleJourneyRef(env.trip2.getId().getId()) .withRecordedCalls(builder -> @@ -331,8 +362,6 @@ void testNegativeDwellTime() { @Test @Disabled("Not supported yet") void testExtraUnknownStop() { - var env = new RealtimeTestEnvironment(); - var updates = new SiriEtBuilder(env.getDateTimeHelper()) .withDatedVehicleJourneyRef(env.trip1.getId().getId()) .withEstimatedCalls(builder -> @@ -357,8 +386,28 @@ private void assertFailure(UpdateResult result, UpdateError.UpdateErrorType erro assertEquals(result.failures().keySet(), Set.of(errorType)); } + private static SiriEtBuilder updatedJourneyBuilder(RealtimeTestEnvironment env) { + return new SiriEtBuilder(env.getDateTimeHelper()) + .withRecordedCalls(builder -> + builder.call(env.stopA1).departAimedActual("00:00:11", "00:00:15") + ) + .withEstimatedCalls(builder -> + builder.call(env.stopB1).arriveAimedExpected("00:00:20", "00:00:25") + ); + } + + private static void assertTripUpdated(RealtimeTestEnvironment env) { + var tripTimes = env.getTripTimesForTrip(env.trip1); + assertEquals(RealTimeState.UPDATED, tripTimes.getRealTimeState()); + assertEquals(11, tripTimes.getScheduledDepartureTime(0)); + assertEquals(15, tripTimes.getDepartureTime(0)); + assertEquals(20, tripTimes.getScheduledArrivalTime(1)); + assertEquals(25, tripTimes.getArrivalTime(1)); + } + private static class RealtimeTestEnvironment { + public static final FeedScopedId SERVICE_ID = TransitModelForTest.id("SERVICE_ID"); private final TransitModelForTest testModel = TransitModelForTest.of(); public final ZoneId timeZone = ZoneId.of(TransitModelForTest.TIME_ZONE_ID); public final Station stationA = testModel.station("A").build(); @@ -411,12 +460,11 @@ public RealtimeTestEnvironment() { ); CalendarServiceData calendarServiceData = new CalendarServiceData(); - var cal_id = TransitModelForTest.id("CAL_1"); calendarServiceData.putServiceDatesForServiceId( - cal_id, + SERVICE_ID, List.of(serviceDate.minusDays(1), serviceDate, serviceDate.plusDays(1)) ); - transitModel.getServiceCodes().put(cal_id, 0); + transitModel.getServiceCodes().put(SERVICE_ID, 0); transitModel.updateCalendarServiceData(true, calendarServiceData, DataImportIssueStore.NOOP); transitModel.index(); @@ -428,7 +476,7 @@ public RealtimeTestEnvironment() { private record Stop(RegularStop stop, int arrivalTime, int departureTime) {} private Trip createTrip(String id, Route route, List stops) { - var trip = Trip.of(id(id)).withRoute(route).build(); + var trip = Trip.of(id(id)).withRoute(route).withServiceId(SERVICE_ID).build(); var tripOnServiceDate = TripOnServiceDate .of(trip.getId()) @@ -534,8 +582,21 @@ public String getFeedId() { } public UpdateResult applyEstimatedTimetable(List updates) { + return applyEstimatedTimetable(updates, null); + } + + public UpdateResult applyEstimatedTimetableWithFuzzyMatcher( + List updates + ) { + return applyEstimatedTimetable(updates, SiriFuzzyTripMatcher.of(getTransitService())); + } + + private UpdateResult applyEstimatedTimetable( + List updates, + SiriFuzzyTripMatcher siriFuzzyTripMatcher + ) { return this.snapshotSource.applyEstimatedTimetable( - null, + siriFuzzyTripMatcher, getEntityResolver(), getFeedId(), false, diff --git a/src/ext/java/org/opentripplanner/ext/siri/SiriFuzzyTripMatcher.java b/src/ext/java/org/opentripplanner/ext/siri/SiriFuzzyTripMatcher.java index 2f2f113fc79..5a2c89c3cb0 100644 --- a/src/ext/java/org/opentripplanner/ext/siri/SiriFuzzyTripMatcher.java +++ b/src/ext/java/org/opentripplanner/ext/siri/SiriFuzzyTripMatcher.java @@ -11,6 +11,7 @@ import java.util.function.BiFunction; import java.util.stream.Collectors; import javax.annotation.Nonnull; +import javax.annotation.Nullable; import org.opentripplanner.framework.time.ServiceDateUtils; import org.opentripplanner.model.Timetable; import org.opentripplanner.model.calendar.CalendarService; @@ -98,6 +99,7 @@ public Trip match( /** * Matches EstimatedVehicleJourney to a set of possible Trips based on tripId */ + @Nullable public TripAndPattern match( EstimatedVehicleJourney journey, EntityResolver entityResolver, @@ -110,6 +112,14 @@ public TripAndPattern match( return null; } + if (calls.getFirst().getAimedDepartureTime() == null) { + LOG.warn( + "Missing aimed departure time on first stop for estimated vehicle journey '{}'", + DebugString.of(journey) + ); + return null; + } + Set trips = null; if ( journey.getVehicleRef() != null && @@ -119,7 +129,7 @@ public TripAndPattern match( } if (trips == null || trips.isEmpty()) { - CallWrapper lastStop = calls.get(calls.size() - 1); + CallWrapper lastStop = calls.getLast(); String lastStopPoint = lastStop.getStopPointRef(); ZonedDateTime arrivalTime = lastStop.getAimedArrivalTime() != null ? lastStop.getAimedArrivalTime() @@ -269,20 +279,21 @@ private Set getCachedTripsByInternalPlanningCode(String internalPlanningCo /** * Finds the correct trip based on OTP-ServiceDate and SIRI-DepartureTime */ - private TripAndPattern getTripAndPatternForJourney( + @Nullable + TripAndPattern getTripAndPatternForJourney( Set trips, List calls, EntityResolver entityResolver, BiFunction getCurrentTimetable, BiFunction getRealtimeAddedTripPattern ) { - var journeyFirstStop = entityResolver.resolveQuay(calls.get(0).getStopPointRef()); - var journeyLastStop = entityResolver.resolveQuay(calls.get(calls.size() - 1).getStopPointRef()); + var journeyFirstStop = entityResolver.resolveQuay(calls.getFirst().getStopPointRef()); + var journeyLastStop = entityResolver.resolveQuay(calls.getLast().getStopPointRef()); if (journeyFirstStop == null || journeyLastStop == null) { return null; } - ZonedDateTime date = calls.get(0).getAimedDepartureTime(); + ZonedDateTime date = calls.getFirst().getAimedDepartureTime(); LocalDate serviceDate = date.toLocalDate(); int departureInSecondsSinceMidnight = ServiceDateUtils.secondsSinceStartOfService( @@ -359,7 +370,7 @@ private Trip getTripForJourney( } if (results.size() == 1) { - return results.get(0); + return results.getFirst(); } else if (results.size() > 1) { // Multiple possible matches - check if lineRef/routeId matches if ( @@ -376,7 +387,7 @@ private Trip getTripForJourney( } // Line does not match any routeId - return first result. - return results.get(0); + return results.getFirst(); } return null;