Skip to content

Commit eaaa9cc

Browse files
vesameskanenvesameskanen
committed
Merge remote-tracking branch 'origin/fix-skipped-removal' into dev-2.x
2 parents 91c8396 + b98c3c8 commit eaaa9cc

File tree

4 files changed

+313
-109
lines changed

4 files changed

+313
-109
lines changed

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

+1-22
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.removeRealtimeAddedTripPatternAndTimetablesForTrip(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

+49-37
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,56 @@ 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+
* If a previous realtime update has changed which trip pattern is used for this trip on the given
266+
* service date, this removes the timetables for this trip on the service date for the trip
267+
* pattern and also the connection of that trip pattern for this trip on the given service date.
268+
* The original trip pattern from the scheduled data will be used for the trip again on this
269+
* service date until a new trip pattern is used for the trip.
270+
*
271+
* @return true if a new trip pattern was used for the trip previously and its connection to the
272+
* trip one the given service date was attempted to removed together with its timetables for the
273+
* trip.
300274
*/
301-
public void removeLastAddedTripPattern(FeedScopedId feedScopedTripId, LocalDate serviceDate) {
302-
realtimeAddedTripPattern.remove(new TripIdAndServiceDate(feedScopedTripId, serviceDate));
275+
public boolean removeRealtimeAddedTripPatternAndTimetablesForTrip(
276+
FeedScopedId tripId,
277+
LocalDate serviceDate
278+
) {
279+
boolean success = false;
280+
281+
final TripPattern pattern = getRealtimeAddedTripPattern(tripId, serviceDate);
282+
if (pattern != null) {
283+
// Remove the previous real-time-added TripPattern from buffer.
284+
// Only one version of the real-time-update should exist
285+
realtimeAddedTripPattern.remove(new TripIdAndServiceDate(tripId, serviceDate));
286+
SortedSet<Timetable> sortedTimetables = this.timetables.get(pattern);
287+
if (sortedTimetables != null) {
288+
TripTimes tripTimesToRemove = null;
289+
for (Timetable timetable : sortedTimetables) {
290+
if (timetable.isValidFor(serviceDate)) {
291+
final TripTimes tripTimes = timetable.getTripTimes(tripId);
292+
if (tripTimes == null) {
293+
LOG.debug("No triptimes to remove for trip {}", tripId);
294+
} else if (tripTimesToRemove != null) {
295+
LOG.debug("Found two triptimes to remove for trip {}", tripId);
296+
} else {
297+
tripTimesToRemove = tripTimes;
298+
}
299+
}
300+
}
301+
302+
if (tripTimesToRemove != null) {
303+
for (Timetable sortedTimetable : sortedTimetables) {
304+
boolean isDirty = sortedTimetable.getTripTimes().remove(tripTimesToRemove);
305+
if (isDirty) {
306+
dirtyTimetables.add(sortedTimetable);
307+
}
308+
}
309+
}
310+
}
311+
success = true;
312+
}
313+
314+
return success;
303315
}
304316

305317
/**

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

+59-43
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.removeRealtimeAddedTripPatternAndTimetablesForTrip(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,21 @@ private Result<UpdateSuccess, UpdateError> handleModifiedTrip(
11221136
private Result<UpdateSuccess, UpdateError> handleCanceledTrip(
11231137
FeedScopedId tripId,
11241138
final LocalDate serviceDate,
1125-
CancelationType markAsDeleted
1139+
CancelationType cancelationType,
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
1128-
final boolean cancelScheduledSuccess = cancelScheduledTrip(tripId, serviceDate, markAsDeleted);
1129-
1130-
// Try to cancel previously added trip
1131-
final boolean cancelPreviouslyAddedSuccess = cancelPreviouslyAddedTrip(
1147+
final boolean cancelScheduledSuccess = cancelScheduledTrip(
11321148
tripId,
11331149
serviceDate,
1134-
markAsDeleted
1150+
cancelationType
11351151
);
11361152

1137-
if (!cancelScheduledSuccess && !cancelPreviouslyAddedSuccess) {
1153+
if (!cancelScheduledSuccess) {
11381154
debug(tripId, "No pattern found for tripId. Skipping cancellation.");
11391155
return UpdateError.result(tripId, NO_TRIP_FOR_CANCELLATION_FOUND);
11401156
}

0 commit comments

Comments
 (0)