Merge pull request opentripplanner#7253 from HSLdevcom/clear-patterns-for-stop

Fix memory leak in `TimetableSnapshot` related to `patternsForStop` and `realTimeAddedReplacedByTripOnServiceDateById`

Author: VillePihlava

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

@@ -175,7 +175,7 @@ public TimetableSnapshot() {
     );
   }
 
-  private TimetableSnapshot(
+  TimetableSnapshot(
     Map<FeedScopedId, SortedSet<Timetable>> timetables,
     Map<TripIdAndServiceDate, TripPattern> realTimeNewTripPatternsForModifiedTrips,
     Map<FeedScopedId, Route> realtimeAddedRoutes,
@@ -428,11 +428,13 @@ public void clear(String feedId) {
       feedId
     );
     boolean addedTripPatternsWereCleared = clearEntriesForRealtimeAddedTrips(feedId);
+    boolean patternsForStopWereCleared = clearPatternsForStop(feedId);
     // If this snapshot was modified, it will be dirty after the clear actions.
     if (
       timetablesWereCleared ||
       newTripPatternsForModifiedTripsWereCleared ||
-      addedTripPatternsWereCleared
+      addedTripPatternsWereCleared ||
+      patternsForStopWereCleared
     ) {
       dirty = true;
     }
@@ -568,7 +570,15 @@ public boolean isEmpty() {
     return (
       dirtyTimetables.isEmpty() &&
       timetables.isEmpty() &&
-      realTimeNewTripPatternsForModifiedTrips.isEmpty()
+      realTimeNewTripPatternsForModifiedTrips.isEmpty() &&
+      patternsForStop.isEmpty() &&
+      realtimeAddedRoutes.isEmpty() &&
+      realTimeAddedTrips.isEmpty() &&
+      realTimeAddedPatternForTrip.isEmpty() &&
+      realTimeAddedPatternsForRoute.isEmpty() &&
+      realTimeAddedTripOnServiceDateById.isEmpty() &&
+      realTimeAddedReplacedByTripOnServiceDateById.isEmpty() &&
+      realTimeAddedTripOnServiceDateForTripAndDay.isEmpty()
     );
   }
 
@@ -615,7 +625,9 @@ private boolean clearNewTripPatternsForModifiedTrips(String feedId) {
   /**
    * Clear all realtime added routes, trip patterns and trips matching the provided feed id.
    *
-   * */
+   * @param feedId feed id to clear out
+   * @return true if realTimeAddedTrips changed as a result of the call
+   */
   private boolean clearEntriesForRealtimeAddedTrips(String feedId) {
     // it is sufficient to test for the removal of added trips, since other indexed entities are
     // added only if a new trip is added.
@@ -625,15 +637,28 @@ private boolean clearEntriesForRealtimeAddedTrips(String feedId) {
     realTimeAddedPatternForTrip.keySet().removeIf(trip -> feedId.equals(trip.getId().getFeedId()));
     realTimeAddedTripOnServiceDateForTripAndDay
       .keySet()
-      .removeIf(tripOnServiceDate -> feedId.equals(tripOnServiceDate.tripId().getFeedId()));
+      .removeIf(tripIdAndServiceDate -> feedId.equals(tripIdAndServiceDate.tripId().getFeedId()));
     realTimeAddedTripOnServiceDateById.keySet().removeIf(id -> feedId.equals(id.getFeedId()));
     realTimeAddedPatternsForRoute
       .keySet()
       .removeIf(route -> feedId.equals(route.getId().getFeedId()));
     realtimeAddedRoutes.keySet().removeIf(id -> feedId.equals(id.getFeedId()));
+    realTimeAddedReplacedByTripOnServiceDateById
+      .keySet()
+      .removeIf(id -> feedId.equals(id.getFeedId()));
     return removedEntry;
   }
 
+  /**
+   * Clear all trip patterns from patternsForStop matching the provided feed id.
+   *
+   * @param feedId feed id to clear out
+   * @return true if patternsForStop changed as a result of the call
+   */
+  private boolean clearPatternsForStop(String feedId) {
+    return patternsForStop.values().removeIf(tripPattern -> feedId.equals(tripPattern.getFeedId()));
+  }
+
   /**
    * Add the patterns to the stop index, only if they come from a real-time pattern.
    */

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

@@ -7,6 +7,12 @@
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
+import com.google.common.collect.ArrayListMultimap;
+import com.google.common.collect.HashMultimap;
+import com.google.common.collect.ImmutableSortedSet;
+import com.google.common.collect.ListMultimap;
+import com.google.common.collect.Multimap;
+import com.google.common.collect.SetMultimap;
 import java.time.LocalDate;
 import java.time.ZoneId;
 import java.util.Collection;
@@ -24,8 +30,13 @@
 import org.opentripplanner._support.time.ZoneIds;
 import org.opentripplanner.core.model.id.FeedScopedId;
 import org.opentripplanner.model.StopTime;
+import org.opentripplanner.transit.model._data.TimetableRepositoryForTest;
 import org.opentripplanner.transit.model.framework.Deduplicator;
+import org.opentripplanner.transit.model.network.Route;
 import org.opentripplanner.transit.model.network.TripPattern;
+import org.opentripplanner.transit.model.site.RegularStop;
+import org.opentripplanner.transit.model.site.StopLocation;
+import org.opentripplanner.transit.model.site.TestStopLocation;
 import org.opentripplanner.transit.service.TimetableRepository;
 
 public class TimetableSnapshotTest {
@@ -169,6 +180,59 @@ void testClear() {
     assertNotNull(snapshot.getRealtimeAddedRoute(pattern.getRoute().getId()));
   }
 
+  @Test
+  void testClearWithAllCollections() {
+    TimetableRepositoryForTest TEST_MODEL = TimetableRepositoryForTest.of();
+    RegularStop STOP_A = TEST_MODEL.stop("A").build();
+    RegularStop STOP_B = TEST_MODEL.stop("B").build();
+
+    FeedScopedId id = new FeedScopedId(feedId, "1.2");
+    TripIdAndServiceDate tripIdAndServiceDate = new TripIdAndServiceDate(id, SERVICE_DATE);
+    TripOnServiceDate tripOnServiceDate = TripOnServiceDate.of(
+      new FeedScopedId(feedId, "triponservicedateid")
+    ).build();
+    TestStopLocation testStopLocation = new TestStopLocation(
+      new FeedScopedId(feedId, "stoplocationid")
+    );
+    Route route = TimetableRepositoryForTest.route(new FeedScopedId(feedId, "routeId")).build();
+    Trip trip = TimetableRepositoryForTest.trip(feedId, "tripId").build();
+    TripPattern tripPattern = TripPattern.of(new FeedScopedId(feedId, "tripPatternId"))
+      .withRoute(route)
+      .withStopPattern(TimetableRepositoryForTest.stopPattern(STOP_A, STOP_B))
+      .withScheduledTimeTableBuilder(builder ->
+        builder.addTripTimes(
+          ScheduledTripTimes.of().withTrip(trip).withDepartureTimes(new int[] { 0, 1 }).build()
+        )
+      )
+      .build();
+
+    Multimap<Route, TripPattern> realTimeAddedPatternsForRoute = ArrayListMultimap.create();
+    realTimeAddedPatternsForRoute.put(route, tripPattern);
+    ListMultimap<FeedScopedId, TripOnServiceDate> realTimeAddedReplacedByTripOnServiceDateById =
+      ArrayListMultimap.create();
+    realTimeAddedReplacedByTripOnServiceDateById.put(id, tripOnServiceDate);
+    SetMultimap<StopLocation, TripPattern> patternsForStop = HashMultimap.create();
+    patternsForStop.put(testStopLocation, tripPattern);
+
+    // The entries do not necessarily make sense, they are only for testing the clear method.
+    TimetableSnapshot snapshot = new TimetableSnapshot(
+      new HashMap<>(Map.of(id, ImmutableSortedSet.of())),
+      new HashMap<>(Map.of(tripIdAndServiceDate, tripPattern)),
+      new HashMap<>(Map.of(id, route)),
+      new HashMap<>(Map.of(id, trip)),
+      new HashMap<>(Map.of(trip, tripPattern)),
+      realTimeAddedPatternsForRoute,
+      new HashMap<>(Map.of(id, tripOnServiceDate)),
+      realTimeAddedReplacedByTripOnServiceDateById,
+      new HashMap<>(Map.of(tripIdAndServiceDate, tripOnServiceDate)),
+      patternsForStop,
+      false
+    );
+    assertFalse(snapshot.isEmpty());
+    snapshot.clear(id.getFeedId());
+    assertTrue(snapshot.isEmpty());
+  }
+
   private static RealTimeTripUpdate createRealTimeTripUpdate(TripPattern pattern, Trip trip) {
     TripTimes updatedTriptimes = TripTimesFactory.tripTimes(
       trip,