Skip to content

Commit

Permalink
Merge pull request #5726 from HSLdevcom/fix-skipped-removal
Browse files Browse the repository at this point in the history 
Fix real-time added patterns persistence with DIFFERENTIAL updates
  • Loading branch information
@optionsome
optionsome authored Jun 11, 2024
2 parents be8ade2 + bc06c14 commit 32bee57
Show file tree
Hide file tree
Showing 7 changed files with 318 additions and 106 deletions.
24 changes: 1 addition & 23 deletions src/ext/java/org/opentripplanner/ext/siri/SiriTimetableSnapshotSource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ private Result<TripUpdate, UpdateError> 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.revertTripToScheduledTripPattern(trip.getId(), serviceDate);

return updateResult;
}
Expand All @@ -295,7 +295,6 @@ private Result<UpdateSuccess, UpdateError> addTripToGraphAndBuffer(TripUpdate tr
/**
* Mark the scheduled trip in the buffer as deleted, given trip on service date
*
* @param serviceDate service date
* @return true if scheduled trip was marked as deleted
*/
private boolean markScheduledTripAsDeleted(Trip trip, final LocalDate serviceDate) {
Expand All @@ -320,25 +319,4 @@ 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;
}
}
86 changes: 48 additions & 38 deletions src/main/java/org/opentripplanner/model/TimetableSnapshot.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,44 +141,10 @@ public Timetable resolve(TripPattern pattern, LocalDate serviceDate) {
return pattern.getScheduledTimetable();
}

public void removeRealtimeUpdatedTripTimes(
TripPattern tripPattern,
FeedScopedId tripId,
LocalDate serviceDate
) {
SortedSet<Timetable> 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.
*
* @param tripId trip id
* @param serviceDate service date
* @return trip pattern created by the updater; null if trip is on the original trip pattern
*/
public TripPattern getRealtimeAddedTripPattern(FeedScopedId tripId, LocalDate serviceDate) {
Expand Down Expand Up @@ -325,11 +291,55 @@ 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.
* If a previous realtime update has changed which trip pattern is associated with the given trip
* on the given service date, this method will dissociate the trip from that pattern and remove
* the trip's timetables from that pattern on that particular service date.
*
* For this service date, the trip will revert to its original trip pattern from the scheduled
* data, remaining on that pattern unless it's changed again by a future realtime update.
*
* @return true if the trip was found to be shifted to a different trip pattern by a realtime
* message and an attempt was made to re-associate it with its originally scheduled trip pattern.
*/
public void removeLastAddedTripPattern(FeedScopedId feedScopedTripId, LocalDate serviceDate) {
realtimeAddedTripPattern.remove(new TripIdAndServiceDate(feedScopedTripId, serviceDate));
public boolean revertTripToScheduledTripPattern(FeedScopedId tripId, LocalDate serviceDate) {
boolean success = false;

final TripPattern pattern = getRealtimeAddedTripPattern(tripId, serviceDate);
if (pattern != null) {
// Dissociate the given trip from any realtime-added pattern.
// The trip will then fall back to its original scheduled pattern.
realtimeAddedTripPattern.remove(new TripIdAndServiceDate(tripId, serviceDate));
// Remove times for the trip from any timetables
// under that now-obsolete realtime-added pattern.
SortedSet<Timetable> sortedTimetables = this.timetables.get(pattern);
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);
}
}
}
}
success = true;
}

return success;
}

/**
Expand Down
126 changes: 85 additions & 41 deletions src/main/java/org/opentripplanner/updater/trip/TimetableSnapshotSource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,15 +190,17 @@ public UpdateResult applyTripUpdates(
// starts for example at 40:00, yesterday would probably be a better guess.
serviceDate = localDateNow();
}

uIndex += 1;
LOG.debug("trip update #{} ({} updates) :", uIndex, tripUpdate.getStopTimeUpdateCount());
LOG.trace("{}", tripUpdate);

// Determine what kind of trip update this is
final TripDescriptor.ScheduleRelationship tripScheduleRelationship = determineTripScheduleRelationship(
tripDescriptor
);
if (!fullDataset) {
purgePatternModifications(tripScheduleRelationship, tripId, serviceDate);
}

uIndex += 1;
LOG.debug("trip update #{} ({} updates) :", uIndex, tripUpdate.getStopTimeUpdateCount());
LOG.trace("{}", tripUpdate);

Result<UpdateSuccess, UpdateError> result;
try {
Expand All @@ -216,8 +218,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,
fullDataset
);
case DELETED -> handleCanceledTrip(
tripId,
serviceDate,
CancelationType.DELETE,
fullDataset
);
case REPLACEMENT -> validateAndHandleModifiedTrip(
tripUpdate,
tripDescriptor,
Expand Down Expand Up @@ -255,6 +267,51 @@ public UpdateResult applyTripUpdates(
return updateResult;
}

/**
* Remove previous realtime updates for this trip. This is necessary to avoid previous stop
* pattern modifications from persisting. If a trip was previously added with the
* ScheduleRelationship ADDED and is now cancelled or deleted, we still want to keep the realtime
* added trip pattern.
*/
private void purgePatternModifications(
TripDescriptor.ScheduleRelationship tripScheduleRelationship,
FeedScopedId tripId,
LocalDate serviceDate
) {
final TripPattern pattern = buffer.getRealtimeAddedTripPattern(tripId, serviceDate);
if (
!isPreviouslyAddedTrip(tripId, pattern, serviceDate) ||
(
tripScheduleRelationship != TripDescriptor.ScheduleRelationship.CANCELED &&
tripScheduleRelationship != TripDescriptor.ScheduleRelationship.DELETED
)
) {
// Remove previous realtime updates for this trip. This is necessary to avoid previous
// stop pattern modifications from persisting. If a trip was previously added with the ScheduleRelationship
// ADDED and is now cancelled or deleted, we still want to keep the realtime added trip pattern.
this.buffer.revertTripToScheduledTripPattern(tripId, serviceDate);
}
}

private boolean isPreviouslyAddedTrip(
FeedScopedId tripId,
TripPattern pattern,
LocalDate serviceDate
) {
if (pattern == null) {
return false;
}
var timetable = buffer.resolve(pattern, serviceDate);
if (timetable == null) {
return false;
}
var tripTimes = timetable.getTripTimes(tripId);
if (tripTimes == null) {
return false;
}
return tripTimes.getRealTimeState() == RealTimeState.ADDED;
}

private static void logUpdateResult(
String feedId,
Map<TripDescriptor.ScheduleRelationship, Integer> failuresByRelationship,
Expand Down Expand Up @@ -327,11 +384,6 @@ private Result<UpdateSuccess, UpdateError> 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()
Expand Down Expand Up @@ -578,10 +630,6 @@ private Result<UpdateSuccess, UpdateError> 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
Expand Down Expand Up @@ -827,8 +875,6 @@ private Result<UpdateSuccess, UpdateError> addTripToGraphAndBuffer(
/**
* Cancel scheduled trip in buffer given trip id on service date
*
* @param tripId trip id
* @param serviceDate service date
* @return true if scheduled trip was cancelled
*/
private boolean cancelScheduledTrip(
Expand Down Expand Up @@ -864,25 +910,22 @@ private boolean cancelScheduledTrip(

/**
* Cancel previously added trip from buffer if there is a previously added trip with given trip id
* (without agency id) on service date. This does not remove the modified/added trip from the
* buffer, it just marks it as canceled. This also does not remove the corresponding vertices and
* edges from the Graph. Any TripPattern that was created for the added/modified trip continues to
* exist, and will be reused if a similar added/modified trip message is received with the same
* route and stop sequence.
* on service date. This does not remove the added trip from the buffer, it just marks it as
* canceled or deleted. Any TripPattern that was created for the added trip continues to exist,
* and will be reused if a similar added trip message is received with the same route and stop
* sequence.
*
* @param tripId trip id without agency id
* @param serviceDate service date
* @return true if a previously added trip was cancelled
*/
private boolean cancelPreviouslyAddedTrip(
final FeedScopedId tripId,
final LocalDate serviceDate,
CancelationType cancelationType
) {
boolean success = false;
boolean cancelledAddedTrip = false;

final TripPattern pattern = buffer.getRealtimeAddedTripPattern(tripId, serviceDate);
if (pattern != null) {
if (isPreviouslyAddedTrip(tripId, pattern, serviceDate)) {
// Cancel trip times for this trip in this pattern
final Timetable timetable = buffer.resolve(pattern, serviceDate);
final int tripIndex = timetable.getTripIndex(tripId);
Expand All @@ -897,11 +940,10 @@ private boolean cancelPreviouslyAddedTrip(
case DELETE -> newTripTimes.deleteTrip();
}
buffer.update(pattern, newTripTimes, serviceDate);
success = true;
cancelledAddedTrip = true;
}
}

return success;
return cancelledAddedTrip;
}

/**
Expand Down Expand Up @@ -996,10 +1038,6 @@ private Result<UpdateSuccess, UpdateError> 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,
Expand All @@ -1014,19 +1052,25 @@ private Result<UpdateSuccess, UpdateError> handleModifiedTrip(
private Result<UpdateSuccess, UpdateError> handleCanceledTrip(
FeedScopedId tripId,
final LocalDate serviceDate,
CancelationType markAsDeleted
CancelationType cancelationType,
boolean fullDataset
) {
// Try to cancel scheduled trip
final boolean cancelScheduledSuccess = cancelScheduledTrip(tripId, serviceDate, markAsDeleted);
var canceledPreviouslyAddedTrip = fullDataset
? false
: cancelPreviouslyAddedTrip(tripId, serviceDate, cancelationType);

// Try to cancel previously added trip
final boolean cancelPreviouslyAddedSuccess = cancelPreviouslyAddedTrip(
// if previously an 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
cancelationType
);

if (!cancelScheduledSuccess && !cancelPreviouslyAddedSuccess) {
if (!cancelScheduledSuccess) {
debug(tripId, "No pattern found for tripId. Skipping cancellation.");
return UpdateError.result(tripId, NO_TRIP_FOR_CANCELLATION_FOUND);
}
Expand Down
13 changes: 10 additions & 3 deletions src/test/java/org/opentripplanner/updater/trip/RealtimeTestEnvironment.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -256,15 +256,22 @@ public UpdateResult applyEstimatedTimetable(List<EstimatedTimetableDeliveryStruc
// GTFS-RT updates

public UpdateResult applyTripUpdate(GtfsRealtime.TripUpdate update) {
return applyTripUpdates(List.of(update));
return applyTripUpdates(List.of(update), false);
}

public UpdateResult applyTripUpdates(List<GtfsRealtime.TripUpdate> updates) {
public UpdateResult applyTripUpdate(GtfsRealtime.TripUpdate update, boolean differential) {
return applyTripUpdates(List.of(update), differential);
}

public UpdateResult applyTripUpdates(
List<GtfsRealtime.TripUpdate> updates,
boolean differential
) {
Objects.requireNonNull(gtfsSource, "Test environment is configured for SIRI only");
return gtfsSource.applyTripUpdates(
null,
BackwardsDelayPropagationType.REQUIRED_NO_DATA,
true,
!differential,
updates,
getFeedId()
);
Expand Down
Loading

0 comments on commit 32bee57

Please sign in to comment.