Merge pull request #6506 from leonardehrenfried/trip-index

Remove Timetable#getTripTimes(index)

application/src/ext-test/java/org/opentripplanner/ext/flex/trip/ScheduledDeviatedTripIntegrationTest.java

@@ -183,7 +183,7 @@ void shouldNotInterpolateFlexTimes() {
 
     assertEquals(3, pattern.numberOfStops());
 
-    var tripTimes = pattern.getScheduledTimetable().getTripTimes(0);
+    var tripTimes = pattern.getScheduledTimetable().getTripTimes().getFirst();
     var arrivalTime = tripTimes.getArrivalTime(1);
 
     assertEquals(StopTime.MISSING_VALUE, arrivalTime);

application/src/ext-test/java/org/opentripplanner/ext/realtimeresolver/RealtimeResolverTest.java

@@ -74,7 +74,8 @@ void testPopulateLegsWithRealtime() {
       .asScheduledTransitLeg()
       .getTripPattern()
       .getScheduledTimetable()
-      .getTripTimes(0)
+      .getTripTimes()
+      .getFirst()
       .getArrivalDelay(1);
     assertEquals(123, leg1ArrivalDelay);
     assertEquals(0, legs.get(0).getTransitAlerts().size());
@@ -128,7 +129,7 @@ void testPopulateLegsWithRealtimeKeepStaySeated() {
   private static TripPattern delay(TripPattern pattern1, int seconds) {
     var originalTimeTable = pattern1.getScheduledTimetable();
 
-    var delayedTripTimes = delay(originalTimeTable.getTripTimes(0), seconds);
+    var delayedTripTimes = delay(originalTimeTable.getTripTimes().getFirst(), seconds);
     var delayedTimetable = Timetable.of()
       .withTripPattern(pattern1)
       .addTripTimes(delayedTripTimes)
@@ -166,7 +167,7 @@ private static TransitService makeTransitService(
     patterns.forEach(pattern -> {
       timetableRepository.addTripPattern(pattern.getId(), pattern);
 
-      var serviceCode = pattern.getScheduledTimetable().getTripTimes(0).getServiceCode();
+      var serviceCode = pattern.getScheduledTimetable().getTripTimes().getFirst().getServiceCode();
       timetableRepository.getServiceCodes().put(pattern.getId(), serviceCode);
 
       calendarServiceData.putServiceDatesForServiceId(pattern.getId(), List.of(serviceDate));

application/src/main/java/org/opentripplanner/model/Timetable.java

@@ -52,39 +52,6 @@ public TimetableBuilder copyOf() {
     return new TimetableBuilder(this);
   }
 
-  /** @return the index of TripTimes for this trip ID in this particular Timetable */
-  public int getTripIndex(FeedScopedId tripId) {
-    int ret = 0;
-    for (TripTimes tt : tripTimes) {
-      // could replace linear search with indexing in stoptime updater, but not necessary
-      // at this point since the updater thread is far from pegged.
-      if (tt.getTrip().getId().equals(tripId)) {
-        return ret;
-      }
-      ret += 1;
-    }
-    return -1;
-  }
-
-  /**
-   * @return the index of TripTimes for this trip ID in this particular Timetable, ignoring
-   * AgencyIds.
-   */
-  public int getTripIndex(String tripId) {
-    int ret = 0;
-    for (TripTimes tt : tripTimes) {
-      if (tt.getTrip().getId().getId().equals(tripId)) {
-        return ret;
-      }
-      ret += 1;
-    }
-    return -1;
-  }
-
-  public TripTimes getTripTimes(int tripIndex) {
-    return tripTimes.get(tripIndex);
-  }
-
   @Nullable
   public TripTimes getTripTimes(Trip trip) {
     for (TripTimes tt : tripTimes) {

application/src/main/java/org/opentripplanner/transit/model/network/TripPattern.java

@@ -27,8 +27,6 @@
 import org.opentripplanner.transit.model.timetable.Direction;
 import org.opentripplanner.transit.model.timetable.Trip;
 import org.opentripplanner.transit.model.timetable.TripTimes;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 // TODO OTP2 instances of this class are still mutable after construction with a builder, this will be refactored in a subsequent step
 /**
@@ -58,8 +56,6 @@ public final class TripPattern
   extends AbstractTransitEntity<TripPattern, TripPatternBuilder>
   implements Cloneable, LogInfo {
 
-  private static final Logger LOG = LoggerFactory.getLogger(TripPattern.class);
-
   private final Route route;
 
   /**

application/src/main/java/org/opentripplanner/transit/service/DefaultTransitService.java

@@ -71,8 +71,6 @@
 import org.opentripplanner.updater.GraphUpdaterStatus;
 import org.opentripplanner.utils.collection.CollectionsView;
 import org.opentripplanner.utils.time.ServiceDateUtils;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 /**
  * Default implementation of the Transit Service and Transit Editor Service.
@@ -82,7 +80,6 @@
  */
 public class DefaultTransitService implements TransitEditorService {
 
-  private static final Logger LOG = LoggerFactory.getLogger(DefaultTransitService.class);
   private final TimetableRepository timetableRepository;
 
   private final TimetableRepositoryIndex timetableRepositoryIndex;

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

@@ -935,17 +935,15 @@ private boolean cancelScheduledTrip(
     if (pattern != null) {
       // Cancel scheduled trip times for this trip in this pattern
       final Timetable timetable = pattern.getScheduledTimetable();
-      final int tripIndex = timetable.getTripIndex(tripId);
-      if (tripIndex == -1) {
+      var tripTimes = timetable.getTripTimes(tripId);
+      if (tripTimes == null) {
         debug(
           tripId,
           serviceDate,
           "Could not cancel scheduled trip because it's not in the timetable"
         );
       } else {
-        final RealTimeTripTimes newTripTimes = timetable
-          .getTripTimes(tripIndex)
-          .copyScheduledTimes();
+        final RealTimeTripTimes newTripTimes = tripTimes.copyScheduledTimes();
         switch (cancelationType) {
           case CANCEL -> newTripTimes.cancelTrip();
           case DELETE -> newTripTimes.deleteTrip();
@@ -981,13 +979,11 @@ private boolean cancelPreviouslyAddedTrip(
     if (isPreviouslyAddedTrip(tripId, pattern, serviceDate)) {
       // Cancel trip times for this trip in this pattern
       final Timetable timetable = snapshotManager.resolve(pattern, serviceDate);
-      final int tripIndex = timetable.getTripIndex(tripId);
-      if (tripIndex == -1) {
+      var tripTimes = timetable.getTripTimes(tripId);
+      if (tripTimes == null) {
         debug(tripId, serviceDate, "Could not cancel previously added trip on {}", serviceDate);
       } else {
-        final RealTimeTripTimes newTripTimes = timetable
-          .getTripTimes(tripIndex)
-          .copyScheduledTimes();
+        final RealTimeTripTimes newTripTimes = tripTimes.copyScheduledTimes();
         switch (cancelationType) {
           case CANCEL -> newTripTimes.cancelTrip();
           case DELETE -> newTripTimes.deleteTrip();

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

@@ -82,15 +82,15 @@ public static Result<TripTimesPatch, UpdateError> createUpdatedTripTimesFromGTFS
 
     var feedScopedTripId = new FeedScopedId(timetable.getPattern().getFeedId(), tripId);
 
-    int tripIndex = timetable.getTripIndex(tripId);
-    if (tripIndex == -1) {
+    var tripTimes = timetable.getTripTimes(feedScopedTripId);
+    if (tripTimes == null) {
       LOG.debug("tripId {} not found in pattern.", tripId);
       return Result.failure(new UpdateError(feedScopedTripId, TRIP_NOT_FOUND_IN_PATTERN));
     } else {
-      LOG.trace("tripId {} found at index {} in timetable.", tripId, tripIndex);
+      LOG.trace("tripId {} found in timetable.", tripId);
     }
 
-    RealTimeTripTimes newTimes = timetable.getTripTimes(tripIndex).copyScheduledTimes();
+    RealTimeTripTimes newTimes = tripTimes.copyScheduledTimes();
     List<Integer> skippedStopIndices = new ArrayList<>();
 
     // The GTFS-RT reference specifies that StopTimeUpdates are sorted by stop_sequence.

application/src/test/java/org/opentripplanner/apis/transmodel/mapping/TripRequestMapperTest.java

@@ -87,7 +87,11 @@ public class TripRequestMapperTest implements PlanTestConstants {
     LocalDate serviceDate = itinerary.startTime().toLocalDate();
     patterns.forEach(pattern -> {
       timetableRepository.addTripPattern(pattern.getId(), pattern);
-      final int serviceCode = pattern.getScheduledTimetable().getTripTimes(0).getServiceCode();
+      final int serviceCode = pattern
+        .getScheduledTimetable()
+        .getTripTimes()
+        .getFirst()
+        .getServiceCode();
       timetableRepository.getServiceCodes().put(pattern.getId(), serviceCode);
       calendarServiceData.putServiceDatesForServiceId(pattern.getId(), List.of(serviceDate));
     });

application/src/test/java/org/opentripplanner/routing/stoptimes/StopTimesHelperTest.java

@@ -37,7 +37,7 @@ public static void setUp() throws Exception {
       transitService.getTrip(new FeedScopedId(feedId, "5.1"))
     );
     var tt = originalPattern.getScheduledTimetable();
-    var newTripTimes = tt.getTripTimes(0).copyScheduledTimes();
+    var newTripTimes = tt.getTripTimes().getFirst().copyScheduledTimes();
     newTripTimes.cancelTrip();
     pattern = originalPattern
       .copy()

application/src/test/java/org/opentripplanner/transit/model/network/grouppriority/TransitGroupPriorityServiceTest.java

@@ -99,11 +99,15 @@ void lookupTransitGroupIdByAgency() {
     assertEquals(EXP_GROUP_ID_BASE, subject.lookupTransitGroupPriorityId(nullTrip));
     assertEquals(
       EXP_GROUP_1,
-      subject.lookupTransitGroupPriorityId(busB2.getScheduledTimetable().getTripTimes(0).getTrip())
+      subject.lookupTransitGroupPriorityId(
+        busB2.getScheduledTimetable().getTripTimes().getFirst().getTrip()
+      )
     );
     assertEquals(
       EXP_GROUP_2,
-      subject.lookupTransitGroupPriorityId(railR3.getScheduledTimetable().getTripTimes(0).getTrip())
+      subject.lookupTransitGroupPriorityId(
+        railR3.getScheduledTimetable().getTripTimes().getFirst().getTrip()
+      )
     );
   }
 
@@ -129,7 +133,9 @@ void lookupTransitPriorityGroupIdByGlobalMode() {
     assertEquals(EXP_GROUP_ID_BASE, subject.lookupTransitGroupPriorityId(nullTrip));
     assertEquals(
       EXP_GROUP_2,
-      subject.lookupTransitGroupPriorityId(railR1.getScheduledTimetable().getTripTimes(0).getTrip())
+      subject.lookupTransitGroupPriorityId(
+        railR1.getScheduledTimetable().getTripTimes().getFirst().getTrip()
+      )
     );
   }
 

application/src/test/java/org/opentripplanner/updater/trip/gtfs/GtfsRealTimeTripUpdateAdapterTest.java

@@ -5,7 +5,9 @@
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNotSame;
+import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.opentripplanner.transit.model._data.TimetableRepositoryForTest.id;
 import static org.opentripplanner.updater.trip.UpdateIncrementality.DIFFERENTIAL;
 import static org.opentripplanner.updater.trip.gtfs.BackwardsDelayPropagationType.REQUIRED_NO_DATA;
 
@@ -202,35 +204,25 @@ public void testHandleModifiedTrip() {
 
       assertNotSame(originalTimetableForToday, originalTimetableScheduled);
 
-      final int originalTripIndexScheduled = originalTimetableScheduled.getTripIndex(
-        modifiedTripId
-      );
-      assertTrue(
-        originalTripIndexScheduled > -1,
-        "Original trip should be found in scheduled time table"
-      );
-      final TripTimes originalTripTimesScheduled = originalTimetableScheduled.getTripTimes(
-        originalTripIndexScheduled
+      var original = originalTimetableScheduled.getTripTimes(
+        new FeedScopedId(feedId, modifiedTripId)
       );
+      assertNotNull(original, "Original trip should be found in scheduled time table");
       assertFalse(
-        originalTripTimesScheduled.isCanceledOrDeleted(),
+        original.isCanceledOrDeleted(),
         "Original trip times should not be canceled in scheduled time table"
       );
-      assertEquals(RealTimeState.SCHEDULED, originalTripTimesScheduled.getRealTimeState());
+      assertEquals(RealTimeState.SCHEDULED, original.getRealTimeState());
 
-      final int originalTripIndexForToday = originalTimetableForToday.getTripIndex(modifiedTripId);
-      assertTrue(
-        originalTripIndexForToday > -1,
-        "Original trip should be found in time table for service date"
-      );
-      final TripTimes originalTripTimesForToday = originalTimetableForToday.getTripTimes(
-        originalTripIndexForToday
+      var originalTT = originalTimetableForToday.getTripTimes(
+        new FeedScopedId(feedId, modifiedTripId)
       );
+      assertNotNull(originalTT, "Original trip should be found in time table for service date");
       assertTrue(
-        originalTripTimesForToday.isDeleted(),
+        originalTT.isDeleted(),
         "Original trip times should be deleted in time table for service date"
       );
-      assertEquals(RealTimeState.DELETED, originalTripTimesForToday.getRealTimeState());
+      assertEquals(RealTimeState.DELETED, originalTT.getRealTimeState());
     }
 
     // New trip pattern
@@ -246,21 +238,12 @@ public void testHandleModifiedTrip() {
 
       assertNotSame(newTimetableForToday, newTimetableScheduled);
 
-      final int newTimetableForTodayModifiedTripIndex = newTimetableForToday.getTripIndex(
-        modifiedTripId
-      );
-      assertTrue(
-        newTimetableForTodayModifiedTripIndex > -1,
-        "New trip should be found in time table for service date"
-      );
-      assertEquals(
-        RealTimeState.MODIFIED,
-        newTimetableForToday.getTripTimes(newTimetableForTodayModifiedTripIndex).getRealTimeState()
-      );
+      var tripTimes = newTimetableForToday.getTripTimes(new FeedScopedId(feedId, modifiedTripId));
+      assertNotNull(tripTimes, "New trip should be found in time table for service date");
+      assertEquals(RealTimeState.MODIFIED, tripTimes.getRealTimeState());
 
-      assertEquals(
-        -1,
-        newTimetableScheduled.getTripIndex(modifiedTripId),
+      assertNull(
+        newTimetableScheduled.getTripTimes(id(modifiedTripId)),
         "New trip should not be found in scheduled time table"
       );
     }

application/src/test/java/org/opentripplanner/updater/trip/gtfs/TripTimesUpdaterTest.java

@@ -47,7 +47,7 @@ public class TripTimesUpdaterTest {
   private static Map<FeedScopedId, TripPattern> patternIndex;
   private static Timetable timetable;
   private static String feedId;
-  private static int trip_1_1_index;
+  private static FeedScopedId tripId;
 
   @BeforeAll
   public static void setUp() throws Exception {
@@ -61,9 +61,9 @@ public static void setUp() throws Exception {
       pattern.scheduledTripsAsStream().forEach(trip -> patternIndex.put(trip.getId(), pattern));
     }
 
-    TripPattern pattern = patternIndex.get(new FeedScopedId(feedId, TRIP_ID));
+    tripId = new FeedScopedId(feedId, TRIP_ID);
+    TripPattern pattern = patternIndex.get(tripId);
     timetable = pattern.getScheduledTimetable();
-    trip_1_1_index = timetable.getTripIndex(new FeedScopedId(feedId, TRIP_ID));
   }
 
   @Test
@@ -173,7 +173,7 @@ public void update() {
         .toEpochSecond()
     );
     var tripUpdate = tripUpdateBuilder.build();
-    assertEquals(20 * 60, timetable.getTripTimes(trip_1_1_index).getArrivalTime(2));
+    assertEquals(20 * 60, timetable.getTripTimes(tripId).getArrivalTime(2));
     var result = TripTimesUpdater.createUpdatedTripTimesFromGTFSRT(
       timetable,
       tripUpdate,
@@ -188,7 +188,7 @@ public void update() {
       var updatedTripTimes = p.getTripTimes();
       assertNotNull(updatedTripTimes);
       timetable = timetable.copyOf().addOrUpdateTripTimes(updatedTripTimes).build();
-      assertEquals(20 * 60 + 120, timetable.getTripTimes(trip_1_1_index).getArrivalTime(2));
+      assertEquals(20 * 60 + 120, timetable.getTripTimes(tripId).getArrivalTime(2));
     });
 
     // update trip arrival time incorrectly

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

@@ -2,10 +2,13 @@
 
 import static com.google.transit.realtime.GtfsRealtime.TripDescriptor.ScheduleRelationship.ADDED;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNotSame;
+import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertSame;
 import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.opentripplanner.transit.model._data.TimetableRepositoryForTest.id;
 import static org.opentripplanner.updater.spi.UpdateResultAssertions.assertSuccess;
 
 import de.mfdz.MfdzRealtimeExtensions.StopTimePropertiesExtension.DropOffPickupType;
@@ -128,12 +131,10 @@ private TripPattern assertAddedTrip(String tripId, RealtimeTestEnvironment env)
     var snapshot = env.getTimetableSnapshot();
 
     TransitService transitService = env.getTransitService();
-    Trip trip = transitService.getTrip(TimetableRepositoryForTest.id(ADDED_TRIP_ID));
+    Trip trip = transitService.getTrip(id(ADDED_TRIP_ID));
     assertNotNull(trip);
     assertNotNull(transitService.findPattern(trip));
-    assertNotNull(
-      transitService.getTripOnServiceDate(TimetableRepositoryForTest.id(ADDED_TRIP_ID))
-    );
+    assertNotNull(transitService.getTripOnServiceDate(id(ADDED_TRIP_ID)));
 
     var stopA = env.timetableRepository.getSiteRepository().getRegularStop(STOP_A1.getId());
     // Get the trip pattern of the added trip which goes through stopA
@@ -148,18 +149,12 @@ private TripPattern assertAddedTrip(String tripId, RealtimeTestEnvironment env)
 
     assertNotSame(forToday, schedule);
 
-    final int forTodayAddedTripIndex = forToday.getTripIndex(tripId);
-    assertTrue(
-      forTodayAddedTripIndex > -1,
-      "Added trip should be found in time table for service date"
-    );
-    assertEquals(
-      RealTimeState.ADDED,
-      forToday.getTripTimes(forTodayAddedTripIndex).getRealTimeState()
-    );
+    var tripTimes = forToday.getTripTimes(id(tripId));
+    assertNotNull(tripTimes, "Added trip should be found in time table for service date");
+    assertEquals(RealTimeState.ADDED, tripTimes.getRealTimeState());
 
-    final int scheduleTripIndex = schedule.getTripIndex(tripId);
-    assertEquals(-1, scheduleTripIndex, "Added trip should not be found in scheduled time table");
+    var scheduledTripTimes = schedule.getTripTimes(id(tripId));
+    assertNull(scheduledTripTimes, "Added trip should not be found in scheduled time table");
     return tripPattern;
   }
 }