Merge pull request #6140 from Skanetrafiken/transit-model-index-immut…

…ability-1 Make TransitModelIndex more immutable
opentripplanner · Oct 11, 2024 · 1e8ff41 · 1e8ff41
2 parents bad81fc + 629562b
commit 1e8ff41
Show file tree

Hide file tree

Showing 7 changed files with 62 additions and 68 deletions.
diff --git a/src/main/java/org/opentripplanner/netex/NetexModule.java b/src/main/java/org/opentripplanner/netex/NetexModule.java
@@ -70,7 +70,7 @@ public void buildGraph() {
         );
         transitBuilder.limitServiceDays(transitPeriodLimit);
         for (var tripOnServiceDate : transitBuilder.getTripOnServiceDates().values()) {
-          transitModel.addTripOnServiceDate(tripOnServiceDate.getId(), tripOnServiceDate);
+          transitModel.addTripOnServiceDate(tripOnServiceDate);
         }
         calendarServiceData.add(transitBuilder.buildCalendarServiceData());
 

diff --git a/src/main/java/org/opentripplanner/transit/service/DefaultTransitService.java b/src/main/java/org/opentripplanner/transit/service/DefaultTransitService.java
@@ -234,12 +234,12 @@ public Collection<Trip> getTripsForStop(StopLocation stop) {
   @Override
   public Collection<Operator> getAllOperators() {
     OTPRequestTimeoutException.checkForTimeout();
-    return this.transitModelIndex.getAllOperators();
+    return this.transitModel.getOperators();
   }
 
   @Override
   public Operator getOperatorForId(FeedScopedId id) {
-    return this.transitModelIndex.getOperatorForId().get(id);
+    return this.transitModelIndex.getOperatorForId(id);
   }
 
   @Override
@@ -296,7 +296,7 @@ public Trip getTripForId(FeedScopedId id) {
   @Nullable
   @Override
   public Trip getScheduledTripForId(FeedScopedId id) {
-    return this.transitModelIndex.getTripForId().get(id);
+    return this.transitModelIndex.getTripForId(id);
   }
 
   @Override
@@ -305,11 +305,11 @@ public Collection<Trip> getAllTrips() {
     TimetableSnapshot currentSnapshot = lazyGetTimeTableSnapShot();
     if (currentSnapshot != null) {
       return new CollectionsView<>(
-        transitModelIndex.getTripForId().values(),
+        transitModelIndex.getAllTrips(),
         currentSnapshot.listRealTimeAddedTrips()
       );
     }
-    return Collections.unmodifiableCollection(transitModelIndex.getTripForId().values());
+    return Collections.unmodifiableCollection(transitModelIndex.getAllTrips());
   }
 
   @Override
@@ -322,7 +322,7 @@ public Collection<Route> getAllRoutes() {
         currentSnapshot.listRealTimeAddedRoutes()
       );
     }
-    return Collections.unmodifiableCollection(transitModelIndex.getAllRoutes());
+    return transitModelIndex.getAllRoutes();
   }
 
   @Override
@@ -334,7 +334,7 @@ public TripPattern getPatternForTrip(Trip trip) {
         return realtimeAddedTripPattern;
       }
     }
-    return this.transitModelIndex.getPatternForTrip().get(trip);
+    return this.transitModelIndex.getPatternForTrip(trip);
   }
 
   @Override
@@ -350,7 +350,7 @@ public TripPattern getPatternForTrip(Trip trip, LocalDate serviceDate) {
   public Collection<TripPattern> getPatternsForRoute(Route route) {
     OTPRequestTimeoutException.checkForTimeout();
     Collection<TripPattern> tripPatterns = new HashSet<>(
-      transitModelIndex.getPatternsForRoute().get(route)
+      transitModelIndex.getPatternsForRoute(route)
     );
     TimetableSnapshot currentSnapshot = lazyGetTimeTableSnapShot();
     if (currentSnapshot != null) {
@@ -491,18 +491,18 @@ public Collection<TripPattern> getPatternsForStop(
   @Override
   public Collection<GroupOfRoutes> getGroupsOfRoutes() {
     OTPRequestTimeoutException.checkForTimeout();
-    return transitModelIndex.getRoutesForGroupOfRoutes().keySet();
+    return transitModelIndex.getAllGroupOfRoutes();
   }
 
   @Override
   public Collection<Route> getRoutesForGroupOfRoutes(GroupOfRoutes groupOfRoutes) {
     OTPRequestTimeoutException.checkForTimeout();
-    return transitModelIndex.getRoutesForGroupOfRoutes().get(groupOfRoutes);
+    return transitModelIndex.getRoutesForGroupOfRoutes(groupOfRoutes);
   }
 
   @Override
   public GroupOfRoutes getGroupOfRoutesForId(FeedScopedId id) {
-    return transitModelIndex.getGroupOfRoutesForId().get(id);
+    return transitModelIndex.getGroupOfRoutesForId(id);
   }
 
   /**
@@ -551,31 +551,29 @@ private TimetableSnapshot lazyGetTimeTableSnapShot() {
   }
 
   @Override
-  public TripOnServiceDate getTripOnServiceDateById(FeedScopedId datedServiceJourneyId) {
+  public TripOnServiceDate getTripOnServiceDateById(FeedScopedId tripOnServiceDateId) {
     TimetableSnapshot currentSnapshot = lazyGetTimeTableSnapShot();
     if (currentSnapshot != null) {
       TripOnServiceDate tripOnServiceDate = currentSnapshot.getRealTimeAddedTripOnServiceDateById(
-        datedServiceJourneyId
+        tripOnServiceDateId
       );
       if (tripOnServiceDate != null) {
         return tripOnServiceDate;
       }
     }
-    return transitModelIndex.getTripOnServiceDateById().get(datedServiceJourneyId);
+    return transitModel.getTripOnServiceDateById(tripOnServiceDateId);
   }
 
   @Override
   public Collection<TripOnServiceDate> getAllTripOnServiceDates() {
     TimetableSnapshot currentSnapshot = lazyGetTimeTableSnapShot();
     if (currentSnapshot != null) {
       return new CollectionsView<>(
-        transitModelIndex.getTripOnServiceDateForTripAndDay().values(),
+        transitModel.getAllTripsOnServiceDates(),
         currentSnapshot.listRealTimeAddedTripOnServiceDate()
       );
     }
-    return Collections.unmodifiableCollection(
-      transitModelIndex.getTripOnServiceDateForTripAndDay().values()
-    );
+    return transitModel.getAllTripsOnServiceDates();
   }
 
   @Override
@@ -591,7 +589,7 @@ public TripOnServiceDate getTripOnServiceDateForTripAndDay(
         return tripOnServiceDate;
       }
     }
-    return transitModelIndex.getTripOnServiceDateForTripAndDay().get(tripIdAndServiceDate);
+    return transitModelIndex.getTripOnServiceDateForTripAndDay(tripIdAndServiceDate);
   }
 
   /**
@@ -603,12 +601,7 @@ public TripOnServiceDate getTripOnServiceDateForTripAndDay(
   @Override
   public List<TripOnServiceDate> getTripOnServiceDates(TripOnServiceDateRequest request) {
     Matcher<TripOnServiceDate> matcher = TripOnServiceDateMatcherFactory.of(request);
-    return transitModelIndex
-      .getTripOnServiceDateForTripAndDay()
-      .values()
-      .stream()
-      .filter(matcher::match)
-      .collect(Collectors.toList());
+    return getAllTripOnServiceDates().stream().filter(matcher::match).toList();
   }
 
   /**

diff --git a/src/main/java/org/opentripplanner/transit/service/TransitModel.java b/src/main/java/org/opentripplanner/transit/service/TransitModel.java
@@ -13,6 +13,7 @@
 import java.time.ZonedDateTime;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
@@ -406,9 +407,9 @@ public TripPattern getTripPatternForId(FeedScopedId id) {
     return tripPatternForId.get(id);
   }
 
-  public void addTripOnServiceDate(FeedScopedId id, TripOnServiceDate tripOnServiceDate) {
+  public void addTripOnServiceDate(TripOnServiceDate tripOnServiceDate) {
     invalidateIndex();
-    tripOnServiceDates.put(id, tripOnServiceDate);
+    tripOnServiceDates.put(tripOnServiceDate.getId(), tripOnServiceDate);
   }
 
   /**
@@ -442,8 +443,12 @@ public Collection<TripPattern> getAllTripPatterns() {
     return tripPatternForId.values();
   }
 
-  public Collection<TripOnServiceDate> getAllTripOnServiceDates() {
-    return tripOnServiceDates.values();
+  public TripOnServiceDate getTripOnServiceDateById(FeedScopedId tripOnServiceDateId) {
+    return tripOnServiceDates.get(tripOnServiceDateId);
+  }
+
+  public Collection<TripOnServiceDate> getAllTripsOnServiceDates() {
+    return Collections.unmodifiableCollection(tripOnServiceDates.values());
   }
 
   /**

diff --git a/src/main/java/org/opentripplanner/transit/service/TransitModelIndex.java b/src/main/java/org/opentripplanner/transit/service/TransitModelIndex.java
@@ -7,6 +7,7 @@
 import gnu.trove.set.hash.TIntHashSet;
 import java.time.LocalDate;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
@@ -47,10 +48,9 @@ class TransitModelIndex {
 
   private final Map<Trip, TripPattern> patternForTrip = new HashMap<>();
   private final Multimap<Route, TripPattern> patternsForRoute = ArrayListMultimap.create();
-  private final Multimap<StopLocation, TripPattern> patternsForStopId = ArrayListMultimap.create();
+  private final Multimap<StopLocation, TripPattern> patternsForStop = ArrayListMultimap.create();
 
   private final Map<LocalDate, TIntSet> serviceCodesRunningForDate = new HashMap<>();
-  private final Map<FeedScopedId, TripOnServiceDate> tripOnServiceDateById = new HashMap<>();
   private final Map<TripIdAndServiceDate, TripOnServiceDate> tripOnServiceDateForTripAndDay = new HashMap<>();
 
   private final Multimap<GroupOfRoutes, Route> routesForGroupOfRoutes = ArrayListMultimap.create();
@@ -78,7 +78,7 @@ class TransitModelIndex {
           tripForId.put(trip.getId(), trip);
         });
       for (StopLocation stop : pattern.getStops()) {
-        patternsForStopId.put(stop, pattern);
+        patternsForStop.put(stop, pattern);
       }
     }
     for (Route route : patternsForRoute.asMap().keySet()) {
@@ -91,8 +91,7 @@ class TransitModelIndex {
       groupOfRoutesForId.put(groupOfRoutes.getId(), groupOfRoutes);
     }
 
-    for (TripOnServiceDate tripOnServiceDate : transitModel.getAllTripOnServiceDates()) {
-      tripOnServiceDateById.put(tripOnServiceDate.getId(), tripOnServiceDate);
+    for (TripOnServiceDate tripOnServiceDate : transitModel.getAllTripsOnServiceDates()) {
       tripOnServiceDateForTripAndDay.put(
         new TripIdAndServiceDate(
           tripOnServiceDate.getTrip().getId(),
@@ -132,56 +131,50 @@ void addRoutes(Route route) {
   /** Dynamically generate the set of Routes passing though a Stop on demand. */
   Set<Route> getRoutesForStop(StopLocation stop) {
     Set<Route> routes = new HashSet<>();
-    for (TripPattern p : getPatternsForStop(stop)) {
+    for (TripPattern p : patternsForStop.get(stop)) {
       routes.add(p.getRoute());
     }
     return routes;
   }
 
   Collection<TripPattern> getPatternsForStop(StopLocation stop) {
-    return patternsForStopId.get(stop);
+    return Collections.unmodifiableCollection(patternsForStop.get(stop));
   }
 
   Collection<Trip> getTripsForStop(StopLocation stop) {
-    return getPatternsForStop(stop)
+    return patternsForStop
+      .get(stop)
       .stream()
       .flatMap(TripPattern::scheduledTripsAsStream)
       .collect(Collectors.toList());
   }
 
-  /**
-   * Get a list of all operators spanning across all feeds.
-   */
-  Collection<Operator> getAllOperators() {
-    return getOperatorForId().values();
+  Operator getOperatorForId(FeedScopedId operatorId) {
+    return operatorForId.get(operatorId);
   }
 
-  Map<FeedScopedId, Operator> getOperatorForId() {
-    return operatorForId;
+  Collection<Trip> getAllTrips() {
+    return Collections.unmodifiableCollection(tripForId.values());
   }
 
-  Map<FeedScopedId, Trip> getTripForId() {
-    return tripForId;
+  Trip getTripForId(FeedScopedId tripId) {
+    return tripForId.get(tripId);
   }
 
-  Map<FeedScopedId, TripOnServiceDate> getTripOnServiceDateById() {
-    return tripOnServiceDateById;
-  }
-
-  Map<TripIdAndServiceDate, TripOnServiceDate> getTripOnServiceDateForTripAndDay() {
-    return tripOnServiceDateForTripAndDay;
+  TripOnServiceDate getTripOnServiceDateForTripAndDay(TripIdAndServiceDate tripIdAndServiceDate) {
+    return tripOnServiceDateForTripAndDay.get(tripIdAndServiceDate);
   }
 
   Collection<Route> getAllRoutes() {
-    return routeForId.values();
+    return Collections.unmodifiableCollection(routeForId.values());
   }
 
-  Map<Trip, TripPattern> getPatternForTrip() {
-    return patternForTrip;
+  TripPattern getPatternForTrip(Trip trip) {
+    return patternForTrip.get(trip);
   }
 
-  Multimap<Route, TripPattern> getPatternsForRoute() {
-    return patternsForRoute;
+  Collection<TripPattern> getPatternsForRoute(Route route) {
+    return Collections.unmodifiableCollection(patternsForRoute.get(route));
   }
 
   Map<LocalDate, TIntSet> getServiceCodesRunningForDate() {
@@ -229,11 +222,15 @@ private void initalizeServiceCodesForDate(TransitModel transitModel) {
     }
   }
 
-  Multimap<GroupOfRoutes, Route> getRoutesForGroupOfRoutes() {
-    return routesForGroupOfRoutes;
+  Collection<GroupOfRoutes> getAllGroupOfRoutes() {
+    return Collections.unmodifiableCollection(groupOfRoutesForId.values());
+  }
+
+  Collection<Route> getRoutesForGroupOfRoutes(GroupOfRoutes groupOfRoutes) {
+    return Collections.unmodifiableCollection(routesForGroupOfRoutes.get(groupOfRoutes));
   }
 
-  Map<FeedScopedId, GroupOfRoutes> getGroupOfRoutesForId() {
-    return groupOfRoutesForId;
+  GroupOfRoutes getGroupOfRoutesForId(FeedScopedId id) {
+    return groupOfRoutesForId.get(id);
   }
 }
diff --git a/...st/java/org/opentripplanner/model/plan/legreference/ScheduledTransitLegReferenceTest.java b/...st/java/org/opentripplanner/model/plan/legreference/ScheduledTransitLegReferenceTest.java
@@ -87,7 +87,6 @@ static void buildTransitService() {
     transitModel.updateCalendarServiceData(true, calendarServiceData, DataImportIssueStore.NOOP);
 
     transitModel.addTripOnServiceDate(
-      TRIP_ON_SERVICE_DATE_ID,
       TripOnServiceDate
         .of(TRIP_ON_SERVICE_DATE_ID)
         .withTrip(trip)

diff --git a/src/test/java/org/opentripplanner/transit/service/TransitModelTest.java b/src/test/java/org/opentripplanner/transit/service/TransitModelTest.java
@@ -42,8 +42,8 @@ void validateTimeZones() {
 
     // Then trip times should be same as in input data
     TransitModelIndex transitModelIndex = transitModel.getTransitModelIndex();
-    Trip trip = transitModelIndex.getTripForId().get(SAMPLE_TRIP_ID);
-    Timetable timetable = transitModelIndex.getPatternForTrip().get(trip).getScheduledTimetable();
+    Trip trip = transitModelIndex.getTripForId(SAMPLE_TRIP_ID);
+    Timetable timetable = transitModelIndex.getPatternForTrip(trip).getScheduledTimetable();
     assertEquals(20 * 60, timetable.getTripTimes(trip).getDepartureTime(0));
 
     // Should throw on second bundle, with different agency time zone
@@ -101,8 +101,8 @@ void validateTimeZonesWithExplicitTimeZone() {
     assertEquals("America/Chicago", transitModel.getTimeZone().getId());
 
     // Then trip times should be on hour less than in input data
-    Trip trip = transitModelIndex.getTripForId().get(SAMPLE_TRIP_ID);
-    Timetable timetable = transitModelIndex.getPatternForTrip().get(trip).getScheduledTimetable();
+    Trip trip = transitModelIndex.getTripForId(SAMPLE_TRIP_ID);
+    Timetable timetable = transitModelIndex.getPatternForTrip(trip).getScheduledTimetable();
     assertEquals(20 * 60 - 60 * 60, timetable.getTripTimes(trip).getDepartureTime(0));
   }
 }
diff --git a/src/test/java/org/opentripplanner/updater/trip/RealtimeTestEnvironmentBuilder.java b/src/test/java/org/opentripplanner/updater/trip/RealtimeTestEnvironmentBuilder.java
@@ -65,7 +65,7 @@ private Trip createTrip(TripInput tripInput) {
       .withServiceDate(SERVICE_DATE)
       .build();
 
-    transitModel.addTripOnServiceDate(tripOnServiceDate.getId(), tripOnServiceDate);
+    transitModel.addTripOnServiceDate(tripOnServiceDate);
 
     if (tripInput.route().getOperator() != null) {
       transitModel.getOperators().add(tripInput.route().getOperator());