From b708a3786d4a6d5181dc9d52b9746c62e1dff270 Mon Sep 17 00:00:00 2001 From: Joel Lappalainen Date: Mon, 4 Mar 2024 22:58:42 +0200 Subject: [PATCH] Fix issue with realtime added patterns persisting with DIFFERENTIAL updates --- .../ext/siri/SiriTimetableSnapshotSource.java | 23 +---- .../model/TimetableSnapshot.java | 94 +++++++++++-------- .../updater/trip/TimetableSnapshotSource.java | 54 ++++++----- 3 files changed, 88 insertions(+), 83 deletions(-) diff --git a/src/ext/java/org/opentripplanner/ext/siri/SiriTimetableSnapshotSource.java b/src/ext/java/org/opentripplanner/ext/siri/SiriTimetableSnapshotSource.java index 345f8deba20..87f100464a5 100644 --- a/src/ext/java/org/opentripplanner/ext/siri/SiriTimetableSnapshotSource.java +++ b/src/ext/java/org/opentripplanner/ext/siri/SiriTimetableSnapshotSource.java @@ -354,7 +354,7 @@ private Result handleModifiedTrip( // Also check whether trip id has been used for previously ADDED/MODIFIED trip message and // remove the previously created trip - removePreviousRealtimeUpdate(trip, serviceDate); + this.buffer.removePreviousRealtimeUpdate(trip.getId(), serviceDate); return updateResult; } @@ -410,27 +410,6 @@ private boolean markScheduledTripAsDeleted(Trip trip, final LocalDate serviceDat return success; } - /** - * Removes previous trip-update from buffer if there is an update with given trip on service date - * - * @param serviceDate service date - * @return true if a previously added trip was removed - */ - private boolean removePreviousRealtimeUpdate(final Trip trip, final LocalDate serviceDate) { - boolean success = false; - - final TripPattern pattern = buffer.getRealtimeAddedTripPattern(trip.getId(), serviceDate); - if (pattern != null) { - // Remove the previous real-time-added TripPattern from buffer. - // Only one version of the real-time-update should exist - buffer.removeLastAddedTripPattern(trip.getId(), serviceDate); - buffer.removeRealtimeUpdatedTripTimes(pattern, trip.getId(), serviceDate); - success = true; - } - - return success; - } - private boolean purgeExpiredData() { final LocalDate today = LocalDate.now(transitModel.getTimeZone()); final LocalDate previously = today.minusDays(2); // Just to be safe... diff --git a/src/main/java/org/opentripplanner/model/TimetableSnapshot.java b/src/main/java/org/opentripplanner/model/TimetableSnapshot.java index 066a27ba15a..937c3e13ffd 100644 --- a/src/main/java/org/opentripplanner/model/TimetableSnapshot.java +++ b/src/main/java/org/opentripplanner/model/TimetableSnapshot.java @@ -20,7 +20,6 @@ import org.opentripplanner.transit.model.network.TripPattern; import org.opentripplanner.transit.model.site.StopLocation; import org.opentripplanner.transit.model.timetable.TripIdAndServiceDate; -import org.opentripplanner.transit.model.timetable.TripOnServiceDate; import org.opentripplanner.transit.model.timetable.TripTimes; import org.opentripplanner.updater.spi.UpdateError; import org.opentripplanner.updater.spi.UpdateSuccess; @@ -111,38 +110,6 @@ public Timetable resolve(TripPattern pattern, LocalDate serviceDate) { return pattern.getScheduledTimetable(); } - public void removeRealtimeUpdatedTripTimes( - TripPattern tripPattern, - FeedScopedId tripId, - LocalDate serviceDate - ) { - SortedSet sortedTimetables = this.timetables.get(tripPattern); - if (sortedTimetables != null) { - TripTimes tripTimesToRemove = null; - for (Timetable timetable : sortedTimetables) { - if (timetable.isValidFor(serviceDate)) { - final TripTimes tripTimes = timetable.getTripTimes(tripId); - if (tripTimes == null) { - LOG.debug("No triptimes to remove for trip {}", tripId); - } else if (tripTimesToRemove != null) { - LOG.debug("Found two triptimes to remove for trip {}", tripId); - } else { - tripTimesToRemove = tripTimes; - } - } - } - - if (tripTimesToRemove != null) { - for (Timetable sortedTimetable : sortedTimetables) { - boolean isDirty = sortedTimetable.getTripTimes().remove(tripTimesToRemove); - if (isDirty) { - dirtyTimetables.add(sortedTimetable); - } - } - } - } - } - /** * Get the current trip pattern given a trip id and a service date, if it has been changed from * the scheduled pattern with an update, for which the stopPattern is different. @@ -295,11 +262,24 @@ public void clear(String feedId) { } /** - * Removes the latest added trip pattern from the cache. This should be done when removing the - * trip times from the timetable the trip has been added to. + * Removes previous trip-update from buffer if there is an update with given trip on service date + * + * @param serviceDate service date + * @return true if a previously added trip was removed */ - public void removeLastAddedTripPattern(FeedScopedId feedScopedTripId, LocalDate serviceDate) { - realtimeAddedTripPattern.remove(new TripIdAndServiceDate(feedScopedTripId, serviceDate)); + public boolean removePreviousRealtimeUpdate(FeedScopedId tripId, LocalDate serviceDate) { + boolean success = false; + + final TripPattern pattern = getRealtimeAddedTripPattern(tripId, serviceDate); + if (pattern != null) { + // Remove the previous real-time-added TripPattern from buffer. + // Only one version of the real-time-update should exist + removeLastAddedTripPattern(tripId, serviceDate); + removeRealtimeUpdatedTripTimes(pattern, tripId, serviceDate); + success = true; + } + + return success; } /** @@ -391,6 +371,46 @@ protected boolean clearRealtimeAddedTripPattern(String feedId) { ); } + private void removeRealtimeUpdatedTripTimes( + TripPattern tripPattern, + FeedScopedId tripId, + LocalDate serviceDate + ) { + SortedSet sortedTimetables = this.timetables.get(tripPattern); + if (sortedTimetables != null) { + TripTimes tripTimesToRemove = null; + for (Timetable timetable : sortedTimetables) { + if (timetable.isValidFor(serviceDate)) { + final TripTimes tripTimes = timetable.getTripTimes(tripId); + if (tripTimes == null) { + LOG.debug("No triptimes to remove for trip {}", tripId); + } else if (tripTimesToRemove != null) { + LOG.debug("Found two triptimes to remove for trip {}", tripId); + } else { + tripTimesToRemove = tripTimes; + } + } + } + + if (tripTimesToRemove != null) { + for (Timetable sortedTimetable : sortedTimetables) { + boolean isDirty = sortedTimetable.getTripTimes().remove(tripTimesToRemove); + if (isDirty) { + dirtyTimetables.add(sortedTimetable); + } + } + } + } + } + + /** + * Removes the latest added trip pattern from the cache. This should be done when removing the + * trip times from the timetable the trip has been added to. + */ + private void removeLastAddedTripPattern(FeedScopedId feedScopedTripId, LocalDate serviceDate) { + realtimeAddedTripPattern.remove(new TripIdAndServiceDate(feedScopedTripId, serviceDate)); + } + /** * Add the patterns to the stop index, only if they come from a modified pattern */ diff --git a/src/main/java/org/opentripplanner/updater/trip/TimetableSnapshotSource.java b/src/main/java/org/opentripplanner/updater/trip/TimetableSnapshotSource.java index 049d4933c51..2c2497044fb 100644 --- a/src/main/java/org/opentripplanner/updater/trip/TimetableSnapshotSource.java +++ b/src/main/java/org/opentripplanner/updater/trip/TimetableSnapshotSource.java @@ -272,6 +272,17 @@ public UpdateResult applyTripUpdates( serviceDate = localDateNow.get(); } + // Check whether trip id has been used for previously ADDED trip message and mark previously + // created trip as DELETED + var canceledPreviouslyAddedTrip = cancelPreviouslyAddedTrip( + tripId, + serviceDate, + CancelationType.DELETE + ); + // Remove previous realtime updates for this trip. This is necessary to avoid previous + // stop pattern modifications from persisting + this.buffer.removePreviousRealtimeUpdate(tripId, serviceDate); + uIndex += 1; LOG.debug("trip update #{} ({} updates) :", uIndex, tripUpdate.getStopTimeUpdateCount()); LOG.trace("{}", tripUpdate); @@ -297,8 +308,18 @@ public UpdateResult applyTripUpdates( tripId, serviceDate ); - case CANCELED -> handleCanceledTrip(tripId, serviceDate, CancelationType.CANCEL); - case DELETED -> handleCanceledTrip(tripId, serviceDate, CancelationType.DELETE); + case CANCELED -> handleCanceledTrip( + tripId, + serviceDate, + CancelationType.CANCEL, + canceledPreviouslyAddedTrip + ); + case DELETED -> handleCanceledTrip( + tripId, + serviceDate, + CancelationType.DELETE, + canceledPreviouslyAddedTrip + ); case REPLACEMENT -> validateAndHandleModifiedTrip( tripUpdate, tripDescriptor, @@ -435,11 +456,6 @@ private Result handleScheduledTrip( return UpdateError.result(tripId, NO_SERVICE_ON_DATE); } - // If this trip_id has been used for previously ADDED/MODIFIED trip message (e.g. when the - // sequence of stops has changed, and is now changing back to the originally scheduled one), - // mark that previously created trip as DELETED. - cancelPreviouslyAddedTrip(tripId, serviceDate, CancelationType.DELETE); - // Get new TripTimes based on scheduled timetable var result = pattern .getScheduledTimetable() @@ -686,10 +702,6 @@ private Result handleAddedTrip( "number of stop should match the number of stop time updates" ); - // Check whether trip id has been used for previously ADDED trip message and mark previously - // created trip as DELETED - cancelPreviouslyAddedTrip(tripId, serviceDate, CancelationType.DELETE); - Route route = getOrCreateRoute(tripDescriptor, tripId); // Create new Trip @@ -1104,10 +1116,6 @@ private Result handleModifiedTrip( var tripId = trip.getId(); cancelScheduledTrip(tripId, serviceDate, CancelationType.DELETE); - // Check whether trip id has been used for previously ADDED/REPLACEMENT trip message and mark it - // as DELETED - cancelPreviouslyAddedTrip(tripId, serviceDate, CancelationType.DELETE); - // Add new trip return addTripToGraphAndBuffer( trip, @@ -1122,19 +1130,17 @@ private Result handleModifiedTrip( private Result handleCanceledTrip( FeedScopedId tripId, final LocalDate serviceDate, - CancelationType markAsDeleted + CancelationType markAsDeleted, + boolean canceledPreviouslyAddedTrip ) { + // if previously a added trip was removed, there can't be a scheduled trip to remove + if (canceledPreviouslyAddedTrip) { + return Result.success(UpdateSuccess.noWarnings()); + } // Try to cancel scheduled trip final boolean cancelScheduledSuccess = cancelScheduledTrip(tripId, serviceDate, markAsDeleted); - // Try to cancel previously added trip - final boolean cancelPreviouslyAddedSuccess = cancelPreviouslyAddedTrip( - tripId, - serviceDate, - markAsDeleted - ); - - if (!cancelScheduledSuccess && !cancelPreviouslyAddedSuccess) { + if (!cancelScheduledSuccess) { debug(tripId, "No pattern found for tripId. Skipping cancellation."); return UpdateError.result(tripId, NO_TRIP_FOR_CANCELLATION_FOUND); }