Merge pull request opentripplanner#7219 from ibi-group/combine-trip-pattern-caches

Use SIRI trip pattern cache in GTFS updater

Author: leonardehrenfried

application/src/main/java/org/opentripplanner/updater/trip/gtfs/GtfsRealTimeTripUpdateAdapter.java

@@ -62,6 +62,8 @@
 import org.opentripplanner.updater.trip.UpdateIncrementality;
 import org.opentripplanner.updater.trip.gtfs.model.AddedRoute;
 import org.opentripplanner.updater.trip.gtfs.model.TripUpdate;
+import org.opentripplanner.updater.trip.siri.SiriTripPatternCache;
+import org.opentripplanner.updater.trip.siri.SiriTripPatternIdGenerator;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.slf4j.event.Level;
@@ -73,8 +75,15 @@ public class GtfsRealTimeTripUpdateAdapter {
 
   private static final Logger LOG = LoggerFactory.getLogger(GtfsRealTimeTripUpdateAdapter.class);
 
-  /** A synchronized cache of trip patterns added to the graph due to GTFS-realtime messages. */
-  private final TripPatternCache tripPatternCache = new TripPatternCache();
+  /**
+   * A synchronized cache of trip patterns added to the timetable repository
+   * due to GTFS-realtime messages.
+   * <p>
+   * This has "Siri" in the name because we are combining the two versions very carefully, step by
+   * step. Once this process is complete, we will clean up the name and move it to an appropriate
+   * package.
+   **/
+  private final SiriTripPatternCache tripPatternCache;
 
   private final ZoneId timeZone;
 
@@ -106,6 +115,10 @@ public GtfsRealTimeTripUpdateAdapter(
       snapshotManager.getTimetableSnapshotBuffer()
     );
     this.deduplicator = timetableRepository.getDeduplicator();
+    this.tripPatternCache = new SiriTripPatternCache(
+      new SiriTripPatternIdGenerator(),
+      transitEditorService::findPattern
+    );
   }
 
   /**
@@ -404,11 +417,7 @@ private Result<UpdateSuccess, UpdateError> handleScheduledTrip(
 
       final Trip trip = transitEditorService.getTrip(tripId);
       // Get cached trip pattern or create one if it doesn't exist yet
-      final TripPattern newPattern = tripPatternCache.getOrCreateTripPattern(
-        newStopPattern,
-        trip,
-        pattern
-      );
+      final TripPattern newPattern = tripPatternCache.getOrCreateTripPattern(newStopPattern, trip);
 
       cancelScheduledTrip(tripId, serviceDate, CancelationType.DELETE);
       return snapshotManager.updateBuffer(
@@ -649,13 +658,8 @@ private Result<UpdateSuccess, UpdateError> addNewOrReplacementTripToSnapshot(
     // Create StopPattern
     final StopPattern stopPattern = tripTimesWithStopPattern.stopPattern();
 
-    final TripPattern originalTripPattern = transitEditorService.findPattern(trip);
     // Get cached trip pattern or create one if it doesn't exist yet
-    final TripPattern pattern = tripPatternCache.getOrCreateTripPattern(
-      stopPattern,
-      trip,
-      originalTripPattern
-    );
+    final TripPattern pattern = tripPatternCache.getOrCreateTripPattern(stopPattern, trip);
 
     trace(
       trip.getId(),

application/src/main/java/org/opentripplanner/updater/trip/siri/SiriTripPatternCache.java

@@ -26,7 +26,7 @@
  *             the thread-safety, but just by looking at a few lines of code I see problems - a strategy needs to be
  *             analysed, designed and documented.
  */
-class SiriTripPatternCache {
+public class SiriTripPatternCache {
 
   /**
    * We cache the trip pattern based on the stop pattern only in order to de-duplicate them.
@@ -80,7 +80,7 @@ public synchronized TripPattern getOrCreateTripPattern(
     // TODO RT_AB: Verify implementation, which is different than the GTFS-RT version.
     //   It can return a TripPattern from the scheduled data, but protective copies are handled in
     //   TimetableSnapshot.update. Better document this aspect of the contract in this method's Javadoc.
-    if (originalTripPattern.getStopPattern().equals(stopPattern)) {
+    if (originalTripPattern != null && originalTripPattern.getStopPattern().equals(stopPattern)) {
       return originalTripPattern;
     }
 

application/src/main/java/org/opentripplanner/updater/trip/siri/SiriTripPatternIdGenerator.java

@@ -14,7 +14,7 @@
  * usage pattern is not enforced due to differing needs in tests.
  * The ID generation is threadsafe, even if that is probably not needed.
  */
-class SiriTripPatternIdGenerator {
+public class SiriTripPatternIdGenerator {
 
   private final AtomicInteger counter = new AtomicInteger(0);
 

application/src/test/java/org/opentripplanner/updater/trip/gtfs/moduletests/addition/AddedThenRemovedTest.java

@@ -45,7 +45,7 @@ void addedThenRemoved() {
 
     assertThat(env.raptorData().summarizePatterns()).containsExactly(
       "F:Pattern1[SCHEDULED]",
-      "F:AddedTrip::rt#1[ADDED]"
+      "F:AddedTrip::001:RT[ADDED]"
     );
 
     // the GTFS updater is configured to clear timetables, so an empty list should remove the

application/src/test/java/org/opentripplanner/updater/trip/gtfs/moduletests/delay/AssignedStopIdsTest.java

@@ -71,7 +71,7 @@ void assignedThenRevertedStopIds() {
       env.tripData(TRIP_1_ID).showTimetable()
     );
     assertTrue(env.tripData(TRIP_1_ID).tripPattern().isStopPatternModifiedInRealTime());
-    assertEquals(List.of("F:Route1::rt#1[UPDATED]"), env.raptorData().summarizePatterns());
+    assertEquals(List.of("F:Route1::001:RT[UPDATED]"), env.raptorData().summarizePatterns());
 
     var tripUpdate2 = rt
       .tripUpdateScheduled(TRIP_1_ID)
@@ -86,7 +86,7 @@ void assignedThenRevertedStopIds() {
       env.tripData(TRIP_1_ID).showTimetable()
     );
     assertTrue(env.tripData(TRIP_1_ID).tripPattern().isStopPatternModifiedInRealTime());
-    assertEquals(List.of("F:Route1::rt#2[UPDATED]"), env.raptorData().summarizePatterns());
+    assertEquals(List.of("F:Route1::002:RT[UPDATED]"), env.raptorData().summarizePatterns());
 
     var tripUpdate3 = rt
       .tripUpdateScheduled(TRIP_1_ID)
@@ -136,7 +136,7 @@ void reuseRealtimeTripPatterns() {
     assertTrue(env.tripData(TRIP_1_ID).tripPattern().isStopPatternModifiedInRealTime());
     assertFalse(env.tripData(TRIP_2_ID).tripPattern().isStopPatternModifiedInRealTime());
     assertEquals(
-      List.of("F:Pattern1[SCHEDULED]", "F:Route1::rt#1[UPDATED]"),
+      List.of("F:Pattern1[SCHEDULED]", "F:Route1::001:RT[UPDATED]"),
       env.raptorData().summarizePatterns()
     );
 
@@ -151,7 +151,10 @@ void reuseRealtimeTripPatterns() {
     );
     assertTrue(env.tripData(TRIP_1_ID).tripPattern().isStopPatternModifiedInRealTime());
     assertTrue(env.tripData(TRIP_2_ID).tripPattern().isStopPatternModifiedInRealTime());
-    assertEquals(List.of("F:Route1::rt#1[UPDATED,UPDATED]"), env.raptorData().summarizePatterns());
+    assertEquals(
+      List.of("F:Route1::001:RT[UPDATED,UPDATED]"),
+      env.raptorData().summarizePatterns()
+    );
 
     assertSuccess(rt.applyTripUpdate(tripUpdate2));
     assertEquals(
@@ -165,7 +168,7 @@ void reuseRealtimeTripPatterns() {
     assertFalse(env.tripData(TRIP_1_ID).tripPattern().isStopPatternModifiedInRealTime());
     assertTrue(env.tripData(TRIP_2_ID).tripPattern().isStopPatternModifiedInRealTime());
     assertEquals(
-      List.of("F:Pattern1[SCHEDULED]", "F:Route1::rt#1[UPDATED]"),
+      List.of("F:Pattern1[SCHEDULED]", "F:Route1::001:RT[UPDATED]"),
       env.raptorData().summarizePatterns()
     );
 
@@ -256,7 +259,7 @@ void reuseRealtimeTripPatternsOnDifferentServiceDates() {
       env.tripData(TRIP_2_ID, SERVICE_DATE_PLUS).tripPattern().isStopPatternModifiedInRealTime()
     );
     assertEquals(
-      List.of("F:Route1::rt#1[UPDATED,UPDATED]"),
+      List.of("F:Route1::001:RT[UPDATED,UPDATED]"),
       env.raptorData(SERVICE_DATE).summarizePatterns()
     );
     assertEquals(
@@ -296,11 +299,11 @@ void reuseRealtimeTripPatternsOnDifferentServiceDates() {
       env.tripData(TRIP_2_ID, SERVICE_DATE_PLUS).tripPattern().isStopPatternModifiedInRealTime()
     );
     assertEquals(
-      List.of("F:Route1::rt#1[UPDATED,UPDATED]"),
+      List.of("F:Route1::001:RT[UPDATED,UPDATED]"),
       env.raptorData(SERVICE_DATE).summarizePatterns()
     );
     assertEquals(
-      List.of("F:Route1::rt#1[UPDATED,UPDATED]"),
+      List.of("F:Route1::001:RT[UPDATED,UPDATED]"),
       env.raptorData(SERVICE_DATE_PLUS).summarizePatterns()
     );
 
@@ -338,7 +341,7 @@ void reuseRealtimeTripPatternsOnDifferentServiceDates() {
       env.raptorData(SERVICE_DATE).summarizePatterns()
     );
     assertEquals(
-      List.of("F:Route1::rt#1[UPDATED,UPDATED]"),
+      List.of("F:Route1::001:RT[UPDATED,UPDATED]"),
       env.raptorData(SERVICE_DATE_PLUS).summarizePatterns()
     );