From 66804c90f7a7cd42779b50d34fb8e2c6c2e11a1d Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Fri, 15 Aug 2025 23:30:22 +0200 Subject: [PATCH 01/14] Fix multi-modal linking --- .../raptoradapter/router/TransitRouter.java | 2 ++ .../router/street/DirectFlexRouter.java | 1 + .../router/street/DirectStreetRouter.java | 2 ++ .../opentripplanner/routing/graph/Graph.java | 8 +++--- .../routing/graph/StreetIndex.java | 28 +++---------------- .../graphfinder/StreetGraphFinder.java | 1 + .../search/TemporaryVerticesContainer.java | 27 +++++++++++++++--- .../service/DefaultTransitService.java | 8 ++++++ .../transit/service/TransitService.java | 2 ++ .../visualizer/GraphVisualizer.java | 2 ++ .../routing/TestHalfEdges.java | 1 + .../algorithm/StreetModeLinkingTest.java | 2 ++ .../router/street/AccessEgressRouterTest.java | 1 + .../core/TemporaryVerticesContainerTest.java | 2 ++ .../integration/BarrierRoutingTest.java | 2 ++ .../integration/BicycleRoutingTest.java | 2 ++ .../street/integration/CarRoutingTest.java | 2 ++ .../SplitEdgeTurnRestrictionsTest.java | 2 ++ .../street/integration/WalkRoutingTest.java | 2 ++ 19 files changed, 65 insertions(+), 32 deletions(-) diff --git a/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/router/TransitRouter.java b/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/router/TransitRouter.java index 68614977797..4dc847da348 100644 --- a/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/router/TransitRouter.java +++ b/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/router/TransitRouter.java @@ -11,6 +11,7 @@ import java.util.List; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionException; +import java.util.stream.Collectors; import java.util.stream.IntStream; import javax.annotation.Nullable; import org.opentripplanner.ext.ridehailing.RideHailingAccessShifter; @@ -382,6 +383,7 @@ private TemporaryVerticesContainer createTemporaryVerticesContainer( return new TemporaryVerticesContainer( serverContext.graph(), serverContext.vertexLinker(), + serverContext.transitService()::findStopOrChildIds, request.from(), request.to(), request.journey().access().mode(), diff --git a/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/router/street/DirectFlexRouter.java b/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/router/street/DirectFlexRouter.java index d1dee233b39..28e37d4a35d 100644 --- a/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/router/street/DirectFlexRouter.java +++ b/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/router/street/DirectFlexRouter.java @@ -30,6 +30,7 @@ public static List route( var temporaryVertices = new TemporaryVerticesContainer( serverContext.graph(), serverContext.vertexLinker(), + serverContext.transitService()::findStopOrChildIds, request.from(), request.to(), request.journey().direct().mode(), diff --git a/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/router/street/DirectStreetRouter.java b/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/router/street/DirectStreetRouter.java index 02d4369bf2b..336c6611672 100644 --- a/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/router/street/DirectStreetRouter.java +++ b/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/router/street/DirectStreetRouter.java @@ -2,6 +2,7 @@ import java.util.Collections; import java.util.List; +import java.util.stream.Collectors; import org.opentripplanner.astar.model.GraphPath; import org.opentripplanner.framework.application.OTPRequestTimeoutException; import org.opentripplanner.framework.geometry.SphericalDistanceLibrary; @@ -36,6 +37,7 @@ public static List route(OtpServerRequestContext serverContext, Route var temporaryVertices = new TemporaryVerticesContainer( serverContext.graph(), serverContext.vertexLinker(), + serverContext.transitService()::findStopOrChildIds, request.from(), request.to(), request.journey().direct().mode(), diff --git a/application/src/main/java/org/opentripplanner/routing/graph/Graph.java b/application/src/main/java/org/opentripplanner/routing/graph/Graph.java index 832f04f82b3..6e03928863d 100644 --- a/application/src/main/java/org/opentripplanner/routing/graph/Graph.java +++ b/application/src/main/java/org/opentripplanner/routing/graph/Graph.java @@ -219,7 +219,7 @@ public TransitStopVertex getStopVertexForStopId(FeedScopedId id) { */ public Set findStopOrChildStopsVertices(FeedScopedId stopId) { requireIndex(); - return streetIndex.getStopOrChildStopsVertices(stopId); + return streetIndex.findStopOrChildStopVertices(stopId); } /** @@ -324,9 +324,9 @@ public Collection findVertices(Envelope env) { /** * Get the street vertices for an id. If the id corresponds to a regular stop we will return the - * coordinate for the stop. - * If the id corresponds to a station we will either return the coordinates of the child stops or - * the station centroid if the station is configured to route to centroid. + * vertex for the stop. + * If the id corresponds to a station centroid if the station is configured to route to centroid + * then that vertex will be returned. */ public Set findStopVertices(FeedScopedId stopId) { requireIndex(); diff --git a/application/src/main/java/org/opentripplanner/routing/graph/StreetIndex.java b/application/src/main/java/org/opentripplanner/routing/graph/StreetIndex.java index 928c8d13227..e1232fe1482 100644 --- a/application/src/main/java/org/opentripplanner/routing/graph/StreetIndex.java +++ b/application/src/main/java/org/opentripplanner/routing/graph/StreetIndex.java @@ -1,8 +1,5 @@ package org.opentripplanner.routing.graph; -import com.google.common.collect.ArrayListMultimap; -import com.google.common.collect.ImmutableSetMultimap; -import com.google.common.collect.Multimap; import java.util.Collection; import java.util.Collections; import java.util.HashMap; @@ -39,7 +36,6 @@ class StreetIndex { private static final Logger LOG = LoggerFactory.getLogger(StreetIndex.class); private final Map stopVerticesById; - private final ImmutableSetMultimap stopVerticesByParentId; /** * This list contains transitStationVertices for the stations that are configured to route to centroid @@ -57,7 +53,6 @@ class StreetIndex { this.vertexIndex = new HashGridSpatialIndex<>(); var stopVertices = graph.getVerticesOfType(TransitStopVertex.class); this.stopVerticesById = indexStopIds(stopVertices); - this.stopVerticesByParentId = indexStationIds(stopVertices); this.stationCentroidVertices = indexStationCentroids(graph); postSetup(graph.getVertices()); @@ -105,14 +100,12 @@ public String toString() { } /** - * @param id Id of Stop, Station, MultiModalStation or GroupOfStations - * @return The associated TransitStopVertex or all underlying TransitStopVertices + * @param id Id of a RegularStaop + * @return The associated TransitStopVertex or the empty set. */ - Set getStopOrChildStopsVertices(FeedScopedId id) { + Set findStopOrChildStopVertices(FeedScopedId id) { if (stopVerticesById.containsKey(id)) { return Set.of(stopVerticesById.get(id)); - } else if (stopVerticesByParentId.containsKey(id)) { - return stopVerticesByParentId.get(id); } else { return Set.of(); } @@ -126,7 +119,7 @@ Set findStopVertices(FeedScopedId id) { if (stationVertex != null) { return Set.of(stationVertex); } - return Collections.unmodifiableSet(getStopOrChildStopsVertices(id)); + return Collections.unmodifiableSet(findStopOrChildStopVertices(id)); } Collection findEdges(Envelope env, Scope scope) { @@ -192,19 +185,6 @@ private static Map indexStopIds( return Map.copyOf(map); } - private static ImmutableSetMultimap indexStationIds( - Collection vertices - ) { - Multimap map = ArrayListMultimap.create(); - vertices - .stream() - .filter(v -> v.getStop().isPartOfStation()) - .forEach(v -> { - map.put(v.getStop().getParentStation().getId(), v); - }); - return ImmutableSetMultimap.copyOf(map); - } - private static Map indexStationCentroids(Graph graph) { return graph .getVerticesOfType(StationCentroidVertex.class) diff --git a/application/src/main/java/org/opentripplanner/routing/graphfinder/StreetGraphFinder.java b/application/src/main/java/org/opentripplanner/routing/graphfinder/StreetGraphFinder.java index 0d881c412e8..063aae24f9c 100644 --- a/application/src/main/java/org/opentripplanner/routing/graphfinder/StreetGraphFinder.java +++ b/application/src/main/java/org/opentripplanner/routing/graphfinder/StreetGraphFinder.java @@ -101,6 +101,7 @@ private void findClosestUsingStreets( var temporaryVertices = new TemporaryVerticesContainer( graph, linker, + id -> List.of(), GenericLocation.fromCoordinate(lat, lon), GenericLocation.UNKNOWN, StreetMode.WALK, diff --git a/application/src/main/java/org/opentripplanner/street/search/TemporaryVerticesContainer.java b/application/src/main/java/org/opentripplanner/street/search/TemporaryVerticesContainer.java index 7d34b735a7a..604b1965f33 100644 --- a/application/src/main/java/org/opentripplanner/street/search/TemporaryVerticesContainer.java +++ b/application/src/main/java/org/opentripplanner/street/search/TemporaryVerticesContainer.java @@ -2,10 +2,13 @@ import com.google.common.collect.Sets; import java.util.ArrayList; +import java.util.Collection; import java.util.HashSet; import java.util.List; import java.util.Set; +import java.util.function.Function; import java.util.function.Predicate; +import java.util.stream.Collectors; import javax.annotation.Nullable; import org.locationtech.jts.geom.Coordinate; import org.locationtech.jts.geom.GeometryFactory; @@ -28,6 +31,10 @@ import org.opentripplanner.street.model.vertex.TemporaryStreetLocation; import org.opentripplanner.street.model.vertex.TransitStopVertex; import org.opentripplanner.street.model.vertex.Vertex; +import org.opentripplanner.transit.model.framework.FeedScopedId; +import org.opentripplanner.transit.model.site.Station; +import org.opentripplanner.transit.model.site.StopLocation; +import org.opentripplanner.transit.service.TransitService; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -48,15 +55,18 @@ public class TemporaryVerticesContainer implements AutoCloseable { private final GenericLocation from; private final GenericLocation to; private final VertexLinker vertexLinker; + private final Function> resolveStopIds; public TemporaryVerticesContainer( Graph graph, VertexLinker linker, + Function> resolveStopIds, GenericLocation from, GenericLocation to, StreetMode accessMode, StreetMode egressMode ) { + this.resolveStopIds = resolveStopIds; this.tempEdges = new HashSet<>(); this.graph = graph; @@ -157,11 +167,20 @@ private Set getStreetVerticesForLocation( } } } else { - // Check if Stop/StopCollection is found by FeedScopeId if (location.stopId != null) { - var streetVertices = graph.findStopVertices(location.stopId); - if (!streetVertices.isEmpty()) { - return streetVertices; + // Check if Stop or station centroid is found + var stopVertices = graph.findStopVertices(location.stopId); + if (!stopVertices.isEmpty()) { + return stopVertices; + } else { + var vertices = resolveStopIds + .apply(location.stopId) + .stream() + .flatMap(id -> graph.findStopOrChildStopsVertices(id).stream().map(Vertex.class::cast)) + .collect(Collectors.toUnmodifiableSet()); + if (!vertices.isEmpty()) { + return vertices; + } } } } diff --git a/application/src/main/java/org/opentripplanner/transit/service/DefaultTransitService.java b/application/src/main/java/org/opentripplanner/transit/service/DefaultTransitService.java index 842fb17b4a4..449fc6557bc 100644 --- a/application/src/main/java/org/opentripplanner/transit/service/DefaultTransitService.java +++ b/application/src/main/java/org/opentripplanner/transit/service/DefaultTransitService.java @@ -291,6 +291,14 @@ public Collection findStopOrChildStops(FeedScopedId id) { return timetableRepository.getSiteRepository().findStopOrChildStops(id); } + @Override + public Collection findStopOrChildIds(FeedScopedId id) { + return findStopOrChildStops(id) + .stream() + .map(StopLocation::getId) + .collect(Collectors.toUnmodifiableSet()); + } + @Override public Collection listStopLocationGroups() { OTPRequestTimeoutException.checkForTimeout(); diff --git a/application/src/main/java/org/opentripplanner/transit/service/TransitService.java b/application/src/main/java/org/opentripplanner/transit/service/TransitService.java index 4037980eca4..789a51829f1 100644 --- a/application/src/main/java/org/opentripplanner/transit/service/TransitService.java +++ b/application/src/main/java/org/opentripplanner/transit/service/TransitService.java @@ -160,6 +160,8 @@ public interface TransitService { */ Collection findStopOrChildStops(FeedScopedId id); + Collection findStopOrChildIds(FeedScopedId id); + Collection listStopLocationGroups(); StopLocationsGroup getStopLocationsGroup(FeedScopedId id); diff --git a/application/src/main/java/org/opentripplanner/visualizer/GraphVisualizer.java b/application/src/main/java/org/opentripplanner/visualizer/GraphVisualizer.java index ec108d193ac..83d1228f548 100644 --- a/application/src/main/java/org/opentripplanner/visualizer/GraphVisualizer.java +++ b/application/src/main/java/org/opentripplanner/visualizer/GraphVisualizer.java @@ -28,6 +28,7 @@ import java.util.LinkedList; import java.util.List; import java.util.Queue; +import java.util.Set; import javax.swing.AbstractListModel; import javax.swing.BoxLayout; import javax.swing.ButtonGroup; @@ -521,6 +522,7 @@ protected void route(String from, String to) { VisibilityMode.TRAVERSE_AREA_EDGES, StreetConstants.DEFAULT_MAX_AREA_NODES ), + id -> Set.of(), request.from(), request.to(), request.journey().direct().mode(), diff --git a/application/src/test/java/org/opentripplanner/routing/TestHalfEdges.java b/application/src/test/java/org/opentripplanner/routing/TestHalfEdges.java index 48a64c4ab06..aa128a7deaa 100644 --- a/application/src/test/java/org/opentripplanner/routing/TestHalfEdges.java +++ b/application/src/test/java/org/opentripplanner/routing/TestHalfEdges.java @@ -567,6 +567,7 @@ public void testTemporaryVerticesContainer() { var container = new TemporaryVerticesContainer( graph, TestVertexLinker.of(graph), + id -> Set.of(), walking.from(), walking.to(), StreetMode.WALK, diff --git a/application/src/test/java/org/opentripplanner/routing/algorithm/StreetModeLinkingTest.java b/application/src/test/java/org/opentripplanner/routing/algorithm/StreetModeLinkingTest.java index 4e921ae10ed..d56a3e0b878 100644 --- a/application/src/test/java/org/opentripplanner/routing/algorithm/StreetModeLinkingTest.java +++ b/application/src/test/java/org/opentripplanner/routing/algorithm/StreetModeLinkingTest.java @@ -227,6 +227,7 @@ private void assertLinking( var temporaryVertices = new TemporaryVerticesContainer( graph, linker, + id -> List.of(), location, ANY_PLACE, streetMode, @@ -244,6 +245,7 @@ private void assertLinking( var temporaryVertices = new TemporaryVerticesContainer( graph, linker, + id -> List.of(), ANY_PLACE, location, streetMode, diff --git a/application/src/test/java/org/opentripplanner/routing/algorithm/raptoradapter/router/street/AccessEgressRouterTest.java b/application/src/test/java/org/opentripplanner/routing/algorithm/raptoradapter/router/street/AccessEgressRouterTest.java index fb861140bd1..afe1ee66746 100644 --- a/application/src/test/java/org/opentripplanner/routing/algorithm/raptoradapter/router/street/AccessEgressRouterTest.java +++ b/application/src/test/java/org/opentripplanner/routing/algorithm/raptoradapter/router/street/AccessEgressRouterTest.java @@ -257,6 +257,7 @@ private Collection findAccessEgressFromTo( var verticesContainer = new TemporaryVerticesContainer( graph, TestVertexLinker.of(graph), + id -> Set.of(), from, to, StreetMode.WALK, diff --git a/application/src/test/java/org/opentripplanner/routing/core/TemporaryVerticesContainerTest.java b/application/src/test/java/org/opentripplanner/routing/core/TemporaryVerticesContainerTest.java index cdeb3cc2a52..40c4dbb0288 100644 --- a/application/src/test/java/org/opentripplanner/routing/core/TemporaryVerticesContainerTest.java +++ b/application/src/test/java/org/opentripplanner/routing/core/TemporaryVerticesContainerTest.java @@ -8,6 +8,7 @@ import java.util.Arrays; import java.util.Collection; import java.util.List; +import java.util.Set; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.locationtech.jts.geom.Coordinate; @@ -63,6 +64,7 @@ public void temporaryChangesRemovedOnClose() { subject = new TemporaryVerticesContainer( g, TestVertexLinker.of(g), + id -> Set.of(), from, to, StreetMode.WALK, diff --git a/application/src/test/java/org/opentripplanner/street/integration/BarrierRoutingTest.java b/application/src/test/java/org/opentripplanner/street/integration/BarrierRoutingTest.java index 543f5d60738..1e3c042cfa5 100644 --- a/application/src/test/java/org/opentripplanner/street/integration/BarrierRoutingTest.java +++ b/application/src/test/java/org/opentripplanner/street/integration/BarrierRoutingTest.java @@ -8,6 +8,7 @@ import java.time.Instant; import java.util.List; +import java.util.Set; import java.util.function.Consumer; import java.util.function.Function; import java.util.stream.Collectors; @@ -186,6 +187,7 @@ private static String computePolyline( var temporaryVertices = new TemporaryVerticesContainer( graph, TestVertexLinker.of(graph), + id -> Set.of(), from, to, streetMode, diff --git a/application/src/test/java/org/opentripplanner/street/integration/BicycleRoutingTest.java b/application/src/test/java/org/opentripplanner/street/integration/BicycleRoutingTest.java index a6e30547be4..a183ce389cd 100644 --- a/application/src/test/java/org/opentripplanner/street/integration/BicycleRoutingTest.java +++ b/application/src/test/java/org/opentripplanner/street/integration/BicycleRoutingTest.java @@ -5,6 +5,7 @@ import static org.opentripplanner.test.support.PolylineAssert.assertThatPolylinesAreEqual; import java.time.Instant; +import java.util.Set; import org.junit.jupiter.api.Test; import org.locationtech.jts.geom.Geometry; import org.opentripplanner.ConstantsForTests; @@ -88,6 +89,7 @@ private static String computePolyline(Graph graph, GenericLocation from, Generic var temporaryVertices = new TemporaryVerticesContainer( graph, TestVertexLinker.of(graph), + id -> Set.of(), request.from(), request.to(), request.journey().direct().mode(), diff --git a/application/src/test/java/org/opentripplanner/street/integration/CarRoutingTest.java b/application/src/test/java/org/opentripplanner/street/integration/CarRoutingTest.java index 8b98db25bd4..b4744f2fcba 100644 --- a/application/src/test/java/org/opentripplanner/street/integration/CarRoutingTest.java +++ b/application/src/test/java/org/opentripplanner/street/integration/CarRoutingTest.java @@ -5,6 +5,7 @@ import static org.opentripplanner.test.support.PolylineAssert.assertThatPolylinesAreEqual; import java.time.Instant; +import java.util.Set; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; @@ -136,6 +137,7 @@ private static String computePolyline(Graph graph, GenericLocation from, Generic var temporaryVertices = new TemporaryVerticesContainer( graph, TestVertexLinker.of(graph), + id -> Set.of(), from, to, StreetMode.CAR, diff --git a/application/src/test/java/org/opentripplanner/street/integration/SplitEdgeTurnRestrictionsTest.java b/application/src/test/java/org/opentripplanner/street/integration/SplitEdgeTurnRestrictionsTest.java index ac81b57d76a..36f4144224e 100644 --- a/application/src/test/java/org/opentripplanner/street/integration/SplitEdgeTurnRestrictionsTest.java +++ b/application/src/test/java/org/opentripplanner/street/integration/SplitEdgeTurnRestrictionsTest.java @@ -6,6 +6,7 @@ import java.time.Instant; import java.time.LocalDateTime; +import java.util.Set; import org.junit.jupiter.api.Test; import org.locationtech.jts.geom.Geometry; import org.opentripplanner.ConstantsForTests; @@ -168,6 +169,7 @@ private static String computeCarPolyline(Graph graph, GenericLocation from, Gene var temporaryVertices = new TemporaryVerticesContainer( graph, TestVertexLinker.of(graph), + id -> Set.of(), from, to, StreetMode.CAR, diff --git a/application/src/test/java/org/opentripplanner/street/integration/WalkRoutingTest.java b/application/src/test/java/org/opentripplanner/street/integration/WalkRoutingTest.java index 6b17272f07a..3b19f232763 100644 --- a/application/src/test/java/org/opentripplanner/street/integration/WalkRoutingTest.java +++ b/application/src/test/java/org/opentripplanner/street/integration/WalkRoutingTest.java @@ -6,6 +6,7 @@ import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.List; +import java.util.Set; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; @@ -89,6 +90,7 @@ private static List> route( var temporaryVertices = new TemporaryVerticesContainer( graph, TestVertexLinker.of(graph), + id -> Set.of(), request.from(), request.to(), request.journey().direct().mode(), From 16a003f8f20973152ac6182b5a2bf3cdac20fb62 Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Fri, 15 Aug 2025 23:52:28 +0200 Subject: [PATCH 02/14] Update test --- .../search/TemporaryVerticesContainer.java | 23 ++++++++++--------- .../router/street/AccessEgressRouterTest.java | 6 ++++- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/application/src/main/java/org/opentripplanner/street/search/TemporaryVerticesContainer.java b/application/src/main/java/org/opentripplanner/street/search/TemporaryVerticesContainer.java index 604b1965f33..79f5630490c 100644 --- a/application/src/main/java/org/opentripplanner/street/search/TemporaryVerticesContainer.java +++ b/application/src/main/java/org/opentripplanner/street/search/TemporaryVerticesContainer.java @@ -32,9 +32,6 @@ import org.opentripplanner.street.model.vertex.TransitStopVertex; import org.opentripplanner.street.model.vertex.Vertex; import org.opentripplanner.transit.model.framework.FeedScopedId; -import org.opentripplanner.transit.model.site.Station; -import org.opentripplanner.transit.model.site.StopLocation; -import org.opentripplanner.transit.service.TransitService; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -115,7 +112,7 @@ public Set getFromStopVertices() { if (from.stopId == null) { return Set.of(); } - return graph.findStopOrChildStopsVertices(from.stopId); + return resolveStopId(from.stopId); } /** @@ -127,7 +124,7 @@ public Set getToStopVertices() { if (to.stopId == null) { return Set.of(); } - return graph.findStopOrChildStopsVertices(to.stopId); + return resolveStopId(to.stopId); } /* PRIVATE METHODS */ @@ -173,13 +170,9 @@ private Set getStreetVerticesForLocation( if (!stopVertices.isEmpty()) { return stopVertices; } else { - var vertices = resolveStopIds - .apply(location.stopId) - .stream() - .flatMap(id -> graph.findStopOrChildStopsVertices(id).stream().map(Vertex.class::cast)) - .collect(Collectors.toUnmodifiableSet()); + var vertices = resolveStopId(location.stopId); if (!vertices.isEmpty()) { - return vertices; + return vertices.stream().map(v -> (Vertex) v).collect(Collectors.toSet()); } } } @@ -201,6 +194,14 @@ private Set getStreetVerticesForLocation( return null; } + private Set resolveStopId(FeedScopedId stopId) { + return resolveStopIds + .apply(stopId) + .stream() + .flatMap(id -> graph.findStopOrChildStopsVertices(id).stream()) + .collect(Collectors.toUnmodifiableSet()); + } + private TraverseMode getTraverseModeForLinker(StreetMode streetMode, boolean endVertex) { TraverseMode nonTransitMode = TraverseMode.WALK; // for park and ride we will start in car mode and walk to the end vertex diff --git a/application/src/test/java/org/opentripplanner/routing/algorithm/raptoradapter/router/street/AccessEgressRouterTest.java b/application/src/test/java/org/opentripplanner/routing/algorithm/raptoradapter/router/street/AccessEgressRouterTest.java index afe1ee66746..2ebc8c9e0f2 100644 --- a/application/src/test/java/org/opentripplanner/routing/algorithm/raptoradapter/router/street/AccessEgressRouterTest.java +++ b/application/src/test/java/org/opentripplanner/routing/algorithm/raptoradapter/router/street/AccessEgressRouterTest.java @@ -20,10 +20,13 @@ import org.opentripplanner.street.search.TemporaryVerticesContainer; import org.opentripplanner.street.search.state.State; import org.opentripplanner.transit.model.framework.FeedScopedId; +import org.opentripplanner.transit.service.DefaultTransitService; +import org.opentripplanner.transit.service.TimetableRepository; class AccessEgressRouterTest extends GraphRoutingTest { private Graph graph; + private TimetableRepository timetableRepository; private TransitStopVertex stopForCentroidRoutingStation; private TransitStopVertex stopForNoCentroidRoutingStation; @@ -80,6 +83,7 @@ public void build() { } ); graph = otpModel.graph(); + timetableRepository = otpModel.timetableRepository(); } @Test @@ -257,7 +261,7 @@ private Collection findAccessEgressFromTo( var verticesContainer = new TemporaryVerticesContainer( graph, TestVertexLinker.of(graph), - id -> Set.of(), + id -> new DefaultTransitService(timetableRepository).findStopOrChildIds(id), from, to, StreetMode.WALK, From ba14d3f0e239c6e4e9b5473a122a28609e07ab4a Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Sat, 16 Aug 2025 09:23:22 +0200 Subject: [PATCH 03/14] Clean up --- .../raptoradapter/router/TransitRouter.java | 1 - .../router/street/DirectStreetRouter.java | 1 - .../transit/service/DefaultTransitService.java | 8 -------- .../transit/service/TransitService.java | 8 +++++++- .../routing/algorithm/StreetModeLinkingTest.java | 5 +++-- .../core/TemporaryVerticesContainerTest.java | 14 ++------------ 6 files changed, 12 insertions(+), 25 deletions(-) diff --git a/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/router/TransitRouter.java b/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/router/TransitRouter.java index 4dc847da348..9d0d3685b88 100644 --- a/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/router/TransitRouter.java +++ b/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/router/TransitRouter.java @@ -11,7 +11,6 @@ import java.util.List; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionException; -import java.util.stream.Collectors; import java.util.stream.IntStream; import javax.annotation.Nullable; import org.opentripplanner.ext.ridehailing.RideHailingAccessShifter; diff --git a/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/router/street/DirectStreetRouter.java b/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/router/street/DirectStreetRouter.java index 336c6611672..1eba7f9bc5b 100644 --- a/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/router/street/DirectStreetRouter.java +++ b/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/router/street/DirectStreetRouter.java @@ -2,7 +2,6 @@ import java.util.Collections; import java.util.List; -import java.util.stream.Collectors; import org.opentripplanner.astar.model.GraphPath; import org.opentripplanner.framework.application.OTPRequestTimeoutException; import org.opentripplanner.framework.geometry.SphericalDistanceLibrary; diff --git a/application/src/main/java/org/opentripplanner/transit/service/DefaultTransitService.java b/application/src/main/java/org/opentripplanner/transit/service/DefaultTransitService.java index 449fc6557bc..842fb17b4a4 100644 --- a/application/src/main/java/org/opentripplanner/transit/service/DefaultTransitService.java +++ b/application/src/main/java/org/opentripplanner/transit/service/DefaultTransitService.java @@ -291,14 +291,6 @@ public Collection findStopOrChildStops(FeedScopedId id) { return timetableRepository.getSiteRepository().findStopOrChildStops(id); } - @Override - public Collection findStopOrChildIds(FeedScopedId id) { - return findStopOrChildStops(id) - .stream() - .map(StopLocation::getId) - .collect(Collectors.toUnmodifiableSet()); - } - @Override public Collection listStopLocationGroups() { OTPRequestTimeoutException.checkForTimeout(); diff --git a/application/src/main/java/org/opentripplanner/transit/service/TransitService.java b/application/src/main/java/org/opentripplanner/transit/service/TransitService.java index 789a51829f1..df9ab39d1cf 100644 --- a/application/src/main/java/org/opentripplanner/transit/service/TransitService.java +++ b/application/src/main/java/org/opentripplanner/transit/service/TransitService.java @@ -11,6 +11,7 @@ import java.util.Map; import java.util.Optional; import java.util.Set; +import java.util.stream.Collectors; import javax.annotation.Nullable; import org.locationtech.jts.geom.Envelope; import org.opentripplanner.ext.flex.FlexIndex; @@ -160,7 +161,12 @@ public interface TransitService { */ Collection findStopOrChildStops(FeedScopedId id); - Collection findStopOrChildIds(FeedScopedId id); + default Set findStopOrChildIds(FeedScopedId id) { + return findStopOrChildStops(id) + .stream() + .map(StopLocation::getId) + .collect(Collectors.toUnmodifiableSet()); + } Collection listStopLocationGroups(); diff --git a/application/src/test/java/org/opentripplanner/routing/algorithm/StreetModeLinkingTest.java b/application/src/test/java/org/opentripplanner/routing/algorithm/StreetModeLinkingTest.java index d56a3e0b878..0aecfab5999 100644 --- a/application/src/test/java/org/opentripplanner/routing/algorithm/StreetModeLinkingTest.java +++ b/application/src/test/java/org/opentripplanner/routing/algorithm/StreetModeLinkingTest.java @@ -15,6 +15,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Locale; +import java.util.Set; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; @@ -227,7 +228,7 @@ private void assertLinking( var temporaryVertices = new TemporaryVerticesContainer( graph, linker, - id -> List.of(), + id -> Set.of(), location, ANY_PLACE, streetMode, @@ -245,7 +246,7 @@ private void assertLinking( var temporaryVertices = new TemporaryVerticesContainer( graph, linker, - id -> List.of(), + id -> Set.of(), ANY_PLACE, location, streetMode, diff --git a/application/src/test/java/org/opentripplanner/routing/core/TemporaryVerticesContainerTest.java b/application/src/test/java/org/opentripplanner/routing/core/TemporaryVerticesContainerTest.java index 40c4dbb0288..6f801a6fe7e 100644 --- a/application/src/test/java/org/opentripplanner/routing/core/TemporaryVerticesContainerTest.java +++ b/application/src/test/java/org/opentripplanner/routing/core/TemporaryVerticesContainerTest.java @@ -16,6 +16,7 @@ import org.locationtech.jts.geom.LineString; import org.opentripplanner.framework.geometry.GeometryUtils; import org.opentripplanner.framework.geometry.SphericalDistanceLibrary; +import org.opentripplanner.framework.i18n.I18NString; import org.opentripplanner.graph_builder.module.linking.TestVertexLinker; import org.opentripplanner.model.GenericLocation; import org.opentripplanner.routing.api.request.StreetMode; @@ -146,19 +147,8 @@ private void originAndDestinationInsertedCorrect() { } private void createStreetEdge(StreetVertex v0, StreetVertex v1, String name) { - LineString geom = gf.createLineString( - new Coordinate[] { v0.getCoordinate(), v1.getCoordinate() } - ); double dist = SphericalDistanceLibrary.distance(v0.getCoordinate(), v1.getCoordinate()); - new StreetEdgeBuilder<>() - .withFromVertex(v0) - .withToVertex(v1) - .withGeometry(geom) - .withName(name) - .withMeterLength(dist) - .withPermission(StreetTraversalPermission.ALL) - .withBack(false) - .buildAndConnect(); + StreetModelForTest.streetEdgeBuilder(v0, v1, dist, StreetTraversalPermission.ALL).withName(I18NString.of(name)).buildAndConnect(); } private void assertVertexEdgeIsNotReferencingTemporaryElements(Vertex src, Edge e, Vertex v) { From 98aefe29311b672faa2d42de6cdef19407002ef5 Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Sat, 16 Aug 2025 09:59:08 +0200 Subject: [PATCH 04/14] Reorder and rename methods --- .../opentripplanner/ext/flex/FlexRouter.java | 2 +- .../opentripplanner/routing/graph/Graph.java | 38 ++++----- .../routing/graph/StreetIndex.java | 78 +++++++++---------- .../graphfinder/StreetGraphFinder.java | 3 +- .../search/TemporaryVerticesContainer.java | 23 +++--- .../transit/service/TransitService.java | 8 +- .../api/DefaultRoutingServiceTest.java | 8 +- .../core/TemporaryVerticesContainerTest.java | 4 +- 8 files changed, 85 insertions(+), 79 deletions(-) diff --git a/application/src/ext/java/org/opentripplanner/ext/flex/FlexRouter.java b/application/src/ext/java/org/opentripplanner/ext/flex/FlexRouter.java index b77ded74011..4fac081b561 100644 --- a/application/src/ext/java/org/opentripplanner/ext/flex/FlexRouter.java +++ b/application/src/ext/java/org/opentripplanner/ext/flex/FlexRouter.java @@ -198,7 +198,7 @@ private class CallbackAdapter implements FlexAccessEgressCallbackAdapter { @Override public TransitStopVertex getStopVertexForStopId(FeedScopedId stopId) { - return graph.getStopVertexForStopId(stopId); + return graph.getStopVertex(stopId); } @Override diff --git a/application/src/main/java/org/opentripplanner/routing/graph/Graph.java b/application/src/main/java/org/opentripplanner/routing/graph/Graph.java index 6e03928863d..3804bac3ee7 100644 --- a/application/src/main/java/org/opentripplanner/routing/graph/Graph.java +++ b/application/src/main/java/org/opentripplanner/routing/graph/Graph.java @@ -7,6 +7,7 @@ import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.stream.Collectors; @@ -207,19 +208,29 @@ public List getVerticesOfType(Class cls) { * Return the vertex corresponding to the stop id, or null. */ @Nullable - public TransitStopVertex getStopVertexForStopId(FeedScopedId id) { + public TransitStopVertex getStopVertex(FeedScopedId id) { requireIndex(); - return streetIndex.findTransitStopVertex(id); + return streetIndex.getStopVertex(id); } /** - * If the {@code id} is a stop id return a set with a single element. - * If it is a station id return a set containing all child stop vertices, or an empty - * set otherwise. + * If the {@code id} is a stop id return the corresponding vertex, otherwise return an empty + * optional. */ - public Set findStopOrChildStopsVertices(FeedScopedId stopId) { + public Optional findStopVertex(FeedScopedId stopId) { requireIndex(); - return streetIndex.findStopOrChildStopVertices(stopId); + return streetIndex.findStopVertex(stopId); + } + + /** + * Get the street vertices for an id. If the id corresponds to a regular stop it will return the + * vertex for the stop. + * If the id corresponds to a station centroid if the station is configured to route to centroid + * then that vertex will be returned. + */ + public Optional findStreetVertex(FeedScopedId stopId) { + requireIndex(); + return streetIndex.findStreetVertex(stopId); } /** @@ -319,18 +330,7 @@ public OpeningHoursCalendarService getOpeningHoursCalendarService() { */ public Collection findVertices(Envelope env) { requireIndex(); - return streetIndex.getVerticesForEnvelope(env); - } - - /** - * Get the street vertices for an id. If the id corresponds to a regular stop we will return the - * vertex for the stop. - * If the id corresponds to a station centroid if the station is configured to route to centroid - * then that vertex will be returned. - */ - public Set findStopVertices(FeedScopedId stopId) { - requireIndex(); - return streetIndex.findStopVertices(stopId); + return streetIndex.findVertices(env); } /** diff --git a/application/src/main/java/org/opentripplanner/routing/graph/StreetIndex.java b/application/src/main/java/org/opentripplanner/routing/graph/StreetIndex.java index e1232fe1482..32cef51dfd2 100644 --- a/application/src/main/java/org/opentripplanner/routing/graph/StreetIndex.java +++ b/application/src/main/java/org/opentripplanner/routing/graph/StreetIndex.java @@ -1,11 +1,10 @@ package org.opentripplanner.routing.graph; import java.util.Collection; -import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Set; +import java.util.Optional; import java.util.stream.Collectors; import javax.annotation.Nullable; import org.locationtech.jts.geom.Coordinate; @@ -58,15 +57,36 @@ class StreetIndex { postSetup(graph.getVertices()); } + /** + * @see Graph#getStopVertex(FeedScopedId) + */ @Nullable - TransitStopVertex findTransitStopVertex(FeedScopedId stopId) { + TransitStopVertex getStopVertex(FeedScopedId stopId) { return stopVerticesById.get(stopId); } + /** + * @see Graph#findStreetVertex(FeedScopedId) + */ + Optional findStopVertex(FeedScopedId id) { + return Optional.ofNullable(stopVerticesById.get(id)); + } + + /** + * @see Graph#findStreetVertex(FeedScopedId) + */ + Optional findStreetVertex(FeedScopedId id) { + var stationVertex = stationCentroidVertices.get(id); + if (stationVertex != null) { + return Optional.of(stationVertex); + } + return findStopVertex(id).map(Vertex.class::cast); + } + /** * Returns the vertices intersecting with the specified envelope. */ - List getVerticesForEnvelope(Envelope envelope) { + List findVertices(Envelope envelope) { List vertices = vertexIndex.query(envelope); // Here we assume vertices list modifiable vertices.removeIf(v -> !envelope.contains(new Coordinate(v.getLon(), v.getLat()))); @@ -74,7 +94,8 @@ List getVerticesForEnvelope(Envelope envelope) { } /** - * Return the edges whose geometry intersect with the specified envelope. Warning: edges disconnected from the graph + * Return the edges whose geometry intersect with the specified envelope. + * Warning: edges disconnected from the graph * will not be indexed. */ Collection findEdges(Envelope envelope) { @@ -88,40 +109,6 @@ Collection findEdges(Envelope envelope) { .toList(); } - @Override - public String toString() { - return ( - getClass().getName() + - " -- edgeTree: " + - edgeIndex.toString() + - " -- verticesTree: " + - vertexIndex.toString() - ); - } - - /** - * @param id Id of a RegularStaop - * @return The associated TransitStopVertex or the empty set. - */ - Set findStopOrChildStopVertices(FeedScopedId id) { - if (stopVerticesById.containsKey(id)) { - return Set.of(stopVerticesById.get(id)); - } else { - return Set.of(); - } - } - - /** - * @see Graph#findStopVertices(FeedScopedId) - */ - Set findStopVertices(FeedScopedId id) { - var stationVertex = stationCentroidVertices.get(id); - if (stationVertex != null) { - return Set.of(stationVertex); - } - return Collections.unmodifiableSet(findStopOrChildStopVertices(id)); - } - Collection findEdges(Envelope env, Scope scope) { return edgeIndex.query(env, scope).toList(); } @@ -141,6 +128,19 @@ void remove(Vertex vertex) { vertexIndex.remove(new Envelope(vertex.getCoordinate()), vertex); } + @Override + public String toString() { + return ( + getClass().getName() + + " -- edgeTree: " + + edgeIndex.toString() + + " -- verticesTree: " + + vertexIndex.toString() + ); + } + + // private methods + private static LineString edgeGeometryOrStraightLine(Edge e) { LineString geometry = e.getGeometry(); if (geometry == null) { diff --git a/application/src/main/java/org/opentripplanner/routing/graphfinder/StreetGraphFinder.java b/application/src/main/java/org/opentripplanner/routing/graphfinder/StreetGraphFinder.java index 063aae24f9c..c70f9e9f376 100644 --- a/application/src/main/java/org/opentripplanner/routing/graphfinder/StreetGraphFinder.java +++ b/application/src/main/java/org/opentripplanner/routing/graphfinder/StreetGraphFinder.java @@ -4,6 +4,7 @@ import java.util.Comparator; import java.util.List; +import java.util.Set; import org.locationtech.jts.geom.Coordinate; import org.opentripplanner.astar.spi.SkipEdgeStrategy; import org.opentripplanner.astar.spi.TraverseVisitor; @@ -101,7 +102,7 @@ private void findClosestUsingStreets( var temporaryVertices = new TemporaryVerticesContainer( graph, linker, - id -> List.of(), + id -> Set.of(), GenericLocation.fromCoordinate(lat, lon), GenericLocation.UNKNOWN, StreetMode.WALK, diff --git a/application/src/main/java/org/opentripplanner/street/search/TemporaryVerticesContainer.java b/application/src/main/java/org/opentripplanner/street/search/TemporaryVerticesContainer.java index 79f5630490c..367d08fef23 100644 --- a/application/src/main/java/org/opentripplanner/street/search/TemporaryVerticesContainer.java +++ b/application/src/main/java/org/opentripplanner/street/search/TemporaryVerticesContainer.java @@ -112,7 +112,7 @@ public Set getFromStopVertices() { if (from.stopId == null) { return Set.of(); } - return resolveStopId(from.stopId); + return findStopVertices(from.stopId); } /** @@ -124,7 +124,7 @@ public Set getToStopVertices() { if (to.stopId == null) { return Set.of(); } - return resolveStopId(to.stopId); + return findStopVertices(to.stopId); } /* PRIVATE METHODS */ @@ -152,7 +152,7 @@ private Set getStreetVerticesForLocation( if (mode.isInCar()) { // Fetch coordinate from stop, if not given in request if (location.stopId != null && location.getCoordinate() == null) { - var stopVertex = graph.getStopVertexForStopId(location.stopId); + var stopVertex = graph.getStopVertex(location.stopId); if (stopVertex != null) { var c = stopVertex.getStop().getCoordinate(); location = new GenericLocation( @@ -166,13 +166,16 @@ private Set getStreetVerticesForLocation( } else { if (location.stopId != null) { // Check if Stop or station centroid is found - var stopVertices = graph.findStopVertices(location.stopId); - if (!stopVertices.isEmpty()) { - return stopVertices; + var stopVertices = graph.findStreetVertex(location.stopId); + if (stopVertices.isPresent()) { + return Set.of(stopVertices.get()); } else { - var vertices = resolveStopId(location.stopId); + var vertices = findStopVertices(location.stopId); if (!vertices.isEmpty()) { - return vertices.stream().map(v -> (Vertex) v).collect(Collectors.toSet()); + return vertices + .stream() + .map(Vertex.class::cast) + .collect(Collectors.toUnmodifiableSet()); } } } @@ -194,11 +197,11 @@ private Set getStreetVerticesForLocation( return null; } - private Set resolveStopId(FeedScopedId stopId) { + private Set findStopVertices(FeedScopedId stopId) { return resolveStopIds .apply(stopId) .stream() - .flatMap(id -> graph.findStopOrChildStopsVertices(id).stream()) + .flatMap(id -> graph.findStopVertex(id).stream()) .collect(Collectors.toUnmodifiableSet()); } diff --git a/application/src/main/java/org/opentripplanner/transit/service/TransitService.java b/application/src/main/java/org/opentripplanner/transit/service/TransitService.java index df9ab39d1cf..3d6c09fc407 100644 --- a/application/src/main/java/org/opentripplanner/transit/service/TransitService.java +++ b/application/src/main/java/org/opentripplanner/transit/service/TransitService.java @@ -162,10 +162,10 @@ public interface TransitService { Collection findStopOrChildStops(FeedScopedId id); default Set findStopOrChildIds(FeedScopedId id) { - return findStopOrChildStops(id) - .stream() - .map(StopLocation::getId) - .collect(Collectors.toUnmodifiableSet()); + return findStopOrChildStops(id) + .stream() + .map(StopLocation::getId) + .collect(Collectors.toUnmodifiableSet()); } Collection listStopLocationGroups(); diff --git a/application/src/test/java/org/opentripplanner/routing/api/DefaultRoutingServiceTest.java b/application/src/test/java/org/opentripplanner/routing/api/DefaultRoutingServiceTest.java index 055b173854d..4181fc34cae 100644 --- a/application/src/test/java/org/opentripplanner/routing/api/DefaultRoutingServiceTest.java +++ b/application/src/test/java/org/opentripplanner/routing/api/DefaultRoutingServiceTest.java @@ -48,7 +48,7 @@ public void testIdLookup() { for (Vertex vertex : graph.getVertices()) { if (vertex instanceof TransitStopVertex) { RegularStop stop = ((TransitStopVertex) vertex).getStop(); - Vertex index_vertex = graph.getStopVertexForStopId(stop.getId()); + Vertex index_vertex = graph.getStopVertex(stop.getId()); assertEquals(index_vertex, vertex); } } @@ -104,9 +104,9 @@ public void testSpatialIndex() { var stopL = transitService.getRegularStop(idL); FeedScopedId idM = new FeedScopedId(feedId, "M"); var stopM = transitService.getRegularStop(idM); - TransitStopVertex stopvJ = graph.getStopVertexForStopId(idJ); - TransitStopVertex stopvL = graph.getStopVertexForStopId(idL); - TransitStopVertex stopvM = graph.getStopVertexForStopId(idM); + TransitStopVertex stopvJ = graph.getStopVertex(idJ); + TransitStopVertex stopvL = graph.getStopVertex(idL); + TransitStopVertex stopvM = graph.getStopVertex(idM); // There are a two other stops within 100 meters of stop J. Envelope env = new Envelope(new Coordinate(stopJ.getLon(), stopJ.getLat())); env.expandBy( diff --git a/application/src/test/java/org/opentripplanner/routing/core/TemporaryVerticesContainerTest.java b/application/src/test/java/org/opentripplanner/routing/core/TemporaryVerticesContainerTest.java index 6f801a6fe7e..34096c15224 100644 --- a/application/src/test/java/org/opentripplanner/routing/core/TemporaryVerticesContainerTest.java +++ b/application/src/test/java/org/opentripplanner/routing/core/TemporaryVerticesContainerTest.java @@ -148,7 +148,9 @@ private void originAndDestinationInsertedCorrect() { private void createStreetEdge(StreetVertex v0, StreetVertex v1, String name) { double dist = SphericalDistanceLibrary.distance(v0.getCoordinate(), v1.getCoordinate()); - StreetModelForTest.streetEdgeBuilder(v0, v1, dist, StreetTraversalPermission.ALL).withName(I18NString.of(name)).buildAndConnect(); + StreetModelForTest.streetEdgeBuilder(v0, v1, dist, StreetTraversalPermission.ALL) + .withName(I18NString.of(name)) + .buildAndConnect(); } private void assertVertexEdgeIsNotReferencingTemporaryElements(Vertex src, Edge e, Vertex v) { From 454795f4c5003346c47bdc29e7f5e6b88892fa33 Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Sat, 16 Aug 2025 16:05:37 +0200 Subject: [PATCH 05/14] Clean up --- .../ext/flex/template/ClosestTripTest.java | 4 +- .../opentripplanner/ext/flex/FlexRouter.java | 2 +- .../flex/template/AbstractFlexTemplate.java | 2 +- .../FlexAccessEgressCallbackAdapter.java | 2 +- .../transit/service/TransitService.java | 9 +++- .../core/TemporaryVerticesContainerTest.java | 6 --- .../TemporaryVerticesContainerTest.java | 43 +++++++++++++++++ .../service/DefaultTransitServiceTest.java | 46 ++++++++++++++++++- 8 files changed, 100 insertions(+), 14 deletions(-) create mode 100644 application/src/test/java/org/opentripplanner/street/search/TemporaryVerticesContainerTest.java diff --git a/application/src/ext-test/java/org/opentripplanner/ext/flex/template/ClosestTripTest.java b/application/src/ext-test/java/org/opentripplanner/ext/flex/template/ClosestTripTest.java index c3b4448531f..13a3d286d9a 100644 --- a/application/src/ext-test/java/org/opentripplanner/ext/flex/template/ClosestTripTest.java +++ b/application/src/ext-test/java/org/opentripplanner/ext/flex/template/ClosestTripTest.java @@ -44,7 +44,7 @@ class ClosestTripTest { private static final FlexAccessEgressCallbackAdapter ADAPTER = new FlexAccessEgressCallbackAdapter() { @Override - public TransitStopVertex getStopVertexForStopId(FeedScopedId id) { + public TransitStopVertex getStopVertex(FeedScopedId id) { return null; } @@ -76,7 +76,7 @@ void doNotFilter() { var trips = closestTrips(matcher); assertThat(trips).hasSize(1); - assertEquals(List.copyOf(trips).getFirst().flexTrip(), FLEX_TRIP); + assertEquals(FLEX_TRIP, List.copyOf(trips).getFirst().flexTrip()); } @Test diff --git a/application/src/ext/java/org/opentripplanner/ext/flex/FlexRouter.java b/application/src/ext/java/org/opentripplanner/ext/flex/FlexRouter.java index 4fac081b561..1e343c19224 100644 --- a/application/src/ext/java/org/opentripplanner/ext/flex/FlexRouter.java +++ b/application/src/ext/java/org/opentripplanner/ext/flex/FlexRouter.java @@ -197,7 +197,7 @@ private List createFlexServiceDates( private class CallbackAdapter implements FlexAccessEgressCallbackAdapter { @Override - public TransitStopVertex getStopVertexForStopId(FeedScopedId stopId) { + public TransitStopVertex getStopVertex(FeedScopedId stopId) { return graph.getStopVertex(stopId); } diff --git a/application/src/ext/java/org/opentripplanner/ext/flex/template/AbstractFlexTemplate.java b/application/src/ext/java/org/opentripplanner/ext/flex/template/AbstractFlexTemplate.java index b2455fba6c8..e59d214930e 100644 --- a/application/src/ext/java/org/opentripplanner/ext/flex/template/AbstractFlexTemplate.java +++ b/application/src/ext/java/org/opentripplanner/ext/flex/template/AbstractFlexTemplate.java @@ -99,7 +99,7 @@ StopLocation getAccessEgressStop() { */ Stream createFlexAccessEgressStream(FlexAccessEgressCallbackAdapter callback) { if (transferStop instanceof RegularStop stop) { - var flexVertex = callback.getStopVertexForStopId(stop.getId()); + var flexVertex = callback.getStopVertex(stop.getId()); return Stream.of(createFlexAccessEgress(new ArrayList<>(), flexVertex, stop)).filter( Objects::nonNull ); diff --git a/application/src/ext/java/org/opentripplanner/ext/flex/template/FlexAccessEgressCallbackAdapter.java b/application/src/ext/java/org/opentripplanner/ext/flex/template/FlexAccessEgressCallbackAdapter.java index 792213cd2c8..50b5d9ed1a3 100644 --- a/application/src/ext/java/org/opentripplanner/ext/flex/template/FlexAccessEgressCallbackAdapter.java +++ b/application/src/ext/java/org/opentripplanner/ext/flex/template/FlexAccessEgressCallbackAdapter.java @@ -18,7 +18,7 @@ */ public interface FlexAccessEgressCallbackAdapter { /** Adapter, look at implementing service for documentation. */ - TransitStopVertex getStopVertexForStopId(FeedScopedId id); + TransitStopVertex getStopVertex(FeedScopedId id); /** Adapter, look at implementing service for documentation. */ Collection getTransfersFromStop(StopLocation stop); diff --git a/application/src/main/java/org/opentripplanner/transit/service/TransitService.java b/application/src/main/java/org/opentripplanner/transit/service/TransitService.java index 3d6c09fc407..efaa0f934a6 100644 --- a/application/src/main/java/org/opentripplanner/transit/service/TransitService.java +++ b/application/src/main/java/org/opentripplanner/transit/service/TransitService.java @@ -157,10 +157,17 @@ public interface TransitService { * Return all stops associated with the given id. If a Station, a MultiModalStation, or a * GroupOfStations matches the id, then all child stops are returned. If the id matches a regular * stop, area stop or stop group, then a list with one item is returned. - * An empty list is if nothing is found. + * An empty collection is returned if nothing is found. */ Collection findStopOrChildStops(FeedScopedId id); + /** + * Return all stop ids associated with the given id. + *

+ * If a Station, a MultiModalStation, or a GroupOfStations matches the id, then all child stops + * are returned. If the id matches a regular stop, area stop or stop group, then a list with one + * item is returned. + */ default Set findStopOrChildIds(FeedScopedId id) { return findStopOrChildStops(id) .stream() diff --git a/application/src/test/java/org/opentripplanner/routing/core/TemporaryVerticesContainerTest.java b/application/src/test/java/org/opentripplanner/routing/core/TemporaryVerticesContainerTest.java index 34096c15224..70556a6cef4 100644 --- a/application/src/test/java/org/opentripplanner/routing/core/TemporaryVerticesContainerTest.java +++ b/application/src/test/java/org/opentripplanner/routing/core/TemporaryVerticesContainerTest.java @@ -11,10 +11,6 @@ import java.util.Set; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.locationtech.jts.geom.Coordinate; -import org.locationtech.jts.geom.GeometryFactory; -import org.locationtech.jts.geom.LineString; -import org.opentripplanner.framework.geometry.GeometryUtils; import org.opentripplanner.framework.geometry.SphericalDistanceLibrary; import org.opentripplanner.framework.i18n.I18NString; import org.opentripplanner.graph_builder.module.linking.TestVertexLinker; @@ -24,7 +20,6 @@ import org.opentripplanner.street.model.StreetTraversalPermission; import org.opentripplanner.street.model._data.StreetModelForTest; import org.opentripplanner.street.model.edge.Edge; -import org.opentripplanner.street.model.edge.StreetEdgeBuilder; import org.opentripplanner.street.model.edge.TemporaryEdge; import org.opentripplanner.street.model.vertex.StreetVertex; import org.opentripplanner.street.model.vertex.TemporaryVertex; @@ -34,7 +29,6 @@ public class TemporaryVerticesContainerTest { - private final GeometryFactory gf = GeometryUtils.getGeometryFactory(); // Given: // - a graph with 3 intersections/vertexes private final Graph g = new Graph(new Deduplicator()); diff --git a/application/src/test/java/org/opentripplanner/street/search/TemporaryVerticesContainerTest.java b/application/src/test/java/org/opentripplanner/street/search/TemporaryVerticesContainerTest.java new file mode 100644 index 00000000000..35da19b65ba --- /dev/null +++ b/application/src/test/java/org/opentripplanner/street/search/TemporaryVerticesContainerTest.java @@ -0,0 +1,43 @@ +package org.opentripplanner.street.search; + +import static org.opentripplanner.routing.api.request.StreetMode.WALK; + +import java.util.Set; +import java.util.stream.Stream; +import org.junit.jupiter.api.Test; +import org.opentripplanner.graph_builder.module.linking.TestVertexLinker; +import org.opentripplanner.model.GenericLocation; +import org.opentripplanner.routing.graph.Graph; +import org.opentripplanner.street.model.vertex.TransitStopVertex; +import org.opentripplanner.transit.model._data.TimetableRepositoryForTest; +import org.opentripplanner.transit.model.site.RegularStop; + +class TemporaryVerticesContainerTest { + + private final TimetableRepositoryForTest testModel = TimetableRepositoryForTest.of(); + private final RegularStop stopA = testModel.stop("A").build(); + private final RegularStop stopB = testModel.stop("B").build(); + private final RegularStop stopC = testModel.stop("C").build(); + + @Test + void foo() { + var graph = new Graph(); + + Stream.of(stopA, stopB, stopC).forEach(s -> + graph.addVertex(TransitStopVertex.of().withStop(s).build()) + ); + + graph.index(); + var container = new TemporaryVerticesContainer( + graph, + TestVertexLinker.of(graph), + id -> Set.of(), + GenericLocation.fromStopId("a", "F", "A"), + GenericLocation.fromStopId("a", "F", "B"), + WALK, + WALK + ); + var x = container.getToVertices(); + System.out.println(x); + } +} diff --git a/application/src/test/java/org/opentripplanner/transit/service/DefaultTransitServiceTest.java b/application/src/test/java/org/opentripplanner/transit/service/DefaultTransitServiceTest.java index 1455158da10..1e5159605a1 100644 --- a/application/src/test/java/org/opentripplanner/transit/service/DefaultTransitServiceTest.java +++ b/application/src/test/java/org/opentripplanner/transit/service/DefaultTransitServiceTest.java @@ -3,6 +3,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.opentripplanner.transit.model._data.TimetableRepositoryForTest.id; import static org.opentripplanner.transit.model.basic.TransitMode.BUS; import static org.opentripplanner.transit.model.basic.TransitMode.FERRY; import static org.opentripplanner.transit.model.basic.TransitMode.RAIL; @@ -16,6 +17,7 @@ import java.util.Set; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; +import org.opentripplanner.framework.geometry.WgsCoordinate; import org.opentripplanner.framework.i18n.I18NString; import org.opentripplanner.graph_builder.issue.api.DataImportIssueStore; import org.opentripplanner.model.RealTimeTripUpdate; @@ -27,6 +29,8 @@ import org.opentripplanner.transit.model.framework.FeedScopedId; import org.opentripplanner.transit.model.network.StopPattern; import org.opentripplanner.transit.model.network.TripPattern; +import org.opentripplanner.transit.model.site.GroupOfStations; +import org.opentripplanner.transit.model.site.MultiModalStation; import org.opentripplanner.transit.model.site.RegularStop; import org.opentripplanner.transit.model.site.Station; import org.opentripplanner.transit.model.site.StopLocation; @@ -52,6 +56,18 @@ class DefaultTransitServiceTest { private static final RegularStop STOP_ONE = TEST_MODEL.stop("Stop_1") .withVehicleType(TRAM) .build(); + + private static final MultiModalStation MM_STATION = MultiModalStation.of(id("mm_s")) + .withChildStations(List.of(STATION)) + .withCoordinate(WgsCoordinate.GREENWICH) + .withName(I18NString.of("MM1")) + .build(); + private static final GroupOfStations GO_STATIONS = GroupOfStations.of(id("gos")) + .addChildStation(MM_STATION) + .withCoordinate(WgsCoordinate.GREENWICH) + .withName(I18NString.of("GOS")) + .build(); + private static final FeedScopedId SERVICE_ID = new FeedScopedId("FEED", "SERVICE"); private static final int SERVICE_CODE = 0; private static final TripPattern FERRY_PATTERN = TEST_MODEL.pattern(FERRY).build(); @@ -66,7 +82,7 @@ class DefaultTransitServiceTest { .withCreatedByRealtimeUpdater(true) .build(); - static FeedScopedId CALENDAR_ID = TimetableRepositoryForTest.id("CAL_1"); + static FeedScopedId CALENDAR_ID = id("CAL_1"); static Trip TRIP = TimetableRepositoryForTest.trip("123") .withHeadsign(I18NString.of("Trip Headsign")) .withServiceId(CALENDAR_ID) @@ -94,7 +110,7 @@ class DefaultTransitServiceTest { .withServiceCode(SERVICE_CODE) .build(); - static FeedScopedId CALENDAR_ID_TWO = TimetableRepositoryForTest.id("CAL_2"); + static FeedScopedId CALENDAR_ID_TWO = id("CAL_2"); static Trip TRIP_TODAY = TimetableRepositoryForTest.trip("12345") .withHeadsign(I18NString.of("Trip Headsign")) .withServiceId(CALENDAR_ID_TWO) @@ -130,6 +146,8 @@ static void setup() { .withRegularStop(STOP_C) .withRegularStop(STOP_ONE) .withStation(STATION) + .withMultiModalStation(MM_STATION) + .withGroupOfStation(GO_STATIONS) .build(); var deduplicator = new Deduplicator(); @@ -315,4 +333,28 @@ void hasTripsForStop() { assertFalse(service.hasScheduledServicesAfter(LocalDate.of(2025, 7, 3), STOP_ONE)); assertFalse(service.hasScheduledServicesAfter(LocalDate.of(2025, 7, 1), STOP_C)); } + + @Test + void stopOrChildId() { + var res = service.findStopOrChildIds(STOP_A.getId()); + assertEquals(Set.of(STOP_A.getId()), res); + } + + @Test + void stationChildIds() { + var res = service.findStopOrChildIds(STATION.getId()); + assertEquals(Set.of(STOP_A.getId(), STOP_B.getId()), res); + } + + @Test + void multiModalStationChildIds() { + var res = service.findStopOrChildIds(MM_STATION.getId()); + assertEquals(Set.of(STOP_A.getId(), STOP_B.getId()), res); + } + + @Test + void groupOfStationsChildIds() { + var res = service.findStopOrChildIds(GO_STATIONS.getId()); + assertEquals(Set.of(STOP_A.getId(), STOP_B.getId()), res); + } } From ab679376b049cb8dbdd14ff8cc23f6d301d217be Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Mon, 18 Aug 2025 10:32:15 +0200 Subject: [PATCH 06/14] Clarify responsibilities --- .../opentripplanner/routing/graph/Graph.java | 9 ++--- .../routing/graph/StreetIndex.java | 14 ++++---- .../search/TemporaryVerticesContainer.java | 33 ++++++++++++------- .../TemporaryVerticesContainerTest.java | 24 +++++++++++--- 4 files changed, 52 insertions(+), 28 deletions(-) diff --git a/application/src/main/java/org/opentripplanner/routing/graph/Graph.java b/application/src/main/java/org/opentripplanner/routing/graph/Graph.java index 3804bac3ee7..eaee889aeca 100644 --- a/application/src/main/java/org/opentripplanner/routing/graph/Graph.java +++ b/application/src/main/java/org/opentripplanner/routing/graph/Graph.java @@ -223,14 +223,15 @@ public Optional findStopVertex(FeedScopedId stopId) { } /** - * Get the street vertices for an id. If the id corresponds to a regular stop it will return the + * Get the vertex for a site id. If the id corresponds to a regular stop it will return the * vertex for the stop. - * If the id corresponds to a station centroid if the station is configured to route to centroid + * If the id corresponds to a station and the station is configured to route to centroid * then that vertex will be returned. + * Otherwise, an empty optional will be returned. */ - public Optional findStreetVertex(FeedScopedId stopId) { + public Optional findStopOrCentroidVertex(FeedScopedId stopId) { requireIndex(); - return streetIndex.findStreetVertex(stopId); + return streetIndex.findStopOrCentroidVertex(stopId); } /** diff --git a/application/src/main/java/org/opentripplanner/routing/graph/StreetIndex.java b/application/src/main/java/org/opentripplanner/routing/graph/StreetIndex.java index 32cef51dfd2..0764210450d 100644 --- a/application/src/main/java/org/opentripplanner/routing/graph/StreetIndex.java +++ b/application/src/main/java/org/opentripplanner/routing/graph/StreetIndex.java @@ -50,8 +50,7 @@ class StreetIndex { StreetIndex(Graph graph) { this.edgeIndex = new EdgeSpatialIndex(); this.vertexIndex = new HashGridSpatialIndex<>(); - var stopVertices = graph.getVerticesOfType(TransitStopVertex.class); - this.stopVerticesById = indexStopIds(stopVertices); + this.stopVerticesById = indexStopIds(graph); this.stationCentroidVertices = indexStationCentroids(graph); postSetup(graph.getVertices()); @@ -66,16 +65,16 @@ TransitStopVertex getStopVertex(FeedScopedId stopId) { } /** - * @see Graph#findStreetVertex(FeedScopedId) + * @see Graph#findStopVertex(FeedScopedId) (FeedScopedId) */ Optional findStopVertex(FeedScopedId id) { return Optional.ofNullable(stopVerticesById.get(id)); } /** - * @see Graph#findStreetVertex(FeedScopedId) + * @see Graph#findStopOrCentroidVertex(FeedScopedId) */ - Optional findStreetVertex(FeedScopedId id) { + Optional findStopOrCentroidVertex(FeedScopedId id) { var stationVertex = stationCentroidVertices.get(id); if (stationVertex != null) { return Optional.of(stationVertex); @@ -175,9 +174,8 @@ private void postSetup(Collection vertices) { LOG.info(progress.completeMessage()); } - private static Map indexStopIds( - Collection vertices - ) { + private static Map indexStopIds(Graph graph) { + var vertices = graph.getVerticesOfType(TransitStopVertex.class); var map = new HashMap(); for (TransitStopVertex it : vertices) { map.put(it.getStop().getId(), it); diff --git a/application/src/main/java/org/opentripplanner/street/search/TemporaryVerticesContainer.java b/application/src/main/java/org/opentripplanner/street/search/TemporaryVerticesContainer.java index 367d08fef23..3927d0363b1 100644 --- a/application/src/main/java/org/opentripplanner/street/search/TemporaryVerticesContainer.java +++ b/application/src/main/java/org/opentripplanner/street/search/TemporaryVerticesContainer.java @@ -37,7 +37,11 @@ /** * This class is responsible for linking the RouteRequest origin and destination to the Graph used - * in the A-Star search, as well as removing them after the search has been done. It implements + * in the A-Star search, as well as removing them after the search has been done. + *

+ * It also facilitates the lookup of site ids to the correct vertices. + *

+ * It implements * AutoCloseable, in order to be able to use the try-with-resources statement, making the clean-up * automatic. */ @@ -52,18 +56,22 @@ public class TemporaryVerticesContainer implements AutoCloseable { private final GenericLocation from; private final GenericLocation to; private final VertexLinker vertexLinker; - private final Function> resolveStopIds; + private final Function> resolveChildStops; + /** + * @param resolveChildStops A function that takes a site id (stop, station, multi-modal station...) + * and returns a list of child stop ids. + */ public TemporaryVerticesContainer( Graph graph, VertexLinker linker, - Function> resolveStopIds, + Function> resolveChildStops, GenericLocation from, GenericLocation to, StreetMode accessMode, StreetMode egressMode ) { - this.resolveStopIds = resolveStopIds; + this.resolveChildStops = resolveChildStops; this.tempEdges = new HashSet<>(); this.graph = graph; @@ -112,7 +120,7 @@ public Set getFromStopVertices() { if (from.stopId == null) { return Set.of(); } - return findStopVertices(from.stopId); + return findChildStopVertices(from.stopId); } /** @@ -124,7 +132,7 @@ public Set getToStopVertices() { if (to.stopId == null) { return Set.of(); } - return findStopVertices(to.stopId); + return findChildStopVertices(to.stopId); } /* PRIVATE METHODS */ @@ -165,12 +173,15 @@ private Set getStreetVerticesForLocation( } } else { if (location.stopId != null) { - // Check if Stop or station centroid is found - var stopVertices = graph.findStreetVertex(location.stopId); + // Check if stop or station centroid is found + // station centroids are a special vertex to indicate that you don't want to route + // to/from the child stops because they are to spread out. + var stopVertices = graph.findStopOrCentroidVertex(location.stopId); if (stopVertices.isPresent()) { return Set.of(stopVertices.get()); } else { - var vertices = findStopVertices(location.stopId); + // in the regular case you want to resolve a (multi-modal) station into its child stops + var vertices = findChildStopVertices(location.stopId); if (!vertices.isEmpty()) { return vertices .stream() @@ -197,8 +208,8 @@ private Set getStreetVerticesForLocation( return null; } - private Set findStopVertices(FeedScopedId stopId) { - return resolveStopIds + private Set findChildStopVertices(FeedScopedId stopId) { + return resolveChildStops .apply(stopId) .stream() .flatMap(id -> graph.findStopVertex(id).stream()) diff --git a/application/src/test/java/org/opentripplanner/street/search/TemporaryVerticesContainerTest.java b/application/src/test/java/org/opentripplanner/street/search/TemporaryVerticesContainerTest.java index 35da19b65ba..9750e28b7e0 100644 --- a/application/src/test/java/org/opentripplanner/street/search/TemporaryVerticesContainerTest.java +++ b/application/src/test/java/org/opentripplanner/street/search/TemporaryVerticesContainerTest.java @@ -1,7 +1,10 @@ package org.opentripplanner.street.search; +import static com.google.common.truth.Truth.assertThat; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.opentripplanner.routing.api.request.StreetMode.WALK; +import java.util.List; import java.util.Set; import java.util.stream.Stream; import org.junit.jupiter.api.Test; @@ -20,7 +23,7 @@ class TemporaryVerticesContainerTest { private final RegularStop stopC = testModel.stop("C").build(); @Test - void foo() { + void stopId() { var graph = new Graph(); Stream.of(stopA, stopB, stopC).forEach(s -> @@ -32,12 +35,23 @@ void foo() { graph, TestVertexLinker.of(graph), id -> Set.of(), - GenericLocation.fromStopId("a", "F", "A"), - GenericLocation.fromStopId("a", "F", "B"), + stopToLocation(stopA), + stopToLocation(stopB), WALK, WALK ); - var x = container.getToVertices(); - System.out.println(x); + var from = container.getFromVertices(); + assertThat(from).hasSize(1); + var fromStop = ((TransitStopVertex) List.copyOf(from).getFirst()).getStop(); + + assertEquals(stopA, fromStop); + } + + private GenericLocation stopToLocation(RegularStop s) { + return GenericLocation.fromStopId( + s.getName().toString(), + s.getId().getFeedId(), + s.getId().getId() + ); } } From 085e0658c445415a970744482eac7a64056cfe31 Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Mon, 18 Aug 2025 11:26:47 +0200 Subject: [PATCH 07/14] Add test for station lookup --- .../search/TemporaryVerticesContainer.java | 8 +- .../TemporaryVerticesContainerTest.java | 110 +++++++++++++++--- 2 files changed, 102 insertions(+), 16 deletions(-) diff --git a/application/src/main/java/org/opentripplanner/street/search/TemporaryVerticesContainer.java b/application/src/main/java/org/opentripplanner/street/search/TemporaryVerticesContainer.java index 3927d0363b1..4739826a881 100644 --- a/application/src/main/java/org/opentripplanner/street/search/TemporaryVerticesContainer.java +++ b/application/src/main/java/org/opentripplanner/street/search/TemporaryVerticesContainer.java @@ -120,7 +120,7 @@ public Set getFromStopVertices() { if (from.stopId == null) { return Set.of(); } - return findChildStopVertices(from.stopId); + return findStopOrChildStopVertices(from.stopId); } /** @@ -132,7 +132,7 @@ public Set getToStopVertices() { if (to.stopId == null) { return Set.of(); } - return findChildStopVertices(to.stopId); + return findStopOrChildStopVertices(to.stopId); } /* PRIVATE METHODS */ @@ -208,6 +208,10 @@ private Set getStreetVerticesForLocation( return null; } + private Set findStopOrChildStopVertices(FeedScopedId stopId) { + return graph.findStopVertex(stopId).map(Set::of).orElseGet(() -> findChildStopVertices(stopId)); + } + private Set findChildStopVertices(FeedScopedId stopId) { return resolveChildStops .apply(stopId) diff --git a/application/src/test/java/org/opentripplanner/street/search/TemporaryVerticesContainerTest.java b/application/src/test/java/org/opentripplanner/street/search/TemporaryVerticesContainerTest.java index 9750e28b7e0..373fcb88416 100644 --- a/application/src/test/java/org/opentripplanner/street/search/TemporaryVerticesContainerTest.java +++ b/application/src/test/java/org/opentripplanner/street/search/TemporaryVerticesContainerTest.java @@ -3,34 +3,71 @@ import static com.google.common.truth.Truth.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.opentripplanner.routing.api.request.StreetMode.WALK; +import static org.opentripplanner.transit.model._data.TimetableRepositoryForTest.id; +import com.google.common.collect.ImmutableMultimap; +import java.util.Arrays; import java.util.List; import java.util.Set; -import java.util.stream.Stream; +import java.util.stream.Collectors; import org.junit.jupiter.api.Test; +import org.opentripplanner.framework.geometry.WgsCoordinate; import org.opentripplanner.graph_builder.module.linking.TestVertexLinker; import org.opentripplanner.model.GenericLocation; import org.opentripplanner.routing.graph.Graph; +import org.opentripplanner.street.model._data.StreetModelForTest; import org.opentripplanner.street.model.vertex.TransitStopVertex; +import org.opentripplanner.street.model.vertex.Vertex; import org.opentripplanner.transit.model._data.TimetableRepositoryForTest; +import org.opentripplanner.transit.model.framework.FeedScopedId; import org.opentripplanner.transit.model.site.RegularStop; class TemporaryVerticesContainerTest { + private static final WgsCoordinate CENTER = new WgsCoordinate(0, 0); + private static final int DISTANCE = 20; + + private static final FeedScopedId OMEGA_ID = id("omega"); + private final TimetableRepositoryForTest testModel = TimetableRepositoryForTest.of(); - private final RegularStop stopA = testModel.stop("A").build(); - private final RegularStop stopB = testModel.stop("B").build(); - private final RegularStop stopC = testModel.stop("C").build(); + private final RegularStop stopA = testModel + .stop("A") + .withCoordinate(CENTER.moveEastMeters(DISTANCE)) + .build(); + private final RegularStop stopB = testModel + .stop("B") + .withCoordinate(CENTER.moveSouthMeters(DISTANCE)) + .build(); + private final RegularStop stopC = testModel + .stop("C") + .withCoordinate(CENTER.moveWestMeters(DISTANCE)) + .build(); + private final RegularStop stopD = testModel + .stop("D") + .withCoordinate(CENTER.moveNorthMeters(DISTANCE)) + .build(); + private final Graph graph = buildGraph(stopA, stopB, stopC, stopD); @Test - void stopId() { - var graph = new Graph(); - - Stream.of(stopA, stopB, stopC).forEach(s -> - graph.addVertex(TransitStopVertex.of().withStop(s).build()) + void coordinates() { + var container = new TemporaryVerticesContainer( + graph, + TestVertexLinker.of(graph), + id -> Set.of(), + GenericLocation.fromCoordinate(stopA.getLat(), stopA.getLon()), + GenericLocation.fromCoordinate(stopD.getLat(), stopD.getLon()), + WALK, + WALK ); + assertThat(container.getFromVertices()).hasSize(1); + assertThat(container.getToVertices()).hasSize(1); - graph.index(); + assertThat(container.getFromStopVertices()).isEmpty(); + assertThat(container.getToStopVertices()).isEmpty(); + } + + @Test + void stopId() { var container = new TemporaryVerticesContainer( graph, TestVertexLinker.of(graph), @@ -40,11 +77,56 @@ void stopId() { WALK, WALK ); - var from = container.getFromVertices(); - assertThat(from).hasSize(1); - var fromStop = ((TransitStopVertex) List.copyOf(from).getFirst()).getStop(); + assertEquals(stopA, toStop(container.getFromVertices())); + assertEquals(stopB, toStop(container.getToVertices())); + + assertEquals(stopA, toStop(container.getFromStopVertices())); + assertEquals(stopB, toStop(container.getToVertices())); + } + + @Test + void stationId() { + var mapping = ImmutableMultimap.builder() + .putAll(OMEGA_ID, stopC.getId(), stopD.getId()) + .build(); + var container = new TemporaryVerticesContainer( + graph, + TestVertexLinker.of(graph), + mapping::get, + GenericLocation.fromStopId("station", OMEGA_ID.getFeedId(), OMEGA_ID.getId()), + stopToLocation(stopB), + WALK, + WALK + ); + assertThat(toStops(container.getFromVertices())).containsExactly(stopC, stopD); + assertThat(toStops(container.getFromStopVertices())).containsExactly(stopC, stopD); + } + + private static Graph buildGraph(RegularStop... stops) { + var graph = new Graph(); + var center = StreetModelForTest.intersectionVertex(CENTER.asJtsCoordinate()); + graph.addVertex(center); + Arrays.stream(stops).forEach(s -> { + graph.addVertex(TransitStopVertex.of().withStop(s).build()); + var vertex = StreetModelForTest.intersectionVertex(s.getCoordinate().asJtsCoordinate()); + StreetModelForTest.streetEdge(vertex, center); + graph.addVertex(vertex); + }); + graph.index(); + graph.calculateConvexHull(); + return graph; + } + + private static RegularStop toStop(Set fromVertices) { + assertThat(fromVertices).hasSize(1); + return ((TransitStopVertex) List.copyOf(fromVertices).getFirst()).getStop(); + } - assertEquals(stopA, fromStop); + private static Set toStops(Set fromVertices) { + return fromVertices + .stream() + .map(v -> ((TransitStopVertex) v).getStop()) + .collect(Collectors.toUnmodifiableSet()); } private GenericLocation stopToLocation(RegularStop s) { From 40e53a13f6f638aca8dec2f636c2360f64388950 Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Mon, 18 Aug 2025 12:28:02 +0200 Subject: [PATCH 08/14] Add test for index --- .../search/TemporaryVerticesContainer.java | 4 +- .../routing/graph/StreetIndexTest.java | 53 +++++++++++++++++++ 2 files changed, 55 insertions(+), 2 deletions(-) create mode 100644 application/src/test/java/org/opentripplanner/routing/graph/StreetIndexTest.java diff --git a/application/src/main/java/org/opentripplanner/street/search/TemporaryVerticesContainer.java b/application/src/main/java/org/opentripplanner/street/search/TemporaryVerticesContainer.java index 4739826a881..96b3b1e01fb 100644 --- a/application/src/main/java/org/opentripplanner/street/search/TemporaryVerticesContainer.java +++ b/application/src/main/java/org/opentripplanner/street/search/TemporaryVerticesContainer.java @@ -173,9 +173,9 @@ private Set getStreetVerticesForLocation( } } else { if (location.stopId != null) { - // Check if stop or station centroid is found + // check if stop or station centroid is found // station centroids are a special vertex to indicate that you don't want to route - // to/from the child stops because they are to spread out. + // to/from the child stops because they are too spread out. var stopVertices = graph.findStopOrCentroidVertex(location.stopId); if (stopVertices.isPresent()) { return Set.of(stopVertices.get()); diff --git a/application/src/test/java/org/opentripplanner/routing/graph/StreetIndexTest.java b/application/src/test/java/org/opentripplanner/routing/graph/StreetIndexTest.java new file mode 100644 index 00000000000..7619a379385 --- /dev/null +++ b/application/src/test/java/org/opentripplanner/routing/graph/StreetIndexTest.java @@ -0,0 +1,53 @@ +package org.opentripplanner.routing.graph; + +import static com.google.common.truth.Truth.assertThat; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.opentripplanner.transit.model._data.TimetableRepositoryForTest.id; + +import org.junit.jupiter.api.Test; +import org.opentripplanner.street.model.vertex.StationCentroidVertex; +import org.opentripplanner.street.model.vertex.TransitStopVertex; +import org.opentripplanner.transit.model._data.TimetableRepositoryForTest; +import org.opentripplanner.transit.model.site.RegularStop; +import org.opentripplanner.transit.model.site.Station; + +class StreetIndexTest { + + private final TimetableRepositoryForTest testModel = TimetableRepositoryForTest.of(); + private final RegularStop stop = testModel.stop("A").build(); + private final TransitStopVertex stopVertex = TransitStopVertex.of().withStop(stop).build(); + private final Station station = testModel + .station("OMEGA") + .withShouldRouteToCentroid(true) + .build(); + private final StationCentroidVertex centroidVertex = new StationCentroidVertex(station); + + @Test + void stopId() { + var streetIndex = buildIndex(); + assertEquals(stopVertex, streetIndex.getStopVertex(stop.getId())); + assertThat(streetIndex.findStopVertex(stop.getId())).hasValue(stopVertex); + } + + @Test + void nonExistentId() { + var streetIndex = buildIndex(); + assertNull(streetIndex.getStopVertex(id("non-existent-stop-id"))); + assertThat(streetIndex.findStopVertex(id("non-existent-stop-id"))).isEmpty(); + } + + @Test + void stationCentroid() { + var streetIndex = buildIndex(); + assertNull(streetIndex.getStopVertex(station.getId())); + assertThat(streetIndex.findStopOrCentroidVertex(station.getId())).hasValue(centroidVertex); + } + + private StreetIndex buildIndex() { + var graph = new Graph(); + graph.addVertex(stopVertex); + graph.addVertex(centroidVertex); + return new StreetIndex(graph); + } +} From 641455bac11c5c74fe07cf959b59b1f85cbdaf03 Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Mon, 18 Aug 2025 12:35:05 +0200 Subject: [PATCH 09/14] Remove toString --- .../opentripplanner/routing/graph/StreetIndex.java | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/application/src/main/java/org/opentripplanner/routing/graph/StreetIndex.java b/application/src/main/java/org/opentripplanner/routing/graph/StreetIndex.java index 0764210450d..72ec06d6f6d 100644 --- a/application/src/main/java/org/opentripplanner/routing/graph/StreetIndex.java +++ b/application/src/main/java/org/opentripplanner/routing/graph/StreetIndex.java @@ -127,17 +127,6 @@ void remove(Vertex vertex) { vertexIndex.remove(new Envelope(vertex.getCoordinate()), vertex); } - @Override - public String toString() { - return ( - getClass().getName() + - " -- edgeTree: " + - edgeIndex.toString() + - " -- verticesTree: " + - vertexIndex.toString() - ); - } - // private methods private static LineString edgeGeometryOrStraightLine(Edge e) { From 1d3765104be5044f68c1d72b93d87412ac63bd8b Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Mon, 18 Aug 2025 14:31:20 +0200 Subject: [PATCH 10/14] Simplify lookup --- .../search/TemporaryVerticesContainer.java | 19 ++++++++----------- .../TemporaryVerticesContainerTest.java | 4 ++-- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/application/src/main/java/org/opentripplanner/street/search/TemporaryVerticesContainer.java b/application/src/main/java/org/opentripplanner/street/search/TemporaryVerticesContainer.java index 96b3b1e01fb..fd6a782bb63 100644 --- a/application/src/main/java/org/opentripplanner/street/search/TemporaryVerticesContainer.java +++ b/application/src/main/java/org/opentripplanner/street/search/TemporaryVerticesContainer.java @@ -56,22 +56,23 @@ public class TemporaryVerticesContainer implements AutoCloseable { private final GenericLocation from; private final GenericLocation to; private final VertexLinker vertexLinker; - private final Function> resolveChildStops; + private final Function> resolveSiteIds; /** - * @param resolveChildStops A function that takes a site id (stop, station, multi-modal station...) - * and returns a list of child stop ids. + * @param resolveSiteIds A function that takes a site id (stop, station, multi-modal station...) + * and returns a list of the input value (if it's a stop) or of the of + * child stop ids if it's a grouping of stops. */ public TemporaryVerticesContainer( Graph graph, VertexLinker linker, - Function> resolveChildStops, + Function> resolveSiteIds, GenericLocation from, GenericLocation to, StreetMode accessMode, StreetMode egressMode ) { - this.resolveChildStops = resolveChildStops; + this.resolveSiteIds = resolveSiteIds; this.tempEdges = new HashSet<>(); this.graph = graph; @@ -181,7 +182,7 @@ private Set getStreetVerticesForLocation( return Set.of(stopVertices.get()); } else { // in the regular case you want to resolve a (multi-modal) station into its child stops - var vertices = findChildStopVertices(location.stopId); + var vertices = findStopOrChildStopVertices(location.stopId); if (!vertices.isEmpty()) { return vertices .stream() @@ -209,11 +210,7 @@ private Set getStreetVerticesForLocation( } private Set findStopOrChildStopVertices(FeedScopedId stopId) { - return graph.findStopVertex(stopId).map(Set::of).orElseGet(() -> findChildStopVertices(stopId)); - } - - private Set findChildStopVertices(FeedScopedId stopId) { - return resolveChildStops + return resolveSiteIds .apply(stopId) .stream() .flatMap(id -> graph.findStopVertex(id).stream()) diff --git a/application/src/test/java/org/opentripplanner/street/search/TemporaryVerticesContainerTest.java b/application/src/test/java/org/opentripplanner/street/search/TemporaryVerticesContainerTest.java index 373fcb88416..b162c0595ec 100644 --- a/application/src/test/java/org/opentripplanner/street/search/TemporaryVerticesContainerTest.java +++ b/application/src/test/java/org/opentripplanner/street/search/TemporaryVerticesContainerTest.java @@ -53,7 +53,7 @@ void coordinates() { var container = new TemporaryVerticesContainer( graph, TestVertexLinker.of(graph), - id -> Set.of(), + Set::of, GenericLocation.fromCoordinate(stopA.getLat(), stopA.getLon()), GenericLocation.fromCoordinate(stopD.getLat(), stopD.getLon()), WALK, @@ -71,7 +71,7 @@ void stopId() { var container = new TemporaryVerticesContainer( graph, TestVertexLinker.of(graph), - id -> Set.of(), + Set::of, stopToLocation(stopA), stopToLocation(stopB), WALK, From e17cac785ec79c8d2581cd6f8025045b9c6cdb65 Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Mon, 18 Aug 2025 18:26:23 +0200 Subject: [PATCH 11/14] Make methods in graph simpler --- .../opentripplanner/routing/graph/Graph.java | 14 +++---- .../routing/graph/StreetIndex.java | 25 +++---------- .../search/TemporaryVerticesContainer.java | 34 +++++++++-------- .../routing/graph/StreetIndexTest.java | 5 +-- .../TemporaryVerticesContainerTest.java | 37 ++++++++++++++++++- 5 files changed, 67 insertions(+), 48 deletions(-) diff --git a/application/src/main/java/org/opentripplanner/routing/graph/Graph.java b/application/src/main/java/org/opentripplanner/routing/graph/Graph.java index eaee889aeca..ad3d5c1e563 100644 --- a/application/src/main/java/org/opentripplanner/routing/graph/Graph.java +++ b/application/src/main/java/org/opentripplanner/routing/graph/Graph.java @@ -22,6 +22,7 @@ import org.opentripplanner.routing.services.notes.StreetNotesService; import org.opentripplanner.street.model.edge.Edge; import org.opentripplanner.street.model.edge.StreetEdge; +import org.opentripplanner.street.model.vertex.StationCentroidVertex; import org.opentripplanner.street.model.vertex.TransitStopVertex; import org.opentripplanner.street.model.vertex.Vertex; import org.opentripplanner.street.model.vertex.VertexLabel; @@ -210,7 +211,7 @@ public List getVerticesOfType(Class cls) { @Nullable public TransitStopVertex getStopVertex(FeedScopedId id) { requireIndex(); - return streetIndex.getStopVertex(id); + return streetIndex.findStopVertex(id).orElse(null); } /** @@ -223,15 +224,12 @@ public Optional findStopVertex(FeedScopedId stopId) { } /** - * Get the vertex for a site id. If the id corresponds to a regular stop it will return the - * vertex for the stop. - * If the id corresponds to a station and the station is configured to route to centroid - * then that vertex will be returned. - * Otherwise, an empty optional will be returned. + * If the {@code id} is a station id and it is configured to route to its center, + * return the corresponding vertex, otherwise return an empty optional. */ - public Optional findStopOrCentroidVertex(FeedScopedId stopId) { + public Optional findStationCentroidVertex(FeedScopedId stopId) { requireIndex(); - return streetIndex.findStopOrCentroidVertex(stopId); + return streetIndex.findStationCentroidVertex(stopId); } /** diff --git a/application/src/main/java/org/opentripplanner/routing/graph/StreetIndex.java b/application/src/main/java/org/opentripplanner/routing/graph/StreetIndex.java index 72ec06d6f6d..b347bba1767 100644 --- a/application/src/main/java/org/opentripplanner/routing/graph/StreetIndex.java +++ b/application/src/main/java/org/opentripplanner/routing/graph/StreetIndex.java @@ -6,7 +6,6 @@ import java.util.Map; import java.util.Optional; import java.util.stream.Collectors; -import javax.annotation.Nullable; import org.locationtech.jts.geom.Coordinate; import org.locationtech.jts.geom.Envelope; import org.locationtech.jts.geom.LineString; @@ -34,7 +33,7 @@ class StreetIndex { private static final Logger LOG = LoggerFactory.getLogger(StreetIndex.class); - private final Map stopVerticesById; + private final Map stopVertices; /** * This list contains transitStationVertices for the stations that are configured to route to centroid @@ -50,36 +49,24 @@ class StreetIndex { StreetIndex(Graph graph) { this.edgeIndex = new EdgeSpatialIndex(); this.vertexIndex = new HashGridSpatialIndex<>(); - this.stopVerticesById = indexStopIds(graph); + this.stopVertices = indexStopIds(graph); this.stationCentroidVertices = indexStationCentroids(graph); postSetup(graph.getVertices()); } - /** - * @see Graph#getStopVertex(FeedScopedId) - */ - @Nullable - TransitStopVertex getStopVertex(FeedScopedId stopId) { - return stopVerticesById.get(stopId); - } - /** * @see Graph#findStopVertex(FeedScopedId) (FeedScopedId) */ Optional findStopVertex(FeedScopedId id) { - return Optional.ofNullable(stopVerticesById.get(id)); + return Optional.ofNullable(stopVertices.get(id)); } /** - * @see Graph#findStopOrCentroidVertex(FeedScopedId) + * @see Graph#findStationCentroidVertex(FeedScopedId) */ - Optional findStopOrCentroidVertex(FeedScopedId id) { - var stationVertex = stationCentroidVertices.get(id); - if (stationVertex != null) { - return Optional.of(stationVertex); - } - return findStopVertex(id).map(Vertex.class::cast); + Optional findStationCentroidVertex(FeedScopedId id) { + return Optional.ofNullable(stationCentroidVertices.get(id)); } /** diff --git a/application/src/main/java/org/opentripplanner/street/search/TemporaryVerticesContainer.java b/application/src/main/java/org/opentripplanner/street/search/TemporaryVerticesContainer.java index fd6a782bb63..6e7cae4f082 100644 --- a/application/src/main/java/org/opentripplanner/street/search/TemporaryVerticesContainer.java +++ b/application/src/main/java/org/opentripplanner/street/search/TemporaryVerticesContainer.java @@ -174,21 +174,25 @@ private Set getStreetVerticesForLocation( } } else { if (location.stopId != null) { - // check if stop or station centroid is found - // station centroids are a special vertex to indicate that you don't want to route - // to/from the child stops because they are too spread out. - var stopVertices = graph.findStopOrCentroidVertex(location.stopId); - if (stopVertices.isPresent()) { - return Set.of(stopVertices.get()); - } else { - // in the regular case you want to resolve a (multi-modal) station into its child stops - var vertices = findStopOrChildStopVertices(location.stopId); - if (!vertices.isEmpty()) { - return vertices - .stream() - .map(Vertex.class::cast) - .collect(Collectors.toUnmodifiableSet()); - } + // check if there is a stop by the given id + var stopVertex = graph.findStopVertex(location.stopId); + if (stopVertex.isPresent()) { + return Set.of(stopVertex.get()); + } + + // station centroids may be used instead of child stop vertices for stations + var centroidVertex = graph.findStationCentroidVertex(location.stopId); + if (centroidVertex.isPresent()) { + return Set.of(centroidVertex.get()); + } + + // in the regular case you want to resolve a (multi-modal) station into its child stops + var childVertices = findStopOrChildStopVertices(location.stopId); + if (!childVertices.isEmpty()) { + return childVertices + .stream() + .map(Vertex.class::cast) + .collect(Collectors.toUnmodifiableSet()); } } } diff --git a/application/src/test/java/org/opentripplanner/routing/graph/StreetIndexTest.java b/application/src/test/java/org/opentripplanner/routing/graph/StreetIndexTest.java index 7619a379385..5e35ad3744a 100644 --- a/application/src/test/java/org/opentripplanner/routing/graph/StreetIndexTest.java +++ b/application/src/test/java/org/opentripplanner/routing/graph/StreetIndexTest.java @@ -26,22 +26,19 @@ class StreetIndexTest { @Test void stopId() { var streetIndex = buildIndex(); - assertEquals(stopVertex, streetIndex.getStopVertex(stop.getId())); assertThat(streetIndex.findStopVertex(stop.getId())).hasValue(stopVertex); } @Test void nonExistentId() { var streetIndex = buildIndex(); - assertNull(streetIndex.getStopVertex(id("non-existent-stop-id"))); assertThat(streetIndex.findStopVertex(id("non-existent-stop-id"))).isEmpty(); } @Test void stationCentroid() { var streetIndex = buildIndex(); - assertNull(streetIndex.getStopVertex(station.getId())); - assertThat(streetIndex.findStopOrCentroidVertex(station.getId())).hasValue(centroidVertex); + assertThat(streetIndex.findStationCentroidVertex(station.getId())).hasValue(centroidVertex); } private StreetIndex buildIndex() { diff --git a/application/src/test/java/org/opentripplanner/street/search/TemporaryVerticesContainerTest.java b/application/src/test/java/org/opentripplanner/street/search/TemporaryVerticesContainerTest.java index b162c0595ec..096f47bc56b 100644 --- a/application/src/test/java/org/opentripplanner/street/search/TemporaryVerticesContainerTest.java +++ b/application/src/test/java/org/opentripplanner/street/search/TemporaryVerticesContainerTest.java @@ -16,20 +16,32 @@ import org.opentripplanner.model.GenericLocation; import org.opentripplanner.routing.graph.Graph; import org.opentripplanner.street.model._data.StreetModelForTest; +import org.opentripplanner.street.model.edge.StreetStationCentroidLink; +import org.opentripplanner.street.model.vertex.StationCentroidVertex; import org.opentripplanner.street.model.vertex.TransitStopVertex; import org.opentripplanner.street.model.vertex.Vertex; import org.opentripplanner.transit.model._data.TimetableRepositoryForTest; import org.opentripplanner.transit.model.framework.FeedScopedId; import org.opentripplanner.transit.model.site.RegularStop; +import org.opentripplanner.transit.model.site.Station; class TemporaryVerticesContainerTest { private static final WgsCoordinate CENTER = new WgsCoordinate(0, 0); private static final int DISTANCE = 20; + private static final FeedScopedId ALPHA_ID = id("alpha"); private static final FeedScopedId OMEGA_ID = id("omega"); private final TimetableRepositoryForTest testModel = TimetableRepositoryForTest.of(); + + private final Station stationAlpha = testModel + .station("alpha") + .withId(ALPHA_ID) + .withCoordinate(CENTER) + .withShouldRouteToCentroid(true) + .build(); + private final RegularStop stopA = testModel .stop("A") .withCoordinate(CENTER.moveEastMeters(DISTANCE)) @@ -46,7 +58,7 @@ class TemporaryVerticesContainerTest { .stop("D") .withCoordinate(CENTER.moveNorthMeters(DISTANCE)) .build(); - private final Graph graph = buildGraph(stopA, stopB, stopC, stopD); + private final Graph graph = buildGraph(stationAlpha, stopA, stopB, stopC, stopD); @Test void coordinates() { @@ -102,10 +114,31 @@ void stationId() { assertThat(toStops(container.getFromStopVertices())).containsExactly(stopC, stopD); } - private static Graph buildGraph(RegularStop... stops) { + @Test + void centroid() { + var container = new TemporaryVerticesContainer( + graph, + TestVertexLinker.of(graph), + Set::of, + GenericLocation.fromStopId("station", ALPHA_ID.getFeedId(), ALPHA_ID.getId()), + stopToLocation(stopB), + WALK, + WALK + ); + var fromVertices = List.copyOf(container.getFromVertices()); + assertThat(fromVertices).hasSize(1); + + var station = ((StationCentroidVertex) fromVertices.getFirst()).getStation(); + assertEquals(station, this.stationAlpha); + } + + private static Graph buildGraph(Station station, RegularStop... stops) { var graph = new Graph(); var center = StreetModelForTest.intersectionVertex(CENTER.asJtsCoordinate()); graph.addVertex(center); + var centroidVertex = new StationCentroidVertex(station); + graph.addVertex(centroidVertex); + StreetStationCentroidLink.createStreetStationLink(centroidVertex, center); Arrays.stream(stops).forEach(s -> { graph.addVertex(TransitStopVertex.of().withStop(s).build()); var vertex = StreetModelForTest.intersectionVertex(s.getCoordinate().asJtsCoordinate()); From 3e4268e46927425cf347cd2fea7036b73f05bcd1 Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Tue, 26 Aug 2025 15:11:41 +0200 Subject: [PATCH 12/14] Update application/src/main/java/org/opentripplanner/routing/graph/Graph.java Co-authored-by: Thomas Gran --- .../src/main/java/org/opentripplanner/routing/graph/Graph.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/application/src/main/java/org/opentripplanner/routing/graph/Graph.java b/application/src/main/java/org/opentripplanner/routing/graph/Graph.java index ad3d5c1e563..5e51fb2f881 100644 --- a/application/src/main/java/org/opentripplanner/routing/graph/Graph.java +++ b/application/src/main/java/org/opentripplanner/routing/graph/Graph.java @@ -224,7 +224,7 @@ public Optional findStopVertex(FeedScopedId stopId) { } /** - * If the {@code id} is a station id and it is configured to route to its center, + * If the {@code stopId} is a station id and it is configured to route to its center, * return the corresponding vertex, otherwise return an empty optional. */ public Optional findStationCentroidVertex(FeedScopedId stopId) { From 1e029c6024b7ead553c7e46b2275f828821e4663 Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Tue, 26 Aug 2025 15:22:32 +0200 Subject: [PATCH 13/14] Convert to List --- .../opentripplanner/transit/service/TransitService.java | 7 ++----- .../transit/service/DefaultTransitServiceTest.java | 9 +++++---- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/application/src/main/java/org/opentripplanner/transit/service/TransitService.java b/application/src/main/java/org/opentripplanner/transit/service/TransitService.java index efaa0f934a6..945016703dc 100644 --- a/application/src/main/java/org/opentripplanner/transit/service/TransitService.java +++ b/application/src/main/java/org/opentripplanner/transit/service/TransitService.java @@ -168,11 +168,8 @@ public interface TransitService { * are returned. If the id matches a regular stop, area stop or stop group, then a list with one * item is returned. */ - default Set findStopOrChildIds(FeedScopedId id) { - return findStopOrChildStops(id) - .stream() - .map(StopLocation::getId) - .collect(Collectors.toUnmodifiableSet()); + default List findStopOrChildIds(FeedScopedId id) { + return findStopOrChildStops(id).stream().map(StopLocation::getId).distinct().toList(); } Collection listStopLocationGroups(); diff --git a/application/src/test/java/org/opentripplanner/transit/service/DefaultTransitServiceTest.java b/application/src/test/java/org/opentripplanner/transit/service/DefaultTransitServiceTest.java index 1e5159605a1..96935f132e0 100644 --- a/application/src/test/java/org/opentripplanner/transit/service/DefaultTransitServiceTest.java +++ b/application/src/test/java/org/opentripplanner/transit/service/DefaultTransitServiceTest.java @@ -1,5 +1,6 @@ package org.opentripplanner.transit.service; +import static com.google.common.truth.Truth.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -337,24 +338,24 @@ void hasTripsForStop() { @Test void stopOrChildId() { var res = service.findStopOrChildIds(STOP_A.getId()); - assertEquals(Set.of(STOP_A.getId()), res); + assertThat(res).containsExactly(STOP_A.getId()); } @Test void stationChildIds() { var res = service.findStopOrChildIds(STATION.getId()); - assertEquals(Set.of(STOP_A.getId(), STOP_B.getId()), res); + assertThat(res).containsExactly(STOP_A.getId(), STOP_B.getId()); } @Test void multiModalStationChildIds() { var res = service.findStopOrChildIds(MM_STATION.getId()); - assertEquals(Set.of(STOP_A.getId(), STOP_B.getId()), res); + assertThat(res).containsExactly(STOP_A.getId(), STOP_B.getId()); } @Test void groupOfStationsChildIds() { var res = service.findStopOrChildIds(GO_STATIONS.getId()); - assertEquals(Set.of(STOP_A.getId(), STOP_B.getId()), res); + assertThat(res).containsExactly(STOP_A.getId(), STOP_B.getId()); } } From fb3832a84fd9a5bed3f5b6dcca606279ec1eefc0 Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Tue, 26 Aug 2025 16:56:54 +0200 Subject: [PATCH 14/14] Replace Set.of with List.of --- .../routing/core/TemporaryVerticesContainerTest.java | 3 +-- .../street/integration/BarrierRoutingTest.java | 3 +-- .../street/integration/BicycleRoutingTest.java | 4 ++-- .../opentripplanner/street/integration/CarRoutingTest.java | 4 ++-- .../street/integration/SplitEdgeTurnRestrictionsTest.java | 4 ++-- .../opentripplanner/street/integration/WalkRoutingTest.java | 3 +-- 6 files changed, 9 insertions(+), 12 deletions(-) diff --git a/application/src/test/java/org/opentripplanner/routing/core/TemporaryVerticesContainerTest.java b/application/src/test/java/org/opentripplanner/routing/core/TemporaryVerticesContainerTest.java index 70556a6cef4..1feb5144a58 100644 --- a/application/src/test/java/org/opentripplanner/routing/core/TemporaryVerticesContainerTest.java +++ b/application/src/test/java/org/opentripplanner/routing/core/TemporaryVerticesContainerTest.java @@ -8,7 +8,6 @@ import java.util.Arrays; import java.util.Collection; import java.util.List; -import java.util.Set; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.opentripplanner.framework.geometry.SphericalDistanceLibrary; @@ -59,7 +58,7 @@ public void temporaryChangesRemovedOnClose() { subject = new TemporaryVerticesContainer( g, TestVertexLinker.of(g), - id -> Set.of(), + id -> List.of(), from, to, StreetMode.WALK, diff --git a/application/src/test/java/org/opentripplanner/street/integration/BarrierRoutingTest.java b/application/src/test/java/org/opentripplanner/street/integration/BarrierRoutingTest.java index 1e3c042cfa5..77cdd38d05e 100644 --- a/application/src/test/java/org/opentripplanner/street/integration/BarrierRoutingTest.java +++ b/application/src/test/java/org/opentripplanner/street/integration/BarrierRoutingTest.java @@ -8,7 +8,6 @@ import java.time.Instant; import java.util.List; -import java.util.Set; import java.util.function.Consumer; import java.util.function.Function; import java.util.stream.Collectors; @@ -187,7 +186,7 @@ private static String computePolyline( var temporaryVertices = new TemporaryVerticesContainer( graph, TestVertexLinker.of(graph), - id -> Set.of(), + id -> List.of(), from, to, streetMode, diff --git a/application/src/test/java/org/opentripplanner/street/integration/BicycleRoutingTest.java b/application/src/test/java/org/opentripplanner/street/integration/BicycleRoutingTest.java index a183ce389cd..063dceeb779 100644 --- a/application/src/test/java/org/opentripplanner/street/integration/BicycleRoutingTest.java +++ b/application/src/test/java/org/opentripplanner/street/integration/BicycleRoutingTest.java @@ -5,7 +5,7 @@ import static org.opentripplanner.test.support.PolylineAssert.assertThatPolylinesAreEqual; import java.time.Instant; -import java.util.Set; +import java.util.List; import org.junit.jupiter.api.Test; import org.locationtech.jts.geom.Geometry; import org.opentripplanner.ConstantsForTests; @@ -89,7 +89,7 @@ private static String computePolyline(Graph graph, GenericLocation from, Generic var temporaryVertices = new TemporaryVerticesContainer( graph, TestVertexLinker.of(graph), - id -> Set.of(), + id -> List.of(), request.from(), request.to(), request.journey().direct().mode(), diff --git a/application/src/test/java/org/opentripplanner/street/integration/CarRoutingTest.java b/application/src/test/java/org/opentripplanner/street/integration/CarRoutingTest.java index b4744f2fcba..2c78b6ead97 100644 --- a/application/src/test/java/org/opentripplanner/street/integration/CarRoutingTest.java +++ b/application/src/test/java/org/opentripplanner/street/integration/CarRoutingTest.java @@ -5,7 +5,7 @@ import static org.opentripplanner.test.support.PolylineAssert.assertThatPolylinesAreEqual; import java.time.Instant; -import java.util.Set; +import java.util.List; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; @@ -137,7 +137,7 @@ private static String computePolyline(Graph graph, GenericLocation from, Generic var temporaryVertices = new TemporaryVerticesContainer( graph, TestVertexLinker.of(graph), - id -> Set.of(), + id -> List.of(), from, to, StreetMode.CAR, diff --git a/application/src/test/java/org/opentripplanner/street/integration/SplitEdgeTurnRestrictionsTest.java b/application/src/test/java/org/opentripplanner/street/integration/SplitEdgeTurnRestrictionsTest.java index 36f4144224e..5a59911ca3f 100644 --- a/application/src/test/java/org/opentripplanner/street/integration/SplitEdgeTurnRestrictionsTest.java +++ b/application/src/test/java/org/opentripplanner/street/integration/SplitEdgeTurnRestrictionsTest.java @@ -6,7 +6,7 @@ import java.time.Instant; import java.time.LocalDateTime; -import java.util.Set; +import java.util.List; import org.junit.jupiter.api.Test; import org.locationtech.jts.geom.Geometry; import org.opentripplanner.ConstantsForTests; @@ -169,7 +169,7 @@ private static String computeCarPolyline(Graph graph, GenericLocation from, Gene var temporaryVertices = new TemporaryVerticesContainer( graph, TestVertexLinker.of(graph), - id -> Set.of(), + id -> List.of(), from, to, StreetMode.CAR, diff --git a/application/src/test/java/org/opentripplanner/street/integration/WalkRoutingTest.java b/application/src/test/java/org/opentripplanner/street/integration/WalkRoutingTest.java index 3b19f232763..8b352cc54ed 100644 --- a/application/src/test/java/org/opentripplanner/street/integration/WalkRoutingTest.java +++ b/application/src/test/java/org/opentripplanner/street/integration/WalkRoutingTest.java @@ -6,7 +6,6 @@ import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.List; -import java.util.Set; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; @@ -90,7 +89,7 @@ private static List> route( var temporaryVertices = new TemporaryVerticesContainer( graph, TestVertexLinker.of(graph), - id -> Set.of(), + id -> List.of(), request.from(), request.to(), request.journey().direct().mode(),