Fix issue with realtime added patterns persisting with DIFFERENTIAL u…

…pdates
HSLdevcom · Mar 4, 2024 · b708a37 · b708a37
1 parent 24807b1
commit b708a37
Show file tree

Hide file tree

Showing 3 changed files with 88 additions and 83 deletions.
diff --git 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<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.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
@@ -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<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.
@@ -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<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);
+          }
+        }
+      }
+    }
+  }
+
+  /**
+   * 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
@@ -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<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()
@@ -686,10 +702,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
@@ -1104,10 +1116,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,
@@ -1122,19 +1130,17 @@ private Result<UpdateSuccess, UpdateError> handleModifiedTrip(
   private Result<UpdateSuccess, UpdateError> 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);
     }