Merge pull request #50 from naviqore/bugfix/NAV-90-create-parent-stop…

…-to-stops-map-in-gtfs-and-raise-warning-when-parent-stop-does-not-exist bugfix/NAV-90-create-parent-stop-to-stops-map-in-gtfs-and-raise-warning-when-parent-stop-does-not-exist
naviqore · Jun 5, 2024 · 4ec44c4 · 4ec44c4
2 parents 13f0985 + b71ce91
commit 4ec44c4
Show file tree

Hide file tree

Showing 5 changed files with 38 additions and 23 deletions.
diff --git a/src/main/java/ch/naviqore/gtfs/schedule/GtfsScheduleParser.java b/src/main/java/ch/naviqore/gtfs/schedule/GtfsScheduleParser.java
@@ -81,13 +81,13 @@ private void parseCalendarDate(CSVRecord record) {
     }
 
     private void parseStop(CSVRecord record) {
-        String parentId = null;
         if (record.isMapped("parent_station")) {
-            parentId = record.get("parent_station");
+            builder.addStop(record.get("stop_id"), record.get("stop_name"), Double.parseDouble(record.get("stop_lat")),
+                    Double.parseDouble(record.get("stop_lon")), record.get("parent_station").trim());
+        } else {
+            builder.addStop(record.get("stop_id"), record.get("stop_name"), Double.parseDouble(record.get("stop_lat")),
+                    Double.parseDouble(record.get("stop_lon")));
         }
-        builder.addStop(record.get("stop_id"), record.get("stop_name"), parentId,
-                Double.parseDouble(record.get("stop_lat")), Double.parseDouble(record.get("stop_lon")));
-
     }
 
     private void parseRoute(CSVRecord record) {

diff --git a/src/main/java/ch/naviqore/gtfs/schedule/model/GtfsScheduleBuilder.java b/src/main/java/ch/naviqore/gtfs/schedule/model/GtfsScheduleBuilder.java
@@ -50,14 +50,24 @@ public GtfsScheduleBuilder addAgency(String id, String name, String url, String
         return this;
     }
 
-    public GtfsScheduleBuilder addStop(String id, String name, @Nullable String parentStop, double lat, double lon) {
+    public GtfsScheduleBuilder addStop(String id, String name, double lat, double lon) {
+        addStop(id, name, lat, lon, "");
+        return this;
+    }
+
+    public GtfsScheduleBuilder addStop(String id, String name, double lat, double lon, String parentStopId) {
         checkNotBuilt();
         if (stops.containsKey(id)) {
             throw new IllegalArgumentException("Stop " + id + " already exists");
         }
         log.debug("Adding stop {}", id);
         Stop stop = new Stop(id, name, new GeoCoordinate(lat, lon));
-        parents.computeIfAbsent(parentStop, ignored -> new ArrayList<>()).add(stop);
+
+        // only add stop id if it is not a blank string
+        if (!parentStopId.isEmpty()) {
+            parents.computeIfAbsent(parentStopId, ignored -> new ArrayList<>()).add(stop);
+        }
+
         stops.put(id, stop);
         return this;
     }
@@ -171,18 +181,9 @@ public GtfsScheduleBuilder addTransfer(String fromStopId, String toStopId, Trans
      */
     public GtfsSchedule build() {
         checkNotBuilt();
-        log.info("Building schedule with {} stops, {} routes and {} trips", stops.size(), routes.size(), trips.size());
 
-        // set parents for child
-        parents.forEach((parentId, children) -> {
-            Stop parent = stops.get(parentId);
-            if (parent == null) {
-                log.warn("Parent {} does not exist", parentId);
-            } else {
-                parent.setChildren(children);
-                children.forEach(child -> child.setParent(parent));
-            }
-        });
+        log.info("Building schedule with {} stops, {} routes and {} trips", stops.size(), routes.size(), trips.size());
+        setParentChildrenStopRelations();
 
         // initialize: make immutable and resize arrays to capacity
         trips.values().parallelStream().forEach(Initializable::initialize);
@@ -197,6 +198,21 @@ public GtfsSchedule build() {
         return schedule;
     }
 
+    private void setParentChildrenStopRelations() {
+        parents.forEach((parentId, children) -> {
+            Stop parent = stops.get(parentId);
+
+            if (parent == null) {
+                throw new IllegalStateException("Parent stop " + parentId + " for children " + children.stream()
+                        .map(Stop::getId)
+                        .toList() + " does not exist.");
+            }
+
+            parent.setChildren(children);
+            children.forEach(child -> child.setParent(parent));
+        });
+    }
+
     /**
      * Resets the builder to its initial state, allowing it to be reused.
      */

diff --git a/src/test/java/ch/naviqore/gtfs/schedule/model/GtfsScheduleTestBuilder.java b/src/test/java/ch/naviqore/gtfs/schedule/model/GtfsScheduleTestBuilder.java
@@ -177,7 +177,7 @@ private void addStops(Route route) {
         for (String stopId : route.stops) {
             if (!addedStops.contains(stopId)) {
                 Stop stop = STOPS.get(stopId);
-                builder.addStop(stop.id, stop.id, stop.id, stop.lat, stop.lon);
+                builder.addStop(stop.id, stop.id, stop.lat, stop.lon);
                 addedStops.add(stopId);
             }
         }

diff --git a/src/test/java/ch/naviqore/service/impl/transfer/SameStationTransferGeneratorTest.java b/src/test/java/ch/naviqore/service/impl/transfer/SameStationTransferGeneratorTest.java
@@ -38,8 +38,8 @@ class CreateTransfers {
         @BeforeEach
         void setUp() {
             GtfsScheduleBuilder builder = GtfsSchedule.builder();
-            builder.addStop("stop1", "Zürich, Stadelhofen", "stop1", 47.366542, 8.548384);
-            builder.addStop("stop2", "Zürich, Opernhaus", "stop2", 47.365030, 8.547976);
+            builder.addStop("stop1", "Zürich, Stadelhofen", 47.366542, 8.548384);
+            builder.addStop("stop2", "Zürich, Opernhaus", 47.365030, 8.547976);
             schedule = builder.build();
         }
 

diff --git a/src/test/java/ch/naviqore/service/impl/transfer/WalkTransferGeneratorTest.java b/src/test/java/ch/naviqore/service/impl/transfer/WalkTransferGeneratorTest.java
@@ -92,8 +92,7 @@ private static void assertTransfersGenerated(List<TransferGenerator.Transfer> tr
     void setUp() {
         GtfsScheduleBuilder builder = GtfsSchedule.builder();
         TEST_STOPS.values()
-                .forEach(stopData -> builder.addStop(stopData.id(), stopData.name(), stopData.id(), stopData.lat(),
-                        stopData.lon()));
+                .forEach(stopData -> builder.addStop(stopData.id(), stopData.name(), stopData.lat(), stopData.lon()));
         schedule = builder.build();
         spatialIndex = new KDTreeBuilder<Stop>().addLocations(schedule.getStops().values()).build();
         generator = new WalkTransferGenerator(DEFAULT_CALCULATOR, DEFAULT_MINIMUM_TRANSFER_TIME,