From 189d7e5fb939184a6b4dbfe1ce09b240adcf68fa Mon Sep 17 00:00:00 2001 From: Merlin Unterfinger Date: Mon, 9 Dec 2024 17:29:26 +0100 Subject: [PATCH 01/18] FIX: 158 - Ensure GTFS and RAPTOR router have the same stop set - Add all stops to RAPTOR, even if they have no departures / stopping trips. - When transfers are generated, it could be that connections from or to stops with no trips are available via a footpath / transfer as first or last leg. --- .../raptor/convert/GtfsToRaptorConverter.java | 34 ++++++++----------- 1 file changed, 14 insertions(+), 20 deletions(-) diff --git a/src/main/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorConverter.java b/src/main/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorConverter.java index 2ba75df..67e897c 100644 --- a/src/main/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorConverter.java +++ b/src/main/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorConverter.java @@ -1,9 +1,6 @@ package ch.naviqore.service.gtfs.raptor.convert; -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.Transfer; +import ch.naviqore.gtfs.schedule.model.*; import ch.naviqore.gtfs.schedule.type.TransferType; import ch.naviqore.raptor.router.RaptorConfig; import ch.naviqore.raptor.router.RaptorRouter; @@ -47,7 +44,13 @@ public GtfsToRaptorConverter(GtfsSchedule schedule, List stopIds = subRoute.getStopsSequence().stream().map(Stop::getId).toList(); - for (String stopId : stopIds) { - if (!addedStops.contains(stopId)) { - builder.addStop(stopId); - addedStops.add(stopId); - } - } - // add sub route as raptor route + List stopIds = subRoute.getStopsSequence().stream().map(Stop::getId).toList(); builder.addRoute(subRoute.getId(), stopIds); // add trips of sub route @@ -88,11 +82,11 @@ private void addRoute(GtfsRoutePartitioner.SubRoute subRoute) { /** * Processes all types of transfers, ensuring the correct order of precedence: *

- * 1. Additional transfers: These transfers have the lowest priority and are processed first. - * 2. Parent-child derived transfers: If a transfer is defined between two parent stops (e.g., A to B), this method - * derives corresponding transfers for their child stops (e.g., A1, A2, ... to B1, B2, ...). - * 3. GTFS schedule-defined transfers: Transfers explicitly defined in the GTFS schedule (e.g., A1 to B2) take the - * highest priority and are applied last, thereby overwriting transfers previously derived from parent stops. + * 1. Additional transfers: These transfers have the lowest priority and are processed first. 2. Parent-child + * derived transfers: If a transfer is defined between two parent stops (e.g., A to B), this method derives + * corresponding transfers for their child stops (e.g., A1, A2, ... to B1, B2, ...). 3. GTFS schedule-defined + * transfers: Transfers explicitly defined in the GTFS schedule (e.g., A1 to B2) take the highest priority and are + * applied last, thereby overwriting transfers previously derived from parent stops. *

* The method ensures that all transfers, whether additional, derived, or explicitly defined, are handled in the * correct priority order. From 07c362cf9beec0ca49eb0b10884b908f6f060a90 Mon Sep 17 00:00:00 2001 From: Merlin Unterfinger Date: Sat, 4 Jan 2025 02:37:00 +0100 Subject: [PATCH 02/18] TEST: 158 - Adjust and refactor GtfsToRaptorConverterIT - Test passes when all stops are added to raptor, even if they have no departures. - The asserts are checking actual transfer ids or keys, which makes it easier to follow the test cases. - Clearly differentiate between same stop transfers and between stop transfers. - Suppress unchecked cast compiler warnings in test cases. --- .../convert/GtfsToRaptorConverterIT.java | 229 ++++++++++-------- 1 file changed, 125 insertions(+), 104 deletions(-) diff --git a/src/test/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorConverterIT.java b/src/test/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorConverterIT.java index d41a33f..fad724c 100644 --- a/src/test/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorConverterIT.java +++ b/src/test/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorConverterIT.java @@ -23,10 +23,7 @@ import java.nio.file.Path; import java.time.DayOfWeek; import java.time.LocalDate; -import java.util.EnumSet; -import java.util.List; -import java.util.Map; -import java.util.Set; +import java.util.*; import java.util.stream.Collectors; import static org.assertj.core.api.Assertions.assertThat; @@ -55,22 +52,23 @@ void shouldConvertGtfsScheduleToRaptor() { @Nested class ManualSchedule { - /* - All tests run with a fixed set of stops and routes in a GTFS schedule as shown below: - --------B1------------C1 - | - A---| (B) ------C (D) - | | - --------B2 -----| (C2) - - Route 1 passes through A - B1 - C1 - Route 2 passes through A - B2 - C - - Stops B, C2 and D have no departures/arrivals and should not be included in the raptor conversion. - Stops B and C are parents of stops B1, B2 and C1, C2, respectively. + /** + * All tests run with a fixed set of stops and routes in a GTFS schedule as shown below: + * 

+         *     |--------B1------------C1
+         *     |
+         * A---|       (B)      |-----C           (D)
+         *     |                |
+         *     |--------B2 -----|    (C2)
+         *
+ *
    + *
  • Route 1: Passes through A - B1 - C1
    • + *
  • Route 2: Passes through A - B2 - C
    • + *
+ * Stops B, C2 and D have no departures/arrivals. Stops B and C are parents of stops B1, B2 and C1, C2, + * respectively. */ - static RaptorBuilderData convertRaptor(List scheduleTransfers, List additionalTransfers) throws NoSuchFieldException, IllegalAccessException { @@ -113,110 +111,110 @@ static RaptorBuilderData convertRaptor(List scheduleTransfers, schedule.getStops().get(transfer.toStopId()), transfer.duration())) .collect(Collectors.toList()); - GtfsToRaptorConverter mapper = new GtfsToRaptorConverter(schedule, additionalTransfersList, + GtfsToRaptorConverter converter = new GtfsToRaptorConverter(schedule, additionalTransfersList, new RaptorConfig()); - mapper.convert(); + converter.convert(); - return new RaptorBuilderData(mapper); + return new RaptorBuilderData(converter); } @Test void noTransfers() throws NoSuchFieldException, IllegalAccessException { RaptorBuilderData data = convertRaptor(List.of(), List.of()); - data.assertNumStops(5); - data.assertNumSameStopTransfers(0); - data.assertNumNonSameStopTransfers(0); - List existingStops = List.of("A", "B1", "B2", "C", "C1"); - for (String existingStop : existingStops) { - data.assertStopExists(existingStop); - if (existingStop.equals("A")) { - data.assertStopHasNumRoutes(existingStop, 2); + + data.assertStops(Set.of("A", "B", "B1", "B2", "C", "C1", "C2", "D")); + data.assertSameStopTransfers(Set.of()); + data.assertBetweenStopTransfers(Set.of()); + + List stopsWithDepartures = List.of("A", "B1", "B2", "C", "C1"); + for (String stopWithDeparture : stopsWithDepartures) { + data.assertStopExists(stopWithDeparture); + if (stopWithDeparture.equals("A")) { + data.assertStopHasNumRoutes(stopWithDeparture, 2); } else { - data.assertStopHasNumRoutes(existingStop, 1); + data.assertStopHasNumRoutes(stopWithDeparture, 1); } } - // they do not have any active trips - List nonExistingStops = List.of("B", "C2", "D"); - for (String nonExistingStop : nonExistingStops) { - data.assertStopNotExists(nonExistingStop); + + // they do not have any departures, but should still exist in the raptor data + List stopsWithoutDepartures = List.of("B", "C2", "D"); + for (String stopWithoutDeparture : stopsWithoutDepartures) { + data.assertStopWithNoDeparturesExists(stopWithoutDeparture); } } @Test - void sameStopTransfersOnAllActiveStops() throws NoSuchFieldException, IllegalAccessException { + void sameStopTransfersOnAllStopsWithDepartures() throws NoSuchFieldException, IllegalAccessException { + List sameStopTransfers = List.of(new Transfer("A", "A", 120), new Transfer("B1", "B1", 120), + new Transfer("B2", "B2", 120), new Transfer("C", "C", 120), new Transfer("C1", "C1", 120)); + + RaptorBuilderData data = convertRaptor(sameStopTransfers, List.of()); + + data.assertStops(Set.of("A", "B", "B1", "B2", "C", "C1", "C2", "D")); + data.assertSameStopTransfers(Set.of("A-120", "B1-120", "B2-120", "C-120", "C1-120", "C2-120")); // since C is also a parent stop, additional transfer C1 -> C and C -> C1 will also be generated - RaptorBuilderData data = convertRaptor( - List.of(new Transfer("A", "A", 120), new Transfer("B1", "B1", 120), new Transfer("B2", "B2", 120), - new Transfer("C", "C", 120), new Transfer("C1", "C1", 120)), List.of()); - data.assertNumStops(5); - data.assertNumSameStopTransfers(5); - // no way to test further as the Raptor.Transfer is not public - data.assertNumNonSameStopTransfers(2); - List existingStops = List.of("A", "B1", "B2", "C", "C1"); - for (String existingStop : existingStops) { - data.assertStopExists(existingStop); - data.assertSameStopTransferDuration(existingStop, 120); - } + data.assertBetweenStopTransfers(Set.of("B-B1", "B-B2", "C-C1", "C-C2", "C1-C", "C1-C2", "C2-C", "C2-C1")); } @Test void sameStopTransfersOnParentStops() throws NoSuchFieldException, IllegalAccessException { - // since B is not active, but B1 and B2 are active, it should create B1-B1, B1-B2, B2-B2, B2-B1 - // and C and C1 are active thus will have C-C1, C-C, C1-C1, C1-C - // even though D is specified, it should not be included and the stop should not be created because it does - // not have any departures - RaptorBuilderData data = convertRaptor( - List.of(new Transfer("A", "A", 120), new Transfer("B", "B", 120), new Transfer("C", "C", 120), - new Transfer("D", "D", 120)), List.of()); - data.assertNumStops(5); - data.assertNumSameStopTransfers(5); - // no way to test further as the Raptor.Transfer is not public - data.assertNumNonSameStopTransfers(4); - List existingStops = List.of("A", "B1", "B2", "C", "C1"); - for (String existingStop : existingStops) { - data.assertStopExists(existingStop); - data.assertSameStopTransferDuration(existingStop, 120); - } + List sameStopTransfers = List.of(new Transfer("A", "A", 120), new Transfer("B", "B", 120), + new Transfer("C", "C", 120), new Transfer("D", "D", 120)); + + RaptorBuilderData data = convertRaptor(sameStopTransfers, List.of()); + + data.assertStops(Set.of("A", "B", "B1", "B2", "C", "C1", "C2", "D")); + data.assertSameStopTransfers( + Set.of("A-120", "B-120", "B1-120", "B2-120", "C-120", "C1-120", "C2-120", "D-120")); + + // D has no departures specified, it should be included but should not have any between stop transfers + // TODO: Do we want this behavior? + data.assertBetweenStopTransfers( + Set.of("B-B1", "B-B2", "B1-B", "B1-B2", "B2-B", "B2-B1", "C-C1", "C-C2", "C1-C", "C1-C2", "C2-C", + "C2-C1")); } @Test void sameStopTransfersOnParentAndChildStops() throws NoSuchFieldException, IllegalAccessException { + List sameStopTransfers = List.of(new Transfer("B", "B", 120), new Transfer("B1", "B1", 60), + new Transfer("B2", "B2", 60)); + + RaptorBuilderData data = convertRaptor(sameStopTransfers, List.of()); + + data.assertStops(Set.of("A", "B", "B1", "B2", "C", "C1", "C2", "D")); // since explicit child same stop transfers are defined, the transfer time between B1-B1 and B2-B2 should // be 60, whereas create B1-B2, B2-B1 should be 120 - RaptorBuilderData data = convertRaptor( - List.of(new Transfer("B", "B", 120), new Transfer("B1", "B1", 60), new Transfer("B2", "B2", 60)), - List.of()); - data.assertNumStops(5); - data.assertNumSameStopTransfers(2); - // no way to test further as the Raptor.Transfer is not public and make sure that transfer time is 120 - data.assertNumNonSameStopTransfers(2); - List stops = List.of("B1", "B2"); - for (String stop : stops) { - data.assertStopExists(stop); - data.assertSameStopTransferDuration(stop, 60); - } + data.assertSameStopTransfers(Set.of("B-120", "B1-60", "B2-60")); + data.assertBetweenStopTransfers(Set.of("B-B1", "B-B2", "B1-B", "B1-B2", "B2-B", "B2-B1")); } @Test void betweenStopTransfersOnParentStops() throws NoSuchFieldException, IllegalAccessException { + List scheduleTransfers = List.of(new Transfer("B", "C", 120), new Transfer("C", "B", 120)); + + RaptorBuilderData data = convertRaptor(scheduleTransfers, List.of()); + + data.assertStops(Set.of("A", "B", "B1", "B2", "C", "C1", "C2", "D")); + data.assertSameStopTransfers(Set.of()); + // since B1, B2, C, and C1 are active following transfers should be derived from B-C: - // B1-C, B1-C1, B2-C, B2-C1, C-B1, C-B2, C1-B1, C1-B2 - RaptorBuilderData data = convertRaptor(List.of(new Transfer("B", "C", 120), new Transfer("C", "B", 120)), - List.of()); - data.assertNumSameStopTransfers(0); - data.assertNumNonSameStopTransfers(8); + data.assertBetweenStopTransfers( + Set.of("B-C", "B-C1", "B-C2", "B1-C", "B1-C1", "B1-C2", "B2-C", "B2-C1", "B2-C2", "C-B", "C-B1", + "C-B2", "C1-B", "C1-B1", "C1-B2", "C2-B", "C2-B1", "C2-B2")); } @Test void additionalTransfers() throws NoSuchFieldException, IllegalAccessException { - RaptorBuilderData data = convertRaptor(List.of(new Transfer("B1", "B1", 120)), - List.of(new Transfer("B1", "B1", 60), new Transfer("B2", "B2", 60), new Transfer("B1", "B2", 120))); - data.assertNumStops(5); - data.assertNumSameStopTransfers(2); - data.assertNumNonSameStopTransfers(1); + List scheduleTransfers = List.of(new Transfer("B1", "B1", 120)); + List additionalTransfers = List.of(new Transfer("B1", "B1", 60), new Transfer("B2", "B2", 60), + new Transfer("B1", "B2", 120)); + + RaptorBuilderData data = convertRaptor(scheduleTransfers, additionalTransfers); + + data.assertStops(Set.of("A", "B", "B1", "B2", "C", "C1", "C2", "D")); // since additional transfers should not be applied if gtfs data exists B1-B1 should remain 120 - data.assertSameStopTransferDuration("B1", 120); - data.assertSameStopTransferDuration("B2", 60); + data.assertSameStopTransfers(Set.of("B1-120", "B2-60")); + data.assertBetweenStopTransfers(Set.of("B-B1", "B1-B2")); } record Transfer(String fromStopId, String toStopId, int duration) { @@ -225,9 +223,11 @@ record Transfer(String fromStopId, String toStopId, int duration) { static class RaptorBuilderData { Map stops; - Map sameStopTransfers; Map> stopRoutes; + List betweenStopTransferIds; + List sameStopTransferKeys; + int stopTimeSize; int routeStopSize; int transferSize; @@ -236,14 +236,39 @@ static class RaptorBuilderData { RaptorRouterBuilder raptorBuilder = getPrivateField(converter, "builder"); stops = getPrivateField(raptorBuilder, "stops"); - sameStopTransfers = getPrivateField(raptorBuilder, "sameStopTransfers"); stopRoutes = getPrivateField(raptorBuilder, "stopRoutes"); + betweenStopTransferIds = getBetweenStopTransferIds(raptorBuilder); + sameStopTransferKeys = getSameStopTransferKeys(raptorBuilder); + stopTimeSize = getPrivateField(raptorBuilder, "stopTimeSize"); routeStopSize = getPrivateField(raptorBuilder, "routeStopSize"); transferSize = getPrivateField(raptorBuilder, "transferSize"); } + private static List getSameStopTransferKeys( + RaptorRouterBuilder raptorBuilder) throws NoSuchFieldException, IllegalAccessException { + Map sameStopTransfers = getPrivateField(raptorBuilder, "sameStopTransfers"); + List keys = new ArrayList<>(); + sameStopTransfers.forEach((key, value) -> keys.add(key + "-" + value)); + + return keys; + } + + @SuppressWarnings("unchecked") + private static List getBetweenStopTransferIds( + RaptorRouterBuilder raptorBuilder) throws NoSuchFieldException, IllegalAccessException { + Map transfers = getPrivateField(raptorBuilder, "transfers"); + + List betweenStopTransferIds = new ArrayList<>(); + for (var entry : transfers.entrySet()) { + Map transfersAtCurrentStop = (Map) entry.getValue(); + betweenStopTransferIds.addAll(transfersAtCurrentStop.keySet()); + } + + return betweenStopTransferIds; + } + /** * A generic method to get the value of a private field from an object. * @@ -254,8 +279,9 @@ static class RaptorBuilderData { * @throws NoSuchFieldException If the field does not exist. * @throws IllegalAccessException If the field is not accessible. */ - public static T getPrivateField(Object object, - String fieldName) throws NoSuchFieldException, IllegalAccessException { + @SuppressWarnings("unchecked") + private static T getPrivateField(Object object, + String fieldName) throws NoSuchFieldException, IllegalAccessException { // Get the class of the object Class clazz = object.getClass(); @@ -269,16 +295,16 @@ public static T getPrivateField(Object object, return (T) field.get(object); } - void assertNumStops(int numStops) { - assertThat(stops.size()).isEqualTo(numStops); + void assertStops(Set ids) { + assertThat(stops.keySet()).containsExactlyInAnyOrderElementsOf(ids); } void assertStopExists(String stopId) { assertThat(stops.containsKey(stopId)).isTrue(); } - void assertStopNotExists(String stopId) { - assertThat(stops.containsKey(stopId)).isFalse(); + void assertStopWithNoDeparturesExists(String stopId) { + assertThat(stops.containsKey(stopId)).isTrue(); } void assertStopHasNumRoutes(String stopId, int numRoutes) { @@ -286,17 +312,12 @@ void assertStopHasNumRoutes(String stopId, int numRoutes) { assertThat(stopRoutes.get(stopId).size()).isEqualTo(numRoutes); } - void assertNumNonSameStopTransfers(int numTransfers) { - assertThat(transferSize).isEqualTo(numTransfers); - } - - void assertNumSameStopTransfers(int numTransfers) { - assertThat(sameStopTransfers.size()).isEqualTo(numTransfers); + void assertSameStopTransfers(Set keys) { + assertThat(sameStopTransferKeys).containsExactlyInAnyOrderElementsOf(keys); } - void assertSameStopTransferDuration(String stopId, int duration) { - assertThat(sameStopTransfers.containsKey(stopId)).isTrue(); - assertThat(sameStopTransfers.get(stopId)).isEqualTo(duration); + void assertBetweenStopTransfers(Set ids) { + assertThat(betweenStopTransferIds).containsExactlyInAnyOrderElementsOf(ids); } } From 027e908ff0e8d6f0ecbef4f5567ee78e65b08795 Mon Sep 17 00:00:00 2001 From: Merlin Unterfinger Date: Sat, 4 Jan 2025 02:49:15 +0100 Subject: [PATCH 03/18] TEST: 158 - Improve test case comment --- .../service/gtfs/raptor/convert/GtfsToRaptorConverterIT.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorConverterIT.java b/src/test/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorConverterIT.java index fad724c..c2a9288 100644 --- a/src/test/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorConverterIT.java +++ b/src/test/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorConverterIT.java @@ -167,8 +167,8 @@ void sameStopTransfersOnParentStops() throws NoSuchFieldException, IllegalAccess data.assertSameStopTransfers( Set.of("A-120", "B-120", "B1-120", "B2-120", "C-120", "C1-120", "C2-120", "D-120")); - // D has no departures specified, it should be included but should not have any between stop transfers - // TODO: Do we want this behavior? + // D has no departures and not transfers specified, it should be included in the raptor data as stop, + // but should not have any between stop transfers data.assertBetweenStopTransfers( Set.of("B-B1", "B-B2", "B1-B", "B1-B2", "B2-B", "B2-B1", "C-C1", "C-C2", "C1-C", "C1-C2", "C2-C", "C2-C1")); From e293533927c9897d95d0662f5c0091cba259d156 Mon Sep 17 00:00:00 2001 From: Merlin Unterfinger Date: Sat, 4 Jan 2025 02:56:13 +0100 Subject: [PATCH 04/18] STYLE: 158 - Format javadoc --- .../raptor/convert/GtfsToRaptorConverter.java | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/main/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorConverter.java b/src/main/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorConverter.java index 67e897c..67ac63b 100644 --- a/src/main/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorConverter.java +++ b/src/main/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorConverter.java @@ -81,12 +81,21 @@ private void addRoute(GtfsRoutePartitioner.SubRoute subRoute) { /** * Processes all types of transfers, ensuring the correct order of precedence: - *

- * 1. Additional transfers: These transfers have the lowest priority and are processed first. 2. Parent-child - * derived transfers: If a transfer is defined between two parent stops (e.g., A to B), this method derives - * corresponding transfers for their child stops (e.g., A1, A2, ... to B1, B2, ...). 3. GTFS schedule-defined - * transfers: Transfers explicitly defined in the GTFS schedule (e.g., A1 to B2) take the highest priority and are - * applied last, thereby overwriting transfers previously derived from parent stops. + *

    + *
  1. + * Additional transfers: These transfers have the lowest priority and are processed first. + *
    2. + *
  2. + * Parent-child derived transfers: If a transfer is defined between two parent stops + * (e.g., A to B), this method derives corresponding transfers for their child stops + * (e.g., A1, A2, ... to B1, B2, ...). + *
    3. + *
  3. + * GTFS schedule-defined transfers: Transfers explicitly defined in the GTFS schedule + * (e.g., A1 to B2) take the highest priority. These transfers are applied last, + * overwriting any transfers previously derived from parent stops. + *
    4. + *
*

* The method ensures that all transfers, whether additional, derived, or explicitly defined, are handled in the * correct priority order. From 8dbf6aa89acea375c8696ab89586e644855f600b Mon Sep 17 00:00:00 2001 From: Merlin Unterfinger Date: Fri, 10 Jan 2025 11:12:21 +0100 Subject: [PATCH 05/18] ENH: 158 - Reuse existing logic for GTFS to RAPTOR conversion - The RAPTOR should not contain all stops, due to the parent stops, which often have not got any departures but would increase the stop array size significantly. --- .../raptor/convert/GtfsToRaptorConverter.java | 42 ++++++++----------- 1 file changed, 18 insertions(+), 24 deletions(-) diff --git a/src/main/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorConverter.java b/src/main/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorConverter.java index 67ac63b..080401f 100644 --- a/src/main/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorConverter.java +++ b/src/main/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorConverter.java @@ -44,12 +44,6 @@ public GtfsToRaptorConverter(GtfsSchedule schedule, List stopIds = subRoute.getStopsSequence().stream().map(Stop::getId).toList(); + for (String stopId : stopIds) { + if (!addedStops.contains(stopId)) { + builder.addStop(stopId); + addedStops.add(stopId); + } + } + + // add sub route as raptor route builder.addRoute(subRoute.getId(), stopIds); // add trips of sub route - for (var trip : subRoute.getTrips()) { + for (Trip trip : subRoute.getTrips()) { builder.addTrip(trip.getId(), subRoute.getId()); List stopTimes = trip.getStopTimes(); for (int i = 0; i < stopTimes.size(); i++) { @@ -81,21 +84,12 @@ private void addRoute(GtfsRoutePartitioner.SubRoute subRoute) { /** * Processes all types of transfers, ensuring the correct order of precedence: - *

    - *
  1. - * Additional transfers: These transfers have the lowest priority and are processed first. - *
    2. - *
  2. - * Parent-child derived transfers: If a transfer is defined between two parent stops - * (e.g., A to B), this method derives corresponding transfers for their child stops - * (e.g., A1, A2, ... to B1, B2, ...). - *
    3. - *
  3. - * GTFS schedule-defined transfers: Transfers explicitly defined in the GTFS schedule - * (e.g., A1 to B2) take the highest priority. These transfers are applied last, - * overwriting any transfers previously derived from parent stops. - *
    4. - *
+ *

+ * 1. Additional transfers: These transfers have the lowest priority and are processed first. 2. Parent-child + * derived transfers: If a transfer is defined between two parent stops (e.g., A to B), this method derives + * corresponding transfers for their child stops (e.g., A1, A2, ... to B1, B2, ...). 3. GTFS schedule-defined + * transfers: Transfers explicitly defined in the GTFS schedule (e.g., A1 to B2) take the highest priority and are + * applied last, thereby overwriting transfers previously derived from parent stops. *

* The method ensures that all transfers, whether additional, derived, or explicitly defined, are handled in the * correct priority order. @@ -251,4 +245,4 @@ private Collection expandTransfersFromStop(Stop stop return parentTransfers.values(); } -} +} \ No newline at end of file From 5d0d015aa92c5daa8e856829e1c6f0435a692433 Mon Sep 17 00:00:00 2001 From: Merlin Unterfinger Date: Fri, 10 Jan 2025 11:13:36 +0100 Subject: [PATCH 06/18] TEST: 158 - Adjust converter test Now only stops with departures are added to the raptor. --- .../convert/GtfsToRaptorConverterIT.java | 54 +++++++++---------- 1 file changed, 24 insertions(+), 30 deletions(-) diff --git a/src/test/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorConverterIT.java b/src/test/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorConverterIT.java index c2a9288..42dddbe 100644 --- a/src/test/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorConverterIT.java +++ b/src/test/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorConverterIT.java @@ -66,8 +66,8 @@ class ManualSchedule { *

  • Route 1: Passes through A - B1 - C1
    • *
  • Route 2: Passes through A - B2 - C
    • * - * Stops B, C2 and D have no departures/arrivals. Stops B and C are parents of stops B1, B2 and C1, C2, - * respectively. + * Stops B, C2 and D have no departures/arrivals and should not be included in the raptor conversion. + * Stops B and C are parents of stops B1, B2 and C1, C2, respectively. */ static RaptorBuilderData convertRaptor(List scheduleTransfers, List additionalTransfers) throws NoSuchFieldException, IllegalAccessException { @@ -121,12 +121,12 @@ static RaptorBuilderData convertRaptor(List scheduleTransfers, @Test void noTransfers() throws NoSuchFieldException, IllegalAccessException { RaptorBuilderData data = convertRaptor(List.of(), List.of()); + Set stopsWithDepartures = Set.of("A", "B1", "B2", "C", "C1"); - data.assertStops(Set.of("A", "B", "B1", "B2", "C", "C1", "C2", "D")); + data.assertStops(stopsWithDepartures); data.assertSameStopTransfers(Set.of()); data.assertBetweenStopTransfers(Set.of()); - List stopsWithDepartures = List.of("A", "B1", "B2", "C", "C1"); for (String stopWithDeparture : stopsWithDepartures) { data.assertStopExists(stopWithDeparture); if (stopWithDeparture.equals("A")) { @@ -136,10 +136,10 @@ void noTransfers() throws NoSuchFieldException, IllegalAccessException { } } - // they do not have any departures, but should still exist in the raptor data - List stopsWithoutDepartures = List.of("B", "C2", "D"); + // they do not have any departures, and therefore should not be included in the raptor data + Set stopsWithoutDepartures = Set.of("B", "C2", "D"); for (String stopWithoutDeparture : stopsWithoutDepartures) { - data.assertStopWithNoDeparturesExists(stopWithoutDeparture); + data.assertStopWithNoDeparturesNotExists(stopWithoutDeparture); } } @@ -150,10 +150,10 @@ void sameStopTransfersOnAllStopsWithDepartures() throws NoSuchFieldException, Il RaptorBuilderData data = convertRaptor(sameStopTransfers, List.of()); - data.assertStops(Set.of("A", "B", "B1", "B2", "C", "C1", "C2", "D")); - data.assertSameStopTransfers(Set.of("A-120", "B1-120", "B2-120", "C-120", "C1-120", "C2-120")); + data.assertStops(Set.of("A", "B1", "B2", "C", "C1")); + data.assertSameStopTransfers(Set.of("A-120", "B1-120", "B2-120", "C-120", "C1-120")); // since C is also a parent stop, additional transfer C1 -> C and C -> C1 will also be generated - data.assertBetweenStopTransfers(Set.of("B-B1", "B-B2", "C-C1", "C-C2", "C1-C", "C1-C2", "C2-C", "C2-C1")); + data.assertBetweenStopTransfers(Set.of("C-C1", "C1-C")); } @Test @@ -163,15 +163,11 @@ void sameStopTransfersOnParentStops() throws NoSuchFieldException, IllegalAccess RaptorBuilderData data = convertRaptor(sameStopTransfers, List.of()); - data.assertStops(Set.of("A", "B", "B1", "B2", "C", "C1", "C2", "D")); - data.assertSameStopTransfers( - Set.of("A-120", "B-120", "B1-120", "B2-120", "C-120", "C1-120", "C2-120", "D-120")); + data.assertStops(Set.of("A", "B1", "B2", "C", "C1")); + data.assertSameStopTransfers(Set.of("A-120", "B1-120", "B2-120", "C-120", "C1-120")); - // D has no departures and not transfers specified, it should be included in the raptor data as stop, - // but should not have any between stop transfers - data.assertBetweenStopTransfers( - Set.of("B-B1", "B-B2", "B1-B", "B1-B2", "B2-B", "B2-B1", "C-C1", "C-C2", "C1-C", "C1-C2", "C2-C", - "C2-C1")); + // B is not active, but B1 and B2 are active and C and C1 are active: + data.assertBetweenStopTransfers(Set.of("B1-B2", "B2-B1", "C-C1", "C1-C")); } @Test @@ -181,11 +177,11 @@ void sameStopTransfersOnParentAndChildStops() throws NoSuchFieldException, Illeg RaptorBuilderData data = convertRaptor(sameStopTransfers, List.of()); - data.assertStops(Set.of("A", "B", "B1", "B2", "C", "C1", "C2", "D")); + data.assertStops(Set.of("A", "B1", "B2", "C", "C1")); // since explicit child same stop transfers are defined, the transfer time between B1-B1 and B2-B2 should - // be 60, whereas create B1-B2, B2-B1 should be 120 - data.assertSameStopTransfers(Set.of("B-120", "B1-60", "B2-60")); - data.assertBetweenStopTransfers(Set.of("B-B1", "B-B2", "B1-B", "B1-B2", "B2-B", "B2-B1")); + // be 60: + data.assertSameStopTransfers(Set.of("B1-60", "B2-60")); + data.assertBetweenStopTransfers(Set.of("B1-B2", "B2-B1")); } @Test @@ -194,13 +190,11 @@ void betweenStopTransfersOnParentStops() throws NoSuchFieldException, IllegalAcc RaptorBuilderData data = convertRaptor(scheduleTransfers, List.of()); - data.assertStops(Set.of("A", "B", "B1", "B2", "C", "C1", "C2", "D")); + data.assertStops(Set.of("A", "B1", "B2", "C", "C1")); data.assertSameStopTransfers(Set.of()); // since B1, B2, C, and C1 are active following transfers should be derived from B-C: - data.assertBetweenStopTransfers( - Set.of("B-C", "B-C1", "B-C2", "B1-C", "B1-C1", "B1-C2", "B2-C", "B2-C1", "B2-C2", "C-B", "C-B1", - "C-B2", "C1-B", "C1-B1", "C1-B2", "C2-B", "C2-B1", "C2-B2")); + data.assertBetweenStopTransfers(Set.of("B1-C", "B1-C1", "B2-C", "B2-C1", "C-B1", "C-B2", "C1-B1", "C1-B2")); } @Test @@ -211,10 +205,10 @@ void additionalTransfers() throws NoSuchFieldException, IllegalAccessException { RaptorBuilderData data = convertRaptor(scheduleTransfers, additionalTransfers); - data.assertStops(Set.of("A", "B", "B1", "B2", "C", "C1", "C2", "D")); + data.assertStops(Set.of("A", "B1", "B2", "C", "C1")); // since additional transfers should not be applied if gtfs data exists B1-B1 should remain 120 data.assertSameStopTransfers(Set.of("B1-120", "B2-60")); - data.assertBetweenStopTransfers(Set.of("B-B1", "B1-B2")); + data.assertBetweenStopTransfers(Set.of("B1-B2")); } record Transfer(String fromStopId, String toStopId, int duration) { @@ -303,8 +297,8 @@ void assertStopExists(String stopId) { assertThat(stops.containsKey(stopId)).isTrue(); } - void assertStopWithNoDeparturesExists(String stopId) { - assertThat(stops.containsKey(stopId)).isTrue(); + void assertStopWithNoDeparturesNotExists(String stopId) { + assertThat(stops.containsKey(stopId)).isFalse(); } void assertStopHasNumRoutes(String stopId, int numRoutes) { From 281c420d21cf90eeaaede1f936af0a03162f5d27 Mon Sep 17 00:00:00 2001 From: Merlin Unterfinger Date: Fri, 10 Jan 2025 18:46:02 +0100 Subject: [PATCH 07/18] TEST: 158 - Only add stops with departures to raptor, catch exceptions from router in service - Move transfer generation to convert package. - Pass generators to converter instead of transfers. - Generator uses a stop collection to build transfers instead of the complete schedule. - Service catches IllegalArgumentException of raptor router and returns an empty list. - Introduce GtfsToRaptorTestSchedule to reuse the ManualSchedule. - Rename isoLines to isolines, be consistent. --- .../app/controller/RoutingController.java | 4 +- .../service/PublicTransitSpringService.java | 8 +- .../service/ConnectionRoutingService.java | 4 +- .../gtfs/raptor/GtfsRaptorService.java | 35 +- .../raptor/GtfsRaptorServiceInitializer.java | 88 ++- .../raptor/convert/GtfsToRaptorConverter.java | 102 ++-- .../raptor/convert/TransferGenerator.java | 30 + .../WalkTransferGenerator.java | 19 +- .../raptor/transfer/TransferGenerator.java | 30 - src/test/java/ch/naviqore/Benchmark.java | 2 +- .../naviqore/app/controller/DummyService.java | 4 +- .../gtfs/raptor/GtfsRaptorServiceIT.java | 521 +++++++++++------- .../GtfsRaptorServiceInitializerIT.java | 46 +- .../convert/GtfsToRaptorConverterIT.java | 62 +-- .../convert/GtfsToRaptorTestSchedule.java | 73 +++ .../WalkTransferGeneratorTest.java | 20 +- 16 files changed, 590 insertions(+), 458 deletions(-) create mode 100644 src/main/java/ch/naviqore/service/gtfs/raptor/convert/TransferGenerator.java rename src/main/java/ch/naviqore/service/gtfs/raptor/{transfer => convert}/WalkTransferGenerator.java (87%) delete mode 100644 src/main/java/ch/naviqore/service/gtfs/raptor/transfer/TransferGenerator.java create mode 100644 src/test/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorTestSchedule.java rename src/test/java/ch/naviqore/service/gtfs/raptor/{transfer => convert}/WalkTransferGeneratorTest.java (96%) diff --git a/src/main/java/ch/naviqore/app/controller/RoutingController.java b/src/main/java/ch/naviqore/app/controller/RoutingController.java index 0b4d9a2..27d2035 100644 --- a/src/main/java/ch/naviqore/app/controller/RoutingController.java +++ b/src/main/java/ch/naviqore/app/controller/RoutingController.java @@ -143,10 +143,10 @@ public List getIsolines(@RequestParam(required = false) String s // determine routing case and get isolines try { if (sourceStop != null) { - return map(service.getIsoLines(sourceStop, dateTime, map(timeType), config), timeType, + return map(service.getIsolines(sourceStop, dateTime, map(timeType), config), timeType, returnConnections); } else { - return map(service.getIsoLines(sourceCoordinate, dateTime, map(timeType), config), timeType, + return map(service.getIsolines(sourceCoordinate, dateTime, map(timeType), config), timeType, returnConnections); } } catch (ConnectionRoutingException e) { diff --git a/src/main/java/ch/naviqore/app/service/PublicTransitSpringService.java b/src/main/java/ch/naviqore/app/service/PublicTransitSpringService.java index 9815e0d..43a50c5 100644 --- a/src/main/java/ch/naviqore/app/service/PublicTransitSpringService.java +++ b/src/main/java/ch/naviqore/app/service/PublicTransitSpringService.java @@ -122,15 +122,15 @@ public List getConnections(GeoCoordinate source, Stop target, LocalD } @Override - public Map getIsoLines(GeoCoordinate source, LocalDateTime time, TimeType timeType, + public Map getIsolines(GeoCoordinate source, LocalDateTime time, TimeType timeType, ConnectionQueryConfig config) throws ConnectionRoutingException { - return delegate.getIsoLines(source, time, timeType, config); + return delegate.getIsolines(source, time, timeType, config); } @Override - public Map getIsoLines(Stop source, LocalDateTime time, TimeType timeType, + public Map getIsolines(Stop source, LocalDateTime time, TimeType timeType, ConnectionQueryConfig config) throws ConnectionRoutingException { - return delegate.getIsoLines(source, time, timeType, config); + return delegate.getIsolines(source, time, timeType, config); } @Override diff --git a/src/main/java/ch/naviqore/service/ConnectionRoutingService.java b/src/main/java/ch/naviqore/service/ConnectionRoutingService.java index 652498e..c1af52e 100644 --- a/src/main/java/ch/naviqore/service/ConnectionRoutingService.java +++ b/src/main/java/ch/naviqore/service/ConnectionRoutingService.java @@ -79,7 +79,7 @@ List getConnections(Stop source, GeoCoordinate target, LocalDateTime * @param config additional configuration for the query * @return a map of stops to the shortest possible connection to each stop from the departure location */ - Map getIsoLines(GeoCoordinate source, LocalDateTime time, TimeType timeType, + Map getIsolines(GeoCoordinate source, LocalDateTime time, TimeType timeType, ConnectionQueryConfig config) throws ConnectionRoutingException; /** @@ -92,6 +92,6 @@ Map getIsoLines(GeoCoordinate source, LocalDateTime time, Time * @param config additional configuration for the query * @return a map of stops to the shortest possible connection to each stop from the departure location */ - Map getIsoLines(Stop source, LocalDateTime time, TimeType timeType, + Map getIsolines(Stop source, LocalDateTime time, TimeType timeType, ConnectionQueryConfig config) throws ConnectionRoutingException; } diff --git a/src/main/java/ch/naviqore/service/gtfs/raptor/GtfsRaptorService.java b/src/main/java/ch/naviqore/service/gtfs/raptor/GtfsRaptorService.java index 71c9e74..2b2b26f 100644 --- a/src/main/java/ch/naviqore/service/gtfs/raptor/GtfsRaptorService.java +++ b/src/main/java/ch/naviqore/service/gtfs/raptor/GtfsRaptorService.java @@ -1,17 +1,12 @@ package ch.naviqore.service.gtfs.raptor; import ch.naviqore.gtfs.schedule.model.GtfsSchedule; -import ch.naviqore.raptor.router.RaptorConfig; import ch.naviqore.raptor.router.RaptorRouter; import ch.naviqore.service.*; import ch.naviqore.service.config.ConnectionQueryConfig; import ch.naviqore.service.config.ServiceConfig; import ch.naviqore.service.exception.*; -import ch.naviqore.service.gtfs.raptor.convert.GtfsToRaptorConverter; -import ch.naviqore.service.gtfs.raptor.convert.GtfsTripMaskProvider; -import ch.naviqore.service.gtfs.raptor.transfer.TransferGenerator; import ch.naviqore.service.walk.WalkCalculator; -import ch.naviqore.utils.cache.EvictionCache; import ch.naviqore.utils.search.SearchIndex; import ch.naviqore.utils.spatial.GeoCoordinate; import ch.naviqore.utils.spatial.index.KDTree; @@ -40,23 +35,14 @@ public class GtfsRaptorService implements PublicTransitService { GtfsRaptorService(ServiceConfig config, GtfsSchedule schedule, KDTree spatialStopIndex, SearchIndex stopSearchIndex, WalkCalculator walkCalculator, - List additionalTransfers) { + RaptorRouter raptorRouter) { this.config = config; this.schedule = schedule; this.validity = new GtfsRaptorValidity(schedule); this.spatialStopIndex = spatialStopIndex; this.stopSearchIndex = stopSearchIndex; this.walkCalculator = walkCalculator; - - EvictionCache.Strategy cacheStrategy = EvictionCache.Strategy.valueOf(config.getCacheEvictionStrategy().name()); - GtfsTripMaskProvider tripMaskProvider = new GtfsTripMaskProvider(schedule, config.getCacheServiceDaySize(), - cacheStrategy); - - // build raptor algorithm - RaptorConfig raptorConfig = new RaptorConfig(config.getRaptorDaysToScan(), config.getRaptorRange(), - config.getTransferTimeSameStopDefault(), config.getCacheServiceDaySize(), cacheStrategy, - tripMaskProvider); - raptorRouter = new GtfsToRaptorConverter(schedule, additionalTransfers, raptorConfig).convert(); + this.raptorRouter = raptorRouter; } @Override @@ -226,7 +212,10 @@ private List getConnections(@Nullable ch.naviqore.gtfs.schedule.mode connections = raptorRouter.routeLatestDeparture(targetStops, sourceStops, TypeMapper.map(config)); } } catch (IllegalArgumentException e) { - throw new ConnectionRoutingException(e); + log.debug("RaptorRouter exception: {}", e.getMessage()); + return List.of(); + // TODO: Introduce exception handling or exception type on in raptor router. + // throw new ConnectionRoutingException(e); } // assemble connection results @@ -305,7 +294,7 @@ public Map getAllChildStopsFromStop(Stop stop) { } @Override - public Map getIsoLines(GeoCoordinate source, LocalDateTime time, TimeType timeType, + public Map getIsolines(GeoCoordinate source, LocalDateTime time, TimeType timeType, ConnectionQueryConfig config) throws ConnectionRoutingException { Map sourceStops = getStopsWithWalkTimeFromLocation(source, time, config.getMaximumWalkingDuration(), timeType); @@ -320,12 +309,14 @@ public Map getIsoLines(GeoCoordinate source, LocalDateTime tim raptorRouter.routeIsolines(sourceStops, TypeMapper.map(timeType), TypeMapper.map(config)), source, config, timeType); } catch (IllegalArgumentException e) { - throw new ConnectionRoutingException(e); + log.debug(e.getMessage()); + return Map.of(); + // TODO: throw new ConnectionRoutingException(e); } } @Override - public Map getIsoLines(Stop source, LocalDateTime time, TimeType timeType, + public Map getIsolines(Stop source, LocalDateTime time, TimeType timeType, ConnectionQueryConfig config) throws ConnectionRoutingException { Map sourceStops = getAllChildStopsFromStop(source, time); @@ -334,7 +325,9 @@ public Map getIsoLines(Stop source, LocalDateTime time, TimeTy raptorRouter.routeIsolines(sourceStops, TypeMapper.map(timeType), TypeMapper.map(config)), null, config, timeType); } catch (IllegalArgumentException e) { - throw new ConnectionRoutingException(e); + log.debug(e.getMessage()); + return Map.of(); + // TODO: throw new ConnectionRoutingException(e); } } diff --git a/src/main/java/ch/naviqore/service/gtfs/raptor/GtfsRaptorServiceInitializer.java b/src/main/java/ch/naviqore/service/gtfs/raptor/GtfsRaptorServiceInitializer.java index 456e6fb..3f06088 100644 --- a/src/main/java/ch/naviqore/service/gtfs/raptor/GtfsRaptorServiceInitializer.java +++ b/src/main/java/ch/naviqore/service/gtfs/raptor/GtfsRaptorServiceInitializer.java @@ -2,26 +2,27 @@ import ch.naviqore.gtfs.schedule.model.GtfsSchedule; import ch.naviqore.gtfs.schedule.model.Stop; -import ch.naviqore.gtfs.schedule.type.TransferType; -import ch.naviqore.service.PublicTransitService; +import ch.naviqore.raptor.router.RaptorConfig; +import ch.naviqore.raptor.router.RaptorRouter; import ch.naviqore.service.config.ServiceConfig; -import ch.naviqore.service.gtfs.raptor.transfer.TransferGenerator; -import ch.naviqore.service.gtfs.raptor.transfer.WalkTransferGenerator; +import ch.naviqore.service.gtfs.raptor.convert.GtfsToRaptorConverter; +import ch.naviqore.service.gtfs.raptor.convert.GtfsTripMaskProvider; +import ch.naviqore.service.gtfs.raptor.convert.TransferGenerator; +import ch.naviqore.service.gtfs.raptor.convert.WalkTransferGenerator; import ch.naviqore.service.walk.BeeLineWalkCalculator; import ch.naviqore.service.walk.WalkCalculator; +import ch.naviqore.utils.cache.EvictionCache; import ch.naviqore.utils.search.SearchIndex; import ch.naviqore.utils.search.SearchIndexBuilder; import ch.naviqore.utils.spatial.index.KDTree; import ch.naviqore.utils.spatial.index.KDTreeBuilder; -import lombok.AccessLevel; -import lombok.Getter; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; -import java.util.*; +import java.util.Collections; +import java.util.List; @RequiredArgsConstructor -@Getter(AccessLevel.PACKAGE) @Slf4j public class GtfsRaptorServiceInitializer { @@ -30,23 +31,20 @@ public class GtfsRaptorServiceInitializer { private final WalkCalculator walkCalculator; private final SearchIndex stopSearchIndex; private final KDTree spatialStopIndex; - private final List additionalTransfers; + private final RaptorRouter raptorRouter; public GtfsRaptorServiceInitializer(ServiceConfig config, GtfsSchedule schedule) { + log.debug("Initializing with config: {}", config); this.config = config; this.schedule = schedule; - log.debug("Initializing with config: {}", config); this.walkCalculator = initializeWalkCalculator(config); - this.stopSearchIndex = generateStopSearchIndex(schedule); - this.spatialStopIndex = generateSpatialStopIndex(schedule); + this.stopSearchIndex = createStopSearchIndex(schedule); + this.spatialStopIndex = createSpatialStopIndex(schedule); // generate transfers if minimum transfer time is not negative; usually -1 to deactivate generators - if (config.getTransferTimeBetweenStopsMinimum() >= 0) { - this.additionalTransfers = generateTransfers(schedule, - createTransferGenerators(config, walkCalculator, spatialStopIndex)); - } else { - this.additionalTransfers = new ArrayList<>(); - } + List transferGenerators = config.getTransferTimeBetweenStopsMinimum() >= 0 ? createTransferGenerators( + config, walkCalculator, spatialStopIndex) : Collections.emptyList(); + this.raptorRouter = createRaptorRouter(config, schedule, transferGenerators); } private static WalkCalculator initializeWalkCalculator(ServiceConfig config) { @@ -56,7 +54,7 @@ private static WalkCalculator initializeWalkCalculator(ServiceConfig config) { }; } - private static SearchIndex generateStopSearchIndex(GtfsSchedule schedule) { + private static SearchIndex createStopSearchIndex(GtfsSchedule schedule) { SearchIndexBuilder builder = SearchIndex.builder(); // only add parent stops and stops without a parent @@ -69,53 +67,33 @@ private static SearchIndex generateStopSearchIndex(GtfsSchedule schedule) return builder.build(); } - private static KDTree generateSpatialStopIndex(GtfsSchedule schedule) { + private static KDTree createSpatialStopIndex(GtfsSchedule schedule) { return new KDTreeBuilder().addLocations(schedule.getStops().values()).build(); } private static List createTransferGenerators(ServiceConfig config, WalkCalculator walkCalculator, KDTree spatialStopIndex) { - ArrayList generators = new ArrayList<>(); - - // add between stops walking transfer generator to generator list - generators.add(new WalkTransferGenerator(walkCalculator, config.getTransferTimeBetweenStopsMinimum(), + return List.of(new WalkTransferGenerator(walkCalculator, config.getTransferTimeBetweenStopsMinimum(), config.getTransferTimeAccessEgress(), config.getWalkingSearchRadius(), spatialStopIndex)); - - return generators; } - private static List generateTransfers(GtfsSchedule schedule, - List transferGenerators) { - // create lookup for GTFS transfers in schedule to prevent adding duplicates later - Set gtfsTransfers = new HashSet<>(); - schedule.getStops().values().forEach(stop -> stop.getTransfers().forEach(transfer -> { - if (transfer.getTransferType() == TransferType.MINIMUM_TIME) { - String key = transfer.getFromStop().getId() + transfer.getToStop().getId(); - gtfsTransfers.add(key); - } - })); - - // run all generators in sequence and collect all generated transfers - List uncheckedGeneratedTransfers = transferGenerators.stream() - .flatMap(generator -> generator.generateTransfers(schedule).stream()) - .toList(); - - // add all generated Transfers to the Lookup if they are not already in the GTFS Transfers or - // where already generated by a preceding generator - Map generatedTransfers = new HashMap<>(); - for (TransferGenerator.Transfer transfer : uncheckedGeneratedTransfers) { - String key = transfer.from().getId() + transfer.to().getId(); - if (!gtfsTransfers.contains(key) && !generatedTransfers.containsKey(key)) { - generatedTransfers.put(key, transfer); - } - } + private static RaptorRouter createRaptorRouter(ServiceConfig config, GtfsSchedule schedule, + List transferGenerators) { + // setup cache and trip mask provider + EvictionCache.Strategy cacheStrategy = EvictionCache.Strategy.valueOf(config.getCacheEvictionStrategy().name()); + GtfsTripMaskProvider tripMaskProvider = new GtfsTripMaskProvider(schedule, config.getCacheServiceDaySize(), + cacheStrategy); + + // configure raptor + RaptorConfig raptorConfig = new RaptorConfig(config.getRaptorDaysToScan(), config.getRaptorRange(), + config.getTransferTimeSameStopDefault(), config.getCacheServiceDaySize(), cacheStrategy, + tripMaskProvider); - return new ArrayList<>(generatedTransfers.values()); + return new GtfsToRaptorConverter(raptorConfig, schedule, transferGenerators).run(); } - public PublicTransitService get() { - return new GtfsRaptorService(config, schedule, spatialStopIndex, stopSearchIndex, walkCalculator, - additionalTransfers); + public GtfsRaptorService get() { + return new GtfsRaptorService(config, schedule, spatialStopIndex, stopSearchIndex, walkCalculator, raptorRouter); } } diff --git a/src/main/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorConverter.java b/src/main/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorConverter.java index 080401f..07f19a3 100644 --- a/src/main/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorConverter.java +++ b/src/main/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorConverter.java @@ -5,7 +5,6 @@ import ch.naviqore.raptor.router.RaptorConfig; import ch.naviqore.raptor.router.RaptorRouter; import ch.naviqore.raptor.router.RaptorRouterBuilder; -import ch.naviqore.service.gtfs.raptor.transfer.TransferGenerator; import lombok.extern.slf4j.Slf4j; import java.util.*; @@ -23,25 +22,26 @@ @Slf4j public class GtfsToRaptorConverter { - private final Set addedStops = new HashSet<>(); - private final RaptorRouterBuilder builder; - private final GtfsRoutePartitioner partitioner; - private final List additionalTransfers; + private final Set addedStops = new HashSet<>(); private final GtfsSchedule schedule; + private final List transferGenerators; - public GtfsToRaptorConverter(GtfsSchedule schedule, RaptorConfig config) { - this(schedule, List.of(), config); + private final GtfsRoutePartitioner partitioner; + private final RaptorRouterBuilder builder; + + public GtfsToRaptorConverter(RaptorConfig config, GtfsSchedule schedule) { + this(config, schedule, List.of()); } - public GtfsToRaptorConverter(GtfsSchedule schedule, List additionalTransfers, - RaptorConfig config) { - this.partitioner = new GtfsRoutePartitioner(schedule); - this.additionalTransfers = additionalTransfers; + public GtfsToRaptorConverter(RaptorConfig config, GtfsSchedule schedule, + List transferGenerators) { this.schedule = schedule; + this.transferGenerators = transferGenerators; + this.partitioner = new GtfsRoutePartitioner(schedule); this.builder = RaptorRouter.builder(config); } - public RaptorRouter convert() { + public RaptorRouter run() { log.info("Converting {} trips from GTFS schedule to Raptor data model", schedule.getTrips().size()); for (Route route : schedule.getRoutes().values()) { @@ -59,16 +59,15 @@ public RaptorRouter convert() { private void addRoute(GtfsRoutePartitioner.SubRoute subRoute) { // add stops of sub route that are not already added - List stopIds = subRoute.getStopsSequence().stream().map(Stop::getId).toList(); - for (String stopId : stopIds) { - if (!addedStops.contains(stopId)) { - builder.addStop(stopId); - addedStops.add(stopId); + for (Stop stop : subRoute.getStopsSequence()) { + if (!addedStops.contains(stop)) { + builder.addStop(stop.getId()); + addedStops.add(stop); } } // add sub route as raptor route - builder.addRoute(subRoute.getId(), stopIds); + builder.addRoute(subRoute.getId(), subRoute.getStopsSequence().stream().map(Stop::getId).toList()); // add trips of sub route for (Trip trip : subRoute.getTrips()) { @@ -85,26 +84,61 @@ private void addRoute(GtfsRoutePartitioner.SubRoute subRoute) { /** * Processes all types of transfers, ensuring the correct order of precedence: *

    - * 1. Additional transfers: These transfers have the lowest priority and are processed first. 2. Parent-child - * derived transfers: If a transfer is defined between two parent stops (e.g., A to B), this method derives - * corresponding transfers for their child stops (e.g., A1, A2, ... to B1, B2, ...). 3. GTFS schedule-defined - * transfers: Transfers explicitly defined in the GTFS schedule (e.g., A1 to B2) take the highest priority and are - * applied last, thereby overwriting transfers previously derived from parent stops. + *

      + *
    1. + * Additional transfers: These transfers have the lowest priority and are processed first. + *
      2. + *
    2. + * Parent-child derived transfers: If a transfer is defined between two parent stops + * (e.g., A to B), this method derives corresponding transfers for their child stops + * (e.g., A1, A2, ... to B1, B2, ...). + *
      3. + *
    3. + * GTFS schedule-defined transfers: Transfers explicitly defined in the GTFS schedule + * (e.g., A1 to B2) take the highest priority. These transfers are applied last, + * overwriting any transfers previously derived from parent stops. + *
      4. + *
    *

    * The method ensures that all transfers, whether additional, derived, or explicitly defined, are handled in the * correct priority order. */ private void processAllTransfers() { - addAdditionalTransfers(); + createAndAddAdditionalTransfers(); processStopAndParentChildTransfers(); addGtfsTransfersWithPrecedence(); } /** - * Adds all additional transfers. + * Create and add additional transfers. */ - private void addAdditionalTransfers() { - for (TransferGenerator.Transfer transfer : additionalTransfers) { + private void createAndAddAdditionalTransfers() { + // create lookup for GTFS transfers in schedule to prevent adding duplicates later + Set gtfsTransfers = new HashSet<>(); + schedule.getStops().values().forEach(stop -> stop.getTransfers().forEach(transfer -> { + if (transfer.getTransferType() == TransferType.MINIMUM_TIME) { + String key = transfer.getFromStop().getId() + transfer.getToStop().getId(); + gtfsTransfers.add(key); + } + })); + + // run all generators in sequence and collect all generated transfers + List uncheckedGeneratedTransfers = transferGenerators.stream() + .flatMap(generator -> generator.generateTransfers(addedStops).stream()) + .toList(); + + // add all generated Transfers to the Lookup if they are not already in the GTFS Transfers or + // where already generated by a preceding generator + Map generatedTransfers = new HashMap<>(); + for (TransferGenerator.Transfer transfer : uncheckedGeneratedTransfers) { + String key = transfer.from().getId() + transfer.to().getId(); + if (!gtfsTransfers.contains(key) && !generatedTransfers.containsKey(key)) { + generatedTransfers.put(key, transfer); + } + } + + // add generated transfers to builder + for (TransferGenerator.Transfer transfer : generatedTransfers.values()) { builder.addTransfer(transfer.from().getId(), transfer.to().getId(), transfer.duration()); } } @@ -113,8 +147,7 @@ private void addAdditionalTransfers() { * Processes transfers for each stop and handles parent-child relationships. */ private void processStopAndParentChildTransfers() { - for (String stopId : addedStops) { - Stop stop = schedule.getStops().get(stopId); + for (Stop stop : addedStops) { processParentAndChildTransfersForStop(stop); } } @@ -176,7 +209,7 @@ private void addToStopChildrenTransfers(Stop stop) { for (Stop toChildStop : stopTransfer.getToStop().getChildren()) { // only add new transfers if the to stop also has departures, else the raptor router does not care // about this stop and the builder will throw an exception. - if (addedStops.contains(toChildStop.getId())) { + if (addedStops.contains(toChildStop)) { builder.addTransfer(stop.getId(), toChildStop.getId(), stopTransfer.getMinTransferTime().get()); } } @@ -188,13 +221,12 @@ private void addToStopChildrenTransfers(Stop stop) { * Adds transfers explicitly defined in the GTFS schedule, ensuring precedence over additional transfers. */ private void addGtfsTransfersWithPrecedence() { - for (String stopId : addedStops) { - Stop stop = schedule.getStops().get(stopId); + for (Stop stop : addedStops) { for (Transfer transfer : stop.getTransfers()) { // only add new transfers if the to stop also has departures, else the raptor router does not care about // this stop and the builder will throw an exception. if (transfer.getTransferType() == TransferType.MINIMUM_TIME && transfer.getMinTransferTime() - .isPresent() && addedStops.contains(transfer.getToStop().getId())) { + .isPresent() && addedStops.contains(transfer.getToStop())) { builder.addTransfer(stop.getId(), transfer.getToStop().getId(), transfer.getMinTransferTime().get()); } @@ -226,11 +258,11 @@ private Collection expandTransfersFromStop(Stop stop Stop toStop = transfer.getToStop(); // only add new transfers if the to stop also has departures, else the raptor router does not care about // this stop and the builder will throw an exception. - if (addedStops.contains(toStop.getId())) { + if (addedStops.contains(toStop)) { otherTransfers.add(new TransferGenerator.Transfer(stop, toStop, transfer.getMinTransferTime().get())); } for (Stop childToStop : toStop.getChildren()) { - if (addedStops.contains(childToStop.getId())) { + if (addedStops.contains(childToStop)) { parentTransfers.put(childToStop, new TransferGenerator.Transfer(stop, childToStop, transfer.getMinTransferTime().get())); } diff --git a/src/main/java/ch/naviqore/service/gtfs/raptor/convert/TransferGenerator.java b/src/main/java/ch/naviqore/service/gtfs/raptor/convert/TransferGenerator.java new file mode 100644 index 0000000..82bb00b --- /dev/null +++ b/src/main/java/ch/naviqore/service/gtfs/raptor/convert/TransferGenerator.java @@ -0,0 +1,30 @@ +package ch.naviqore.service.gtfs.raptor.convert; + +import ch.naviqore.gtfs.schedule.model.GtfsSchedule; +import ch.naviqore.gtfs.schedule.model.Stop; + +import java.util.Collection; +import java.util.List; + +/** + * Abstracts the generation of transfers between stops from a GTFS schedule. + *

    + * Is only intended to be used in the {@link GtfsToRaptorConverter}, as source to provide additional generated transfers + * not present in the {@link GtfsSchedule} schedule. + */ +public interface TransferGenerator { + + /** + * Generates minimum time transfers between stops in the GTFS schedule. + * + * @param stops Stops of the GTFS schedule to generate transfers for. + * @return List of minimum time transfers. + */ + List generateTransfers(Collection stops); + + /** + * Represents a minimum time transfer between two stops. + */ + record Transfer(Stop from, Stop to, int duration) { + } +} diff --git a/src/main/java/ch/naviqore/service/gtfs/raptor/transfer/WalkTransferGenerator.java b/src/main/java/ch/naviqore/service/gtfs/raptor/convert/WalkTransferGenerator.java similarity index 87% rename from src/main/java/ch/naviqore/service/gtfs/raptor/transfer/WalkTransferGenerator.java rename to src/main/java/ch/naviqore/service/gtfs/raptor/convert/WalkTransferGenerator.java index 8819b20..94949fe 100644 --- a/src/main/java/ch/naviqore/service/gtfs/raptor/transfer/WalkTransferGenerator.java +++ b/src/main/java/ch/naviqore/service/gtfs/raptor/convert/WalkTransferGenerator.java @@ -1,4 +1,4 @@ -package ch.naviqore.service.gtfs.raptor.transfer; +package ch.naviqore.service.gtfs.raptor.convert; import ch.naviqore.gtfs.schedule.model.GtfsSchedule; import ch.naviqore.gtfs.schedule.model.Stop; @@ -7,8 +7,8 @@ import lombok.extern.slf4j.Slf4j; import java.util.ArrayList; +import java.util.Collection; import java.util.List; -import java.util.Map; /** * Implements a transfer generator that creates minimum time transfers between stops where the {@link GtfsSchedule} does @@ -32,6 +32,7 @@ public class WalkTransferGenerator implements TransferGenerator { * be shorter. Accounts for access and egress of vehicle, building, stairways, etc. * @param accessEgressTime Time needed to access or egress a public transit trip. * @param searchRadius Search radius in meters, the maximum beeline walking distance between stops. + * @param spatialStopIndex Spatial index containing the stops. */ public WalkTransferGenerator(WalkCalculator walkCalculator, int minimumTransferTime, int accessEgressTime, int searchRadius, KDTree spatialStopIndex) { @@ -40,6 +41,7 @@ public WalkTransferGenerator(WalkCalculator walkCalculator, int minimumTransferT if (accessEgressTime < 0) throw new IllegalArgumentException("accessEgressTime is negative"); if (searchRadius <= 0) throw new IllegalArgumentException("searchRadius is negative or zero"); if (spatialStopIndex == null) throw new IllegalArgumentException("spatialStopIndex is null"); + this.walkCalculator = walkCalculator; this.minimumTransferTime = minimumTransferTime; this.accessEgressTime = accessEgressTime; @@ -52,22 +54,22 @@ public WalkTransferGenerator(WalkCalculator walkCalculator, int minimumTransferT * {@link GtfsSchedule} and the stops are within walking distance. Uses the {@link WalkCalculator} to calculate * walking times between stops. * - * @param schedule GTFS schedule to generate transfers for. + * @param stops Stops of the GTFS schedule to generate transfers for. * @return List of minimum time transfers. */ @Override - public List generateTransfers(GtfsSchedule schedule) { - Map stops = schedule.getStops(); - + public List generateTransfers(Collection stops) { log.info("Generating transfers between {} stops", stops.size()); - List transfers = stops.values().parallelStream().flatMap(fromStop -> { + List transfers = stops.parallelStream().flatMap(fromStop -> { List nearbyStops = spatialStopIndex.rangeSearch(fromStop, searchRadius); + // since spatial index contains all stops of the schedule, we only consider stops with departures return nearbyStops.stream() + .filter(stops::contains) .filter(toStop -> !toStop.equals(fromStop)) .map(toStop -> createTransfer(fromStop, toStop)); }).toList(); - log.info("Generated {} transfers between {} stops", transfers.size(), stops.size()); + log.info("Generated {} transfers between {} stops", transfers.size(), stops.size()); return new ArrayList<>(transfers); } @@ -77,6 +79,7 @@ private TransferGenerator.Transfer createTransfer(Stop fromStop, Stop toStop) { // get total transfer duration by adding access and egress time (twice) to the walk duration // and taking the maximum value between this total and the minimum transfer time. int transferDuration = Math.max(walk.duration() + 2 * accessEgressTime, minimumTransferTime); + return new TransferGenerator.Transfer(fromStop, toStop, transferDuration); } } diff --git a/src/main/java/ch/naviqore/service/gtfs/raptor/transfer/TransferGenerator.java b/src/main/java/ch/naviqore/service/gtfs/raptor/transfer/TransferGenerator.java deleted file mode 100644 index 24b480e..0000000 --- a/src/main/java/ch/naviqore/service/gtfs/raptor/transfer/TransferGenerator.java +++ /dev/null @@ -1,30 +0,0 @@ -package ch.naviqore.service.gtfs.raptor.transfer; - -import ch.naviqore.gtfs.schedule.model.GtfsSchedule; -import ch.naviqore.gtfs.schedule.model.Stop; -import ch.naviqore.service.gtfs.raptor.convert.GtfsToRaptorConverter; - -import java.util.List; - -/** - * Abstracts the generation of transfers between stops from a GTFS schedule. - */ -public interface TransferGenerator { - - /** - * Generates minimum time transfers between stops in the GTFS schedule, which can be added to the - * {@link GtfsToRaptorConverter} . - * - * @param schedule GTFS schedule to generate transfers for. - * @return List of minimum time transfers. - */ - List generateTransfers(GtfsSchedule schedule); - - /** - * Represents a minimum time transfer between two stops. Is only intended to be used in the - * {@link GtfsToRaptorConverter}, as source to provide additional generated transfers not present in the - * {@link GtfsSchedule} schedule. - */ - record Transfer(Stop from, Stop to, int duration) { - } -} diff --git a/src/test/java/ch/naviqore/Benchmark.java b/src/test/java/ch/naviqore/Benchmark.java index 8cb4083..879b408 100644 --- a/src/test/java/ch/naviqore/Benchmark.java +++ b/src/test/java/ch/naviqore/Benchmark.java @@ -82,7 +82,7 @@ private static GtfsSchedule initializeSchedule() throws IOException, Interrupted private static RaptorAlgorithm initializeRaptor(GtfsSchedule schedule) throws InterruptedException { RaptorConfig config = new RaptorConfig(MAX_DAYS_TO_SCAN, RAPTOR_RANGE, SAME_STOP_TRANSFER_TIME, MAX_DAYS_TO_SCAN, EvictionCache.Strategy.LRU, new GtfsTripMaskProvider(schedule)); - RaptorRouter raptor = new GtfsToRaptorConverter(schedule, config).convert(); + RaptorRouter raptor = new GtfsToRaptorConverter(config, schedule).run(); manageResources(); for (int dayIndex = 0; dayIndex < MAX_DAYS_TO_SCAN; dayIndex++) { diff --git a/src/test/java/ch/naviqore/app/controller/DummyService.java b/src/test/java/ch/naviqore/app/controller/DummyService.java index 6fdd0be..cfa7310 100644 --- a/src/test/java/ch/naviqore/app/controller/DummyService.java +++ b/src/test/java/ch/naviqore/app/controller/DummyService.java @@ -128,7 +128,7 @@ public List getConnections(Stop source, GeoCoordinate target, LocalD } @Override - public Map getIsoLines(GeoCoordinate source, LocalDateTime time, TimeType timeType, + public Map getIsolines(GeoCoordinate source, LocalDateTime time, TimeType timeType, ConnectionQueryConfig config) { Map connections = new HashMap<>(); for (Stop stop : STOPS) { @@ -146,7 +146,7 @@ public Map getIsoLines(GeoCoordinate source, LocalDateTime tim } @Override - public Map getIsoLines(Stop source, LocalDateTime time, TimeType timeType, + public Map getIsolines(Stop source, LocalDateTime time, TimeType timeType, ConnectionQueryConfig config) { Map connections = new HashMap<>(); for (Stop stop : STOPS) { diff --git a/src/test/java/ch/naviqore/service/gtfs/raptor/GtfsRaptorServiceIT.java b/src/test/java/ch/naviqore/service/gtfs/raptor/GtfsRaptorServiceIT.java index 211e85c..476076c 100644 --- a/src/test/java/ch/naviqore/service/gtfs/raptor/GtfsRaptorServiceIT.java +++ b/src/test/java/ch/naviqore/service/gtfs/raptor/GtfsRaptorServiceIT.java @@ -2,10 +2,12 @@ import ch.naviqore.gtfs.schedule.GtfsScheduleReader; import ch.naviqore.gtfs.schedule.GtfsScheduleTestData; +import ch.naviqore.gtfs.schedule.model.GtfsSchedule; import ch.naviqore.service.*; import ch.naviqore.service.config.ConnectionQueryConfig; import ch.naviqore.service.config.ServiceConfig; import ch.naviqore.service.exception.*; +import ch.naviqore.service.gtfs.raptor.convert.GtfsToRaptorTestSchedule; import ch.naviqore.service.repo.GtfsScheduleRepository; import ch.naviqore.utils.spatial.GeoCoordinate; import org.junit.jupiter.api.BeforeEach; @@ -21,303 +23,412 @@ import java.util.List; import java.util.Map; +import static ch.naviqore.service.config.ServiceConfig.*; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.*; class GtfsRaptorServiceIT { private GtfsRaptorService service; - @BeforeEach - void setUp(@TempDir Path tempDir) throws IOException, InterruptedException { - File zipFile = GtfsScheduleTestData.prepareZipDataset(tempDir); - - // implement repo for gtfs schedule file reader - GtfsScheduleRepository repo = () -> new GtfsScheduleReader().read(zipFile.toString()); - GtfsRaptorServiceInitializer initializer = new GtfsRaptorServiceInitializer( - new ServiceConfig(zipFile.getAbsolutePath()), repo.get()); - - // create service from initialized data - service = new GtfsRaptorService(initializer.getConfig(), initializer.getSchedule(), - initializer.getSpatialStopIndex(), initializer.getStopSearchIndex(), initializer.getWalkCalculator(), - initializer.getAdditionalTransfers()); - } - @Nested - class Validity { + class FromZip { - @Test - void shouldReturnCorrectStartDate() { - LocalDate expectedStartDate = LocalDate.of(2007, 1, 1); - assertEquals(expectedStartDate, service.getValidity().getStartDate(), - "The start date of the validity should match the earliest start date in the schedule."); - } - - @Test - void shouldReturnCorrectEndDate() { - LocalDate expectedEndDate = LocalDate.of(2010, 12, 31); - assertEquals(expectedEndDate, service.getValidity().getEndDate(), - "The end date of the validity should match the latest end date in the schedule."); - } - - @Test - void shouldBeWithinValidityPeriod() { - LocalDate dateWithinValidity = LocalDate.of(2008, 6, 1); - assertTrue(service.getValidity().isWithin(dateWithinValidity), - "The date should be within the validity period."); - } - - @Test - void shouldIncludeStartDateInValidityPeriod() { - LocalDate startDate = service.getValidity().getStartDate(); - assertTrue(service.getValidity().isWithin(startDate), - "The start date should be considered within the validity period."); - } - - @Test - void shouldIncludeEndDateInValidityPeriod() { - LocalDate endDate = service.getValidity().getEndDate(); - assertTrue(service.getValidity().isWithin(endDate), - "The end date should be considered within the validity period."); - } + @BeforeEach + void setUp(@TempDir Path tempDir) throws IOException, InterruptedException { + File zipFile = GtfsScheduleTestData.prepareZipDataset(tempDir); - @Test - void shouldNotBeWithinValidityPeriodBeforeStart() { - LocalDate dateBeforeValidity = LocalDate.of(2006, 12, 13); - assertFalse(service.getValidity().isWithin(dateBeforeValidity), - "The date before the start date should not be within the validity period."); - } + // implement repo for gtfs schedule file reader + GtfsScheduleRepository repo = () -> new GtfsScheduleReader().read(zipFile.toString()); + GtfsRaptorServiceInitializer initializer = new GtfsRaptorServiceInitializer( + new ServiceConfig(zipFile.getAbsolutePath()), repo.get()); - @Test - void shouldNotBeWithinValidityPeriodAfterEnd() { - LocalDate dateAfterValidity = LocalDate.of(2011, 1, 1); - assertFalse(service.getValidity().isWithin(dateAfterValidity), - "The date after the end date should not be within the validity period."); + service = initializer.get(); } - } - - @Nested - class ScheduleInformation { @Nested - class SearchStopByName { + class Validity { @Test - void shouldFindStopByName() { - List stops = service.getStops("Furnace Creek Resort", SearchType.CONTAINS); - assertFalse(stops.isEmpty(), "Expected to find stops matching the name."); - assertEquals("Furnace Creek Resort (Demo)", stops.getFirst().getName()); + void shouldReturnCorrectStartDate() { + LocalDate expectedStartDate = LocalDate.of(2007, 1, 1); + assertEquals(expectedStartDate, service.getValidity().getStartDate(), + "The start date of the validity should match the earliest start date in the schedule."); } @Test - void shouldNotFindNonExistingStopByName() { - List stops = service.getStops("NonExistingStop", SearchType.CONTAINS); - assertTrue(stops.isEmpty(), "Expected no stops to be found."); + void shouldReturnCorrectEndDate() { + LocalDate expectedEndDate = LocalDate.of(2010, 12, 31); + assertEquals(expectedEndDate, service.getValidity().getEndDate(), + "The end date of the validity should match the latest end date in the schedule."); } - } - @Nested - class NearestStop { @Test - void shouldFindNearestStop() { - Stop stop = service.getNearestStop(new GeoCoordinate(36.425288, -117.133162)).orElseThrow(); - assertEquals("Furnace Creek Resort (Demo)", stop.getName()); + void shouldBeWithinValidityPeriod() { + LocalDate dateWithinValidity = LocalDate.of(2008, 6, 1); + assertTrue(service.getValidity().isWithin(dateWithinValidity), + "The date should be within the validity period."); } @Test - void shouldFindNearestStops() { - List stops = service.getNearestStops(new GeoCoordinate(36.425288, -117.133162), Integer.MAX_VALUE, - Integer.MAX_VALUE); - assertFalse(stops.isEmpty(), "Expected to find nearest stops."); - assertTrue(stops.size() > 1, "Expected to find more than one stop."); + void shouldIncludeStartDateInValidityPeriod() { + LocalDate startDate = service.getValidity().getStartDate(); + assertTrue(service.getValidity().isWithin(startDate), + "The start date should be considered within the validity period."); } - } - - @Nested - class GetById { @Test - void shouldFindStopById() throws StopNotFoundException { - Stop stop = service.getStopById("FUR_CREEK_RES"); - assertEquals("Furnace Creek Resort (Demo)", stop.getName()); + void shouldIncludeEndDateInValidityPeriod() { + LocalDate endDate = service.getValidity().getEndDate(); + assertTrue(service.getValidity().isWithin(endDate), + "The end date should be considered within the validity period."); } @Test - void shouldNotFindMissingStopById() { - assertThrows(StopNotFoundException.class, () -> service.getStopById("NON_EXISTENT_STOP")); + void shouldNotBeWithinValidityPeriodBeforeStart() { + LocalDate dateBeforeValidity = LocalDate.of(2006, 12, 13); + assertFalse(service.getValidity().isWithin(dateBeforeValidity), + "The date before the start date should not be within the validity period."); } @Test - void shouldFindActiveTripById() throws TripNotFoundException, TripNotActiveException { - Trip trip = service.getTripById("AB1", LocalDate.of(2008, 5, 15)); - assertNotNull(trip); - assertEquals("to Bullfrog", trip.getHeadSign()); + void shouldNotBeWithinValidityPeriodAfterEnd() { + LocalDate dateAfterValidity = LocalDate.of(2011, 1, 1); + assertFalse(service.getValidity().isWithin(dateAfterValidity), + "The date after the end date should not be within the validity period."); } + } - @Test - void shouldNotFindInactiveTripById() { - assertThrows(TripNotActiveException.class, () -> service.getTripById("AB1", LocalDate.of(2023, 5, 15))); - } + @Nested + class ScheduleInformation { - @Test - void shouldNotFindMissingTripById() { - assertThrows(TripNotFoundException.class, - () -> service.getTripById("NON_EXISTENT_TRIP", LocalDate.of(2023, 5, 15))); - } + @Nested + class SearchStopByName { - @Test - void shouldThrowTripNotActiveException() { - assertThrows(TripNotActiveException.class, - () -> service.getTripById("AAMV1", LocalDate.of(2023, 5, 15))); - } + @Test + void shouldFindStopByName() { + List stops = service.getStops("Furnace Creek Resort", SearchType.CONTAINS); + assertFalse(stops.isEmpty(), "Expected to find stops matching the name."); + assertEquals("Furnace Creek Resort (Demo)", stops.getFirst().getName()); + } - @Test - void shouldFindRouteById() throws RouteNotFoundException { - Route route = service.getRouteById("AB"); - assertNotNull(route); - assertEquals("Airport - Bullfrog", route.getName()); + @Test + void shouldNotFindNonExistingStopByName() { + List stops = service.getStops("NonExistingStop", SearchType.CONTAINS); + assertTrue(stops.isEmpty(), "Expected no stops to be found."); + } } - @Test - void shouldNotFindMissingRouteById() { - assertThrows(RouteNotFoundException.class, () -> service.getRouteById("NON_EXISTENT_ROUTE")); + @Nested + class NearestStop { + @Test + void shouldFindNearestStop() { + Stop stop = service.getNearestStop(new GeoCoordinate(36.425288, -117.133162)).orElseThrow(); + assertEquals("Furnace Creek Resort (Demo)", stop.getName()); + } + + @Test + void shouldFindNearestStops() { + List stops = service.getNearestStops(new GeoCoordinate(36.425288, -117.133162), + Integer.MAX_VALUE, Integer.MAX_VALUE); + assertFalse(stops.isEmpty(), "Expected to find nearest stops."); + assertTrue(stops.size() > 1, "Expected to find more than one stop."); + } } - } - } - @Nested - class Routing { + @Nested + class GetById { - private ConnectionQueryConfig config; + @Test + void shouldFindStopById() throws StopNotFoundException { + Stop stop = service.getStopById("FUR_CREEK_RES"); + assertEquals("Furnace Creek Resort (Demo)", stop.getName()); + } - @BeforeEach - void setUp() { - config = new ConnectionQueryConfig(10 * 60, 2 * 60, 4, 24 * 60 * 60, false, false, null); - } + @Test + void shouldNotFindMissingStopById() { + assertThrows(StopNotFoundException.class, () -> service.getStopById("NON_EXISTENT_STOP")); + } - @Nested - class Connections { + @Test + void shouldFindActiveTripById() throws TripNotFoundException, TripNotActiveException { + Trip trip = service.getTripById("AB1", LocalDate.of(2008, 5, 15)); + assertNotNull(trip); + assertEquals("to Bullfrog", trip.getHeadSign()); + } - @Nested - class FromLocation { + @Test + void shouldNotFindInactiveTripById() { + assertThrows(TripNotActiveException.class, + () -> service.getTripById("AB1", LocalDate.of(2023, 5, 15))); + } + + @Test + void shouldNotFindMissingTripById() { + assertThrows(TripNotFoundException.class, + () -> service.getTripById("NON_EXISTENT_TRIP", LocalDate.of(2023, 5, 15))); + } @Test - void shouldGetConnections() throws ConnectionRoutingException { - List connections = service.getConnections(new GeoCoordinate(36.425288, -117.133162), - new GeoCoordinate(36.88108, -116.81797), LocalDateTime.of(2008, 5, 15, 8, 0), - TimeType.DEPARTURE, config); - assertFalse(connections.isEmpty(), "Expected to find connections."); + void shouldThrowTripNotActiveException() { + assertThrows(TripNotActiveException.class, + () -> service.getTripById("AAMV1", LocalDate.of(2023, 5, 15))); } @Test - void shouldHandleNoNearestTargetStop() throws ConnectionRoutingException { - List connections = service.getConnections(new GeoCoordinate(0.0, 0.0), - new GeoCoordinate(36.88108, -116.81797), LocalDateTime.of(2008, 5, 15, 8, 0), - TimeType.DEPARTURE, config); - assertTrue(connections.isEmpty(), - "Expected no connections to be found when no nearest stop exists."); + void shouldFindRouteById() throws RouteNotFoundException { + Route route = service.getRouteById("AB"); + assertNotNull(route); + assertEquals("Airport - Bullfrog", route.getName()); } @Test - void shouldHandleInactiveDate() throws ConnectionRoutingException { - List connections = service.getConnections(new GeoCoordinate(36.425288, -117.133162), - new GeoCoordinate(36.88108, -116.81797), LocalDateTime.of(2023, 5, 15, 8, 0), - TimeType.DEPARTURE, config); - assertTrue(connections.isEmpty(), - "Expected no connections to be found when source stop has no active trip on the date."); + void shouldNotFindMissingRouteById() { + assertThrows(RouteNotFoundException.class, () -> service.getRouteById("NON_EXISTENT_ROUTE")); } } + } - @Nested - class FromStop { + @Nested + class Routing { - private Stop source; + private ConnectionQueryConfig config; + + @BeforeEach + void setUp() { + config = new ConnectionQueryConfig(10 * 60, 2 * 60, 4, 24 * 60 * 60, false, false, null); + } - @BeforeEach - void setUp() throws StopNotFoundException { - source = service.getStopById("FUR_CREEK_RES"); + @Nested + class Connections { + + @Nested + class FromLocation { + + @Test + void shouldGetConnections() throws ConnectionRoutingException { + List connections = service.getConnections(new GeoCoordinate(36.425288, -117.133162), + new GeoCoordinate(36.88108, -116.81797), LocalDateTime.of(2008, 5, 15, 8, 0), + TimeType.DEPARTURE, config); + assertFalse(connections.isEmpty(), "Expected to find connections."); + } + + @Test + void shouldHandleNoNearestTargetStop() throws ConnectionRoutingException { + List connections = service.getConnections(new GeoCoordinate(0.0, 0.0), + new GeoCoordinate(36.88108, -116.81797), LocalDateTime.of(2008, 5, 15, 8, 0), + TimeType.DEPARTURE, config); + assertTrue(connections.isEmpty(), + "Expected no connections to be found when no nearest stop exists."); + } + + @Test + void shouldHandleInactiveDate() throws ConnectionRoutingException { + List connections = service.getConnections(new GeoCoordinate(36.425288, -117.133162), + new GeoCoordinate(36.88108, -116.81797), LocalDateTime.of(2023, 5, 15, 8, 0), + TimeType.DEPARTURE, config); + assertTrue(connections.isEmpty(), + "Expected no connections to be found when source stop has no active trip on the date."); + } } - @Test - void shouldGetConnections() throws ConnectionRoutingException { - List connections = service.getConnections(source, - new GeoCoordinate(36.88108, -116.81797), LocalDateTime.of(2008, 5, 15, 8, 0), - TimeType.DEPARTURE, config); - assertFalse(connections.isEmpty(), "Expected to find connections."); + @Nested + class FromStop { + + private Stop source; + + @BeforeEach + void setUp() throws StopNotFoundException { + source = service.getStopById("FUR_CREEK_RES"); + } + + @Test + void shouldGetConnections() throws ConnectionRoutingException { + List connections = service.getConnections(source, + new GeoCoordinate(36.88108, -116.81797), LocalDateTime.of(2008, 5, 15, 8, 0), + TimeType.DEPARTURE, config); + assertFalse(connections.isEmpty(), "Expected to find connections."); + } + + @Test + void shouldHandleNoNearestTargetStop() throws ConnectionRoutingException { + List connections = service.getConnections(source, new GeoCoordinate(-89, 0), + LocalDateTime.of(2008, 5, 15, 8, 0), TimeType.DEPARTURE, config); + assertTrue(connections.isEmpty(), + "Expected no connections to be found when no nearest stop exists."); + } + + @Test + void shouldHandleInactiveDate() throws ConnectionRoutingException { + List connections = service.getConnections(source, + new GeoCoordinate(36.88108, -116.81797), LocalDateTime.of(2023, 5, 15, 8, 0), + TimeType.DEPARTURE, config); + assertTrue(connections.isEmpty(), + "Expected no connections to be found when source stop has no active trip on the date."); + } } + } - @Test - void shouldHandleNoNearestTargetStop() throws ConnectionRoutingException { - List connections = service.getConnections(source, new GeoCoordinate(-89, 0), - LocalDateTime.of(2008, 5, 15, 8, 0), TimeType.DEPARTURE, config); - assertTrue(connections.isEmpty(), - "Expected no connections to be found when no nearest stop exists."); + @Nested + class Isoline { + + @Nested + class FromLocation { + + @Test + void shouldGetIsolines() throws ConnectionRoutingException { + Map connections = service.getIsolines( + new GeoCoordinate(36.425288, -117.133162), LocalDateTime.of(2008, 5, 15, 8, 0), + TimeType.DEPARTURE, config); + assertFalse(connections.isEmpty(), "Expected to find connections."); + } + + @Test + void shouldHandleNoIsolinesFound() throws ConnectionRoutingException { + Map connections = service.getIsolines(new GeoCoordinate(0.0, 0.0), + LocalDateTime.of(2008, 5, 15, 8, 0), TimeType.DEPARTURE, config); + assertTrue(connections.isEmpty(), + "Expected no isolines to be found when no nearest stop exists."); + } + + @Test + void shouldHandleInactiveDate() throws ConnectionRoutingException { + Map connections = service.getIsolines( + new GeoCoordinate(36.425288, -117.133162), LocalDateTime.of(2023, 5, 15, 8, 0), + TimeType.DEPARTURE, config); + assertTrue(connections.isEmpty(), + "Expected no isolines to be found when no active trips exist on the date."); + } } - @Test - void shouldHandleInactiveDate() throws ConnectionRoutingException { - List connections = service.getConnections(source, - new GeoCoordinate(36.88108, -116.81797), LocalDateTime.of(2023, 5, 15, 8, 0), - TimeType.DEPARTURE, config); - assertTrue(connections.isEmpty(), - "Expected no connections to be found when source stop has no active trip on the date."); + @Nested + class FromStop { + + private Stop source; + + @BeforeEach + void setUp() throws StopNotFoundException { + source = service.getStopById("FUR_CREEK_RES"); + } + + @Test + void shouldGetIsolines() throws ConnectionRoutingException { + Map connections = service.getIsolines(source, + LocalDateTime.of(2008, 5, 15, 8, 0), TimeType.DEPARTURE, config); + assertFalse(connections.isEmpty(), "Expected to find connections."); + } + + @Test + void shouldHandleInactiveDate() throws ConnectionRoutingException { + Map connections = service.getIsolines(source, + LocalDateTime.of(2023, 5, 15, 8, 0), TimeType.DEPARTURE, config); + assertTrue(connections.isEmpty(), + "Expected no isolines to be found when no active trips exist on the date."); + } } } } + } + + @Nested + class ManualSchedule { + + private static final LocalDateTime DATE_TIME = LocalDateTime.MIN; + private static final ConnectionQueryConfig QUERY_CONFIG = new ConnectionQueryConfig(10 * 60, 2 * 60, 4, + 24 * 60 * 60, false, false, null); + + @BeforeEach + void setUp() { + GtfsToRaptorTestSchedule builder = new GtfsToRaptorTestSchedule(); + GtfsSchedule schedule = builder.build(); + + // do not create any transfers since all coordinates in the test schedule are at origin 0,0. + ServiceConfig serviceConfig = new ServiceConfig("NONE", DEFAULT_GTFS_STATIC_UPDATE_CRON, + DEFAULT_TRANSFER_TIME_SAME_STOP_DEFAULT, -1, DEFAULT_TRANSFER_TIME_ACCESS_EGRESS, + DEFAULT_WALKING_SEARCH_RADIUS, DEFAULT_WALKING_CALCULATOR_TYPE, DEFAULT_WALKING_SPEED, + DEFAULT_WALKING_DURATION_MINIMUM, DEFAULT_MAX_DAYS_TO_SCAN, DEFAULT_RAPTOR_RANGE, + DEFAULT_CACHE_SIZE, DEFAULT_CACHE_EVICTION_STRATEGY); + + GtfsRaptorServiceInitializer initializer = new GtfsRaptorServiceInitializer(serviceConfig, schedule); + + service = initializer.get(); + } @Nested - class Isoline { + class Connection { + + @Test + void shouldThrowOnInvalidStop() throws StopNotFoundException, ConnectionRoutingException { + assertThrows(StopNotFoundException.class, + () -> service.getConnections(service.getStopById("NOT_A_STOP"), service.getStopById("C"), + DATE_TIME, TimeType.DEPARTURE, QUERY_CONFIG)); + } @Nested - class FromLocation { + class StopWithoutDepartures { + + @Test + void target_departure() throws ConnectionRoutingException, StopNotFoundException { + List connections = service.getConnections(service.getStopById("A"), + service.getStopById("D"), DATE_TIME, TimeType.DEPARTURE, QUERY_CONFIG); + + assertThat(connections).isEmpty(); + } @Test - void shouldGetIsolines() throws ConnectionRoutingException { - Map connections = service.getIsoLines(new GeoCoordinate(36.425288, -117.133162), - LocalDateTime.of(2008, 5, 15, 8, 0), TimeType.DEPARTURE, config); - assertFalse(connections.isEmpty(), "Expected to find connections."); + void target_arrival() throws ConnectionRoutingException, StopNotFoundException { + List connections = service.getConnections(service.getStopById("A"), + service.getStopById("D"), DATE_TIME, TimeType.ARRIVAL, QUERY_CONFIG); + + assertThat(connections).isEmpty(); } @Test - void shouldHandleNoIsolinesFound() throws ConnectionRoutingException { - Map connections = service.getIsoLines(new GeoCoordinate(0.0, 0.0), - LocalDateTime.of(2008, 5, 15, 8, 0), TimeType.DEPARTURE, config); - assertTrue(connections.isEmpty(), "Expected no isolines to be found when no nearest stop exists."); + void source_departure() throws ConnectionRoutingException, StopNotFoundException { + List connections = service.getConnections(service.getStopById("D"), + service.getStopById("A"), DATE_TIME, TimeType.DEPARTURE, QUERY_CONFIG); + + assertThat(connections).isEmpty(); } @Test - void shouldHandleInactiveDate() throws ConnectionRoutingException { - Map connections = service.getIsoLines(new GeoCoordinate(36.425288, -117.133162), - LocalDateTime.of(2023, 5, 15, 8, 0), TimeType.DEPARTURE, config); - assertTrue(connections.isEmpty(), - "Expected no isolines to be found when no active trips exist on the date."); + void source_arrival() throws ConnectionRoutingException, StopNotFoundException { + List connections = service.getConnections(service.getStopById("D"), + service.getStopById("A"), DATE_TIME, TimeType.ARRIVAL, QUERY_CONFIG); + + assertThat(connections).isEmpty(); } } + } - @Nested - class FromStop { + @Nested + class Isolines { - private Stop source; + @Test + void shouldThrowOnInvalidStop() throws StopNotFoundException, ConnectionRoutingException { + assertThrows(StopNotFoundException.class, + () -> service.getIsolines(service.getStopById("NOT_A_STOP"), DATE_TIME, TimeType.DEPARTURE, + QUERY_CONFIG)); + } - @BeforeEach - void setUp() throws StopNotFoundException { - source = service.getStopById("FUR_CREEK_RES"); - } + @Nested + class StopWithoutDepartures { @Test - void shouldGetIsolines() throws ConnectionRoutingException { - Map connections = service.getIsoLines(source, LocalDateTime.of(2008, 5, 15, 8, 0), - TimeType.DEPARTURE, config); - assertFalse(connections.isEmpty(), "Expected to find connections."); + void departure() throws ConnectionRoutingException, StopNotFoundException { + Map isolines = service.getIsolines(service.getStopById("D"), + DATE_TIME, TimeType.DEPARTURE, QUERY_CONFIG); + assertThat(isolines).isEmpty(); } @Test - void shouldHandleInactiveDate() throws ConnectionRoutingException { - Map connections = service.getIsoLines(source, LocalDateTime.of(2023, 5, 15, 8, 0), - TimeType.DEPARTURE, config); - assertTrue(connections.isEmpty(), - "Expected no isolines to be found when no active trips exist on the date."); + void arrival() throws ConnectionRoutingException, StopNotFoundException { + Map isolines = service.getIsolines(service.getStopById("D"), + DATE_TIME, TimeType.ARRIVAL, QUERY_CONFIG); + assertThat(isolines).isEmpty(); } } + } } diff --git a/src/test/java/ch/naviqore/service/gtfs/raptor/GtfsRaptorServiceInitializerIT.java b/src/test/java/ch/naviqore/service/gtfs/raptor/GtfsRaptorServiceInitializerIT.java index e67b6ab..5c398b6 100644 --- a/src/test/java/ch/naviqore/service/gtfs/raptor/GtfsRaptorServiceInitializerIT.java +++ b/src/test/java/ch/naviqore/service/gtfs/raptor/GtfsRaptorServiceInitializerIT.java @@ -2,14 +2,8 @@ import ch.naviqore.gtfs.schedule.GtfsScheduleReader; import ch.naviqore.gtfs.schedule.GtfsScheduleTestData; -import ch.naviqore.gtfs.schedule.model.GtfsSchedule; -import ch.naviqore.gtfs.schedule.model.Stop; import ch.naviqore.service.config.ServiceConfig; -import ch.naviqore.service.gtfs.raptor.transfer.TransferGenerator; import ch.naviqore.service.repo.GtfsScheduleRepository; -import ch.naviqore.service.walk.WalkCalculator; -import ch.naviqore.utils.search.SearchIndex; -import ch.naviqore.utils.spatial.index.KDTree; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; @@ -17,9 +11,8 @@ import java.io.File; import java.io.IOException; import java.nio.file.Path; -import java.util.List; -import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.*; class GtfsRaptorServiceInitializerIT { @@ -34,38 +27,13 @@ void setUp(@TempDir Path tempDir) throws IOException, InterruptedException { } @Test - void shouldSetConfigCorrectly() { - assertNotNull(initializer.getConfig()); - } - - @Test - void shouldInitializeWalkCalculator() { - WalkCalculator walkCalculator = initializer.getWalkCalculator(); - assertNotNull(walkCalculator); - } - - @Test - void shouldReadGtfsScheduleCorrectly() { - GtfsSchedule schedule = initializer.getSchedule(); - assertNotNull(schedule); - } + void shouldInitializeServiceCorrectly() { + GtfsRaptorService service = initializer.get(); - @Test - void shouldGenerateStopSearchIndex() { - SearchIndex stopSearchIndex = initializer.getStopSearchIndex(); - assertNotNull(stopSearchIndex); - } - - @Test - void shouldGenerateSpatialStopIndex() { - KDTree spatialStopIndex = initializer.getSpatialStopIndex(); - assertNotNull(spatialStopIndex); - } - - @Test - void shouldGenerateAdditionalTransfers() { - List additionalTransfers = initializer.getAdditionalTransfers(); - assertNotNull(additionalTransfers); + assertNotNull(service); + assertTrue(service.hasTravelModeInformation()); + assertFalse(service.hasAccessibilityInformation()); + assertFalse(service.hasBikeInformation()); } } diff --git a/src/test/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorConverterIT.java b/src/test/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorConverterIT.java index 42dddbe..4918cf8 100644 --- a/src/test/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorConverterIT.java +++ b/src/test/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorConverterIT.java @@ -3,15 +3,10 @@ import ch.naviqore.gtfs.schedule.GtfsScheduleReader; import ch.naviqore.gtfs.schedule.GtfsScheduleTestData; import ch.naviqore.gtfs.schedule.model.GtfsSchedule; -import ch.naviqore.gtfs.schedule.model.GtfsScheduleBuilder; -import ch.naviqore.gtfs.schedule.type.AccessibilityInformation; -import ch.naviqore.gtfs.schedule.type.RouteType; -import ch.naviqore.gtfs.schedule.type.ServiceDayTime; import ch.naviqore.gtfs.schedule.type.TransferType; import ch.naviqore.raptor.RaptorAlgorithm; import ch.naviqore.raptor.router.RaptorConfig; import ch.naviqore.raptor.router.RaptorRouterBuilder; -import ch.naviqore.service.gtfs.raptor.transfer.TransferGenerator; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; @@ -21,9 +16,10 @@ import java.io.IOException; import java.lang.reflect.Field; import java.nio.file.Path; -import java.time.DayOfWeek; -import java.time.LocalDate; -import java.util.*; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.Set; import java.util.stream.Collectors; import static org.assertj.core.api.Assertions.assertThat; @@ -43,8 +39,8 @@ void setUp(@TempDir Path tempDir) throws IOException { @Test void shouldConvertGtfsScheduleToRaptor() { - GtfsToRaptorConverter mapper = new GtfsToRaptorConverter(schedule, new RaptorConfig()); - RaptorAlgorithm raptor = mapper.convert(); + GtfsToRaptorConverter mapper = new GtfsToRaptorConverter(new RaptorConfig(), schedule); + RaptorAlgorithm raptor = mapper.run(); assertThat(raptor).isNotNull(); } @@ -71,49 +67,23 @@ class ManualSchedule { */ static RaptorBuilderData convertRaptor(List scheduleTransfers, List additionalTransfers) throws NoSuchFieldException, IllegalAccessException { - - GtfsScheduleBuilder scheduleBuilder = GtfsSchedule.builder(); - scheduleBuilder.addCalendar("always", EnumSet.allOf(DayOfWeek.class), LocalDate.MIN, LocalDate.MAX); - scheduleBuilder.addAgency("agency", "Some Agency", "", "America/New_York"); - - scheduleBuilder.addStop("A", "A", 0.0, 0.0); - scheduleBuilder.addStop("B", "B", 0.0, 0.0); - scheduleBuilder.addStop("B1", "B1", 0.0, 0.0, "B", AccessibilityInformation.UNKNOWN); - scheduleBuilder.addStop("B2", "B2", 0.0, 0.0, "B", AccessibilityInformation.UNKNOWN); - scheduleBuilder.addStop("C", "C", 0.0, 0.0); - scheduleBuilder.addStop("C1", "C1", 0.0, 0.0, "C", AccessibilityInformation.UNKNOWN); - scheduleBuilder.addStop("C2", "C2", 0.0, 0.0, "C", AccessibilityInformation.UNKNOWN); - scheduleBuilder.addStop("D", "D", 0.0, 0.0); - - // Route 1 goes from A, B1, C1 - scheduleBuilder.addRoute("R1", "agency", "R1", "R1", RouteType.parse(1)); - scheduleBuilder.addTrip("T1", "R1", "always", "C1"); - scheduleBuilder.addStopTime("T1", "A", new ServiceDayTime(0), new ServiceDayTime(0)); - scheduleBuilder.addStopTime("T1", "B1", new ServiceDayTime(0), new ServiceDayTime(0)); - scheduleBuilder.addStopTime("T1", "C1", new ServiceDayTime(0), new ServiceDayTime(0)); - - // Route 2 goes from A, B2, C - scheduleBuilder.addRoute("R2", "agency", "R2", "R2", RouteType.parse(1)); - scheduleBuilder.addTrip("T2", "R2", "always", "C"); - scheduleBuilder.addStopTime("T2", "A", new ServiceDayTime(0), new ServiceDayTime(0)); - scheduleBuilder.addStopTime("T2", "B2", new ServiceDayTime(0), new ServiceDayTime(0)); - scheduleBuilder.addStopTime("T2", "C", new ServiceDayTime(0), new ServiceDayTime(0)); - + // build GTFS test schedule + GtfsToRaptorTestSchedule builder = new GtfsToRaptorTestSchedule(); for (Transfer transfer : scheduleTransfers) { - scheduleBuilder.addTransfer(transfer.fromStopId, transfer.toStopId, TransferType.MINIMUM_TIME, + builder.addTransfer(transfer.fromStopId, transfer.toStopId, TransferType.MINIMUM_TIME, transfer.duration); } + GtfsSchedule schedule = builder.build(); - GtfsSchedule schedule = scheduleBuilder.build(); - - List additionalTransfersList = additionalTransfers.stream() + List transferGenerators = List.of(stops -> additionalTransfers.stream() .map(transfer -> new TransferGenerator.Transfer(schedule.getStops().get(transfer.fromStopId()), schedule.getStops().get(transfer.toStopId()), transfer.duration())) - .collect(Collectors.toList()); + .collect(Collectors.toList())); - GtfsToRaptorConverter converter = new GtfsToRaptorConverter(schedule, additionalTransfersList, - new RaptorConfig()); - converter.convert(); + // run converter + GtfsToRaptorConverter converter = new GtfsToRaptorConverter(new RaptorConfig(), schedule, + transferGenerators); + converter.run(); return new RaptorBuilderData(converter); } diff --git a/src/test/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorTestSchedule.java b/src/test/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorTestSchedule.java new file mode 100644 index 0000000..987882c --- /dev/null +++ b/src/test/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorTestSchedule.java @@ -0,0 +1,73 @@ +package ch.naviqore.service.gtfs.raptor.convert; + +import ch.naviqore.gtfs.schedule.model.GtfsSchedule; +import ch.naviqore.gtfs.schedule.model.GtfsScheduleBuilder; +import ch.naviqore.gtfs.schedule.type.AccessibilityInformation; +import ch.naviqore.gtfs.schedule.type.RouteType; +import ch.naviqore.gtfs.schedule.type.ServiceDayTime; +import ch.naviqore.gtfs.schedule.type.TransferType; + +import java.time.DayOfWeek; +import java.time.LocalDate; +import java.util.EnumSet; + +/** + * GTFS schedule builder for testing. Builds a schedule as below + * 

    + *     |--------B1------------C1
+ *     |
+ * A---|       (B)      |-----C           (D)
+ *     |                |
+ *     |--------B2 -----|    (C2)
+ *
    + *
      + *
    • Route 1: Passes through A - B1 - C1
      • + *
    • Route 2: Passes through A - B2 - C
      • + *
    + * Stops B, C2 and D have no departures/arrivals and should not be included in the raptor conversion. + * Stops B and C are parents of stops B1, B2 and C1, C2, respectively. + */ +public class GtfsToRaptorTestSchedule { + + private final GtfsScheduleBuilder builder = GtfsSchedule.builder(); + + public GtfsToRaptorTestSchedule() { + setup(); + } + + private void setup() { + builder.addCalendar("always", EnumSet.allOf(DayOfWeek.class), LocalDate.MIN, LocalDate.MAX); + builder.addAgency("agency", "Some Agency", "", "America/New_York"); + + builder.addStop("A", "A", 0.0, 0.0); + builder.addStop("B", "B", 0.0, 0.0); + builder.addStop("B1", "B1", 0.0, 0.0, "B", AccessibilityInformation.UNKNOWN); + builder.addStop("B2", "B2", 0.0, 0.0, "B", AccessibilityInformation.UNKNOWN); + builder.addStop("C", "C", 0.0, 0.0); + builder.addStop("C1", "C1", 0.0, 0.0, "C", AccessibilityInformation.UNKNOWN); + builder.addStop("C2", "C2", 0.0, 0.0, "C", AccessibilityInformation.UNKNOWN); + builder.addStop("D", "D", 0.0, 0.0); + + // Route 1 goes from A, B1, C1 + builder.addRoute("R1", "agency", "R1", "R1", RouteType.parse(1)); + builder.addTrip("T1", "R1", "always", "C1"); + builder.addStopTime("T1", "A", new ServiceDayTime(0), new ServiceDayTime(0)); + builder.addStopTime("T1", "B1", new ServiceDayTime(0), new ServiceDayTime(0)); + builder.addStopTime("T1", "C1", new ServiceDayTime(0), new ServiceDayTime(0)); + + // Route 2 goes from A, B2, C + builder.addRoute("R2", "agency", "R2", "R2", RouteType.parse(1)); + builder.addTrip("T2", "R2", "always", "C"); + builder.addStopTime("T2", "A", new ServiceDayTime(0), new ServiceDayTime(0)); + builder.addStopTime("T2", "B2", new ServiceDayTime(0), new ServiceDayTime(0)); + builder.addStopTime("T2", "C", new ServiceDayTime(0), new ServiceDayTime(0)); + } + + public void addTransfer(String fromStopId, String toStopId, TransferType type, int duration) { + builder.addTransfer(fromStopId, toStopId, type, duration); + } + + public GtfsSchedule build() { + return builder.build(); + } +} diff --git a/src/test/java/ch/naviqore/service/gtfs/raptor/transfer/WalkTransferGeneratorTest.java b/src/test/java/ch/naviqore/service/gtfs/raptor/convert/WalkTransferGeneratorTest.java similarity index 96% rename from src/test/java/ch/naviqore/service/gtfs/raptor/transfer/WalkTransferGeneratorTest.java rename to src/test/java/ch/naviqore/service/gtfs/raptor/convert/WalkTransferGeneratorTest.java index 2aba0cf..7dce734 100644 --- a/src/test/java/ch/naviqore/service/gtfs/raptor/transfer/WalkTransferGeneratorTest.java +++ b/src/test/java/ch/naviqore/service/gtfs/raptor/convert/WalkTransferGeneratorTest.java @@ -1,4 +1,4 @@ -package ch.naviqore.service.gtfs.raptor.transfer; +package ch.naviqore.service.gtfs.raptor.convert; import ch.naviqore.gtfs.schedule.model.GtfsSchedule; import ch.naviqore.gtfs.schedule.model.GtfsScheduleBuilder; @@ -14,10 +14,7 @@ import java.time.DayOfWeek; import java.time.LocalDate; -import java.util.EnumSet; -import java.util.List; -import java.util.Map; -import java.util.NoSuchElementException; +import java.util.*; import static org.junit.jupiter.api.Assertions.*; @@ -177,9 +174,16 @@ void nullSpatialIndex_shouldThrowException() { @Nested class CreateTransfers { + private Set stops; + + @BeforeEach + void setUp() { + stops = new HashSet<>(schedule.getStops().values()); + } + @Test void expectedBehavior() { - List transfers = generator.generateTransfers(schedule); + List transfers = generator.generateTransfers(stops); assertNoSameStopTransfers(transfers); assertTransfersGenerated(transfers, "stop1", "stop2"); assertTransfersGenerated(transfers, "stop1", "stop3"); @@ -190,7 +194,7 @@ void expectedBehavior() { void expectedBehavior_withIncreasedSearchRadius() { WalkTransferGenerator generator = new WalkTransferGenerator(DEFAULT_CALCULATOR, DEFAULT_MINIMUM_TRANSFER_TIME, DEFAULT_ACCESS_EGRESS_TIME, 1000, spatialIndex); - List transfers = generator.generateTransfers(schedule); + List transfers = generator.generateTransfers(stops); assertNoSameStopTransfers(transfers); assertTransfersGenerated(transfers, "stop1", "stop2"); assertTransfersGenerated(transfers, "stop1", "stop3"); @@ -202,7 +206,7 @@ void expectedBehavior_shouldBeMinimumTransferTime() { int minimumTransferTime = 2000; WalkTransferGenerator highMinTimeGenerator = new WalkTransferGenerator(DEFAULT_CALCULATOR, minimumTransferTime, DEFAULT_ACCESS_EGRESS_TIME, DEFAULT_SEARCH_RADIUS, spatialIndex); - List transfers = highMinTimeGenerator.generateTransfers(schedule); + List transfers = highMinTimeGenerator.generateTransfers(stops); assertNoSameStopTransfers(transfers); assertTransfersGenerated(transfers, "stop1", "stop2"); assertTransfersGenerated(transfers, "stop1", "stop3"); From 26f29a6ebdbfcec42fd80d8c3e56c8e7830cf3cd Mon Sep 17 00:00:00 2001 From: Lukas Connolly Date: Sun, 12 Jan 2025 20:59:53 +0100 Subject: [PATCH 08/18] REFACTOR: 158 - Simplify transfer generation in gtfs raptor converter. --- .../raptor/convert/GtfsToRaptorConverter.java | 39 +++++-------------- 1 file changed, 9 insertions(+), 30 deletions(-) diff --git a/src/main/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorConverter.java b/src/main/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorConverter.java index 07f19a3..49857bd 100644 --- a/src/main/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorConverter.java +++ b/src/main/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorConverter.java @@ -104,43 +104,22 @@ private void addRoute(GtfsRoutePartitioner.SubRoute subRoute) { * correct priority order. */ private void processAllTransfers() { - createAndAddAdditionalTransfers(); + createAndAddTransfersFromTransferGenerators(); processStopAndParentChildTransfers(); addGtfsTransfersWithPrecedence(); } /** - * Create and add additional transfers. + * Create and add transfers from transfer generators. */ - private void createAndAddAdditionalTransfers() { - // create lookup for GTFS transfers in schedule to prevent adding duplicates later - Set gtfsTransfers = new HashSet<>(); - schedule.getStops().values().forEach(stop -> stop.getTransfers().forEach(transfer -> { - if (transfer.getTransferType() == TransferType.MINIMUM_TIME) { - String key = transfer.getFromStop().getId() + transfer.getToStop().getId(); - gtfsTransfers.add(key); - } - })); - - // run all generators in sequence and collect all generated transfers - List uncheckedGeneratedTransfers = transferGenerators.stream() + private void createAndAddTransfersFromTransferGenerators() { + // reverse order ensures that the lowest priority transfers are overwritten if a higher priority transfer + // generator generated transfer or gtfs based transfer exists. + transferGenerators.stream() + .sorted(Collections.reverseOrder()) .flatMap(generator -> generator.generateTransfers(addedStops).stream()) - .toList(); - - // add all generated Transfers to the Lookup if they are not already in the GTFS Transfers or - // where already generated by a preceding generator - Map generatedTransfers = new HashMap<>(); - for (TransferGenerator.Transfer transfer : uncheckedGeneratedTransfers) { - String key = transfer.from().getId() + transfer.to().getId(); - if (!gtfsTransfers.contains(key) && !generatedTransfers.containsKey(key)) { - generatedTransfers.put(key, transfer); - } - } - - // add generated transfers to builder - for (TransferGenerator.Transfer transfer : generatedTransfers.values()) { - builder.addTransfer(transfer.from().getId(), transfer.to().getId(), transfer.duration()); - } + .forEach(transfer -> builder.addTransfer(transfer.from().getId(), transfer.to().getId(), + transfer.duration())); } /** From 036b5de231a0f38a289cc35761d5ba1b498c4a32 Mon Sep 17 00:00:00 2001 From: Lukas Connolly Date: Sun, 12 Jan 2025 22:37:27 +0100 Subject: [PATCH 09/18] TEST: 158 - Add test case for multiple transfer generators. --- .../convert/GtfsToRaptorConverterIT.java | 68 +++++++++++++++---- 1 file changed, 54 insertions(+), 14 deletions(-) diff --git a/src/test/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorConverterIT.java b/src/test/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorConverterIT.java index 4918cf8..77417c6 100644 --- a/src/test/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorConverterIT.java +++ b/src/test/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorConverterIT.java @@ -3,6 +3,7 @@ import ch.naviqore.gtfs.schedule.GtfsScheduleReader; import ch.naviqore.gtfs.schedule.GtfsScheduleTestData; import ch.naviqore.gtfs.schedule.model.GtfsSchedule; +import ch.naviqore.gtfs.schedule.model.Stop; import ch.naviqore.gtfs.schedule.type.TransferType; import ch.naviqore.raptor.RaptorAlgorithm; import ch.naviqore.raptor.router.RaptorConfig; @@ -16,10 +17,7 @@ import java.io.IOException; import java.lang.reflect.Field; import java.nio.file.Path; -import java.util.ArrayList; -import java.util.List; -import java.util.Map; -import java.util.Set; +import java.util.*; import java.util.stream.Collectors; import static org.assertj.core.api.Assertions.assertThat; @@ -66,7 +64,7 @@ class ManualSchedule { * Stops B and C are parents of stops B1, B2 and C1, C2, respectively. */ static RaptorBuilderData convertRaptor(List scheduleTransfers, - List additionalTransfers) throws NoSuchFieldException, IllegalAccessException { + List transferGenerators) throws NoSuchFieldException, IllegalAccessException { // build GTFS test schedule GtfsToRaptorTestSchedule builder = new GtfsToRaptorTestSchedule(); for (Transfer transfer : scheduleTransfers) { @@ -75,11 +73,6 @@ static RaptorBuilderData convertRaptor(List scheduleTransfers, } GtfsSchedule schedule = builder.build(); - List transferGenerators = List.of(stops -> additionalTransfers.stream() - .map(transfer -> new TransferGenerator.Transfer(schedule.getStops().get(transfer.fromStopId()), - schedule.getStops().get(transfer.toStopId()), transfer.duration())) - .collect(Collectors.toList())); - // run converter GtfsToRaptorConverter converter = new GtfsToRaptorConverter(new RaptorConfig(), schedule, transferGenerators); @@ -170,20 +163,67 @@ void betweenStopTransfersOnParentStops() throws NoSuchFieldException, IllegalAcc @Test void additionalTransfers() throws NoSuchFieldException, IllegalAccessException { List scheduleTransfers = List.of(new Transfer("B1", "B1", 120)); - List additionalTransfers = List.of(new Transfer("B1", "B1", 60), new Transfer("B2", "B2", 60), - new Transfer("B1", "B2", 120)); - RaptorBuilderData data = convertRaptor(scheduleTransfers, additionalTransfers); + // Create a list of TransferGenerators that generate specific transfers + List transferGenerators = List.of(new SimpleTransferGenerator( + List.of(new Transfer("B1", "B1", 60), new Transfer("B2", "B2", 60), + new Transfer("B1", "B2", 120)))); + + RaptorBuilderData data = convertRaptor(scheduleTransfers, transferGenerators); data.assertStops(Set.of("A", "B1", "B2", "C", "C1")); - // since additional transfers should not be applied if gtfs data exists B1-B1 should remain 120 + // since LL should not be applied if gtfs data exists B1-B1 should remain 120 data.assertSameStopTransfers(Set.of("B1-120", "B2-60")); data.assertBetweenStopTransfers(Set.of("B1-B2")); } + @Test + void multipleTransferGenerators() throws NoSuchFieldException, IllegalAccessException { + List scheduleTransfers = List.of(new Transfer("B1", "B1", 120)); + + TransferGenerator transferGenerator1 = new SimpleTransferGenerator( + List.of(new Transfer("B1", "B1", 90), new Transfer("B2", "B2", 90))); + + TransferGenerator transferGenerator2 = new SimpleTransferGenerator( + List.of(new Transfer("B1", "B1", 60), new Transfer("B2", "B2", 60), new Transfer("C1", "C1", 60))); + + List transferGenerators = List.of(transferGenerator1, transferGenerator2); + + RaptorBuilderData data = convertRaptor(scheduleTransfers, transferGenerators); + data.assertStops(Set.of("A", "B1", "B2", "C", "C1")); + // because schedule transfers have the highest priority B1-B1 should be 120 + // in case of multiple transfer generators the first should have the highest and the last should have the + // lowest priority, therefore B2-B2 should be 90 (defined in both) and C1-C1 should be 60 (only defined + // in last transfer generator) + data.assertSameStopTransfers(Set.of("B1-120", "B2-90", "C1-60")); + + } + record Transfer(String fromStopId, String toStopId, int duration) { } + class SimpleTransferGenerator implements TransferGenerator { + + private List transfers; + + public SimpleTransferGenerator(List transfers) { + this.transfers = transfers; + } + + @Override + public List generateTransfers(Collection stops) { + return this.transfers.stream() + .map(transfer -> new Transfer(stops.stream() + .filter(stop -> stop.getId().equals(transfer.fromStopId())) + .findFirst() + .orElseThrow(), stops.stream() + .filter(stop -> stop.getId().equals(transfer.toStopId())) + .findFirst() + .orElseThrow(), transfer.duration)) + .toList(); + } + } + static class RaptorBuilderData { Map stops; From 45c3650fb715235298ccb2530d0598c6d507077d Mon Sep 17 00:00:00 2001 From: Lukas Connolly Date: Sun, 12 Jan 2025 22:40:59 +0100 Subject: [PATCH 10/18] FIX: 158 - Fix reversing list order of transfer generators for proper precedence order. --- .../service/gtfs/raptor/convert/GtfsToRaptorConverter.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/main/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorConverter.java b/src/main/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorConverter.java index 49857bd..23c2fca 100644 --- a/src/main/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorConverter.java +++ b/src/main/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorConverter.java @@ -8,6 +8,7 @@ import lombok.extern.slf4j.Slf4j; import java.util.*; +import java.util.stream.Collectors; /** * Maps GTFS schedule to Raptor @@ -116,7 +117,10 @@ private void createAndAddTransfersFromTransferGenerators() { // reverse order ensures that the lowest priority transfers are overwritten if a higher priority transfer // generator generated transfer or gtfs based transfer exists. transferGenerators.stream() - .sorted(Collections.reverseOrder()) + .collect(Collectors.collectingAndThen(Collectors.toList(), list -> { + Collections.reverse(list); + return list.stream(); + })) .flatMap(generator -> generator.generateTransfers(addedStops).stream()) .forEach(transfer -> builder.addTransfer(transfer.from().getId(), transfer.to().getId(), transfer.duration())); From 87b1bd7d6bf0abcaa7aef7e70c0a0f5f6368c639 Mon Sep 17 00:00:00 2001 From: Lukas Connolly Date: Sun, 12 Jan 2025 23:00:51 +0100 Subject: [PATCH 11/18] ENH: 158 - Add custom exceptions for raptor router algorithm. --- .../ch/naviqore/raptor/RaptorAlgorithm.java | 29 +++++++++++++++++++ .../naviqore/raptor/router/RaptorRouter.java | 16 +++++----- 2 files changed, 37 insertions(+), 8 deletions(-) diff --git a/src/main/java/ch/naviqore/raptor/RaptorAlgorithm.java b/src/main/java/ch/naviqore/raptor/RaptorAlgorithm.java index 28d3da4..6be4a2d 100644 --- a/src/main/java/ch/naviqore/raptor/RaptorAlgorithm.java +++ b/src/main/java/ch/naviqore/raptor/RaptorAlgorithm.java @@ -14,6 +14,9 @@ public interface RaptorAlgorithm { * @param config Query configuration * @return a list of pareto-optimal earliest arrival connections sorted in ascending order by the number of route * legs (rounds) + * @throws InvalidStopException if departure or arrival stops are invalid + * @throws InvalidTimeException if departure or arrival times are invalid + * @throws IllegalArgumentException for other argument related errors */ List routeEarliestArrival(Map departureStops, Map arrivalStops, QueryConfig config); @@ -26,6 +29,9 @@ List routeEarliestArrival(Map departureStops, * @param config Query configuration * @return a list of pareto-optimal latest departure connections sorted in ascending order by the number of route * legs (rounds) + * @throws InvalidStopException if departure or arrival stops are invalid + * @throws InvalidTimeException if departure or arrival times are invalid + * @throws IllegalArgumentException for other argument related errors */ List routeLatestDeparture(Map departureStops, Map arrivalStops, QueryConfig config); @@ -38,8 +44,31 @@ List routeLatestDeparture(Map departureStops, Map routeIsolines(Map sourceStops, TimeType timeType, QueryConfig config); + class InvalidStopException extends IllegalArgumentException { + public InvalidStopException(String message) { + super(message); + } + + public InvalidStopException(String message, Throwable cause) { + super(message, cause); + } + } + + class InvalidTimeException extends IllegalArgumentException { + public InvalidTimeException(String message) { + super(message); + } + + public InvalidTimeException(String message, Throwable cause) { + super(message, cause); + } + } + } diff --git a/src/main/java/ch/naviqore/raptor/router/RaptorRouter.java b/src/main/java/ch/naviqore/raptor/router/RaptorRouter.java index 2c1c863..b3050c3 100644 --- a/src/main/java/ch/naviqore/raptor/router/RaptorRouter.java +++ b/src/main/java/ch/naviqore/raptor/router/RaptorRouter.java @@ -149,24 +149,24 @@ private static class InputValidator { private static void checkNonNullOrEmptyStops(Map stops, String labelSource) { if (stops == null) { - throw new IllegalArgumentException(String.format("%s stops must not be null.", labelSource)); + throw new InvalidStopException(String.format("%s stops must not be null.", labelSource)); } if (stops.isEmpty()) { - throw new IllegalArgumentException(String.format("%s stops must not be empty.", labelSource)); + throw new InvalidStopException(String.format("%s stops must not be empty.", labelSource)); } } private static void validateSourceStopTimes(Map sourceStops) { // check that no null values are present if (sourceStops.values().stream().anyMatch(Objects::isNull)) { - throw new IllegalArgumentException("Source stop times must not be null."); + throw new InvalidTimeException("Source stop times must not be null."); } // get min and max values LocalDateTime min = sourceStops.values().stream().min(LocalDateTime::compareTo).orElseThrow(); LocalDateTime max = sourceStops.values().stream().max(LocalDateTime::compareTo).orElseThrow(); if (Duration.between(min, max).getSeconds() > MAX_DIFFERENCE_IN_SOURCE_STOP_TIMES) { - throw new IllegalArgumentException("Difference between source stop times must be less than 24 hours."); + throw new InvalidTimeException("Difference between source stop times must be less than 24 hours."); } } @@ -176,7 +176,7 @@ private static void validateStopPermutations(Map sourceStops, // ensure departure and arrival stops are not the same if (!Collections.disjoint(sourceStops.keySet(), targetStops.keySet())) { - throw new IllegalArgumentException("Source and target stop IDs must not be the same."); + throw new InvalidStopException("Source and target stop IDs must not be the same."); } } @@ -191,14 +191,14 @@ private static void validateWalkingTimeToTarget(int walkingDurationToTarget) { * Validate the stops provided in the query. This method will check that the map of stop ids and their * corresponding departure / walk to target times are valid. This is done by checking if the map is not empty * and then checking each entry if the stop id is present in the lookup. If not it is removed from the query. If - * no valid stops are found an IllegalArgumentException is thrown. + * no valid stops are found an InvalidStopException is thrown. * * @param stops the stops to validate. * @return a map of valid stop IDs and their corresponding departure / walk to target times. */ private Map validateStopsAndGetIndices(Map stops) { if (stops.isEmpty()) { - throw new IllegalArgumentException("At least one stop ID must be provided."); + throw new InvalidStopException("At least one stop ID must be provided."); } // loop over all stop pairs and check if stop exists in raptor, then validate departure time @@ -215,7 +215,7 @@ private Map validateStopsAndGetIndices(Map st } if (validStopIds.isEmpty()) { - throw new IllegalArgumentException("No valid stops provided."); + throw new InvalidStopException("No valid stops provided."); } return validStopIds; From 68f3c9df97fde04ae935781ad38ff0a4e75c5b85 Mon Sep 17 00:00:00 2001 From: Lukas Connolly Date: Sun, 12 Jan 2025 23:10:30 +0100 Subject: [PATCH 12/18] ENH: 158 - Catch specifically InvalidStopExceptions in service. --- .../gtfs/raptor/GtfsRaptorService.java | 25 +++++++++++-------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/src/main/java/ch/naviqore/service/gtfs/raptor/GtfsRaptorService.java b/src/main/java/ch/naviqore/service/gtfs/raptor/GtfsRaptorService.java index 2b2b26f..bd8dd20 100644 --- a/src/main/java/ch/naviqore/service/gtfs/raptor/GtfsRaptorService.java +++ b/src/main/java/ch/naviqore/service/gtfs/raptor/GtfsRaptorService.java @@ -1,6 +1,7 @@ package ch.naviqore.service.gtfs.raptor; import ch.naviqore.gtfs.schedule.model.GtfsSchedule; +import ch.naviqore.raptor.RaptorAlgorithm; import ch.naviqore.raptor.router.RaptorRouter; import ch.naviqore.service.*; import ch.naviqore.service.config.ConnectionQueryConfig; @@ -211,11 +212,12 @@ private List getConnections(@Nullable ch.naviqore.gtfs.schedule.mode } else { connections = raptorRouter.routeLatestDeparture(targetStops, sourceStops, TypeMapper.map(config)); } - } catch (IllegalArgumentException e) { - log.debug("RaptorRouter exception: {}", e.getMessage()); + } catch (RaptorAlgorithm.InvalidStopException e){ + // TODO: try location based routing instead + log.debug("{}: {}", e.getClass().getSimpleName(), e.getMessage()); return List.of(); - // TODO: Introduce exception handling or exception type on in raptor router. - // throw new ConnectionRoutingException(e); + } catch (IllegalArgumentException e) { + throw new ConnectionRoutingException(e); } // assemble connection results @@ -308,10 +310,11 @@ public Map getIsolines(GeoCoordinate source, LocalDateTime tim return mapToStopConnectionMap( raptorRouter.routeIsolines(sourceStops, TypeMapper.map(timeType), TypeMapper.map(config)), source, config, timeType); - } catch (IllegalArgumentException e) { - log.debug(e.getMessage()); + } catch (RaptorAlgorithm.InvalidStopException e){ + log.debug("{}: {}", e.getClass().getSimpleName(), e.getMessage()); return Map.of(); - // TODO: throw new ConnectionRoutingException(e); + } catch (IllegalArgumentException e) { + throw new ConnectionRoutingException(e); } } @@ -324,10 +327,12 @@ public Map getIsolines(Stop source, LocalDateTime time, TimeTy return mapToStopConnectionMap( raptorRouter.routeIsolines(sourceStops, TypeMapper.map(timeType), TypeMapper.map(config)), null, config, timeType); - } catch (IllegalArgumentException e) { - log.debug(e.getMessage()); + } catch (RaptorAlgorithm.InvalidStopException e){ + // TODO: Try location based iso line routing? + log.debug("{}: {}", e.getClass().getSimpleName(), e.getMessage()); return Map.of(); - // TODO: throw new ConnectionRoutingException(e); + } catch (IllegalArgumentException e) { + throw new ConnectionRoutingException(e); } } From 7755d7365c7268177067c388d99045edbc4299bc Mon Sep 17 00:00:00 2001 From: Lukas Connolly Date: Sun, 12 Jan 2025 23:11:55 +0100 Subject: [PATCH 13/18] STYLE: 158 - Rename mapper to converter to be more precise. --- .../service/gtfs/raptor/convert/GtfsToRaptorConverterIT.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorConverterIT.java b/src/test/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorConverterIT.java index 77417c6..730c849 100644 --- a/src/test/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorConverterIT.java +++ b/src/test/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorConverterIT.java @@ -37,8 +37,8 @@ void setUp(@TempDir Path tempDir) throws IOException { @Test void shouldConvertGtfsScheduleToRaptor() { - GtfsToRaptorConverter mapper = new GtfsToRaptorConverter(new RaptorConfig(), schedule); - RaptorAlgorithm raptor = mapper.run(); + GtfsToRaptorConverter converter = new GtfsToRaptorConverter(new RaptorConfig(), schedule); + RaptorAlgorithm raptor = converter.run(); assertThat(raptor).isNotNull(); } From 015c1e7d14845c7cfe6f0f79dea591265401667c Mon Sep 17 00:00:00 2001 From: Merlin Unterfinger Date: Mon, 13 Jan 2025 11:23:18 +0100 Subject: [PATCH 14/18] STYLE: 158 - Refactor project --- .../ch/naviqore/raptor/RaptorAlgorithm.java | 14 +++--- .../gtfs/raptor/GtfsRaptorService.java | 6 +-- .../convert/GtfsToRaptorConverterIT.java | 45 +++++++++---------- 3 files changed, 33 insertions(+), 32 deletions(-) diff --git a/src/main/java/ch/naviqore/raptor/RaptorAlgorithm.java b/src/main/java/ch/naviqore/raptor/RaptorAlgorithm.java index 6be4a2d..871217e 100644 --- a/src/main/java/ch/naviqore/raptor/RaptorAlgorithm.java +++ b/src/main/java/ch/naviqore/raptor/RaptorAlgorithm.java @@ -14,8 +14,8 @@ public interface RaptorAlgorithm { * @param config Query configuration * @return a list of pareto-optimal earliest arrival connections sorted in ascending order by the number of route * legs (rounds) - * @throws InvalidStopException if departure or arrival stops are invalid - * @throws InvalidTimeException if departure or arrival times are invalid + * @throws InvalidStopException if departure or arrival stops are invalid + * @throws InvalidTimeException if departure or arrival times are invalid * @throws IllegalArgumentException for other argument related errors */ List routeEarliestArrival(Map departureStops, Map arrivalStops, @@ -29,8 +29,8 @@ List routeEarliestArrival(Map departureStops, * @param config Query configuration * @return a list of pareto-optimal latest departure connections sorted in ascending order by the number of route * legs (rounds) - * @throws InvalidStopException if departure or arrival stops are invalid - * @throws InvalidTimeException if departure or arrival times are invalid + * @throws InvalidStopException if departure or arrival stops are invalid + * @throws InvalidTimeException if departure or arrival times are invalid * @throws IllegalArgumentException for other argument related errors */ List routeLatestDeparture(Map departureStops, Map arrivalStops, @@ -44,8 +44,8 @@ List routeLatestDeparture(Map departureStops, Map routeIsolines(Map sourceStops, TimeType timeType, @@ -61,6 +61,8 @@ public InvalidStopException(String message, Throwable cause) { } } + class MyException extends IllegalArgumentException {} + class InvalidTimeException extends IllegalArgumentException { public InvalidTimeException(String message) { super(message); diff --git a/src/main/java/ch/naviqore/service/gtfs/raptor/GtfsRaptorService.java b/src/main/java/ch/naviqore/service/gtfs/raptor/GtfsRaptorService.java index bd8dd20..9fd5ab5 100644 --- a/src/main/java/ch/naviqore/service/gtfs/raptor/GtfsRaptorService.java +++ b/src/main/java/ch/naviqore/service/gtfs/raptor/GtfsRaptorService.java @@ -212,7 +212,7 @@ private List getConnections(@Nullable ch.naviqore.gtfs.schedule.mode } else { connections = raptorRouter.routeLatestDeparture(targetStops, sourceStops, TypeMapper.map(config)); } - } catch (RaptorAlgorithm.InvalidStopException e){ + } catch (RaptorAlgorithm.InvalidStopException e) { // TODO: try location based routing instead log.debug("{}: {}", e.getClass().getSimpleName(), e.getMessage()); return List.of(); @@ -310,7 +310,7 @@ public Map getIsolines(GeoCoordinate source, LocalDateTime tim return mapToStopConnectionMap( raptorRouter.routeIsolines(sourceStops, TypeMapper.map(timeType), TypeMapper.map(config)), source, config, timeType); - } catch (RaptorAlgorithm.InvalidStopException e){ + } catch (RaptorAlgorithm.InvalidStopException e) { log.debug("{}: {}", e.getClass().getSimpleName(), e.getMessage()); return Map.of(); } catch (IllegalArgumentException e) { @@ -327,7 +327,7 @@ public Map getIsolines(Stop source, LocalDateTime time, TimeTy return mapToStopConnectionMap( raptorRouter.routeIsolines(sourceStops, TypeMapper.map(timeType), TypeMapper.map(config)), null, config, timeType); - } catch (RaptorAlgorithm.InvalidStopException e){ + } catch (RaptorAlgorithm.InvalidStopException e) { // TODO: Try location based iso line routing? log.debug("{}: {}", e.getClass().getSimpleName(), e.getMessage()); return Map.of(); diff --git a/src/test/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorConverterIT.java b/src/test/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorConverterIT.java index 730c849..b03ca50 100644 --- a/src/test/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorConverterIT.java +++ b/src/test/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorConverterIT.java @@ -18,7 +18,6 @@ import java.lang.reflect.Field; import java.nio.file.Path; import java.util.*; -import java.util.stream.Collectors; import static org.assertj.core.api.Assertions.assertThat; @@ -202,28 +201,6 @@ void multipleTransferGenerators() throws NoSuchFieldException, IllegalAccessExce record Transfer(String fromStopId, String toStopId, int duration) { } - class SimpleTransferGenerator implements TransferGenerator { - - private List transfers; - - public SimpleTransferGenerator(List transfers) { - this.transfers = transfers; - } - - @Override - public List generateTransfers(Collection stops) { - return this.transfers.stream() - .map(transfer -> new Transfer(stops.stream() - .filter(stop -> stop.getId().equals(transfer.fromStopId())) - .findFirst() - .orElseThrow(), stops.stream() - .filter(stop -> stop.getId().equals(transfer.toStopId())) - .findFirst() - .orElseThrow(), transfer.duration)) - .toList(); - } - } - static class RaptorBuilderData { Map stops; @@ -326,6 +303,28 @@ void assertBetweenStopTransfers(Set ids) { } + class SimpleTransferGenerator implements TransferGenerator { + + private final List transfers; + + public SimpleTransferGenerator(List transfers) { + this.transfers = transfers; + } + + @Override + public List generateTransfers(Collection stops) { + return this.transfers.stream() + .map(transfer -> new Transfer(stops.stream() + .filter(stop -> stop.getId().equals(transfer.fromStopId())) + .findFirst() + .orElseThrow(), stops.stream() + .filter(stop -> stop.getId().equals(transfer.toStopId())) + .findFirst() + .orElseThrow(), transfer.duration)) + .toList(); + } + } + } } \ No newline at end of file From f3c2a41f15137be7a526308e2435556c747b98a5 Mon Sep 17 00:00:00 2001 From: Merlin Unterfinger Date: Mon, 13 Jan 2025 11:24:11 +0100 Subject: [PATCH 15/18] STYLE: 158 - Remove unused constructors --- src/main/java/ch/naviqore/raptor/RaptorAlgorithm.java | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/main/java/ch/naviqore/raptor/RaptorAlgorithm.java b/src/main/java/ch/naviqore/raptor/RaptorAlgorithm.java index 871217e..c4439b3 100644 --- a/src/main/java/ch/naviqore/raptor/RaptorAlgorithm.java +++ b/src/main/java/ch/naviqore/raptor/RaptorAlgorithm.java @@ -55,22 +55,12 @@ class InvalidStopException extends IllegalArgumentException { public InvalidStopException(String message) { super(message); } - - public InvalidStopException(String message, Throwable cause) { - super(message, cause); - } } - class MyException extends IllegalArgumentException {} - class InvalidTimeException extends IllegalArgumentException { public InvalidTimeException(String message) { super(message); } - - public InvalidTimeException(String message, Throwable cause) { - super(message, cause); - } } } From 0a5489595c8d65246fa809944dd9bf76958ad123 Mon Sep 17 00:00:00 2001 From: Merlin Unterfinger Date: Mon, 13 Jan 2025 11:42:01 +0100 Subject: [PATCH 16/18] STYLE: 158 - Analyze complete project and solve code issues --- .../service/gtfs/raptor/TypeMapper.java | 31 ++++++------------- .../java/ch/naviqore/app/TestRestRequest.java | 2 +- .../app/controller/RoutingControllerTest.java | 4 +-- .../raptor/router/RaptorRouterTest.java | 10 +++--- .../gtfs/raptor/GtfsRaptorServiceIT.java | 4 +-- 5 files changed, 18 insertions(+), 33 deletions(-) diff --git a/src/main/java/ch/naviqore/service/gtfs/raptor/TypeMapper.java b/src/main/java/ch/naviqore/service/gtfs/raptor/TypeMapper.java index 9a8a264..57fb7f4 100644 --- a/src/main/java/ch/naviqore/service/gtfs/raptor/TypeMapper.java +++ b/src/main/java/ch/naviqore/service/gtfs/raptor/TypeMapper.java @@ -137,28 +137,15 @@ public static EnumSet map(EnumSet tra private static TravelMode map(RouteType routeType) { DefaultRouteType defaultRouteType = RouteTypeMapper.map(routeType); - switch (defaultRouteType) { - case BUS: - case TROLLEYBUS: - return TravelMode.BUS; - case TRAM: - case CABLE_TRAM: - return TravelMode.TRAM; - case RAIL: - case MONORAIL: - return TravelMode.RAIL; - case FERRY: - return TravelMode.SHIP; - case SUBWAY: - return TravelMode.SUBWAY; - case AERIAL_LIFT: - return TravelMode.AERIAL_LIFT; - case FUNICULAR: - return TravelMode.FUNICULAR; - default: - // should never happen - throw new IllegalArgumentException("Route type not supported"); - } + return switch (defaultRouteType) { + case BUS, TROLLEYBUS -> TravelMode.BUS; + case TRAM, CABLE_TRAM -> TravelMode.TRAM; + case RAIL, MONORAIL -> TravelMode.RAIL; + case FERRY -> TravelMode.SHIP; + case SUBWAY -> TravelMode.SUBWAY; + case AERIAL_LIFT -> TravelMode.AERIAL_LIFT; + case FUNICULAR -> TravelMode.FUNICULAR; + }; } private static Leg createPublicTransitLeg(ch.naviqore.raptor.Leg leg, GtfsSchedule schedule, int distance) { diff --git a/src/test/java/ch/naviqore/app/TestRestRequest.java b/src/test/java/ch/naviqore/app/TestRestRequest.java index b304768..4fffa1b 100644 --- a/src/test/java/ch/naviqore/app/TestRestRequest.java +++ b/src/test/java/ch/naviqore/app/TestRestRequest.java @@ -20,7 +20,7 @@ public TestRestRequest(TestRestTemplate template) { } public TestRestRequest setPort(int port) { - this.uriBuilder = UriComponentsBuilder.fromHttpUrl(String.format("http://localhost:%d", port)); + this.uriBuilder = UriComponentsBuilder.fromUriString(String.format("http://localhost:%d", port)); return this; } diff --git a/src/test/java/ch/naviqore/app/controller/RoutingControllerTest.java b/src/test/java/ch/naviqore/app/controller/RoutingControllerTest.java index f3f6caa..6a8abdd 100644 --- a/src/test/java/ch/naviqore/app/controller/RoutingControllerTest.java +++ b/src/test/java/ch/naviqore/app/controller/RoutingControllerTest.java @@ -331,7 +331,7 @@ void testFromStopReturnConnectionsTrue() { Connection connection = stopConnection.getConnection(); // make sure each connection has a departure time after/equal the expected start time assertFalse(connection.getLegs().getFirst().getDepartureTime().isBefore(expectedStartTime)); - assertEquals(connection.getLegs().getFirst().getFromStop().getId(), sourceStopId); + assertEquals(sourceStopId, connection.getLegs().getFirst().getFromStop().getId()); Trip trip = stopConnection.getConnectingLeg().getTrip(); if (trip != null) { @@ -444,7 +444,7 @@ void testFromStopReturnConnectionsTrueTimeTypeArrival() { // make sure each connection has an arrival time after/equal the expected start time assertFalse(connection.getLegs().getLast().getArrivalTime().isAfter(expectedArrivalTime)); - assertEquals(connection.getLegs().getLast().getToStop().getId(), sourceStopId); + assertEquals(sourceStopId, connection.getLegs().getLast().getToStop().getId()); Trip trip = connectingLeg.getTrip(); if (trip != null) { diff --git a/src/test/java/ch/naviqore/raptor/router/RaptorRouterTest.java b/src/test/java/ch/naviqore/raptor/router/RaptorRouterTest.java index 54835ac..3852bef 100644 --- a/src/test/java/ch/naviqore/raptor/router/RaptorRouterTest.java +++ b/src/test/java/ch/naviqore/raptor/router/RaptorRouterTest.java @@ -477,8 +477,8 @@ void findConnectionWithZeroTravelTimeTripsAndConsequentWalkTransfer(RaptorRouter Connection connection = connections.getFirst(); Leg firstRouteLeg = connection.getLegs().getFirst(); Leg walkTransferLeg = connection.getLegs().get(1); - assertEquals(firstRouteLeg.getType(), Leg.Type.ROUTE); - assertEquals(walkTransferLeg.getType(), Leg.Type.WALK_TRANSFER); + assertEquals(Leg.Type.ROUTE, firstRouteLeg.getType()); + assertEquals(Leg.Type.WALK_TRANSFER, walkTransferLeg.getType()); assertEquals(firstRouteLeg.getDepartureTime(), firstRouteLeg.getArrivalTime(), "Departure time at C should be equal to arrival time at D"); assertEquals(firstRouteLeg.getDepartureTime(), walkTransferLeg.getDepartureTime(), @@ -921,21 +921,19 @@ void throwErrorForInvalidWalkToTargetTimeFromOneOfManyTargetStops() { @Test void throwErrorNullSourceStops() { - Map sourceStops = null; Map targetStops = Map.of(STOP_H, 0); assertThrows(IllegalArgumentException.class, - () -> RaptorRouterTestHelpers.routeEarliestArrival(raptor, sourceStops, targetStops), + () -> RaptorRouterTestHelpers.routeEarliestArrival(raptor, null, targetStops), "Source stops cannot be null"); } @Test void throwErrorNullTargetStops() { Map sourceStops = Map.of(STOP_A, START_OF_DAY); - Map targetStops = null; assertThrows(IllegalArgumentException.class, - () -> RaptorRouterTestHelpers.routeEarliestArrival(raptor, sourceStops, targetStops), + () -> RaptorRouterTestHelpers.routeEarliestArrival(raptor, sourceStops, null), "Target stops cannot be null"); } diff --git a/src/test/java/ch/naviqore/service/gtfs/raptor/GtfsRaptorServiceIT.java b/src/test/java/ch/naviqore/service/gtfs/raptor/GtfsRaptorServiceIT.java index 476076c..20d0aeb 100644 --- a/src/test/java/ch/naviqore/service/gtfs/raptor/GtfsRaptorServiceIT.java +++ b/src/test/java/ch/naviqore/service/gtfs/raptor/GtfsRaptorServiceIT.java @@ -358,7 +358,7 @@ void setUp() { class Connection { @Test - void shouldThrowOnInvalidStop() throws StopNotFoundException, ConnectionRoutingException { + void shouldThrowOnInvalidStop() { assertThrows(StopNotFoundException.class, () -> service.getConnections(service.getStopById("NOT_A_STOP"), service.getStopById("C"), DATE_TIME, TimeType.DEPARTURE, QUERY_CONFIG)); @@ -405,7 +405,7 @@ void source_arrival() throws ConnectionRoutingException, StopNotFoundException { class Isolines { @Test - void shouldThrowOnInvalidStop() throws StopNotFoundException, ConnectionRoutingException { + void shouldThrowOnInvalidStop() { assertThrows(StopNotFoundException.class, () -> service.getIsolines(service.getStopById("NOT_A_STOP"), DATE_TIME, TimeType.DEPARTURE, QUERY_CONFIG)); From 20eec36edf7302aca54d65770e5afa25838f234d Mon Sep 17 00:00:00 2001 From: Merlin Unterfinger Date: Mon, 13 Jan 2025 11:44:33 +0100 Subject: [PATCH 17/18] STYLE: 158 - Remove snake_case in test cases --- .../naviqore/service/gtfs/raptor/GtfsRaptorServiceIT.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/test/java/ch/naviqore/service/gtfs/raptor/GtfsRaptorServiceIT.java b/src/test/java/ch/naviqore/service/gtfs/raptor/GtfsRaptorServiceIT.java index 20d0aeb..2ad82d1 100644 --- a/src/test/java/ch/naviqore/service/gtfs/raptor/GtfsRaptorServiceIT.java +++ b/src/test/java/ch/naviqore/service/gtfs/raptor/GtfsRaptorServiceIT.java @@ -368,7 +368,7 @@ void shouldThrowOnInvalidStop() { class StopWithoutDepartures { @Test - void target_departure() throws ConnectionRoutingException, StopNotFoundException { + void targetDeparture() throws ConnectionRoutingException, StopNotFoundException { List connections = service.getConnections(service.getStopById("A"), service.getStopById("D"), DATE_TIME, TimeType.DEPARTURE, QUERY_CONFIG); @@ -376,7 +376,7 @@ void target_departure() throws ConnectionRoutingException, StopNotFoundException } @Test - void target_arrival() throws ConnectionRoutingException, StopNotFoundException { + void targetArrival() throws ConnectionRoutingException, StopNotFoundException { List connections = service.getConnections(service.getStopById("A"), service.getStopById("D"), DATE_TIME, TimeType.ARRIVAL, QUERY_CONFIG); @@ -384,7 +384,7 @@ void target_arrival() throws ConnectionRoutingException, StopNotFoundException { } @Test - void source_departure() throws ConnectionRoutingException, StopNotFoundException { + void sourceDeparture() throws ConnectionRoutingException, StopNotFoundException { List connections = service.getConnections(service.getStopById("D"), service.getStopById("A"), DATE_TIME, TimeType.DEPARTURE, QUERY_CONFIG); @@ -392,7 +392,7 @@ void source_departure() throws ConnectionRoutingException, StopNotFoundException } @Test - void source_arrival() throws ConnectionRoutingException, StopNotFoundException { + void sourceArrival() throws ConnectionRoutingException, StopNotFoundException { List connections = service.getConnections(service.getStopById("D"), service.getStopById("A"), DATE_TIME, TimeType.ARRIVAL, QUERY_CONFIG); From 23a4e2d1a74756b4381a878b94d3a60021c5782f Mon Sep 17 00:00:00 2001 From: Merlin Unterfinger Date: Mon, 13 Jan 2025 14:46:01 +0100 Subject: [PATCH 18/18] STYLE: 158 - Reorder methods in public transit service, and change to private if no outside access is needed --- .../gtfs/raptor/GtfsRaptorService.java | 101 +++++++++--------- 1 file changed, 53 insertions(+), 48 deletions(-) diff --git a/src/main/java/ch/naviqore/service/gtfs/raptor/GtfsRaptorService.java b/src/main/java/ch/naviqore/service/gtfs/raptor/GtfsRaptorService.java index 9fd5ab5..3fbd3e4 100644 --- a/src/main/java/ch/naviqore/service/gtfs/raptor/GtfsRaptorService.java +++ b/src/main/java/ch/naviqore/service/gtfs/raptor/GtfsRaptorService.java @@ -117,6 +117,43 @@ public List getNextDepartures(Stop stop, LocalDateTime from, @Nullable .toList(); } + @Override + public Stop getStopById(String stopId) throws StopNotFoundException { + ch.naviqore.gtfs.schedule.model.Stop stop = schedule.getStops().get(stopId); + + if (stop == null) { + throw new StopNotFoundException(stopId); + } + + return TypeMapper.map(stop); + } + + @Override + public Trip getTripById(String tripId, LocalDate date) throws TripNotFoundException, TripNotActiveException { + ch.naviqore.gtfs.schedule.model.Trip trip = schedule.getTrips().get(tripId); + + if (trip == null) { + throw new TripNotFoundException(tripId); + } + + if (!trip.getCalendar().isServiceAvailable(date)) { + throw new TripNotActiveException(tripId, date); + } + + return TypeMapper.map(trip, date); + } + + @Override + public Route getRouteById(String routeId) throws RouteNotFoundException { + ch.naviqore.gtfs.schedule.model.Route route = schedule.getRoutes().get(routeId); + + if (route == null) { + throw new RouteNotFoundException(routeId); + } + + return TypeMapper.map(route); + } + // returns all stops to be included in search private List getAllStopIdsForStop(Stop stop) { ch.naviqore.gtfs.schedule.model.Stop scheduleStop = schedule.getStops().get(stop.getId()); @@ -134,6 +171,12 @@ private List getAllStopIdsForStop(Stop stop) { } } + @Override + public RoutingFeatures getRoutingFeatures() { + return new RoutingFeatures(true, true, true, true, schedule.hasTripAccessibilityInformation(), + schedule.hasTripBikeInformation(), true); + } + @Override public List getConnections(Stop source, Stop target, LocalDateTime time, TimeType timeType, ConnectionQueryConfig config) throws ConnectionRoutingException { @@ -141,12 +184,6 @@ public List getConnections(Stop source, Stop target, LocalDateTime t null, time, timeType, config); } - @Override - public RoutingFeatures getRoutingFeatures() { - return new RoutingFeatures(true, true, true, true, schedule.hasTripAccessibilityInformation(), - schedule.hasTripBikeInformation(), true); - } - @Override public List getConnections(GeoCoordinate source, GeoCoordinate target, LocalDateTime time, TimeType timeType, @@ -248,8 +285,8 @@ private List getConnections(@Nullable ch.naviqore.gtfs.schedule.mode return result; } - public Map getStopsWithWalkTimeFromLocation(GeoCoordinate location, LocalDateTime startTime, - int maxWalkDuration, TimeType timeType) { + private Map getStopsWithWalkTimeFromLocation(GeoCoordinate location, LocalDateTime startTime, + int maxWalkDuration, TimeType timeType) { Map stopsWithWalkTime = getStopsWithWalkTimeFromLocation(location, maxWalkDuration); return stopsWithWalkTime.entrySet() .stream() @@ -259,7 +296,7 @@ public Map getStopsWithWalkTimeFromLocation(GeoCoordinate } - public Map getStopsWithWalkTimeFromLocation(GeoCoordinate location, int maxWalkDuration) { + private Map getStopsWithWalkTimeFromLocation(GeoCoordinate location, int maxWalkDuration) { List nearestStops = new ArrayList<>( spatialStopIndex.rangeSearch(location, config.getWalkingSearchRadius())); @@ -274,24 +311,27 @@ public Map getStopsWithWalkTimeFromLocation(GeoCoordinate locat stopsWithWalkTime.put(stop.getId(), walkDuration); } } + return stopsWithWalkTime; } - public Map getAllChildStopsFromStop(Stop stop, LocalDateTime time) { + private Map getAllChildStopsFromStop(Stop stop, LocalDateTime time) { List stopIds = getAllStopIdsForStop(stop); Map stopWithDateTime = new HashMap<>(); for (String stopId : stopIds) { stopWithDateTime.put(stopId, time); } + return stopWithDateTime; } - public Map getAllChildStopsFromStop(Stop stop) { + private Map getAllChildStopsFromStop(Stop stop) { List stopIds = getAllStopIdsForStop(stop); Map stopsWithWalkTime = new HashMap<>(); for (String stopId : stopIds) { stopsWithWalkTime.put(stopId, 0); } + return stopsWithWalkTime; } @@ -376,6 +416,7 @@ private Map mapToStopConnectionMap(Map mapToStopConnectionMap(Map