Skip to content

Commit 0518c5e

Browse files
optionsomeoptionsome
committed
Merge branch 'fix-skipped-removal' into dev-2.x
2 parents 0eb0b2a + 7cdbe3d commit 0518c5e

File tree

4 files changed

+319
-111
lines changed

4 files changed

+319
-111
lines changed

‎src/ext/java/org/opentripplanner/ext/siri/SiriTimetableSnapshotSource.java

Lines changed: 1 addition & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -354,7 +354,7 @@ private Result<TripUpdate, UpdateError> handleModifiedTrip(
354354

355355
// Also check whether trip id has been used for previously ADDED/MODIFIED trip message and
356356
// remove the previously created trip
357-
removePreviousRealtimeUpdate(trip, serviceDate);
357+
this.buffer.removePreviousRealtimeUpdate(trip.getId(), serviceDate);
358358

359359
return updateResult;
360360
}
@@ -410,27 +410,6 @@ private boolean markScheduledTripAsDeleted(Trip trip, final LocalDate serviceDat
410410
return success;
411411
}
412412

413-
/**
414-
* Removes previous trip-update from buffer if there is an update with given trip on service date
415-
*
416-
* @param serviceDate service date
417-
* @return true if a previously added trip was removed
418-
*/
419-
private boolean removePreviousRealtimeUpdate(final Trip trip, final LocalDate serviceDate) {
420-
boolean success = false;
421-
422-
final TripPattern pattern = buffer.getRealtimeAddedTripPattern(trip.getId(), serviceDate);
423-
if (pattern != null) {
424-
// Remove the previous real-time-added TripPattern from buffer.
425-
// Only one version of the real-time-update should exist
426-
buffer.removeLastAddedTripPattern(trip.getId(), serviceDate);
427-
buffer.removeRealtimeUpdatedTripTimes(pattern, trip.getId(), serviceDate);
428-
success = true;
429-
}
430-
431-
return success;
432-
}
433-
434413
private boolean purgeExpiredData() {
435414
final LocalDate today = LocalDate.now(transitModel.getTimeZone());
436415
final LocalDate previously = today.minusDays(2); // Just to be safe...

‎src/main/java/org/opentripplanner/model/TimetableSnapshot.java

Lines changed: 57 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
import org.opentripplanner.transit.model.network.TripPattern;
2121
import org.opentripplanner.transit.model.site.StopLocation;
2222
import org.opentripplanner.transit.model.timetable.TripIdAndServiceDate;
23-
import org.opentripplanner.transit.model.timetable.TripOnServiceDate;
2423
import org.opentripplanner.transit.model.timetable.TripTimes;
2524
import org.opentripplanner.updater.spi.UpdateError;
2625
import org.opentripplanner.updater.spi.UpdateSuccess;
@@ -111,38 +110,6 @@ public Timetable resolve(TripPattern pattern, LocalDate serviceDate) {
111110
return pattern.getScheduledTimetable();
112111
}
113112

114-
public void removeRealtimeUpdatedTripTimes(
115-
TripPattern tripPattern,
116-
FeedScopedId tripId,
117-
LocalDate serviceDate
118-
) {
119-
SortedSet<Timetable> sortedTimetables = this.timetables.get(tripPattern);
120-
if (sortedTimetables != null) {
121-
TripTimes tripTimesToRemove = null;
122-
for (Timetable timetable : sortedTimetables) {
123-
if (timetable.isValidFor(serviceDate)) {
124-
final TripTimes tripTimes = timetable.getTripTimes(tripId);
125-
if (tripTimes == null) {
126-
LOG.debug("No triptimes to remove for trip {}", tripId);
127-
} else if (tripTimesToRemove != null) {
128-
LOG.debug("Found two triptimes to remove for trip {}", tripId);
129-
} else {
130-
tripTimesToRemove = tripTimes;
131-
}
132-
}
133-
}
134-
135-
if (tripTimesToRemove != null) {
136-
for (Timetable sortedTimetable : sortedTimetables) {
137-
boolean isDirty = sortedTimetable.getTripTimes().remove(tripTimesToRemove);
138-
if (isDirty) {
139-
dirtyTimetables.add(sortedTimetable);
140-
}
141-
}
142-
}
143-
}
144-
}
145-
146113
/**
147114
* Get the current trip pattern given a trip id and a service date, if it has been changed from
148115
* the scheduled pattern with an update, for which the stopPattern is different.
@@ -295,11 +262,24 @@ public void clear(String feedId) {
295262
}
296263

297264
/**
298-
* Removes the latest added trip pattern from the cache. This should be done when removing the
299-
* trip times from the timetable the trip has been added to.
265+
* Removes previous trip-update from buffer if there is an update with given trip on service date
266+
*
267+
* @param serviceDate service date
268+
* @return true if a previously added trip was removed
300269
*/
301-
public void removeLastAddedTripPattern(FeedScopedId feedScopedTripId, LocalDate serviceDate) {
302-
realtimeAddedTripPattern.remove(new TripIdAndServiceDate(feedScopedTripId, serviceDate));
270+
public boolean removePreviousRealtimeUpdate(FeedScopedId tripId, LocalDate serviceDate) {
271+
boolean success = false;
272+
273+
final TripPattern pattern = getRealtimeAddedTripPattern(tripId, serviceDate);
274+
if (pattern != null) {
275+
// Remove the previous real-time-added TripPattern from buffer.
276+
// Only one version of the real-time-update should exist
277+
removeLastAddedTripPattern(tripId, serviceDate);
278+
removeRealtimeUpdatedTripTimes(pattern, tripId, serviceDate);
279+
success = true;
280+
}
281+
282+
return success;
303283
}
304284

305285
/**
@@ -391,6 +371,46 @@ protected boolean clearRealtimeAddedTripPattern(String feedId) {
391371
);
392372
}
393373

374+
private void removeRealtimeUpdatedTripTimes(
375+
TripPattern tripPattern,
376+
FeedScopedId tripId,
377+
LocalDate serviceDate
378+
) {
379+
SortedSet<Timetable> sortedTimetables = this.timetables.get(tripPattern);
380+
if (sortedTimetables != null) {
381+
TripTimes tripTimesToRemove = null;
382+
for (Timetable timetable : sortedTimetables) {
383+
if (timetable.isValidFor(serviceDate)) {
384+
final TripTimes tripTimes = timetable.getTripTimes(tripId);
385+
if (tripTimes == null) {
386+
LOG.debug("No triptimes to remove for trip {}", tripId);
387+
} else if (tripTimesToRemove != null) {
388+
LOG.debug("Found two triptimes to remove for trip {}", tripId);
389+
} else {
390+
tripTimesToRemove = tripTimes;
391+
}
392+
}
393+
}
394+
395+
if (tripTimesToRemove != null) {
396+
for (Timetable sortedTimetable : sortedTimetables) {
397+
boolean isDirty = sortedTimetable.getTripTimes().remove(tripTimesToRemove);
398+
if (isDirty) {
399+
dirtyTimetables.add(sortedTimetable);
400+
}
401+
}
402+
}
403+
}
404+
}
405+
406+
/**
407+
* Removes the latest added trip pattern from the cache. This should be done when removing the
408+
* trip times from the timetable the trip has been added to.
409+
*/
410+
private void removeLastAddedTripPattern(FeedScopedId feedScopedTripId, LocalDate serviceDate) {
411+
realtimeAddedTripPattern.remove(new TripIdAndServiceDate(feedScopedTripId, serviceDate));
412+
}
413+
394414
/**
395415
* Add the patterns to the stop index, only if they come from a modified pattern
396416
*/

‎src/main/java/org/opentripplanner/updater/trip/TimetableSnapshotSource.java

Lines changed: 57 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -271,15 +271,28 @@ public UpdateResult applyTripUpdates(
271271
// starts for example at 40:00, yesterday would probably be a better guess.
272272
serviceDate = localDateNow.get();
273273
}
274-
275-
uIndex += 1;
276-
LOG.debug("trip update #{} ({} updates) :", uIndex, tripUpdate.getStopTimeUpdateCount());
277-
LOG.trace("{}", tripUpdate);
278-
279274
// Determine what kind of trip update this is
280275
final TripDescriptor.ScheduleRelationship tripScheduleRelationship = determineTripScheduleRelationship(
281276
tripDescriptor
282277
);
278+
var canceledPreviouslyAddedTrip = false;
279+
if (!fullDataset) {
280+
// Check whether trip id has been used for previously ADDED trip message and mark previously
281+
// created trip as DELETED unless schedule relationship is CANCELED, then as CANCEL
282+
var cancelationType = tripScheduleRelationship ==
283+
TripDescriptor.ScheduleRelationship.CANCELED
284+
? CancelationType.CANCEL
285+
: CancelationType.DELETE;
286+
canceledPreviouslyAddedTrip =
287+
cancelPreviouslyAddedTrip(tripId, serviceDate, cancelationType);
288+
// Remove previous realtime updates for this trip. This is necessary to avoid previous
289+
// stop pattern modifications from persisting
290+
this.buffer.removePreviousRealtimeUpdate(tripId, serviceDate);
291+
}
292+
293+
uIndex += 1;
294+
LOG.debug("trip update #{} ({} updates) :", uIndex, tripUpdate.getStopTimeUpdateCount());
295+
LOG.trace("{}", tripUpdate);
283296

284297
Result<UpdateSuccess, UpdateError> result;
285298
try {
@@ -297,8 +310,18 @@ public UpdateResult applyTripUpdates(
297310
tripId,
298311
serviceDate
299312
);
300-
case CANCELED -> handleCanceledTrip(tripId, serviceDate, CancelationType.CANCEL);
301-
case DELETED -> handleCanceledTrip(tripId, serviceDate, CancelationType.DELETE);
313+
case CANCELED -> handleCanceledTrip(
314+
tripId,
315+
serviceDate,
316+
CancelationType.CANCEL,
317+
canceledPreviouslyAddedTrip
318+
);
319+
case DELETED -> handleCanceledTrip(
320+
tripId,
321+
serviceDate,
322+
CancelationType.DELETE,
323+
canceledPreviouslyAddedTrip
324+
);
302325
case REPLACEMENT -> validateAndHandleModifiedTrip(
303326
tripUpdate,
304327
tripDescriptor,
@@ -347,6 +370,26 @@ public UpdateResult applyTripUpdates(
347370
return updateResult;
348371
}
349372

373+
/**
374+
* This shouldn't be used outside of this class for other purposes than testing where the forced
375+
* snapshot commit can guarantee consistent behaviour.
376+
*/
377+
TimetableSnapshot getTimetableSnapshot(final boolean force) {
378+
final long now = System.currentTimeMillis();
379+
if (force || now - lastSnapshotTime > maxSnapshotFrequency.toMillis()) {
380+
if (force || buffer.isDirty()) {
381+
LOG.debug("Committing {}", buffer);
382+
snapshot = buffer.commit(transitLayerUpdater, force);
383+
} else {
384+
LOG.debug("Buffer was unchanged, keeping old snapshot.");
385+
}
386+
lastSnapshotTime = System.currentTimeMillis();
387+
} else {
388+
LOG.debug("Snapshot frequency exceeded. Reusing snapshot {}", snapshot);
389+
}
390+
return snapshot;
391+
}
392+
350393
private static void logUpdateResult(
351394
String feedId,
352395
Map<TripDescriptor.ScheduleRelationship, Integer> failuresByRelationship,
@@ -367,22 +410,6 @@ private static void logUpdateResult(
367410
});
368411
}
369412

370-
private TimetableSnapshot getTimetableSnapshot(final boolean force) {
371-
final long now = System.currentTimeMillis();
372-
if (force || now - lastSnapshotTime > maxSnapshotFrequency.toMillis()) {
373-
if (force || buffer.isDirty()) {
374-
LOG.debug("Committing {}", buffer);
375-
snapshot = buffer.commit(transitLayerUpdater, force);
376-
} else {
377-
LOG.debug("Buffer was unchanged, keeping old snapshot.");
378-
}
379-
lastSnapshotTime = System.currentTimeMillis();
380-
} else {
381-
LOG.debug("Snapshot frequency exceeded. Reusing snapshot {}", snapshot);
382-
}
383-
return snapshot;
384-
}
385-
386413
/**
387414
* Determine how the trip update should be handled.
388415
*
@@ -435,11 +462,6 @@ private Result<UpdateSuccess, UpdateError> handleScheduledTrip(
435462
return UpdateError.result(tripId, NO_SERVICE_ON_DATE);
436463
}
437464

438-
// If this trip_id has been used for previously ADDED/MODIFIED trip message (e.g. when the
439-
// sequence of stops has changed, and is now changing back to the originally scheduled one),
440-
// mark that previously created trip as DELETED.
441-
cancelPreviouslyAddedTrip(tripId, serviceDate, CancelationType.DELETE);
442-
443465
// Get new TripTimes based on scheduled timetable
444466
var result = pattern
445467
.getScheduledTimetable()
@@ -686,10 +708,6 @@ private Result<UpdateSuccess, UpdateError> handleAddedTrip(
686708
"number of stop should match the number of stop time updates"
687709
);
688710

689-
// Check whether trip id has been used for previously ADDED trip message and mark previously
690-
// created trip as DELETED
691-
cancelPreviouslyAddedTrip(tripId, serviceDate, CancelationType.DELETE);
692-
693711
Route route = getOrCreateRoute(tripDescriptor, tripId);
694712

695713
// Create new Trip
@@ -1104,10 +1122,6 @@ private Result<UpdateSuccess, UpdateError> handleModifiedTrip(
11041122
var tripId = trip.getId();
11051123
cancelScheduledTrip(tripId, serviceDate, CancelationType.DELETE);
11061124

1107-
// Check whether trip id has been used for previously ADDED/REPLACEMENT trip message and mark it
1108-
// as DELETED
1109-
cancelPreviouslyAddedTrip(tripId, serviceDate, CancelationType.DELETE);
1110-
11111125
// Add new trip
11121126
return addTripToGraphAndBuffer(
11131127
trip,
@@ -1122,19 +1136,17 @@ private Result<UpdateSuccess, UpdateError> handleModifiedTrip(
11221136
private Result<UpdateSuccess, UpdateError> handleCanceledTrip(
11231137
FeedScopedId tripId,
11241138
final LocalDate serviceDate,
1125-
CancelationType markAsDeleted
1139+
CancelationType markAsDeleted,
1140+
boolean canceledPreviouslyAddedTrip
11261141
) {
1142+
// if previously a added trip was removed, there can't be a scheduled trip to remove
1143+
if (canceledPreviouslyAddedTrip) {
1144+
return Result.success(UpdateSuccess.noWarnings());
1145+
}
11271146
// Try to cancel scheduled trip
11281147
final boolean cancelScheduledSuccess = cancelScheduledTrip(tripId, serviceDate, markAsDeleted);
11291148

1130-
// Try to cancel previously added trip
1131-
final boolean cancelPreviouslyAddedSuccess = cancelPreviouslyAddedTrip(
1132-
tripId,
1133-
serviceDate,
1134-
markAsDeleted
1135-
);
1136-
1137-
if (!cancelScheduledSuccess && !cancelPreviouslyAddedSuccess) {
1149+
if (!cancelScheduledSuccess) {
11381150
debug(tripId, "No pattern found for tripId. Skipping cancellation.");
11391151
return UpdateError.result(tripId, NO_TRIP_FOR_CANCELLATION_FOUND);
11401152
}

0 commit comments

Comments
 (0)