Merge pull request #144 from naviqore/134-api-returns-empty-array-whe…

…n-valid-trips-should-exist

134 api returns empty array when valid trips should exist

src/main/java/ch/naviqore/raptor/router/RaptorRouterBuilder.java

@@ -26,7 +26,7 @@ public class RaptorRouterBuilder {
     private final RaptorConfig config;
     private final Map<String, Integer> stops = new HashMap<>();
     private final Map<String, RouteBuilder> routeBuilders = new HashMap<>();
-    private final Map<String, List<Transfer>> transfers = new HashMap<>();
+    private final Map<String, Map<String, Transfer>> transfers = new HashMap<>();
     private final Map<String, Integer> sameStopTransfers = new HashMap<>();
     private final Map<String, Set<String>> stopRoutes = new HashMap<>();
 
@@ -100,9 +100,12 @@ public RaptorRouterBuilder addTransfer(String sourceStopId, String targetStopId,
             return this;
         }
 
-        transfers.computeIfAbsent(sourceStopId, k -> new ArrayList<>())
-                .add(new Transfer(stops.get(targetStopId), duration));
-        transferSize++;
+        Map<String, Transfer> stopTransfers = transfers.computeIfAbsent(sourceStopId, k -> new HashMap<>());
+        String transferKey = sourceStopId + "-" + targetStopId;
+        if (!stopTransfers.containsKey(transferKey)) {
+            transferSize++;
+        }
+        stopTransfers.put(transferKey, new Transfer(stops.get(targetStopId), duration));
 
         return this;
     }
@@ -161,8 +164,9 @@ private StopContext buildStopContext(Lookup lookup) {
             }
 
             // get the number of (optional) transfers
-            List<Transfer> currentTransfers = transfers.get(stopId);
-            int numberOfTransfers = currentTransfers == null ? 0 : currentTransfers.size();
+            Map<String, Transfer> currentTransfersMap = transfers.get(stopId);
+            Collection<Transfer> currentTransfers = currentTransfersMap == null ? Collections.emptyList() : currentTransfersMap.values();
+            int numberOfTransfers = currentTransfers.size();
 
             int sameStopTransferTime = sameStopTransfers.getOrDefault(stopId, config.getDefaultSameStopTransferTime());
 
@@ -171,10 +175,8 @@ private StopContext buildStopContext(Lookup lookup) {
                     numberOfTransfers == 0 ? NO_INDEX : transferIdx, numberOfTransfers);
 
             // add transfer entry to transfer array if there are any
-            if (currentTransfers != null) {
-                for (Transfer transfer : currentTransfers) {
-                    transferArr[transferIdx++] = transfer;
-                }
+            for (Transfer transfer : currentTransfers) {
+                transferArr[transferIdx++] = transfer;
             }
 
             // add route index entries to stop route array

src/main/java/ch/naviqore/service/gtfs/raptor/convert/GtfsToRaptorConverter.java

@@ -11,9 +11,7 @@
 import ch.naviqore.service.gtfs.raptor.transfer.TransferGenerator;
 import lombok.extern.slf4j.Slf4j;
 
-import java.util.HashSet;
-import java.util.List;
-import java.util.Set;
+import java.util.*;
 
 /**
  * Maps GTFS schedule to Raptor
@@ -55,7 +53,7 @@ public RaptorRouter convert() {
             }
         }
 
-        addTransfers();
+        processAllTransfers();
 
         return builder.build();
     }
@@ -87,27 +85,167 @@ private void addRoute(GtfsRoutePartitioner.SubRoute subRoute) {
         }
     }
 
-    // add raptor transfer for each gtfs or additional transfer
-    private void addTransfers() {
+    /**
+     * Processes all types of transfers, ensuring the correct order of precedence:
+     * <p>
+     * 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.
+     * <p>
+     * The method ensures that all transfers, whether additional, derived, or explicitly defined, are handled in the
+     * correct priority order.
+     */
+    private void processAllTransfers() {
+        addAdditionalTransfers();
+        processStopAndParentChildTransfers();
+        addGtfsTransfersWithPrecedence();
+    }
 
-        // add all additional transfers
+    /**
+     * Adds all additional transfers.
+     */
+    private void addAdditionalTransfers() {
         for (TransferGenerator.Transfer transfer : additionalTransfers) {
             builder.addTransfer(transfer.from().getId(), transfer.to().getId(), transfer.duration());
         }
+    }
 
-        // transfers from gtfs have precedence; already added additional transfers with the same source and target stop
-        // will be overwritten, avoids costly lookups.
+    /**
+     * Processes transfers for each stop and handles parent-child relationships.
+     */
+    private void processStopAndParentChildTransfers() {
         for (String stopId : addedStops) {
             Stop stop = schedule.getStops().get(stopId);
+            processParentAndChildTransfersForStop(stop);
+        }
+    }
+
+    /**
+     * Processes parent and child transfers for a given stop, ensuring proper precedence.
+     * <p>
+     * Assuming transfers A-A, A-B are defined, and both stops are parent stops with 2 children (A1, A2, B1, B2) and
+     * there are also active departures on the parent stop B. For stop A1 following transfers will be derived: (A-A):
+     * A1-A1, A1-A2, (A-B): A1-B, A1-B1, A1-B2
+     * <p>
+     * Alternatively, this method can also handle the case when parent stops have departures but no transfers specified.
+     * For example if transfers A1-A2, A1-B1 are defined, following transfers for stop A may be derived: (A1-A2): A-A,
+     * A-A1, A-A2, (A1-B1): A-B, A-B1, A-B2.
+     *
+     * @param stop the stop for which parent and child transfers are being processed
+     */
+    private void processParentAndChildTransfersForStop(Stop stop) {
+        // this checks if parent stop is present (i.e. checks if stop in question is child stop) and if true, will
+        // derive apply all parent transfers to child stop.
+        stop.getParent().ifPresent(parentStop -> applyTransfersFromOtherStop(stop, parentStop));
+
+        // alternatively, if stop in question is parent stop, all transfers from child stops to other stops should also
+        // be applied to parent stop.
+        for (Stop childStop : stop.getChildren()) {
+            applyTransfersFromOtherStop(stop, childStop);
+        }
+
+        // this is used to make sure that all to stop children/parents are also included for transfer defined on the
+        // current stop.
+        addToStopChildrenTransfers(stop);
+    }
+
+    /**
+     * This method gets all possible transfers from the provider stop and adds copies for the consumer stop. E.g. if A1
+     * is the consumer stop, A is the provider stop and A has a transfer to B (A-B), this method will create the
+     * transfer A1-B and potentially A1-B1, A1-B2,... if B has child stops.
+     *
+     * @param consumerStop the stop of interest where transfers should be derived for
+     * @param providerStop the stop from which transfers should be derived from for the consumerStop
+     */
+    private void applyTransfersFromOtherStop(Stop consumerStop, Stop providerStop) {
+        Collection<TransferGenerator.Transfer> transfers = expandTransfersFromStop(providerStop);
+        for (TransferGenerator.Transfer transfer : transfers) {
+            builder.addTransfer(consumerStop.getId(), transfer.to().getId(), transfer.duration());
+        }
+    }
+
+    /**
+     * Adds transfers to all children stops of all transfer destinations. E.g. if stop B has stops B1 and B2 as children
+     * and stop A has transfer A-B defined, this method will newly create transfers A-B1, A-B2.
+     *
+     * @param stop the stop to process all to stops for
+     */
+    private void addToStopChildrenTransfers(Stop stop) {
+        for (Transfer stopTransfer : stop.getTransfers()) {
+            if (stopTransfer.getTransferType() == TransferType.MINIMUM_TIME && stopTransfer.getMinTransferTime()
+                    .isPresent()) {
+                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())) {
+                        builder.addTransfer(stop.getId(), toChildStop.getId(), stopTransfer.getMinTransferTime().get());
+                    }
+                }
+            }
+        }
+    }
 
+    /**
+     * 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 (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()) {
+                        .isPresent() && addedStops.contains(transfer.getToStop().getId())) {
                     builder.addTransfer(stop.getId(), transfer.getToStop().getId(),
                             transfer.getMinTransferTime().get());
                 }
             }
         }
+    }
+
+    /**
+     * Collects all possible transfers from a given stop. This method loops over all the stops transfers and checks if
+     * the "to stops" have any children and parents and adds transfers to those if needed. For example if Stop A has the
+     * following transfers: A-A1 (A1 is child of A), A-A (note A has also a child A2), A-B (B has B1, B2 as children)
+     * and A-B1. This method will return all the previously defined transfers: A-A1, A-A, A-B and A-B1 and derive the
+     * following: A-A2, A-B2. Note: even though A-A1 and A-B1 could be derived from A-A and A-B, those are not added
+     * because these were already explicitly defined.
+     *
+     * @param stop the stop of interest
+     * @return a collection of complete transfers from stop
+     */
+    private Collection<TransferGenerator.Transfer> expandTransfersFromStop(Stop stop) {
+        Map<Stop, TransferGenerator.Transfer> parentTransfers = new HashMap<>();
+        // to ensure explicitly defined transfers take precedence, those are collected separately and applied in the
+        // end, potentially overwriting other lower priority transfers
+        List<TransferGenerator.Transfer> otherTransfers = new ArrayList<>();
+
+        for (Transfer transfer : stop.getTransfers()) {
+            if (transfer.getTransferType() != TransferType.MINIMUM_TIME || transfer.getMinTransferTime().isEmpty()) {
+                continue;
+            }
+            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())) {
+                otherTransfers.add(new TransferGenerator.Transfer(stop, toStop, transfer.getMinTransferTime().get()));
+            }
+            for (Stop childToStop : toStop.getChildren()) {
+                if (addedStops.contains(childToStop.getId())) {
+                    parentTransfers.put(childToStop,
+                            new TransferGenerator.Transfer(stop, childToStop, transfer.getMinTransferTime().get()));
+                }
+            }
+        }
 
+        // Overwrite transfers derived from children when explicit transfer declaration exists
+        for (TransferGenerator.Transfer transfer : otherTransfers) {
+            parentTransfers.put(transfer.to(), transfer);
+        }
+
+        return parentTransfers.values();
     }
+
 }