Remove old trip pattern cache implementation

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

@@ -50,8 +50,8 @@
 import org.opentripplanner.updater.trip.TimetableSnapshotManager;
 import org.opentripplanner.updater.trip.UpdateIncrementality;
 import org.opentripplanner.updater.trip.gtfs.model.TripUpdate;
-import org.opentripplanner.updater.trip.siri.SiriTripPatternCache;
-import org.opentripplanner.updater.trip.siri.SiriTripPatternIdGenerator;
+import org.opentripplanner.updater.trip.patterncache.TripPatternCache;
+import org.opentripplanner.updater.trip.patterncache.TripPatternIdGenerator;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -70,7 +70,7 @@ public class GtfsRealTimeTripUpdateAdapter {
    * 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 TripPatternCache tripPatternCache;
 
   /**
    * Long-lived transit editor service that has access to the timetable snapshot buffer.
@@ -102,8 +102,8 @@ public GtfsRealTimeTripUpdateAdapter(
     );
     this.deduplicator = deduplicator;
     this.tripTimesUpdater = new TripTimesUpdater(timetableRepository.getTimeZone(), deduplicator);
-    this.tripPatternCache = new SiriTripPatternCache(
-      new SiriTripPatternIdGenerator(),
+    this.tripPatternCache = new TripPatternCache(
+      new TripPatternIdGenerator(),
       transitEditorService::findPattern
     );
   }

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

@@ -1,4 +1,4 @@
-package org.opentripplanner.updater.trip.siri;
+package org.opentripplanner.updater.trip.patterncache;
 
 import java.util.HashMap;
 import java.util.Map;
@@ -8,25 +8,19 @@
 import org.opentripplanner.transit.model.timetable.Trip;
 
 /**
- * Threadsafe mechanism for tracking any TripPatterns added to the graph via SIRI realtime messages.
+ * Threadsafe mechanism for tracking any TripPatterns added to the graph via realtime messages.
  * This tracks only patterns added by realtime messages, not ones that already existed from the
- * scheduled NeTEx. This is a "cache" in the sense that it will keep returning the same TripPattern
+ * scheduled NeTEx or GTFS. This is a "cache" in the sense that it will keep returning the same TripPattern
  * when presented with the same StopPattern, so if realtime messages add many trips passing through
  * the same sequence of stops, they will all end up on this same TripPattern.
- * <p>
- * Note that there are two versions of this class, this one for GTFS-RT and another for SIRI.
- * See additional comments in the Javadoc of the GTFS-RT version of this class, whose name is
- * simply TripPatternCache.
- * TODO RT_AB: To the extent that double SIRI/GTFS implementations are kept, prefix all names
- *             with GTFS or SIRI or NETEX rather than having no prefix on the GTFS versions.
  *  TODO RT_TG: There is no clear strategy for what should be in the cache and the transit model and the flow
- *             between them. The NeTEx and a GTFS version of this should be merged. Having NeTex and GTFS
- *             specific indexes inside is ok. With the increased usage of DatedServiceJourneys, this should probably
+ *             between them.
+ *             With the increased usage of DatedServiceJourneys, this should probably
  *             be part of the main model - not a separate cache. It is possible that this class works when it comes to
  *             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.
  */
-public class SiriTripPatternCache {
+public class TripPatternCache {
 
   /**
    * We cache the trip pattern based on the stop pattern only in order to de-duplicate them.
@@ -36,21 +30,17 @@ public class SiriTripPatternCache {
    */
   private final Map<StopPattern, TripPattern> cache = new HashMap<>();
 
-  // TODO RT_AB: generalize this so we can generate IDs for SIRI or GTFS-RT sources.
-  private final SiriTripPatternIdGenerator tripPatternIdGenerator;
+  private final TripPatternIdGenerator tripPatternIdGenerator;
 
   private final Function<Trip, TripPattern> getPatternForTrip;
 
   /**
-   * TODO RT_AB: This class could potentially be reused for both SIRI and GTFS-RT, which may
-   *   involve injecting a different ID generator and pattern fetching method.
-   *
-   * @param getPatternForTrip SiriTripPatternCache needs only this one feature of TransitService, so we retain
+   * @param getPatternForTrip TripPatternCache needs only this one feature of TransitService, so we retain
    *                          only this function reference to effectively narrow the interface. This should also facilitate
    *                          testing.
    */
-  public SiriTripPatternCache(
-    SiriTripPatternIdGenerator tripPatternIdGenerator,
+  public TripPatternCache(
+    TripPatternIdGenerator tripPatternIdGenerator,
     Function<Trip, TripPattern> getPatternForTrip
   ) {
     this.tripPatternIdGenerator = tripPatternIdGenerator;
@@ -60,12 +50,6 @@ public SiriTripPatternCache(
   /**
    * Get cached trip pattern or create one if it doesn't exist yet.
    *
-   * TODO RT_AB: Improve documentation and/or merge with GTFS version of this class.
-   *  This was clearly derived from a method from TripPatternCache. This is the only non-dead-code
-   *  public method on this class, and mirrors the only public method on the GTFS-RT version of
-   *  TripPatternCache. It also explains why this class is called a "cache". It allows reusing the
-   *  same TripPattern instance when many different trips are created or updated with the same pattern.
-   *
    * @param stopPattern stop pattern to retrieve/create trip pattern
    * @param trip        Trip containing route of new trip pattern in case a new trip pattern will be
    *                    created
@@ -77,9 +61,6 @@ public synchronized TripPattern getOrCreateTripPattern(
   ) {
     TripPattern originalTripPattern = getPatternForTrip.apply(trip);
 
-    // 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 != null && originalTripPattern.getStopPattern().equals(stopPattern)) {
       return originalTripPattern;
     }
@@ -98,7 +79,6 @@ public synchronized TripPattern getOrCreateTripPattern(
         .withRealTimeStopPatternModified()
         .withOriginalTripPattern(originalTripPattern)
         .build();
-      // TODO: Add pattern to timetableRepository index?
 
       // Add pattern to cache
       cache.put(stopPattern, tripPattern);

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

@@ -1,4 +1,4 @@
-package org.opentripplanner.updater.trip.siri;
+package org.opentripplanner.updater.trip.patterncache;
 
 import java.util.concurrent.atomic.AtomicInteger;
 import org.opentripplanner.core.model.id.FeedScopedId;
@@ -8,13 +8,13 @@
 import org.opentripplanner.transit.model.timetable.Trip;
 
 /**
- * This class generates new unique IDs for TripPatterns created in response to real-time updates
- * from the SIRI updaters. In non-test usage it is important to create only one instance of this
- * class, and inject that single instance wherever it is needed. However, this single-instance
- * usage pattern is not enforced due to differing needs in tests.
+ * This class generates new unique IDs for TripPatterns created in response to real-time updates.
+ * In non-test usage it is important to create only one instance of this class, and inject that
+ * single instance wherever it is needed. However, this single-instance usage pattern is not enforced
+ * due to differing needs in tests.
  * The ID generation is threadsafe, even if that is probably not needed.
  */
-public class SiriTripPatternIdGenerator {
+public class TripPatternIdGenerator {
 
   private final AtomicInteger counter = new AtomicInteger(0);
 
@@ -23,7 +23,7 @@ public class SiriTripPatternIdGenerator {
    * roughly follows the format of {@link GenerateTripPatternsOperation}. The generator suffixes the
    * ID with 'RT' to indicate that this trip pattern is generated in response to a realtime message.
    */
-  FeedScopedId generateUniqueTripPatternId(Trip trip) {
+  public FeedScopedId generateUniqueTripPatternId(Trip trip) {
     Route route = trip.getRoute();
     FeedScopedId routeId = route.getId();
     Direction direction = trip.getDirection();

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

@@ -31,6 +31,8 @@
 import org.opentripplanner.updater.spi.UpdateSuccess;
 import org.opentripplanner.updater.trip.TimetableSnapshotManager;
 import org.opentripplanner.updater.trip.UpdateIncrementality;
+import org.opentripplanner.updater.trip.patterncache.TripPatternCache;
+import org.opentripplanner.updater.trip.patterncache.TripPatternIdGenerator;
 import org.opentripplanner.utils.lang.StringUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -48,13 +50,12 @@ public class SiriRealTimeTripUpdateAdapter {
    * Use an id generator to generate TripPattern ids for new TripPatterns created by RealTime
    * updates.
    */
-  private final SiriTripPatternIdGenerator tripPatternIdGenerator =
-    new SiriTripPatternIdGenerator();
+  private final TripPatternIdGenerator tripPatternIdGenerator = new TripPatternIdGenerator();
   /**
    * A synchronized cache of trip patterns that are added to the graph due to GTFS-real-time
    * messages.
    */
-  private final SiriTripPatternCache tripPatternCache;
+  private final TripPatternCache tripPatternCache;
 
   /**
    * Long-lived transit editor service that has access to the timetable snapshot buffer.
@@ -77,7 +78,7 @@ public SiriRealTimeTripUpdateAdapter(
       timetableRepository,
       snapshotManager.getTimetableSnapshotBuffer()
     );
-    this.tripPatternCache = new SiriTripPatternCache(
+    this.tripPatternCache = new TripPatternCache(
       tripPatternIdGenerator,
       transitEditorService::findPattern
     );