Fix duplicated dirty timetables

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

@@ -13,11 +13,9 @@
 import java.util.Comparator;
 import java.util.ConcurrentModificationException;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.Iterator;
 import java.util.Map;
 import java.util.Map.Entry;
-import java.util.Set;
 import java.util.SortedSet;
 import java.util.TreeSet;
 import javax.annotation.Nullable;
@@ -83,10 +81,14 @@ public class TimetableSnapshot {
 
   /**
    * During the construction phase of the TimetableSnapshot, before it is considered immutable and
-   * used in routing, this Set holds all timetables that have been modified and are waiting to be
-   * indexed. This field will be set to null when the TimetableSnapshot becomes read-only.
+   * used in routing, this Map holds all timetables that have been modified and are waiting to be
+   * indexed.
+   * A real-time timetable overrides the scheduled timetable of a TripPattern for only a single
+   * service date. There can be only one overriding timetable per TripPattern and per service date.
+   * This is enforced by indexing the map with a pair (TripPattern, service date).
+   * This map is cleared when the TimetableSnapshot becomes read-only.
    */
-  private final Set<Timetable> dirtyTimetables = new HashSet<>();
+  private final Map<TripPatternAndServiceDate, Timetable> dirtyTimetables = new HashMap<>();
 
   /**
    * For each TripPattern (sequence of stops on a particular Route) for which we have received a
@@ -383,7 +385,7 @@ public TimetableSnapshot commit(TransitLayerUpdater transitLayerUpdater, boolean
     );
 
     if (transitLayerUpdater != null) {
-      transitLayerUpdater.update(dirtyTimetables, timetables);
+      transitLayerUpdater.update(dirtyTimetables.values(), timetables);
     }
 
     this.dirtyTimetables.clear();
@@ -600,7 +602,12 @@ private void swapTimetable(TripPattern pattern, Timetable original, Timetable up
     }
     sortedTimetables.add(updated);
     timetables.put(pattern, ImmutableSortedSet.copyOfSorted(sortedTimetables));
-    dirtyTimetables.add(updated);
+
+    // if the timetable was already modified by a previous real-time update in the same snapshot
+    // and for the same service date,
+    // then the previously updated timetable is superseded by the new one
+    dirtyTimetables.put(new TripPatternAndServiceDate(pattern, updated.getServiceDate()), updated);
+
     dirty = true;
   }
 
@@ -617,4 +624,9 @@ public int compare(Timetable t1, Timetable t2) {
       return t1.getServiceDate().compareTo(t2.getServiceDate());
     }
   }
+
+  /**
+   * A pair made of a TripPattern and one of the service dates it is running on.
+   */
+  private record TripPatternAndServiceDate(TripPattern tripPattern, LocalDate serviceDate) {}
 }

application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/mappers/TransitLayerUpdater.java

@@ -60,7 +60,7 @@ public TransitLayerUpdater(TransitEditorService transitService) {
   }
 
   public void update(
-    Set<Timetable> updatedTimetables,
+    Collection<Timetable> updatedTimetables,
     Map<TripPattern, SortedSet<Timetable>> timetables
   ) {
     if (!transitService.hasRealtimeTransitLayer()) {

application/src/test/java/org/opentripplanner/model/TimetableSnapshotTest.java

@@ -1,5 +1,6 @@
 package org.opentripplanner.model;
 
+import static com.google.common.truth.Truth.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotEquals;
@@ -14,25 +15,35 @@
 import com.google.transit.realtime.GtfsRealtime.TripUpdate;
 import java.time.LocalDate;
 import java.time.ZoneId;
+import java.util.Collection;
 import java.util.ConcurrentModificationException;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
+import java.util.SortedSet;
+import java.util.concurrent.atomic.AtomicBoolean;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
 import org.opentripplanner.ConstantsForTests;
 import org.opentripplanner.TestOtpModel;
 import org.opentripplanner._support.time.ZoneIds;
+import org.opentripplanner.routing.algorithm.raptoradapter.transit.mappers.TransitLayerUpdater;
+import org.opentripplanner.transit.model.framework.Deduplicator;
 import org.opentripplanner.transit.model.framework.FeedScopedId;
 import org.opentripplanner.transit.model.framework.Result;
 import org.opentripplanner.transit.model.network.TripPattern;
+import org.opentripplanner.transit.model.timetable.Trip;
+import org.opentripplanner.transit.model.timetable.TripOnServiceDate;
 import org.opentripplanner.transit.model.timetable.TripTimes;
+import org.opentripplanner.transit.model.timetable.TripTimesFactory;
 import org.opentripplanner.transit.service.TransitModel;
 import org.opentripplanner.updater.spi.UpdateError;
 import org.opentripplanner.updater.trip.BackwardsDelayPropagationType;
 
 public class TimetableSnapshotTest {
 
   private static final ZoneId timeZone = ZoneIds.GMT;
+  public static final LocalDate SERVICE_DATE = LocalDate.of(2024, 1, 1);
   private static Map<FeedScopedId, TripPattern> patternIndex;
   static String feedId;
 
@@ -261,6 +272,49 @@ public void testPurge() {
     assertFalse(resolver.isDirty());
   }
 
+  @Test
+  void testUniqueDirtyTimetablesAfterMultipleUpdates() {
+    TimetableSnapshot snapshot = new TimetableSnapshot();
+    TripPattern pattern = patternIndex.get(new FeedScopedId(feedId, "1.1"));
+    Trip trip = pattern.scheduledTripsAsStream().findFirst().orElseThrow();
+
+    TripTimes updatedTriptimes = TripTimesFactory.tripTimes(
+      trip,
+      List.of(new StopTime()),
+      new Deduplicator()
+    );
+    RealTimeTripUpdate realTimeTripUpdate = new RealTimeTripUpdate(
+      pattern,
+      updatedTriptimes,
+      SERVICE_DATE,
+      TripOnServiceDate.of(trip.getId()).withTrip(trip).withServiceDate(SERVICE_DATE).build(),
+      true,
+      true
+    );
+
+    snapshot.update(realTimeTripUpdate);
+    snapshot.update(realTimeTripUpdate);
+    assertTrue(snapshot.isDirty());
+
+    AtomicBoolean updateIsCalled = new AtomicBoolean();
+
+    TransitLayerUpdater transitLayer = new TransitLayerUpdater(null) {
+      @Override
+      public void update(
+        Collection<Timetable> updatedTimetables,
+        Map<TripPattern, SortedSet<Timetable>> timetables
+      ) {
+        updateIsCalled.set(true);
+        assertThat(updatedTimetables).hasSize(1);
+        assertThat(timetables).hasSize(1);
+      }
+    };
+
+    snapshot.commit(transitLayer, true);
+
+    assertTrue(updateIsCalled.get());
+  }
+
   @Test
   void testCannotUpdateReadOnlyTimetableSnapshot() {
     TimetableSnapshot committedSnapshot = createCommittedSnapshot();