refactor: Encapsulate timetable inside TimetableSnapshot.

This call to the updates listener exposed the internal map of timetable. The callee did only use the map to fetch data, so we could easily change the callback from a map to a function.

NOTE! There is a functional change in this commit! The `TimetableSnapshot#clearTimetables(...)` uses the TripPattern.id.feedScope to filter, not the TripPattern.route.id.feedScope. These should have tha same value, but it is possible to mess up here.

```Java
private boolean clearTimetables(String feedId) {
  return timetables.keySet().removeIf(id -> feedId.equals(id.getFeedId()));
}
```

Author: t2gran

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

@@ -10,11 +10,13 @@
 import java.util.Map;
 import java.util.Set;
 import java.util.SortedSet;
+import java.util.function.Function;
 import java.util.stream.Collectors;
 import org.opentripplanner.framework.application.OTPFeature;
 import org.opentripplanner.routing.algorithm.raptoradapter.transit.RaptorTransitData;
 import org.opentripplanner.routing.algorithm.raptoradapter.transit.TripPatternForDate;
 import org.opentripplanner.routing.algorithm.raptoradapter.transit.constrainedtransfer.TransferIndexGenerator;
+import org.opentripplanner.transit.model.framework.FeedScopedId;
 import org.opentripplanner.transit.model.network.TripPattern;
 import org.opentripplanner.transit.model.timetable.Timetable;
 import org.opentripplanner.transit.model.timetable.TimetableSnapshotUpdateSucessListener;
@@ -95,7 +97,7 @@ public RealTimeRaptorTransitDataUpdater(TimetableRepository timetableRepository)
   @Override
   public void update(
     Collection<Timetable> updatedTimetables,
-    Map<TripPattern, SortedSet<Timetable>> timetables
+    Function<FeedScopedId, SortedSet<Timetable>> timetableProvider
   ) {
     if (!timetableRepository.hasRealtimeRaptorTransitData()) {
       return;
@@ -218,7 +220,7 @@ public void update(
           if (!pattern.isCreatedByRealtimeUpdater()) {
             continue;
           }
-          var oldTimeTable = timetables.get(pattern);
+          var oldTimeTable = timetableProvider.apply(pattern.getId());
           if (oldTimeTable != null) {
             var toRemove = oldTimeTable
               .stream()

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

@@ -102,7 +102,7 @@ public class TimetableSnapshot {
    * The compound key approach better reflects the fact that there should be only one Timetable per
    * TripPattern and date.
    */
-  private final Map<TripPattern, SortedSet<Timetable>> timetables;
+  private final Map<FeedScopedId, SortedSet<Timetable>> timetables;
 
   /**
    * For cases where the trip pattern (sequence of stops visited) has been changed by a realtime
@@ -168,7 +168,7 @@ public TimetableSnapshot() {
   }
 
   private TimetableSnapshot(
-    Map<TripPattern, SortedSet<Timetable>> timetables,
+    Map<FeedScopedId, SortedSet<Timetable>> timetables,
     Map<TripIdAndServiceDate, TripPattern> realTimeNewTripPatternsForModifiedTrips,
     Map<FeedScopedId, Route> realtimeAddedRoutes,
     Map<FeedScopedId, Trip> realtimeAddedTrips,
@@ -196,7 +196,7 @@ private TimetableSnapshot(
    * the originally scheduled timetable if there are no updates in this snapshot.
    */
   public Timetable resolve(TripPattern pattern, @Nullable LocalDate serviceDate) {
-    SortedSet<Timetable> sortedTimetables = timetables.get(pattern);
+    SortedSet<Timetable> sortedTimetables = timetables.get(pattern.getId());
 
     if (sortedTimetables != null && serviceDate != null) {
       for (Timetable timetable : sortedTimetables) {
@@ -390,7 +390,7 @@ public TimetableSnapshot commit(
     );
 
     if (updatesEventListener != null) {
-      updatesEventListener.update(dirtyTimetables.values(), timetables);
+      updatesEventListener.update(dirtyTimetables.values(), timetables::get);
     }
 
     this.dirtyTimetables.clear();
@@ -445,7 +445,7 @@ public boolean revertTripToScheduledTripPattern(FeedScopedId tripId, LocalDate s
       realTimeNewTripPatternsForModifiedTrips.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);
+      SortedSet<Timetable> sortedTimetables = this.timetables.get(pattern.getId());
       if (sortedTimetables != null) {
         TripTimes tripTimesToRemove = null;
         for (Timetable timetable : sortedTimetables) {
@@ -488,9 +488,9 @@ public boolean purgeExpiredData(LocalDate serviceDate) {
     validateNotReadOnly();
 
     boolean modified = false;
-    for (Iterator<TripPattern> it = timetables.keySet().iterator(); it.hasNext();) {
-      TripPattern pattern = it.next();
-      SortedSet<Timetable> sortedTimetables = timetables.get(pattern);
+    for (Iterator<FeedScopedId> it = timetables.keySet().iterator(); it.hasNext();) {
+      FeedScopedId patternId = it.next();
+      SortedSet<Timetable> sortedTimetables = timetables.get(patternId);
       SortedSet<Timetable> toKeepTimetables = new TreeSet<>(new SortedTimetableComparator());
       for (Timetable timetable : sortedTimetables) {
         if (serviceDate.isBefore(timetable.getServiceDate())) {
@@ -503,7 +503,7 @@ public boolean purgeExpiredData(LocalDate serviceDate) {
       if (toKeepTimetables.isEmpty()) {
         it.remove();
       } else {
-        timetables.put(pattern, ImmutableSortedSet.copyOfSorted(toKeepTimetables));
+        timetables.put(patternId, ImmutableSortedSet.copyOfSorted(toKeepTimetables));
       }
     }
 
@@ -557,7 +557,7 @@ public boolean isEmpty() {
    * @return true if the timetable changed as a result of the call
    */
   private boolean clearTimetables(String feedId) {
-    return timetables.keySet().removeIf(tripPattern -> feedId.equals(tripPattern.getFeedId()));
+    return timetables.keySet().removeIf(id -> feedId.equals(id.getFeedId()));
   }
 
   /**
@@ -617,7 +617,7 @@ private void addPatternToIndex(TripPattern tripPattern) {
    * The SortedSet is made immutable to prevent change after snapshot publication.
    */
   private void swapTimetable(TripPattern pattern, Timetable original, Timetable updated) {
-    SortedSet<Timetable> sortedTimetables = timetables.get(pattern);
+    SortedSet<Timetable> sortedTimetables = timetables.get(pattern.getId());
     if (sortedTimetables == null) {
       sortedTimetables = new TreeSet<>(new SortedTimetableComparator());
     } else {
@@ -632,7 +632,7 @@ private void swapTimetable(TripPattern pattern, Timetable original, Timetable up
       sortedTimetables.remove(original);
     }
     sortedTimetables.add(updated);
-    timetables.put(pattern, ImmutableSortedSet.copyOfSorted(sortedTimetables));
+    timetables.put(pattern.getId(), ImmutableSortedSet.copyOfSorted(sortedTimetables));
 
     // if the timetable was already modified by a previous real-time update in the same snapshot
     // and for the same service date,

application/src/main/java/org/opentripplanner/transit/model/timetable/TimetableSnapshotUpdateSucessListener.java

@@ -1,9 +1,9 @@
 package org.opentripplanner.transit.model.timetable;
 
 import java.util.Collection;
-import java.util.Map;
 import java.util.SortedSet;
-import org.opentripplanner.transit.model.network.TripPattern;
+import java.util.function.Function;
+import org.opentripplanner.transit.model.framework.FeedScopedId;
 
 /**
  * When the {@link TimetableSnapshot} is updated and the commit method has completed this
@@ -13,6 +13,6 @@
 public interface TimetableSnapshotUpdateSucessListener {
   void update(
     Collection<Timetable> updatedTimetables,
-    Map<TripPattern, SortedSet<Timetable>> timetables
+    Function<FeedScopedId, SortedSet<Timetable>> timetables
   );
 }

application/src/test/java/org/opentripplanner/transit/model/TimetableRepositoryArchitectureTest.java

@@ -16,7 +16,6 @@
 import org.opentripplanner._support.arch.ArchComponent;
 import org.opentripplanner._support.arch.Package;
 
-@Disabled
 public class TimetableRepositoryArchitectureTest {
 
   private static final Package TRANSIT_FRAMEWORK = TRANSIT_MODEL.subPackage("framework");

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

@@ -16,13 +16,13 @@
 import java.util.Map;
 import java.util.SortedSet;
 import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.function.Function;
 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.model.StopTime;
-import org.opentripplanner.routing.algorithm.raptoradapter.transit.mappers.RealTimeRaptorTransitDataUpdater;
 import org.opentripplanner.transit.model.framework.Deduplicator;
 import org.opentripplanner.transit.model.framework.FeedScopedId;
 import org.opentripplanner.transit.model.network.TripPattern;
@@ -84,21 +84,18 @@ void testUniqueDirtyTimetablesAfterMultipleUpdates() {
 
     AtomicBoolean updateIsCalled = new AtomicBoolean();
 
-    RealTimeRaptorTransitDataUpdater raptorTransitData = new RealTimeRaptorTransitDataUpdater(
-      null
-    ) {
+    var updateListener = new TimetableSnapshotUpdateSucessListener() {
       @Override
       public void update(
         Collection<Timetable> updatedTimetables,
-        Map<TripPattern, SortedSet<Timetable>> timetables
+        Function<FeedScopedId, SortedSet<Timetable>> timetableProvider
       ) {
         updateIsCalled.set(true);
         assertThat(updatedTimetables).hasSize(1);
-        assertThat(timetables).hasSize(1);
       }
     };
 
-    snapshot.commit(raptorTransitData, true);
+    snapshot.commit(updateListener, true);
 
     assertTrue(updateIsCalled.get());
   }