Addressing review feedback: Implementing changes

src/main/java/ch/naviqore/raptor/GtfsRoutePartitioner.java

@@ -0,0 +1,105 @@
+package ch.naviqore.raptor;
+
+import ch.naviqore.gtfs.schedule.model.*;
+import lombok.AccessLevel;
+import lombok.Getter;
+import lombok.RequiredArgsConstructor;
+import lombok.extern.log4j.Log4j2;
+
+import java.util.*;
+import java.util.stream.Collectors;
+
+/**
+ * Splits the routes of a GTFS schedule into sub-routes. In a GTFS schedule, a route can have multiple trips with
+ * different stop sequences. This class groups trips with the same stop sequence into sub-routes and assigns them the
+ * parent route.
+ *
+ * @author munterfi
+ */
+@Log4j2
+public class GtfsRoutePartitioner {
+    private final Map<Route, Map<String, SubRoute>> subRoutes = new HashMap<>();
+
+    public GtfsRoutePartitioner(GtfsSchedule schedule) {
+        log.info("Partitioning GTFS schedule with {} routes into sub-routes", schedule.getRoutes().size());
+        schedule.getRoutes().values().forEach(this::processRoute);
+        log.info("Found {} sub-routes in schedule", subRoutes.values().stream().mapToInt(Map::size).sum());
+    }
+
+    private void processRoute(Route route) {
+        Map<String, SubRoute> sequenceKeyToSubRoute = new HashMap<>();
+        route.getTrips().forEach(trip -> {
+            String key = generateStopSequenceKey(trip);
+            SubRoute subRoute = sequenceKeyToSubRoute.computeIfAbsent(key,
+                    s -> new SubRoute(String.format("%s_sr%d", route.getId(), sequenceKeyToSubRoute.size() + 1), route,
+                            key, extractStopSequence(trip)));
+            subRoute.addTrip(trip);
+        });
+        subRoutes.put(route, sequenceKeyToSubRoute);
+        log.debug("Route {} split into {} sub-routes", route.getId(), sequenceKeyToSubRoute.size());
+    }
+
+    private String generateStopSequenceKey(Trip trip) {
+        return trip.getStopTimes().stream().map(t -> t.stop().getId()).collect(Collectors.joining("-"));
+    }
+
+    private List<Stop> extractStopSequence(Trip trip) {
+        List<Stop> sequence = new ArrayList<>();
+        for (StopTime stopTime : trip.getStopTimes()) {
+            sequence.add(stopTime.stop());
+        }
+        return sequence;
+    }
+
+    public List<SubRoute> getSubRoutes(Route route) {
+        Map<String, SubRoute> currentSubRoutes = subRoutes.get(route);
+        if (currentSubRoutes == null) {
+            throw new IllegalArgumentException("Route " + route.getId() + " not found in schedule");
+        }
+        return new ArrayList<>(currentSubRoutes.values());
+    }
+
+    public SubRoute getSubRoute(Trip trip) {
+        Map<String, SubRoute> currentSubRoutes = subRoutes.get(trip.getRoute());
+        if (currentSubRoutes == null) {
+            throw new IllegalArgumentException("Trip " + trip.getId() + " not found in schedule");
+        }
+        String key = generateStopSequenceKey(trip);
+        return currentSubRoutes.get(key);
+    }
+
+    /**
+     * A sub-route belongs to a route, but has a unique stop sequence.
+     */
+    @RequiredArgsConstructor
+    @Getter
+    public static class SubRoute {
+        private final String id;
+        private final Route route;
+        @Getter(AccessLevel.NONE)
+        private final String stopSequenceKey;
+        private final List<Stop> stopsSequence;
+        private final List<Trip> trips = new ArrayList<>();
+
+        private void addTrip(Trip trip) {
+            trips.add(trip);
+        }
+
+        @Override
+        public boolean equals(Object obj) {
+            if (obj == this) return true;
+            if (obj == null || obj.getClass() != this.getClass()) return false;
+            var that = (SubRoute) obj;
+            return Objects.equals(this.id, that.id);
+        }
+
+        @Override
+        public int hashCode() {
+            return Objects.hash(id);
+        }
+
+        public String toString() {
+            return "SubRoute[" + "id=" + id + ", " + "route=" + route + ", " + "stopSequence=" + stopSequenceKey + ']';
+        }
+    }
+}

src/main/java/ch/naviqore/raptor/GtfsToRaptorConverter.java

@@ -1,9 +1,11 @@
 package ch.naviqore.raptor;
 
-import ch.naviqore.gtfs.schedule.model.*;
+import ch.naviqore.gtfs.schedule.model.GtfsSchedule;
+import ch.naviqore.gtfs.schedule.model.Stop;
+import ch.naviqore.gtfs.schedule.model.StopTime;
+import ch.naviqore.gtfs.schedule.model.Trip;
 import ch.naviqore.raptor.model.Raptor;
 import ch.naviqore.raptor.model.RaptorBuilder;
-import lombok.RequiredArgsConstructor;
 import lombok.extern.log4j.Log4j2;
 
 import java.time.LocalDate;
@@ -16,34 +18,39 @@
  *
  * @author munterfi
  */
-@RequiredArgsConstructor
 @Log4j2
 public class GtfsToRaptorConverter {
 
+    private final Set<GtfsRoutePartitioner.SubRoute> subRoutes = new HashSet<>();
     private final Set<Stop> stops = new HashSet<>();
-    private final Set<Route> routes = new HashSet<>();
-    private final RaptorBuilder builder;
+    private final RaptorBuilder builder = Raptor.builder();
+    private final GtfsRoutePartitioner partitioner;
+    private final GtfsSchedule schedule;
 
-    public Raptor convert(GtfsSchedule schedule, LocalDate date) {
+    public GtfsToRaptorConverter(GtfsSchedule schedule) {
+        this.partitioner = new GtfsRoutePartitioner(schedule);
+        this.schedule = schedule;
+    }
+
+    public Raptor convert(LocalDate date) {
         List<Trip> activeTrips = schedule.getActiveTrips(date);
         log.info("Converting {} active trips from GTFS schedule to Raptor model", activeTrips.size());
         for (Trip trip : activeTrips) {
-            Route route = trip.getRoute();
-            if (!routes.contains(route)) {
-                routes.add(route);
-                builder.addRoute(route.getId());
-                // TODO: Add test for consistency of route stops. Since in GTFS are defined per trip, but Raptor
-                //  builder expects them to be the same for all trips of a route.
+            // Route route = trip.getRoute();
+            GtfsRoutePartitioner.SubRoute subRoute = partitioner.getSubRoute(trip);
+            if (!subRoutes.contains(subRoute)) {
+                subRoutes.add(subRoute);
+                builder.addRoute(subRoute.getId());
                 for (StopTime stopTime : trip.getStopTimes()) {
                     if (!stops.contains(stopTime.stop())) {
                         stops.add(stopTime.stop());
                         builder.addStop(stopTime.stop().getId());
                     }
-                    builder.addRouteStop(stopTime.stop().getId(), route.getId());
+                    builder.addRouteStop(stopTime.stop().getId(), subRoute.getId());
                 }
             }
             for (StopTime stopTime : trip.getStopTimes()) {
-                builder.addStopTime(stopTime.stop().getId(), route.getId(), stopTime.arrival().getTotalSeconds(),
+                builder.addStopTime(stopTime.stop().getId(), subRoute.getId(), stopTime.arrival().getTotalSeconds(),
                         stopTime.departure().getTotalSeconds());
             }
         }

src/test/java/ch/naviqore/Benchmark.java

@@ -55,7 +55,7 @@ private static GtfsSchedule initializeSchedule() throws IOException, Interrupted
     }
 
     private static Raptor initializeRaptor(GtfsSchedule schedule) throws InterruptedException {
-        Raptor raptor = new GtfsToRaptorConverter(Raptor.builder()).convert(schedule, DATE);
+        Raptor raptor = new GtfsToRaptorConverter(schedule).convert(DATE);
         manageResources();
         return raptor;
     }

src/test/java/ch/naviqore/gtfs/schedule/model/GtfsScheduleTest.java

@@ -120,14 +120,14 @@ void shouldReturnNextDeparturesOnWeekday() {
 
             // assert departures times are correct
             List<ServiceDayTime> expectedDepartures = List.of(ServiceDayTime.parse("08:00:00"),
-                    ServiceDayTime.parse("08:00:00"), ServiceDayTime.parse("08:09:00"),
+                    ServiceDayTime.parse("08:03:00"), ServiceDayTime.parse("08:09:00"),
                     ServiceDayTime.parse("08:09:00"), ServiceDayTime.parse("08:15:00"));
             assertThat(departures).hasSize(LIMIT)
                     .extracting(StopTime::departure)
                     .containsExactlyElementsOf(expectedDepartures);
 
             // assert trips are correct
-            List<String> expectedTripIds = List.of("route3_wd_f_16", "route3_wd_r_16", "route3_wd_f_17",
+            List<String> expectedTripIds = List.of("route3_wd_f_16", "route3_wd_r_17", "route3_wd_f_17",
                     "route3_wd_r_17", "route1_wd_f_7");
             List<String> tripIds = departures.stream().map(stopTime -> stopTime.trip().getId()).toList();
             assertThat(tripIds).containsExactlyElementsOf(expectedTripIds);
@@ -161,14 +161,14 @@ void shouldReturnNextDeparturesAfterMidnight() {
 
             // assert departures times are correct
             List<ServiceDayTime> expectedDepartures = List.of(ServiceDayTime.parse("24:00:00"),
-                    ServiceDayTime.parse("24:00:00"), ServiceDayTime.parse("24:09:00"),
+                    ServiceDayTime.parse("24:03:00"), ServiceDayTime.parse("24:09:00"),
                     ServiceDayTime.parse("24:09:00"), ServiceDayTime.parse("24:15:00"));
             assertThat(departures).hasSize(LIMIT)
                     .extracting(StopTime::departure)
                     .containsExactlyElementsOf(expectedDepartures);
 
             // assert trips are correct
-            List<String> expectedTripIds = List.of("route3_wd_f_80", "route3_wd_r_80", "route3_wd_f_81",
+            List<String> expectedTripIds = List.of("route3_wd_f_80", "route3_wd_r_81", "route3_wd_f_81",
                     "route3_wd_r_81", "route1_wd_f_39");
             List<String> tripIds = departures.stream().map(stopTime -> stopTime.trip().getId()).toList();
             assertThat(tripIds).containsExactlyElementsOf(expectedTripIds);

src/test/java/ch/naviqore/gtfs/schedule/model/GtfsScheduleTestBuilder.java

@@ -140,7 +140,7 @@ private void addTrips(Route route, boolean weekday, boolean reverse) {
             String tripId = String.format("%s_%s_%s_%s", route.id, weekdayPostfix, directionPostfix, ++tripCount);
             builder.addTrip(tripId, route.id, weekday ? "weekdays" : "weekends");
             int departureTime = tripDepartureTime;
-            for (String stopId : route.stops) {
+            for (String stopId : routeStops) {
                 builder.addStopTime(tripId, stopId, new ServiceDayTime(departureTime - dwellTime),
                         new ServiceDayTime(departureTime));
                 departureTime += travelTime + dwellTime;

src/test/java/ch/naviqore/raptor/GtfsRoutePartitionerTest.java

@@ -0,0 +1,45 @@
+package ch.naviqore.raptor;
+
+import ch.naviqore.gtfs.schedule.model.*;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+@ExtendWith(GtfsScheduleTestExtension.class)
+class GtfsRoutePartitionerTest {
+
+    private GtfsSchedule schedule;
+    private GtfsRoutePartitioner partitioner;
+
+    @BeforeEach
+    void setUp(GtfsScheduleTestBuilder builder) {
+        schedule = builder.withAddAgency()
+                .withAddCalendars()
+                .withAddCalendarDates()
+                .withAddInterCity()
+                .withAddUnderground()
+                .withAddBus()
+                .build();
+        partitioner = new GtfsRoutePartitioner(schedule);
+    }
+
+    @Test
+    void getSubRoutes() {
+        assertThat(partitioner.getSubRoutes(schedule.getRoutes().get("route1"))).as("SubRoutes").hasSize(2);
+        assertThat(partitioner.getSubRoutes(schedule.getRoutes().get("route2"))).as("SubRoutes").hasSize(1);
+        assertThat(partitioner.getSubRoutes(schedule.getRoutes().get("route3"))).as("SubRoutes").hasSize(2);
+    }
+
+    @Test
+    void getSubRoute() {
+        for (Route route : schedule.getRoutes().values()) {
+            for (Trip trip : route.getTrips()) {
+                GtfsRoutePartitioner.SubRoute subRoute = partitioner.getSubRoute(trip);
+                assertThat(subRoute).as("SubRoute for trip ID " + trip.getId() + " in route " + route.getId())
+                        .isNotNull();
+            }
+        }
+    }
+}

src/test/java/ch/naviqore/raptor/GtfsToRaptorConverterIT.java

@@ -13,6 +13,8 @@
 import java.nio.file.Path;
 import java.time.LocalDate;
 
+import static org.assertj.core.api.Assertions.assertThat;
+
 class GtfsToRaptorConverterIT {
 
     private static final LocalDate DATE = LocalDate.of(2009, 4, 26);
@@ -26,7 +28,8 @@ void setUp(@TempDir Path tempDir) throws IOException {
 
     @Test
     void shouldConvertGtfsScheduleToRaptor() {
-        GtfsToRaptorConverter mapper = new GtfsToRaptorConverter(Raptor.builder());
-        Raptor raptor = mapper.convert(schedule, DATE);
+        GtfsToRaptorConverter mapper = new GtfsToRaptorConverter(schedule);
+        Raptor raptor = mapper.convert(DATE);
+        assertThat(raptor).isNotNull();
     }
 }