diff --git a/application/src/main/java/org/opentripplanner/model/Timetable.java b/application/src/main/java/org/opentripplanner/model/Timetable.java index c437a60b0d5..edba1ff4b69 100644 --- a/application/src/main/java/org/opentripplanner/model/Timetable.java +++ b/application/src/main/java/org/opentripplanner/model/Timetable.java @@ -20,6 +20,7 @@ import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Optional; import javax.annotation.Nullable; import org.opentripplanner.transit.model.framework.DataValidationException; @@ -483,4 +484,16 @@ private static TripTimes getRepresentativeTripTimes( return null; } } + + /** + * Get a copy of the scheduled timetable valid for the specified service date only + */ + public Timetable copyForServiceDate(LocalDate date) { + if (serviceDate != null) { + throw new RuntimeException( + "Can only copy scheduled timetable for a specific date if a date hasn't been specified yet." + ); + } + return copyOf().withServiceDate(date).build(); + } } diff --git a/application/src/main/java/org/opentripplanner/model/TimetableSnapshot.java b/application/src/main/java/org/opentripplanner/model/TimetableSnapshot.java index 44ce1e5cb18..11969cf250b 100644 --- a/application/src/main/java/org/opentripplanner/model/TimetableSnapshot.java +++ b/application/src/main/java/org/opentripplanner/model/TimetableSnapshot.java @@ -14,10 +14,12 @@ import java.util.Comparator; import java.util.ConcurrentModificationException; import java.util.HashMap; +import java.util.HashSet; import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Map.Entry; +import java.util.Set; import java.util.SortedSet; import java.util.TreeSet; import java.util.function.Predicate; @@ -100,7 +102,7 @@ public class TimetableSnapshot { * include ones from the scheduled GTFS, as well as ones added by realtime messages and * tracked by the TripPatternCache.

* Note that the keys do not include all scheduled TripPatterns, only those for which we have at - * least one update.

+ * least one update, and those for which we had updates before but just recently cleared.

* The members of the SortedSet (the Timetable for a particular day) are treated as copy-on-write * when we're updating them. If an update will modify the timetable for a particular day, that * timetable is replicated before any modifications are applied to avoid affecting any previous @@ -111,6 +113,7 @@ public class TimetableSnapshot { * TripPattern and date. */ private final Map> timetables; + private final Set patternAndServiceDatesToBeRestored = new HashSet<>(); /** * For cases where the trip pattern (sequence of stops visited) has been changed by a realtime @@ -395,9 +398,21 @@ public TimetableSnapshot commit(TransitLayerUpdater transitLayerUpdater, boolean ); if (transitLayerUpdater != null) { + for (var patternAndServiceDate : patternAndServiceDatesToBeRestored) { + if (!dirtyTimetables.containsKey(patternAndServiceDate)) { + var pattern = patternAndServiceDate.tripPattern(); + var scheduledTimetable = pattern.getScheduledTimetable(); + dirtyTimetables.put( + patternAndServiceDate, + scheduledTimetable.copyForServiceDate(patternAndServiceDate.serviceDate) + ); + } + } + transitLayerUpdater.update(dirtyTimetables.values(), timetables); } + patternAndServiceDatesToBeRestored.clear(); this.dirtyTimetables.clear(); this.dirty = false; @@ -563,7 +578,25 @@ public boolean isEmpty() { * @return true if the timetable changed as a result of the call */ private boolean clearTimetables(String feedId) { - return timetables.keySet().removeIf(tripPattern -> feedId.equals(tripPattern.getFeedId())); + var entriesToBeRemoved = timetables + .entrySet() + .stream() + .filter(entry -> feedId.equals(entry.getKey().getFeedId())) + .collect(Collectors.toSet()); + patternAndServiceDatesToBeRestored.addAll( + entriesToBeRemoved + .stream() + .flatMap(entry -> + entry + .getValue() + .stream() + .map(timetable -> + new TripPatternAndServiceDate(entry.getKey(), timetable.getServiceDate()) + ) + ) + .toList() + ); + return timetables.entrySet().removeAll(entriesToBeRemoved); } /** diff --git a/application/src/test/java/org/opentripplanner/model/TimetableSnapshotTest.java b/application/src/test/java/org/opentripplanner/model/TimetableSnapshotTest.java index 4f3e12c1368..cceea964f7d 100644 --- a/application/src/test/java/org/opentripplanner/model/TimetableSnapshotTest.java +++ b/application/src/test/java/org/opentripplanner/model/TimetableSnapshotTest.java @@ -20,6 +20,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.SortedSet; import java.util.concurrent.atomic.AtomicBoolean; import org.junit.jupiter.api.BeforeAll; @@ -31,7 +32,10 @@ import org.opentripplanner.transit.model.framework.Deduplicator; import org.opentripplanner.transit.model.framework.FeedScopedId; import org.opentripplanner.transit.model.framework.Result; +import org.opentripplanner.transit.model.network.StopPattern; import org.opentripplanner.transit.model.network.TripPattern; +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.TripIdAndServiceDate; import org.opentripplanner.transit.model.timetable.TripOnServiceDate; @@ -46,7 +50,7 @@ public class TimetableSnapshotTest { private static final ZoneId timeZone = ZoneIds.GMT; public static final LocalDate SERVICE_DATE = LocalDate.of(2024, 1, 1); private static Map patternIndex; - static String feedId; + private static String feedId; @BeforeAll public static void setUp() throws Exception { @@ -412,6 +416,133 @@ void testClear() { assertNotNull(snapshot.getRealtimeAddedRoute(pattern.getRoute().getId())); } + /** + * This test checks that the original timetable is given to TransitLayerUpdater for previously + * added patterns after the buffer is cleared. + *

+ * Refer to bug #6197 for details. + */ + @Test + void testTransitLayerUpdateAfterClear() { + var resolver = new TimetableSnapshot(); + + // make an updated trip + var pattern = patternIndex.get(new FeedScopedId(feedId, "1.1")); + var trip = pattern.scheduledTripsAsStream().findFirst().orElseThrow(); + var scheduledTimetable = pattern.getScheduledTimetable(); + var updatedTripTimes = Objects + .requireNonNull(scheduledTimetable.getTripTimes(trip)) + .copyScheduledTimes(); + for (var i = 0; i < updatedTripTimes.getNumStops(); ++i) { + updatedTripTimes.updateArrivalDelay(i, 30); + updatedTripTimes.updateDepartureDelay(i, 30); + } + updatedTripTimes.setRealTimeState(RealTimeState.UPDATED); + var realTimeTripUpdate = new RealTimeTripUpdate( + pattern, + updatedTripTimes, + SERVICE_DATE, + null, + false, + false + ); + + var addedDepartureStopTime = new StopTime(); + var addedArrivalStopTime = new StopTime(); + addedDepartureStopTime.setDepartureTime(0); + addedDepartureStopTime.setArrivalTime(0); + addedDepartureStopTime.setStop(RegularStop.of(new FeedScopedId(feedId, "XX"), () -> 0).build()); + addedArrivalStopTime.setDepartureTime(300); + addedArrivalStopTime.setArrivalTime(300); + addedArrivalStopTime.setStop(RegularStop.of(new FeedScopedId(feedId, "YY"), () -> 1).build()); + var addedStopTimes = List.of(addedDepartureStopTime, addedArrivalStopTime); + var addedStopPattern = new StopPattern(addedStopTimes); + var route = patternIndex.values().stream().findFirst().orElseThrow().getRoute(); + var addedTripPattern = TripPattern + .of(new FeedScopedId(feedId, "1.1")) + .withRoute(route) + .withStopPattern(addedStopPattern) + .withCreatedByRealtimeUpdater(true) + .build(); + var addedTripTimes = TripTimesFactory.tripTimes( + Trip.of(new FeedScopedId(feedId, "addedTrip")).withRoute(route).build(), + addedStopTimes, + new Deduplicator() + ); + var addedTripUpdate = new RealTimeTripUpdate( + addedTripPattern, + addedTripTimes, + SERVICE_DATE, + null, + true, + false + ); + + var transitLayerUpdater = new TransitLayerUpdater(null) { + int count = 0; + + /** + * Test that the TransitLayerUpdater receives correct updated timetables upon commit + *

+ * This method is called 3 times. + * When count = 0, the buffer contains one added and one updated trip, and the timetables + * should reflect this fact. + * When count = 1, the buffer is empty, however, this method should still receive the + * timetables of the previous added and updated patterns in order to restore them to the + * initial scheduled timetable. + * When count = 2, the buffer is still empty, and no changes should be made. + */ + @Override + public void update( + Collection updatedTimetables, + Map> timetables + ) { + assertThat(updatedTimetables).hasSize(count == 2 ? 0 : 2); + + updatedTimetables.forEach(timetable -> { + var timetablePattern = timetable.getPattern(); + assertEquals(SERVICE_DATE, timetable.getServiceDate()); + if (timetablePattern == pattern) { + if (count == 1) { + // the timetable for the existing pattern should revert to the original + assertEquals(scheduledTimetable.getTripTimes(), timetable.getTripTimes()); + } else { + // the timetable for the existing pattern should contain the modified times + assertEquals( + RealTimeState.UPDATED, + Objects.requireNonNull(timetable.getTripTimes(trip)).getRealTimeState() + ); + } + } else if (timetablePattern == addedTripPattern) { + if (count == 1) { + // the timetable for the added pattern should be empty after clearing + assertThat(timetable.getTripTimes()).isEmpty(); + } else { + // the timetable for the added pattern should contain the times for 1 added trip + assertThat(timetable.getTripTimes()).hasSize(1); + } + } else { + throw new RuntimeException("unknown pattern passed to transit layer updater"); + } + }); + ++count; + } + }; + + resolver.update(realTimeTripUpdate); + resolver.update(addedTripUpdate); + resolver.commit(transitLayerUpdater, true); + + resolver.clear(feedId); + resolver.clear(feedId); + resolver.clear(feedId); + assertTrue(resolver.commit(transitLayerUpdater, true).isEmpty()); + + resolver.clear(feedId); + resolver.clear(feedId); + assertTrue(resolver.commit(transitLayerUpdater, true).isEmpty()); + } + private static TimetableSnapshot createCommittedSnapshot() { TimetableSnapshot timetableSnapshot = new TimetableSnapshot(); return timetableSnapshot.commit(null, true);