From 4ae96474fb7e0c56b086b595245bbcc752b5b8b7 Mon Sep 17 00:00:00 2001 From: evansiroky Date: Tue, 7 May 2019 14:16:49 -0700 Subject: [PATCH 01/12] Cherry-pick street splitting improvements from Coord branch See this commit: https://github.com/opentripplanner/OpenTripPlanner/pull/2596/commits/cadbeb351b3e40a22155d58341a4333c868492c6 --- .../linking/SimpleStreetSplitter.java | 149 +++++++++++------- .../graph_builder/linking/StreetSplitter.java | 11 ++ .../linking/TransitToStreetNetworkModule.java | 2 +- .../module/StreetLinkerModule.java | 2 +- .../impl/StreetVertexIndexServiceImpl.java | 13 +- .../services/StreetVertexIndexService.java | 26 +-- .../updater/bike_park/BikeParkUpdater.java | 16 +- .../bike_rental/BikeRentalUpdater.java | 34 ++-- .../linking/SimpleStreetSplitterTest.java | 6 +- .../graph_builder/module/FakeGraph.java | 2 +- 10 files changed, 162 insertions(+), 99 deletions(-) create mode 100644 src/main/java/org/opentripplanner/graph_builder/linking/StreetSplitter.java diff --git a/src/main/java/org/opentripplanner/graph_builder/linking/SimpleStreetSplitter.java b/src/main/java/org/opentripplanner/graph_builder/linking/SimpleStreetSplitter.java index 0d92de6534d..2e40586444a 100644 --- a/src/main/java/org/opentripplanner/graph_builder/linking/SimpleStreetSplitter.java +++ b/src/main/java/org/opentripplanner/graph_builder/linking/SimpleStreetSplitter.java @@ -19,7 +19,6 @@ import org.opentripplanner.graph_builder.annotation.BikeParkUnlinked; import org.opentripplanner.graph_builder.annotation.BikeRentalStationUnlinked; import org.opentripplanner.graph_builder.annotation.StopUnlinked; -import org.opentripplanner.graph_builder.services.DefaultStreetEdgeFactory; import org.opentripplanner.graph_builder.services.StreetEdgeFactory; import org.opentripplanner.openstreetmap.model.OSMWithTags; import org.opentripplanner.graph_builder.annotation.StopLinkedTooFar; @@ -73,7 +72,7 @@ * See discussion in pull request #1922, follow up issue #1934, and the original issue calling for replacement of * the stop linker, #1305. */ -public class SimpleStreetSplitter { +public class SimpleStreetSplitter implements StreetSplitter { private static final Logger LOG = LoggerFactory.getLogger(SimpleStreetSplitter.class); @@ -85,6 +84,11 @@ public class SimpleStreetSplitter { public static final int WARNING_DISTANCE_METERS = 20; + // DESTRUCTIVE_SPLIT means edges are split and new edges are created (used when linking transit stops etc. during graph building) + // NON_DESTRUCTIVE_SPLIT means new temporary edges are created and no edges are deleted (Used when searching for origin/destination) + public static final boolean DESTRUCTIVE_SPLIT = true; + public static final boolean NON_DESTRUCTIVE_SPLIT = false; + /** if there are two ways and the distances to them differ by less than this value, we link to both of them */ public static final double DUPLICATE_WAY_EPSILON_METERS = 0.001; @@ -96,30 +100,26 @@ public class SimpleStreetSplitter { private static GeometryFactory geometryFactory = GeometryUtils.getGeometryFactory(); - //If true edges are split and new edges are created (used when linking transit stops etc. during graph building) - //If false new temporary edges are created and no edges are deleted (Used when searching for origin/destination) - private final boolean destructiveSplitting; - /** * Construct a new SimpleStreetSplitter. * NOTE: Only one SimpleStreetSplitter should be active on a graph at any given time. * * @param hashGridSpatialIndex If not null this index is used instead of creating new one * @param transitStopIndex Index of all transitStops which is generated in {@link org.opentripplanner.routing.impl.StreetVertexIndexServiceImpl} - * @param destructiveSplitting If true splitting is permanent (Used when linking transit stops etc.) when false Splitting is only for duration of a request. Since they are made from temporary vertices and edges. */ public SimpleStreetSplitter(Graph graph, HashGridSpatialIndex hashGridSpatialIndex, - SpatialIndex transitStopIndex, boolean destructiveSplitting) { + SpatialIndex transitStopIndex) { this.graph = graph; this.transitStopIndex = transitStopIndex; - this.destructiveSplitting = destructiveSplitting; - this.edgeFactory = new DefaultStreetEdgeFactory(); //We build a spatial index if it isn't provided if (hashGridSpatialIndex == null) { // build a nice private spatial index, since we're adding and removing edges idx = new HashGridSpatialIndex<>(); for (StreetEdge se : Iterables.filter(graph.getEdges(), StreetEdge.class)) { + // Note: Although it's up to the caller of idx.insert() to make it thread-safe, we don't need to + // worry about it here as it's inside the constructor of SimpleStreetSplitter + // and it's guaranteed that there is only one thread doing this. idx.insert(se.getGeometry(), se); } } else { @@ -136,14 +136,14 @@ public SimpleStreetSplitter(Graph graph, HashGridSpatialIndex hashGridSpat * @param graph */ public SimpleStreetSplitter(Graph graph) { - this(graph, null, null, true); + this(graph, null, null); } /** Link all relevant vertices to the street network */ - public void link () { + public void linkAllStationsToGraph() { for (Vertex v : graph.getVertices()) { if (v instanceof TransitStop || v instanceof BikeRentalStationVertex || v instanceof BikeParkVertex) - if (!link(v)) { + if (!linkToClosestWalkableEdge(v, DESTRUCTIVE_SPLIT)) { if (v instanceof TransitStop) LOG.warn(graph.addBuilderAnnotation(new StopUnlinked((TransitStop) v))); else if (v instanceof BikeRentalStationVertex) @@ -154,13 +154,39 @@ else if (v instanceof BikeParkVertex) } } - /** Link this vertex into the graph to the closest walkable edge */ - public boolean link (Vertex vertex) { - return link(vertex, TraverseMode.WALK, null); + /** + * Link this vertex into the graph to the closest walkable edge + * @param vertex The vertex to be linked. + * @param destructiveSplitting If true splitting is permanent (Used when linking transit stops etc.) when + * false Splitting is only for duration of a request. Since they are made from + * temporary vertices and edges. + * @return + */ + public boolean linkToClosestWalkableEdge(Vertex vertex, final boolean destructiveSplitting) { + return linkToGraph(vertex, TraverseMode.WALK, null, destructiveSplitting); + } + + public boolean linkToGraph(Vertex vertex, TraverseMode traverseMode, RoutingRequest options, + final boolean destructiveSplitting) { + final TraverseModeSet traverseModeSet = new TraverseModeSet(traverseMode); + if (traverseMode == TraverseMode.BICYCLE) { + traverseModeSet.setWalk(true); + } + return linkToGraph(vertex, traverseModeSet, options, destructiveSplitting); } - /** Link this vertex into the graph */ - public boolean link(Vertex vertex, TraverseMode traverseMode, RoutingRequest options) { + /** + * Link this vertex into the graph + * @param vertex The vertex to be linked. + * @param traverseModeSet The traverse modes. + * @param options The routing options. + * @param destructiveSplitting If true splitting is permanent (Used when linking transit stops etc.) when + * false Splitting is only for duration of a request. Since they are made from + * temporary vertices and edges. + * @return + */ + public boolean linkToGraph(Vertex vertex, TraverseModeSet traverseModeSet, RoutingRequest options, + final boolean destructiveSplitting) { // find nearby street edges // TODO: we used to use an expanding-envelope search, which is more efficient in // dense areas. but first let's see how inefficient this is. I suspect it's not too @@ -177,12 +203,6 @@ public boolean link(Vertex vertex, TraverseMode traverseMode, RoutingRequest opt final double DUPLICATE_WAY_EPSILON_DEGREES = SphericalDistanceLibrary.metersToDegrees(DUPLICATE_WAY_EPSILON_METERS); - final TraverseModeSet traverseModeSet; - if (traverseMode == TraverseMode.BICYCLE) { - traverseModeSet = new TraverseModeSet(traverseMode, TraverseMode.WALK); - } else { - traverseModeSet = new TraverseModeSet(traverseMode); - } // We sort the list of candidate edges by distance to the stop // This should remove any issues with things coming out of the spatial index in different orders // Then we link to everything that is within DUPLICATE_WAY_EPSILON_METERS of of the best distance @@ -262,7 +282,7 @@ public boolean link(Vertex vertex, TraverseMode traverseMode, RoutingRequest opt for (TransitStop stop: bestStops) { LOG.debug("Linking vertex to stop: {}", stop.getName()); - makeTemporaryEdges((TemporaryStreetLocation)vertex, stop); + makeTemporaryEdges((TemporaryStreetLocation)vertex, stop, destructiveSplitting); } return true; } @@ -283,7 +303,7 @@ public boolean link(Vertex vertex, TraverseMode traverseMode, RoutingRequest opt .get(candidateEdges.get(i - 1).getId()) < DUPLICATE_WAY_EPSILON_DEGREES); for (StreetEdge edge : bestEdges) { - link(vertex, edge, xscale, options); + linkToEdge(vertex, edge, xscale, options, destructiveSplitting); } // Warn if a linkage was made, but the linkage was suspiciously long. @@ -322,38 +342,48 @@ private void linkTransitToAreaVertices(Vertex splitterVertex, AreaEdgeList area) } } - /** split the edge and link in the transit stop */ - private void link(Vertex tstop, StreetEdge edge, double xscale, RoutingRequest options) { + /** + * Split the edge and link in the transit stop. + * @param vertex An object of Vertex to be linked to an edge. + * @param edge An object of StreetEdge to be linked to. + * @param xscale The longitude scale factor in Equirectangular projection. + * @param options An object of RoutingRequest + * @param destructiveSplitting If true splitting is permanent (Used when linking transit stops etc.) when + * false Splitting is only for duration of a request. Since they are made from + * temporary vertices and edges. + */ + private void linkToEdge(Vertex vertex, StreetEdge edge, double xscale, RoutingRequest options, + final boolean destructiveSplitting) { // TODO: we've already built this line string, we should save it LineString orig = edge.getGeometry(); LineString transformed = equirectangularProject(orig, xscale); LocationIndexedLine il = new LocationIndexedLine(transformed); - LinearLocation ll = il.project(new Coordinate(tstop.getLon() * xscale, tstop.getLat())); + LinearLocation ll = il.project(new Coordinate(vertex.getLon() * xscale, vertex.getLat())); // if we're very close to one end of the line or the other, or endwise, don't bother to split, // cut to the chase and link directly // We use a really tiny epsilon here because we only want points that actually snap to exactly the same location on the // street to use the same vertices. Otherwise the order the stops are loaded in will affect where they are snapped. if (ll.getSegmentIndex() == 0 && ll.getSegmentFraction() < 1e-8) { - makeLinkEdges(tstop, (StreetVertex) edge.getFromVertex()); + makeLinkEdges(vertex, (StreetVertex) edge.getFromVertex(), destructiveSplitting); } // -1 converts from count to index. Because of the fencepost problem, npoints - 1 is the "segment" // past the last point else if (ll.getSegmentIndex() == orig.getNumPoints() - 1) { - makeLinkEdges(tstop, (StreetVertex) edge.getToVertex()); + makeLinkEdges(vertex, (StreetVertex) edge.getToVertex(), destructiveSplitting); } // nPoints - 2: -1 to correct for index vs count, -1 to account for fencepost problem else if (ll.getSegmentIndex() == orig.getNumPoints() - 2 && ll.getSegmentFraction() > 1 - 1e-8) { - makeLinkEdges(tstop, (StreetVertex) edge.getToVertex()); + makeLinkEdges(vertex, (StreetVertex) edge.getToVertex(), destructiveSplitting); } else { TemporaryVertex temporaryVertex = null; boolean endVertex = false; - if (tstop instanceof TemporaryVertex) { - temporaryVertex = (TemporaryVertex) tstop; + if (vertex instanceof TemporaryVertex) { + temporaryVertex = (TemporaryVertex) vertex; endVertex = temporaryVertex.isEndVertex(); } @@ -363,12 +393,12 @@ else if (ll.getSegmentIndex() == orig.getNumPoints() - 2 && ll.getSegmentFractio options.canSplitEdge(edge); } // split the edge, get the split vertex - SplitterVertex v0 = split(edge, ll, temporaryVertex != null, endVertex); - makeLinkEdges(tstop, v0); + SplitterVertex v0 = split(edge, ll, temporaryVertex != null, endVertex, destructiveSplitting); + makeLinkEdges(vertex, v0, destructiveSplitting); // If splitter vertex is part of area; link splittervertex to all other vertexes in area, this creates // edges that were missed by WalkableAreaBuilder - if (edge instanceof AreaEdge && tstop instanceof TransitStop && this.addExtraEdgesToAreas) { + if (edge instanceof AreaEdge && vertex instanceof TransitStop && this.addExtraEdgesToAreas) { linkTransitToAreaVertices(v0, ((AreaEdge) edge).getArea()); } } @@ -382,9 +412,14 @@ else if (ll.getSegmentIndex() == orig.getNumPoints() - 2 && ll.getSegmentFractio * @param ll fraction at which to split the edge * @param temporarySplit if true this is temporary split at origin/destinations search and only temporary edges vertices are created * @param endVertex if this is temporary edge this is true if this is end vertex otherwise it doesn't matter + * @param destructiveSplitting If true splitting is permanent (Used when linking transit stops etc.) when + * false Splitting is only for duration of a request. Since they are made from + * temporary vertices and edges. * @return Splitter vertex with added new edges */ - private SplitterVertex split (StreetEdge edge, LinearLocation ll, boolean temporarySplit, boolean endVertex) { + private SplitterVertex split (StreetEdge edge, LinearLocation ll, boolean temporarySplit, boolean endVertex, + final boolean destructiveSplitting) { + LineString geometry = edge.getGeometry(); // create the geometries @@ -404,9 +439,11 @@ private SplitterVertex split (StreetEdge edge, LinearLocation ll, boolean tempor if (destructiveSplitting) { // update indices of new edges - idx.insert(edges.first.getGeometry(), edges.first); - idx.insert(edges.second.getGeometry(), edges.second); - + synchronized (this) { + // Note: Write operations are not synchronized in HashGridSpatialIndex, hence the lock. + idx.insert(edges.first.getGeometry(), edges.first); + idx.insert(edges.second.getGeometry(), edges.second); + } // (no need to remove original edge, we filter it when it comes out of the index) // remove original edge from the graph @@ -418,20 +455,20 @@ private SplitterVertex split (StreetEdge edge, LinearLocation ll, boolean tempor } /** Make the appropriate type of link edges from a vertex */ - private void makeLinkEdges(Vertex from, StreetVertex to) { + private void makeLinkEdges(Vertex from, StreetVertex to, final boolean destructiveSplitting) { if (from instanceof TemporaryStreetLocation) { - makeTemporaryEdges((TemporaryStreetLocation) from, to); + makeTemporaryEdges((TemporaryStreetLocation) from, to, destructiveSplitting); } else if (from instanceof TransitStop) { - makeTransitLinkEdges((TransitStop) from, to); + makeTransitLinkEdges((TransitStop) from, to, destructiveSplitting); } else if (from instanceof BikeRentalStationVertex) { - makeBikeRentalLinkEdges((BikeRentalStationVertex) from, to); + makeBikeRentalLinkEdges((BikeRentalStationVertex) from, to, destructiveSplitting); } else if (from instanceof BikeParkVertex) { - makeBikeParkEdges((BikeParkVertex) from, to); + makeBikeParkEdges((BikeParkVertex) from, to, destructiveSplitting); } } /** Make temporary edges to origin/destination vertex in origin/destination search **/ - private void makeTemporaryEdges(TemporaryStreetLocation from, Vertex to) { + private void makeTemporaryEdges(TemporaryStreetLocation from, Vertex to, final boolean destructiveSplitting) { if (destructiveSplitting) { throw new RuntimeException("Destructive splitting is used on temporary edges. Something is wrong!"); } @@ -448,7 +485,7 @@ private void makeTemporaryEdges(TemporaryStreetLocation from, Vertex to) { } /** Make bike park edges */ - private void makeBikeParkEdges(BikeParkVertex from, StreetVertex to) { + private void makeBikeParkEdges(BikeParkVertex from, StreetVertex to, final boolean destructiveSplitting) { if (!destructiveSplitting) { throw new RuntimeException("Bike park edges are created with non destructive splitting!"); } @@ -461,10 +498,10 @@ private void makeBikeParkEdges(BikeParkVertex from, StreetVertex to) { new StreetBikeParkLink(to, from); } - /** + /** * Make street transit link edges, unless they already exist. */ - private void makeTransitLinkEdges (TransitStop tstop, StreetVertex v) { + private void makeTransitLinkEdges (TransitStop tstop, StreetVertex v, final boolean destructiveSplitting) { if (!destructiveSplitting) { throw new RuntimeException("Transitedges are created with non destructive splitting!"); } @@ -480,7 +517,8 @@ private void makeTransitLinkEdges (TransitStop tstop, StreetVertex v) { } /** Make link edges for bike rental */ - private void makeBikeRentalLinkEdges (BikeRentalStationVertex from, StreetVertex to) { + private void makeBikeRentalLinkEdges (BikeRentalStationVertex from, StreetVertex to, + final boolean destructiveSplitting) { if (!destructiveSplitting) { throw new RuntimeException("Bike rental edges are created with non destructive splitting!"); } @@ -533,11 +571,8 @@ private static LineString equirectangularProject(LineString geometry, double xsc * @param endVertex true if this is destination vertex * @return */ - public Vertex getClosestVertex(GenericLocation location, RoutingRequest options, - boolean endVertex) { - if (destructiveSplitting) { - throw new RuntimeException("Origin and destination search is used with destructive splitting. Something is wrong!"); - } + public Vertex linkOriginDestination(GenericLocation location, RoutingRequest options, boolean endVertex) { + if (endVertex) { LOG.debug("Finding end vertex for {}", location); } else { @@ -576,7 +611,7 @@ else if (modes.getBicycle()) nonTransitMode = TraverseMode.BICYCLE; } - if(!link(closest, nonTransitMode, options)) { + if(!linkToGraph(closest, nonTransitMode, options, NON_DESTRUCTIVE_SPLIT)) { LOG.warn("Couldn't link {}", location); } return closest; diff --git a/src/main/java/org/opentripplanner/graph_builder/linking/StreetSplitter.java b/src/main/java/org/opentripplanner/graph_builder/linking/StreetSplitter.java new file mode 100644 index 00000000000..1e6a3ca5550 --- /dev/null +++ b/src/main/java/org/opentripplanner/graph_builder/linking/StreetSplitter.java @@ -0,0 +1,11 @@ +package org.opentripplanner.graph_builder.linking; + +import org.opentripplanner.routing.graph.Vertex; + +/** + * This interface allows passing a SimpleStreetSplitter object to the Graph without making Graph depend on + * SimpleStreetSplitter (avoiding circular dependency). + */ +public interface StreetSplitter { + boolean linkToClosestWalkableEdge (Vertex vertex, boolean destructiveSplitting); +} diff --git a/src/main/java/org/opentripplanner/graph_builder/linking/TransitToStreetNetworkModule.java b/src/main/java/org/opentripplanner/graph_builder/linking/TransitToStreetNetworkModule.java index 12a25b5ec8b..c5e7bcbdc4c 100644 --- a/src/main/java/org/opentripplanner/graph_builder/linking/TransitToStreetNetworkModule.java +++ b/src/main/java/org/opentripplanner/graph_builder/linking/TransitToStreetNetworkModule.java @@ -33,7 +33,7 @@ public void buildGraph(Graph graph, HashMap, Object> extra) { //linker.createLinkage(); SimpleStreetSplitter splitter = new SimpleStreetSplitter(graph); - splitter.link(); + splitter.linkAllStationsToGraph(); // don't split streets //SampleStopLinker linker = new SampleStopLinker(graph); diff --git a/src/main/java/org/opentripplanner/graph_builder/module/StreetLinkerModule.java b/src/main/java/org/opentripplanner/graph_builder/module/StreetLinkerModule.java index bbd3640bacd..f3cf1061a95 100644 --- a/src/main/java/org/opentripplanner/graph_builder/module/StreetLinkerModule.java +++ b/src/main/java/org/opentripplanner/graph_builder/module/StreetLinkerModule.java @@ -46,7 +46,7 @@ public void buildGraph(Graph graph, HashMap, Object> extra) { LOG.info("Linking transit stops, bike rental stations, bike parking areas, and park-and-rides to graph . . ."); SimpleStreetSplitter linker = new SimpleStreetSplitter(graph); linker.setAddExtraEdgesToAreas(this.addExtraEdgesToAreas); - linker.link(); + linker.linkAllStationsToGraph(); } //Calculates convex hull of a graph which is shown in routerInfo API point graph.calculateConvexHull(); diff --git a/src/main/java/org/opentripplanner/routing/impl/StreetVertexIndexServiceImpl.java b/src/main/java/org/opentripplanner/routing/impl/StreetVertexIndexServiceImpl.java index d840a0bafd2..3301246ce65 100644 --- a/src/main/java/org/opentripplanner/routing/impl/StreetVertexIndexServiceImpl.java +++ b/src/main/java/org/opentripplanner/routing/impl/StreetVertexIndexServiceImpl.java @@ -14,6 +14,7 @@ import org.opentripplanner.common.model.GenericLocation; import org.opentripplanner.common.model.P2; import org.opentripplanner.graph_builder.linking.SimpleStreetSplitter; +import org.opentripplanner.graph_builder.linking.StreetSplitter; import org.opentripplanner.routing.core.RoutingRequest; import org.opentripplanner.routing.edgetype.*; import org.opentripplanner.routing.graph.Edge; @@ -90,10 +91,10 @@ public StreetVertexIndexServiceImpl(Graph graph, boolean hashGrid) { if (!hashGrid) { ((STRtree) edgeTree).build(); ((STRtree) transitStopTree).build(); - simpleStreetSplitter = new SimpleStreetSplitter(this.graph, null, null, false); + simpleStreetSplitter = new SimpleStreetSplitter(this.graph, null, null); } else { simpleStreetSplitter = new SimpleStreetSplitter(this.graph, - (HashGridSpatialIndex) edgeTree, transitStopTree, false); + (HashGridSpatialIndex) edgeTree, transitStopTree); } } @@ -316,8 +317,7 @@ public Vertex getVertexForLocation(GenericLocation loc, RoutingRequest options, boolean endVertex) { Coordinate c = loc.getCoordinate(); if (c != null) { - //return getClosestVertex(loc, options, endVertex); - return simpleStreetSplitter.getClosestVertex(loc, options, endVertex); + return simpleStreetSplitter.linkOriginDestination(loc, options, endVertex); } // No Coordinate available. @@ -367,4 +367,9 @@ public Vertex getSampleVertexAt(Coordinate coordinate, boolean dest) { return v; } + + @Override + public StreetSplitter getStreetSplitter() { + return simpleStreetSplitter; + } } diff --git a/src/main/java/org/opentripplanner/routing/services/StreetVertexIndexService.java b/src/main/java/org/opentripplanner/routing/services/StreetVertexIndexService.java index b4f1bbfae67..5de3e74e7f3 100644 --- a/src/main/java/org/opentripplanner/routing/services/StreetVertexIndexService.java +++ b/src/main/java/org/opentripplanner/routing/services/StreetVertexIndexService.java @@ -3,6 +3,7 @@ import org.locationtech.jts.geom.Coordinate; import org.locationtech.jts.geom.Envelope; import org.opentripplanner.common.model.GenericLocation; +import org.opentripplanner.graph_builder.linking.StreetSplitter; import org.opentripplanner.routing.core.RoutingRequest; import org.opentripplanner.routing.graph.Edge; import org.opentripplanner.routing.graph.Vertex; @@ -19,7 +20,7 @@ public interface StreetVertexIndexService { * @param envelope * @return */ - public Collection getVerticesForEnvelope(Envelope envelope); + Collection getVerticesForEnvelope(Envelope envelope); /** * Return the edges whose geometry intersect with the specified envelope. Warning: edges w/o @@ -28,20 +29,20 @@ public interface StreetVertexIndexService { * @param envelope * @return */ - public Collection getEdgesForEnvelope(Envelope envelope); + Collection getEdgesForEnvelope(Envelope envelope); /** * @param coordinate * @param radiusMeters * @return The transit stops within a certain radius of the given location. */ - public List getNearbyTransitStops(Coordinate coordinate, double radiusMeters); + List getNearbyTransitStops(Coordinate coordinate, double radiusMeters); /** * @param envelope * @return The transit stops within an envelope. */ - public List getTransitStopForEnvelope(Envelope envelope); + List getTransitStopForEnvelope(Envelope envelope); /** * Finds the appropriate vertex for this location. @@ -51,11 +52,18 @@ public interface StreetVertexIndexService { * @param endVertex: whether this is a start vertex (if it's false) or end vertex (if it's true) * @return */ - public Vertex getVertexForLocation(GenericLocation place, RoutingRequest options, - boolean endVertex); + Vertex getVertexForLocation(GenericLocation place, RoutingRequest options, boolean endVertex); - /** Get a vertex at a given coordinate, using the same logic as in Samples. Used in Analyst - * so that origins and destinations are linked the same way. */ - public Vertex getSampleVertexAt(Coordinate coordinate, boolean dest); + /** + * Get a vertex at a given coordinate, using the same logic as in Samples. Used in Analyst + * so that origins and destinations are linked the same way. + */ + Vertex getSampleVertexAt(Coordinate coordinate, boolean dest); + /** + * Getter method for StreetSplitter + * + * @return StreetSplitter + */ + StreetSplitter getStreetSplitter(); } diff --git a/src/main/java/org/opentripplanner/updater/bike_park/BikeParkUpdater.java b/src/main/java/org/opentripplanner/updater/bike_park/BikeParkUpdater.java index 42f3165d8f1..058732e8cb4 100644 --- a/src/main/java/org/opentripplanner/updater/bike_park/BikeParkUpdater.java +++ b/src/main/java/org/opentripplanner/updater/bike_park/BikeParkUpdater.java @@ -1,7 +1,6 @@ package org.opentripplanner.updater.bike_park; import java.util.ArrayList; -import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -9,10 +8,9 @@ import java.util.Map.Entry; import java.util.Set; import java.util.concurrent.ExecutionException; -import java.util.prefs.Preferences; import com.fasterxml.jackson.databind.JsonNode; -import org.opentripplanner.graph_builder.linking.SimpleStreetSplitter; +import org.opentripplanner.graph_builder.linking.StreetSplitter; import org.opentripplanner.routing.bike_park.BikePark; import org.opentripplanner.routing.bike_rental.BikeRentalStationService; import org.opentripplanner.routing.edgetype.BikeParkEdge; @@ -25,6 +23,8 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import static org.opentripplanner.graph_builder.linking.SimpleStreetSplitter.*; + /** * Graph updater that dynamically sets availability information on bike parking lots. * This updater fetches data from a single BikeParkDataSource. @@ -47,7 +47,7 @@ public class BikeParkUpdater extends PollingGraphUpdater { private Graph graph; - private SimpleStreetSplitter linker; + private StreetSplitter splitter; private BikeRentalStationService bikeService; @@ -84,8 +84,8 @@ protected void configurePolling(Graph graph, JsonNode config) throws Exception { @Override public void setup(Graph graph) { - // Creation of network linker library will not modify the graph - linker = new SimpleStreetSplitter(graph); + splitter = graph.streetIndex.getStreetSplitter(); + // Adding a bike park station service needs a graph writer runnable bikeService = graph.getService(BikeRentalStationService.class, true); } @@ -127,9 +127,9 @@ public void run(Graph graph) { BikeParkVertex bikeParkVertex = verticesByPark.get(bikePark); if (bikeParkVertex == null) { bikeParkVertex = new BikeParkVertex(graph, bikePark); - if (!linker.link(bikeParkVertex)) { + if (!splitter.linkToClosestWalkableEdge(bikeParkVertex, DESTRUCTIVE_SPLIT)) { // the toString includes the text "Bike park" - LOG.warn("{} not near any streets; it will not be usable.", bikePark); + LOG.warn("Ignoring {} since it's not near any streets; it will not be usable.", bikePark); } verticesByPark.put(bikePark, bikeParkVertex); new BikeParkEdge(bikeParkVertex); diff --git a/src/main/java/org/opentripplanner/updater/bike_rental/BikeRentalUpdater.java b/src/main/java/org/opentripplanner/updater/bike_rental/BikeRentalUpdater.java index 4d1ba5b324a..8ccf22778f6 100644 --- a/src/main/java/org/opentripplanner/updater/bike_rental/BikeRentalUpdater.java +++ b/src/main/java/org/opentripplanner/updater/bike_rental/BikeRentalUpdater.java @@ -1,16 +1,7 @@ package org.opentripplanner.updater.bike_rental; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.HashMap; -import java.util.HashSet; -import java.util.List; -import java.util.Map; -import java.util.Map.Entry; -import java.util.Set; -import java.util.concurrent.ExecutionException; import com.fasterxml.jackson.databind.JsonNode; -import org.opentripplanner.graph_builder.linking.SimpleStreetSplitter; +import org.opentripplanner.graph_builder.linking.StreetSplitter; import org.opentripplanner.routing.bike_rental.BikeRentalStation; import org.opentripplanner.routing.bike_rental.BikeRentalStationService; import org.opentripplanner.routing.edgetype.RentABikeOffEdge; @@ -23,9 +14,19 @@ import org.opentripplanner.updater.PollingGraphUpdater; import org.slf4j.Logger; import org.slf4j.LoggerFactory; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; import java.util.Map.Entry; +import java.util.Set; import java.util.concurrent.ExecutionException; +import static org.opentripplanner.graph_builder.linking.SimpleStreetSplitter.DESTRUCTIVE_SPLIT; + /** * Dynamic bike-rental station updater which updates the Graph with bike rental stations from one BikeRentalDataSource. */ @@ -37,11 +38,13 @@ public class BikeRentalUpdater extends PollingGraphUpdater { private static final String DEFAULT_NETWORK_LIST = "default"; - Map verticesByStation = new HashMap(); + Map verticesByStation = new HashMap<>(); private BikeRentalDataSource source; - private SimpleStreetSplitter linker; + private Graph graph; + + private StreetSplitter splitter; private BikeRentalStationService service; @@ -118,8 +121,8 @@ protected void configurePolling (Graph graph, JsonNode config) throws Exception @Override public void setup(Graph graph) throws InterruptedException, ExecutionException { - // Creation of network linker library will not modify the graph - linker = new SimpleStreetSplitter(graph); + splitter = graph.streetIndex.getStreetSplitter(); + // Adding a bike rental station service needs a graph writer runnable service = graph.getService(BikeRentalStationService.class, true); } @@ -155,6 +158,7 @@ public void run(Graph graph) { // Apply stations to graph Set stationSet = new HashSet<>(); Set defaultNetworks = new HashSet<>(Arrays.asList(network)); + LOG.info("Updating {} rental bike stations.", stations.size()); /* add any new stations and update bike counts for existing stations */ for (BikeRentalStation station : stations) { if (station.networks == null) { @@ -166,7 +170,7 @@ public void run(Graph graph) { BikeRentalStationVertex vertex = verticesByStation.get(station); if (vertex == null) { vertex = new BikeRentalStationVertex(graph, station); - if (!linker.link(vertex)) { + if (!splitter.linkToClosestWalkableEdge(vertex, DESTRUCTIVE_SPLIT)) { // the toString includes the text "Bike rental station" LOG.warn("{} not near any streets; it will not be usable.", station); } diff --git a/src/test/java/org/opentripplanner/graph_builder/linking/SimpleStreetSplitterTest.java b/src/test/java/org/opentripplanner/graph_builder/linking/SimpleStreetSplitterTest.java index 37fb4fc50ce..a3313a776fb 100644 --- a/src/test/java/org/opentripplanner/graph_builder/linking/SimpleStreetSplitterTest.java +++ b/src/test/java/org/opentripplanner/graph_builder/linking/SimpleStreetSplitterTest.java @@ -22,7 +22,7 @@ public class SimpleStreetSplitterTest { @Before public void buildSpy(){ Graph graph = new Graph(); - SimpleStreetSplitter simpleStreetSplitter = new SimpleStreetSplitter(graph, null, null,false); + SimpleStreetSplitter simpleStreetSplitter = new SimpleStreetSplitter(graph, null, null); spySimpleStreetSplitter = spy(simpleStreetSplitter); } @@ -37,7 +37,7 @@ public void testFindEndVertexForParkAndRide(){ routingRequest.setMode(TraverseMode.CAR); routingRequest.parkAndRide = true; - spySimpleStreetSplitter.getClosestVertex(genericLocation, routingRequest, true); - verify(spySimpleStreetSplitter).link(any(Vertex.class), eq(TraverseMode.WALK), eq(routingRequest)); + spySimpleStreetSplitter.linkOriginDestination(genericLocation, routingRequest, true); + verify(spySimpleStreetSplitter).linkToGraph(any(Vertex.class), eq(TraverseMode.WALK), eq(routingRequest), eq(false)); } } diff --git a/src/test/java/org/opentripplanner/graph_builder/module/FakeGraph.java b/src/test/java/org/opentripplanner/graph_builder/module/FakeGraph.java index 55a62f16299..8568b8c9d8e 100644 --- a/src/test/java/org/opentripplanner/graph_builder/module/FakeGraph.java +++ b/src/test/java/org/opentripplanner/graph_builder/module/FakeGraph.java @@ -133,7 +133,7 @@ public static void addExtraStops (Graph g) { /** link the stops in the graph */ public static void link (Graph g) { SimpleStreetSplitter linker = new SimpleStreetSplitter(g); - linker.link(); + linker.linkAllStationsToGraph(); } } From 31dda0fa29a9c8899d3cdf5096351a9e4faa39c6 Mon Sep 17 00:00:00 2001 From: evansiroky Date: Mon, 6 May 2019 23:00:57 -0700 Subject: [PATCH 02/12] fix a few things that broke after the cherry-pick --- .../graph_builder/linking/LinkStopToPlatformTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/java/org/opentripplanner/graph_builder/linking/LinkStopToPlatformTest.java b/src/test/java/org/opentripplanner/graph_builder/linking/LinkStopToPlatformTest.java index b07a02be453..25f863269e0 100644 --- a/src/test/java/org/opentripplanner/graph_builder/linking/LinkStopToPlatformTest.java +++ b/src/test/java/org/opentripplanner/graph_builder/linking/LinkStopToPlatformTest.java @@ -73,7 +73,7 @@ public void before() { @Test public void testLinkStopWithoutExtraEdges() { SimpleStreetSplitter splitter = new SimpleStreetSplitter(graph); - splitter.link(); + splitter.linkAllStationsToGraph(); assertEquals(16, graph.getEdges().size()); } @@ -82,7 +82,7 @@ public void testLinkStopWithoutExtraEdges() { public void testLinkStopWithExtraEdges() { SimpleStreetSplitter splitter = new SimpleStreetSplitter(graph); splitter.setAddExtraEdgesToAreas(true); - splitter.link(); + splitter.linkAllStationsToGraph(); assertEquals(38, graph.getEdges().size()); } From 3a77bb00190542b9ab036a8db448e2bdce0ced68 Mon Sep 17 00:00:00 2001 From: evansiroky Date: Mon, 6 May 2019 23:01:38 -0700 Subject: [PATCH 03/12] add a test for snapping issue Refs https://github.com/opentripplanner/OpenTripPlanner/issues/2758 --- .../linking/SimpleStreetSplitterTest.java | 83 ++++++++++++++++++- 1 file changed, 81 insertions(+), 2 deletions(-) diff --git a/src/test/java/org/opentripplanner/graph_builder/linking/SimpleStreetSplitterTest.java b/src/test/java/org/opentripplanner/graph_builder/linking/SimpleStreetSplitterTest.java index a3313a776fb..841c02dedf1 100644 --- a/src/test/java/org/opentripplanner/graph_builder/linking/SimpleStreetSplitterTest.java +++ b/src/test/java/org/opentripplanner/graph_builder/linking/SimpleStreetSplitterTest.java @@ -1,21 +1,37 @@ package org.opentripplanner.graph_builder.linking; - +import com.vividsolutions.jts.geom.Coordinate; +import com.vividsolutions.jts.geom.GeometryFactory; +import com.vividsolutions.jts.geom.LineString; import org.junit.Before; import org.junit.Test; +import org.opentripplanner.common.geometry.GeometryUtils; +import org.opentripplanner.common.geometry.SphericalDistanceLibrary; import org.opentripplanner.common.model.GenericLocation; +import org.opentripplanner.routing.bike_rental.BikeRentalStation; +import org.opentripplanner.routing.core.RoutingContext; import org.opentripplanner.routing.core.RoutingRequest; import org.opentripplanner.routing.core.TraverseMode; +import org.opentripplanner.routing.edgetype.StreetEdge; +import org.opentripplanner.routing.edgetype.StreetTraversalPermission; import org.opentripplanner.routing.graph.Graph; import org.opentripplanner.routing.graph.Vertex; +import org.opentripplanner.routing.impl.DefaultStreetVertexIndexFactory; +import org.opentripplanner.routing.vertextype.BikeRentalStationVertex; +import org.opentripplanner.routing.vertextype.IntersectionVertex; +import org.opentripplanner.routing.vertextype.StreetVertex; +import org.opentripplanner.util.NonLocalizedString; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.any; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; +import static org.opentripplanner.graph_builder.linking.SimpleStreetSplitter.DESTRUCTIVE_SPLIT; public class SimpleStreetSplitterTest { - + private final GeometryFactory gf = GeometryUtils.getGeometryFactory(); private SimpleStreetSplitter spySimpleStreetSplitter; @@ -40,4 +56,67 @@ public void testFindEndVertexForParkAndRide(){ spySimpleStreetSplitter.linkOriginDestination(genericLocation, routingRequest, true); verify(spySimpleStreetSplitter).linkToGraph(any(Vertex.class), eq(TraverseMode.WALK), eq(routingRequest), eq(false)); } + + /** + * Tests whether the proper edge is linked in a request where the original StreetEdge has been + * split to insert a bike rental station. + */ + @Test + public void canLinkToEdgeSplitForBikeRental() { + // Given: + // - a graph with 3 intersections/vertices + Graph g = new Graph(); + + StreetVertex a = new IntersectionVertex(g, "A", 1.0, 1.0); + StreetVertex b = new IntersectionVertex(g, "B", 0.0, 1.0); + StreetVertex c = new IntersectionVertex(g, "C", 1.0, 0.0); + + // - And 3 streets between the vertices + + createStreetEdge(a, b, "a -> b"); + createStreetEdge(b, a, "b -> a"); + createStreetEdge(a, c, "a -> c"); + g.index(new DefaultStreetVertexIndexFactory()); + + // When - a bike rental station between A and B that has been inserted into the graph + BikeRentalStation brstation = new BikeRentalStation(); + brstation.id = "bike_1"; + brstation.x = 0.5; + brstation.y = 1.001; + brstation.name = new NonLocalizedString("bike_1"); + BikeRentalStationVertex bikeRentalStationVertex = new BikeRentalStationVertex(g, brstation); + StreetSplitter splitter = g.streetIndex.getStreetSplitter(); + splitter.linkToClosestWalkableEdge(bikeRentalStationVertex, DESTRUCTIVE_SPLIT); + + // - And travel *origin* is very close to the road from B to A + GenericLocation from = new GenericLocation(0.999, 0.4); + + // - and *destination* is slightly off 0.7 degrees on road from C to A + GenericLocation to = new GenericLocation(0.701, 1.001); + + // - And A request + RoutingRequest request = new RoutingRequest(); + request.from = from; + request.to = to; + + // When - the context is created + RoutingContext subject = new RoutingContext(request, g); + + // Then - the origin and destination is set properly + assertEquals("Origin", subject.fromVertex.getName()); + assertEquals("Destination", subject.toVertex.getName()); + + // - And the from vertex should have outgoing edges + assertTrue( + "From vertex should have at least 1 outgoing edge", + subject.fromVertex.getOutgoing().size() > 0 + ); + } + + 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 StreetEdge(v0, v1, geom, name, dist, StreetTraversalPermission.ALL, false); + } } From b13a6e4de0af231908a74e7cab3b3f24968a4382 Mon Sep 17 00:00:00 2001 From: evansiroky Date: Tue, 7 May 2019 09:39:12 -0700 Subject: [PATCH 04/12] Make splitter work with areas --- .../graph_builder/linking/SimpleStreetSplitter.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/opentripplanner/graph_builder/linking/SimpleStreetSplitter.java b/src/main/java/org/opentripplanner/graph_builder/linking/SimpleStreetSplitter.java index 2e40586444a..8eee56e2dd3 100644 --- a/src/main/java/org/opentripplanner/graph_builder/linking/SimpleStreetSplitter.java +++ b/src/main/java/org/opentripplanner/graph_builder/linking/SimpleStreetSplitter.java @@ -19,6 +19,7 @@ import org.opentripplanner.graph_builder.annotation.BikeParkUnlinked; import org.opentripplanner.graph_builder.annotation.BikeRentalStationUnlinked; import org.opentripplanner.graph_builder.annotation.StopUnlinked; +import org.opentripplanner.graph_builder.services.DefaultStreetEdgeFactory; import org.opentripplanner.graph_builder.services.StreetEdgeFactory; import org.opentripplanner.openstreetmap.model.OSMWithTags; import org.opentripplanner.graph_builder.annotation.StopLinkedTooFar; @@ -80,7 +81,7 @@ public class SimpleStreetSplitter implements StreetSplitter { private Boolean addExtraEdgesToAreas = false; - private StreetEdgeFactory edgeFactory; + private StreetEdgeFactory edgeFactory = new DefaultStreetEdgeFactory(); public static final int WARNING_DISTANCE_METERS = 20; From 47dcfb0dccc62f39bf89a5e5f56770bb156a42aa Mon Sep 17 00:00:00 2001 From: evansiroky Date: Tue, 7 May 2019 14:26:19 -0700 Subject: [PATCH 05/12] update to jts imports in splitter test --- .../graph_builder/linking/SimpleStreetSplitterTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/java/org/opentripplanner/graph_builder/linking/SimpleStreetSplitterTest.java b/src/test/java/org/opentripplanner/graph_builder/linking/SimpleStreetSplitterTest.java index 841c02dedf1..ff732f08e4e 100644 --- a/src/test/java/org/opentripplanner/graph_builder/linking/SimpleStreetSplitterTest.java +++ b/src/test/java/org/opentripplanner/graph_builder/linking/SimpleStreetSplitterTest.java @@ -1,10 +1,10 @@ package org.opentripplanner.graph_builder.linking; -import com.vividsolutions.jts.geom.Coordinate; -import com.vividsolutions.jts.geom.GeometryFactory; -import com.vividsolutions.jts.geom.LineString; import org.junit.Before; import org.junit.Test; +import org.locationtech.jts.geom.Coordinate; +import org.locationtech.jts.geom.GeometryFactory; +import org.locationtech.jts.geom.LineString; import org.opentripplanner.common.geometry.GeometryUtils; import org.opentripplanner.common.geometry.SphericalDistanceLibrary; import org.opentripplanner.common.model.GenericLocation; From f7b6c61e0e93db5b3f9b17f25a0bc618953abef0 Mon Sep 17 00:00:00 2001 From: evansiroky Date: Tue, 7 May 2019 14:41:49 -0700 Subject: [PATCH 06/12] add entry to changelog regarding StreetSplitter refactor --- docs/Changelog.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/Changelog.md b/docs/Changelog.md index 539d75695c5..6a0f269b56f 100644 --- a/docs/Changelog.md +++ b/docs/Changelog.md @@ -13,6 +13,7 @@ - Remove CarFreeAtoZ from list of deployments - Fix XML response serialization (#2685) - Refactor InterleavedBidirectionalHeuristic (#2671) +- Refactor StreetSplitter (#2758) ## 1.3 (2018-08-03) From cd8b240267607d4de4cfcd2d5c50169101efb7d2 Mon Sep 17 00:00:00 2001 From: evansiroky Date: Fri, 10 May 2019 16:50:06 -0700 Subject: [PATCH 07/12] Refactor street splitting code - rename SimpleStreetSplitter to StreetSplitter as it is the only StreetSplitter in use - add some checks to make sure duplicate indexing doesn't occur on the same graph - refactor some areas where duplicate indexing was occurring --- .../linking/SimpleStreetSplitter.java | 629 ----------------- .../graph_builder/linking/StreetSplitter.java | 637 +++++++++++++++++- .../linking/TransitToStreetNetworkModule.java | 47 -- .../module/PruneFloatingIslands.java | 27 +- .../module/StreetLinkerModule.java | 16 +- .../routing/core/RoutingRequest.java | 4 +- .../opentripplanner/routing/graph/Graph.java | 5 + .../impl/StreetVertexIndexServiceImpl.java | 60 +- .../updater/bike_park/BikeParkUpdater.java | 3 +- .../bike_rental/BikeRentalUpdater.java | 2 +- .../opentripplanner/ConstantsForTests.java | 15 +- .../analyst/InitialStopsTest.java | 12 +- .../linking/LinkStopToPlatformTest.java | 4 +- ...itterTest.java => StreetSplitterTest.java} | 14 +- .../graph_builder/module/FakeGraph.java | 17 +- .../module/linking/LinkingTest.java | 26 +- .../module/osm/TestIntermediatePlaces.java | 10 +- .../routing/graph/GraphSerializationTest.java | 1 - 18 files changed, 733 insertions(+), 796 deletions(-) delete mode 100644 src/main/java/org/opentripplanner/graph_builder/linking/SimpleStreetSplitter.java delete mode 100644 src/main/java/org/opentripplanner/graph_builder/linking/TransitToStreetNetworkModule.java rename src/test/java/org/opentripplanner/graph_builder/linking/{SimpleStreetSplitterTest.java => StreetSplitterTest.java} (89%) diff --git a/src/main/java/org/opentripplanner/graph_builder/linking/SimpleStreetSplitter.java b/src/main/java/org/opentripplanner/graph_builder/linking/SimpleStreetSplitter.java deleted file mode 100644 index 8eee56e2dd3..00000000000 --- a/src/main/java/org/opentripplanner/graph_builder/linking/SimpleStreetSplitter.java +++ /dev/null @@ -1,629 +0,0 @@ -package org.opentripplanner.graph_builder.linking; - -import com.google.common.collect.Iterables; -import org.locationtech.jts.geom.Coordinate; -import org.locationtech.jts.geom.Envelope; -import org.locationtech.jts.geom.GeometryFactory; -import org.locationtech.jts.geom.LineString; -import org.locationtech.jts.index.SpatialIndex; -import org.locationtech.jts.linearref.LinearLocation; -import org.locationtech.jts.linearref.LocationIndexedLine; -import gnu.trove.map.TIntDoubleMap; -import gnu.trove.map.hash.TIntDoubleHashMap; -import jersey.repackaged.com.google.common.collect.Lists; -import org.opentripplanner.common.geometry.GeometryUtils; -import org.opentripplanner.common.geometry.HashGridSpatialIndex; -import org.opentripplanner.common.geometry.SphericalDistanceLibrary; -import org.opentripplanner.common.model.GenericLocation; -import org.opentripplanner.common.model.P2; -import org.opentripplanner.graph_builder.annotation.BikeParkUnlinked; -import org.opentripplanner.graph_builder.annotation.BikeRentalStationUnlinked; -import org.opentripplanner.graph_builder.annotation.StopUnlinked; -import org.opentripplanner.graph_builder.services.DefaultStreetEdgeFactory; -import org.opentripplanner.graph_builder.services.StreetEdgeFactory; -import org.opentripplanner.openstreetmap.model.OSMWithTags; -import org.opentripplanner.graph_builder.annotation.StopLinkedTooFar; -import org.opentripplanner.routing.core.RoutingRequest; -import org.opentripplanner.routing.core.TraverseMode; -import org.opentripplanner.routing.core.TraverseModeSet; -import org.opentripplanner.routing.edgetype.StreetBikeParkLink; -import org.opentripplanner.routing.edgetype.StreetBikeRentalLink; -import org.opentripplanner.routing.edgetype.StreetEdge; -import org.opentripplanner.routing.edgetype.StreetTransitLink; -import org.opentripplanner.routing.edgetype.TemporaryFreeEdge; -import org.opentripplanner.routing.edgetype.AreaEdgeList; -import org.opentripplanner.routing.edgetype.AreaEdge; -import org.opentripplanner.routing.edgetype.StreetTraversalPermission; -import org.opentripplanner.routing.graph.Edge; -import org.opentripplanner.routing.graph.Graph; -import org.opentripplanner.routing.graph.Vertex; -import org.opentripplanner.routing.location.TemporaryStreetLocation; -import org.opentripplanner.routing.vertextype.BikeParkVertex; -import org.opentripplanner.routing.vertextype.BikeRentalStationVertex; -import org.opentripplanner.routing.vertextype.SplitterVertex; -import org.opentripplanner.routing.vertextype.StreetVertex; -import org.opentripplanner.routing.vertextype.TemporarySplitterVertex; -import org.opentripplanner.routing.vertextype.TemporaryVertex; -import org.opentripplanner.routing.vertextype.TransitStop; -import org.opentripplanner.routing.vertextype.IntersectionVertex; -import org.opentripplanner.util.I18NString; -import org.opentripplanner.util.LocalizedString; -import org.opentripplanner.util.NonLocalizedString; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; -import java.util.UUID; -import java.util.stream.Collectors; - -/** - * This class links transit stops to streets by splitting the streets (unless the stop is extremely close to the street - * intersection). - * - * It is intended to eventually completely replace the existing stop linking code, which had been through so many - * revisions and adaptations to different street and turn representations that it was very glitchy. This new code is - * also intended to be deterministic in linking to streets, independent of the order in which the JVM decides to - * iterate over Maps and even in the presence of points that are exactly halfway between multiple candidate linking - * points. - * - * It would be wise to keep this new incarnation of the linking code relatively simple, considering what happened before. - * - * See discussion in pull request #1922, follow up issue #1934, and the original issue calling for replacement of - * the stop linker, #1305. - */ -public class SimpleStreetSplitter implements StreetSplitter { - - private static final Logger LOG = LoggerFactory.getLogger(SimpleStreetSplitter.class); - - public static final int MAX_SEARCH_RADIUS_METERS = 1000; - - private Boolean addExtraEdgesToAreas = false; - - private StreetEdgeFactory edgeFactory = new DefaultStreetEdgeFactory(); - - public static final int WARNING_DISTANCE_METERS = 20; - - // DESTRUCTIVE_SPLIT means edges are split and new edges are created (used when linking transit stops etc. during graph building) - // NON_DESTRUCTIVE_SPLIT means new temporary edges are created and no edges are deleted (Used when searching for origin/destination) - public static final boolean DESTRUCTIVE_SPLIT = true; - public static final boolean NON_DESTRUCTIVE_SPLIT = false; - - /** if there are two ways and the distances to them differ by less than this value, we link to both of them */ - public static final double DUPLICATE_WAY_EPSILON_METERS = 0.001; - - private Graph graph; - - private HashGridSpatialIndex idx; - - private SpatialIndex transitStopIndex; - - private static GeometryFactory geometryFactory = GeometryUtils.getGeometryFactory(); - - /** - * Construct a new SimpleStreetSplitter. - * NOTE: Only one SimpleStreetSplitter should be active on a graph at any given time. - * - * @param hashGridSpatialIndex If not null this index is used instead of creating new one - * @param transitStopIndex Index of all transitStops which is generated in {@link org.opentripplanner.routing.impl.StreetVertexIndexServiceImpl} - */ - public SimpleStreetSplitter(Graph graph, HashGridSpatialIndex hashGridSpatialIndex, - SpatialIndex transitStopIndex) { - this.graph = graph; - this.transitStopIndex = transitStopIndex; - - //We build a spatial index if it isn't provided - if (hashGridSpatialIndex == null) { - // build a nice private spatial index, since we're adding and removing edges - idx = new HashGridSpatialIndex<>(); - for (StreetEdge se : Iterables.filter(graph.getEdges(), StreetEdge.class)) { - // Note: Although it's up to the caller of idx.insert() to make it thread-safe, we don't need to - // worry about it here as it's inside the constructor of SimpleStreetSplitter - // and it's guaranteed that there is only one thread doing this. - idx.insert(se.getGeometry(), se); - } - } else { - idx = hashGridSpatialIndex; - } - - } - - /** - * Construct a new SimpleStreetSplitter. Be aware that only one SimpleStreetSplitter should be - * active on a graph at any given time. - * - * SimpleStreetSplitter generates index on graph and splits destructively (used in transit splitter) - * @param graph - */ - public SimpleStreetSplitter(Graph graph) { - this(graph, null, null); - } - - /** Link all relevant vertices to the street network */ - public void linkAllStationsToGraph() { - for (Vertex v : graph.getVertices()) { - if (v instanceof TransitStop || v instanceof BikeRentalStationVertex || v instanceof BikeParkVertex) - if (!linkToClosestWalkableEdge(v, DESTRUCTIVE_SPLIT)) { - if (v instanceof TransitStop) - LOG.warn(graph.addBuilderAnnotation(new StopUnlinked((TransitStop) v))); - else if (v instanceof BikeRentalStationVertex) - LOG.warn(graph.addBuilderAnnotation(new BikeRentalStationUnlinked((BikeRentalStationVertex) v))); - else if (v instanceof BikeParkVertex) - LOG.warn(graph.addBuilderAnnotation(new BikeParkUnlinked((BikeParkVertex) v))); - }; - } - } - - /** - * Link this vertex into the graph to the closest walkable edge - * @param vertex The vertex to be linked. - * @param destructiveSplitting If true splitting is permanent (Used when linking transit stops etc.) when - * false Splitting is only for duration of a request. Since they are made from - * temporary vertices and edges. - * @return - */ - public boolean linkToClosestWalkableEdge(Vertex vertex, final boolean destructiveSplitting) { - return linkToGraph(vertex, TraverseMode.WALK, null, destructiveSplitting); - } - - public boolean linkToGraph(Vertex vertex, TraverseMode traverseMode, RoutingRequest options, - final boolean destructiveSplitting) { - final TraverseModeSet traverseModeSet = new TraverseModeSet(traverseMode); - if (traverseMode == TraverseMode.BICYCLE) { - traverseModeSet.setWalk(true); - } - return linkToGraph(vertex, traverseModeSet, options, destructiveSplitting); - } - - /** - * Link this vertex into the graph - * @param vertex The vertex to be linked. - * @param traverseModeSet The traverse modes. - * @param options The routing options. - * @param destructiveSplitting If true splitting is permanent (Used when linking transit stops etc.) when - * false Splitting is only for duration of a request. Since they are made from - * temporary vertices and edges. - * @return - */ - public boolean linkToGraph(Vertex vertex, TraverseModeSet traverseModeSet, RoutingRequest options, - final boolean destructiveSplitting) { - // find nearby street edges - // TODO: we used to use an expanding-envelope search, which is more efficient in - // dense areas. but first let's see how inefficient this is. I suspect it's not too - // bad and the gains in simplicity are considerable. - final double radiusDeg = SphericalDistanceLibrary.metersToDegrees(MAX_SEARCH_RADIUS_METERS); - - Envelope env = new Envelope(vertex.getCoordinate()); - - // Perform a simple local equirectangular projection, so distances are expressed in degrees latitude. - final double xscale = Math.cos(vertex.getLat() * Math.PI / 180); - - // Expand more in the longitude direction than the latitude direction to account for converging meridians. - env.expandBy(radiusDeg / xscale, radiusDeg); - - final double DUPLICATE_WAY_EPSILON_DEGREES = SphericalDistanceLibrary.metersToDegrees(DUPLICATE_WAY_EPSILON_METERS); - - // We sort the list of candidate edges by distance to the stop - // This should remove any issues with things coming out of the spatial index in different orders - // Then we link to everything that is within DUPLICATE_WAY_EPSILON_METERS of of the best distance - // so that we capture back edges and duplicate ways. - List candidateEdges = idx.query(env).stream() - .filter(streetEdge -> streetEdge instanceof StreetEdge) - .map(edge -> (StreetEdge) edge) - // note: not filtering by radius here as distance calculation is expensive - // we do that below. - .filter(edge -> edge.canTraverse(traverseModeSet) && - // only link to edges still in the graph. - edge.getToVertex().getIncoming().contains(edge)) - .collect(Collectors.toList()); - - // Make a map of distances to all edges. - final TIntDoubleMap distances = new TIntDoubleHashMap(); - for (StreetEdge e : candidateEdges) { - distances.put(e.getId(), distance(vertex, e, xscale)); - } - - // Sort the list. - Collections.sort(candidateEdges, (o1, o2) -> { - double diff = distances.get(o1.getId()) - distances.get(o2.getId()); - // A Comparator must return an integer but our distances are doubles. - if (diff < 0) - return -1; - if (diff > 0) - return 1; - return 0; - }); - - // find the closest candidate edges - if (candidateEdges.isEmpty() || distances.get(candidateEdges.get(0).getId()) > radiusDeg) { - // We only link to stops if we are searching for origin/destination and for that we need transitStopIndex. - if (destructiveSplitting || transitStopIndex == null) { - return false; - } - LOG.debug("No street edge was found for {}", vertex); - // We search for closest stops (since this is only used in origin/destination linking if no edges were found) - // in the same way the closest edges are found. - List candidateStops = new ArrayList<>(); - transitStopIndex.query(env).forEach(candidateStop -> - candidateStops.add((TransitStop) candidateStop) - ); - - final TIntDoubleMap stopDistances = new TIntDoubleHashMap(); - - for (TransitStop t : candidateStops) { - stopDistances.put(t.getIndex(), distance(vertex, t, xscale)); - } - - Collections.sort(candidateStops, (o1, o2) -> { - double diff = stopDistances.get(o1.getIndex()) - stopDistances.get(o2.getIndex()); - if (diff < 0) { - return -1; - } - if (diff > 0) { - return 1; - } - return 0; - }); - if (candidateStops.isEmpty() || stopDistances.get(candidateStops.get(0).getIndex()) > radiusDeg) { - LOG.debug("Stops aren't close either!"); - return false; - } else { - List bestStops = Lists.newArrayList(); - // Add stops until there is a break of epsilon meters. - // we do this to enforce determinism. if there are a lot of stops that are all extremely close to each other, - // we want to be sure that we deterministically link to the same ones every time. Any hard cutoff means things can - // fall just inside or beyond the cutoff depending on floating-point operations. - int i = 0; - do { - bestStops.add(candidateStops.get(i++)); - } while (i < candidateStops.size() && - stopDistances.get(candidateStops.get(i).getIndex()) - stopDistances - .get(candidateStops.get(i - 1).getIndex()) < DUPLICATE_WAY_EPSILON_DEGREES); - - for (TransitStop stop: bestStops) { - LOG.debug("Linking vertex to stop: {}", stop.getName()); - makeTemporaryEdges((TemporaryStreetLocation)vertex, stop, destructiveSplitting); - } - return true; - } - } else { - - // find the best edges - List bestEdges = Lists.newArrayList(); - - // add edges until there is a break of epsilon meters. - // we do this to enforce determinism. if there are a lot of edges that are all extremely close to each other, - // we want to be sure that we deterministically link to the same ones every time. Any hard cutoff means things can - // fall just inside or beyond the cutoff depending on floating-point operations. - int i = 0; - do { - bestEdges.add(candidateEdges.get(i++)); - } while (i < candidateEdges.size() && - distances.get(candidateEdges.get(i).getId()) - distances - .get(candidateEdges.get(i - 1).getId()) < DUPLICATE_WAY_EPSILON_DEGREES); - - for (StreetEdge edge : bestEdges) { - linkToEdge(vertex, edge, xscale, options, destructiveSplitting); - } - - // Warn if a linkage was made, but the linkage was suspiciously long. - if (vertex instanceof TransitStop) { - double distanceDegreesLatitude = distances.get(candidateEdges.get(0).getId()); - int distanceMeters = (int)SphericalDistanceLibrary.degreesLatitudeToMeters(distanceDegreesLatitude); - if (distanceMeters > WARNING_DISTANCE_METERS) { - // Registering an annotation but not logging because tests produce thousands of these warnings. - graph.addBuilderAnnotation(new StopLinkedTooFar((TransitStop)vertex, distanceMeters)); - } - } - - return true; - } - } - - // Link to all vertices in area/platform - private void linkTransitToAreaVertices(Vertex splitterVertex, AreaEdgeList area) { - List vertices = new ArrayList<>(); - - for (AreaEdge areaEdge : area.getEdges()) { - if (!vertices.contains(areaEdge.getToVertex())) vertices.add(areaEdge.getToVertex()); - if (!vertices.contains(areaEdge.getFromVertex())) vertices.add(areaEdge.getFromVertex()); - } - - for (Vertex vertex : vertices) { - if (vertex instanceof StreetVertex && !vertex.equals(splitterVertex)) { - LineString line = geometryFactory.createLineString(new Coordinate[] { splitterVertex.getCoordinate(), vertex.getCoordinate()}); - double length = SphericalDistanceLibrary.distance(splitterVertex.getCoordinate(), - vertex.getCoordinate()); - I18NString name = new LocalizedString("", new OSMWithTags()); - - edgeFactory.createAreaEdge((IntersectionVertex) splitterVertex, (IntersectionVertex) vertex, line, name, length,StreetTraversalPermission.PEDESTRIAN_AND_BICYCLE, false, area); - edgeFactory.createAreaEdge((IntersectionVertex) vertex, (IntersectionVertex) splitterVertex, line, name, length,StreetTraversalPermission.PEDESTRIAN_AND_BICYCLE, false, area); - } - } - } - - /** - * Split the edge and link in the transit stop. - * @param vertex An object of Vertex to be linked to an edge. - * @param edge An object of StreetEdge to be linked to. - * @param xscale The longitude scale factor in Equirectangular projection. - * @param options An object of RoutingRequest - * @param destructiveSplitting If true splitting is permanent (Used when linking transit stops etc.) when - * false Splitting is only for duration of a request. Since they are made from - * temporary vertices and edges. - */ - private void linkToEdge(Vertex vertex, StreetEdge edge, double xscale, RoutingRequest options, - final boolean destructiveSplitting) { - // TODO: we've already built this line string, we should save it - LineString orig = edge.getGeometry(); - LineString transformed = equirectangularProject(orig, xscale); - LocationIndexedLine il = new LocationIndexedLine(transformed); - LinearLocation ll = il.project(new Coordinate(vertex.getLon() * xscale, vertex.getLat())); - - // if we're very close to one end of the line or the other, or endwise, don't bother to split, - // cut to the chase and link directly - // We use a really tiny epsilon here because we only want points that actually snap to exactly the same location on the - // street to use the same vertices. Otherwise the order the stops are loaded in will affect where they are snapped. - if (ll.getSegmentIndex() == 0 && ll.getSegmentFraction() < 1e-8) { - makeLinkEdges(vertex, (StreetVertex) edge.getFromVertex(), destructiveSplitting); - } - // -1 converts from count to index. Because of the fencepost problem, npoints - 1 is the "segment" - // past the last point - else if (ll.getSegmentIndex() == orig.getNumPoints() - 1) { - makeLinkEdges(vertex, (StreetVertex) edge.getToVertex(), destructiveSplitting); - } - - // nPoints - 2: -1 to correct for index vs count, -1 to account for fencepost problem - else if (ll.getSegmentIndex() == orig.getNumPoints() - 2 && ll.getSegmentFraction() > 1 - 1e-8) { - makeLinkEdges(vertex, (StreetVertex) edge.getToVertex(), destructiveSplitting); - } - - else { - - TemporaryVertex temporaryVertex = null; - boolean endVertex = false; - if (vertex instanceof TemporaryVertex) { - temporaryVertex = (TemporaryVertex) vertex; - endVertex = temporaryVertex.isEndVertex(); - - } - //This throws runtime TrivialPathException if same edge is split in origin and destination link - //It is only used in origin/destination linking since otherwise options is null - if (options != null) { - options.canSplitEdge(edge); - } - // split the edge, get the split vertex - SplitterVertex v0 = split(edge, ll, temporaryVertex != null, endVertex, destructiveSplitting); - makeLinkEdges(vertex, v0, destructiveSplitting); - - // If splitter vertex is part of area; link splittervertex to all other vertexes in area, this creates - // edges that were missed by WalkableAreaBuilder - if (edge instanceof AreaEdge && vertex instanceof TransitStop && this.addExtraEdgesToAreas) { - linkTransitToAreaVertices(v0, ((AreaEdge) edge).getArea()); - } - } - } - - - /** - * Split the street edge at the given fraction - * - * @param edge to be split - * @param ll fraction at which to split the edge - * @param temporarySplit if true this is temporary split at origin/destinations search and only temporary edges vertices are created - * @param endVertex if this is temporary edge this is true if this is end vertex otherwise it doesn't matter - * @param destructiveSplitting If true splitting is permanent (Used when linking transit stops etc.) when - * false Splitting is only for duration of a request. Since they are made from - * temporary vertices and edges. - * @return Splitter vertex with added new edges - */ - private SplitterVertex split (StreetEdge edge, LinearLocation ll, boolean temporarySplit, boolean endVertex, - final boolean destructiveSplitting) { - - LineString geometry = edge.getGeometry(); - - // create the geometries - Coordinate splitPoint = ll.getCoordinate(geometry); - - // every edge can be split exactly once, so this is a valid label - SplitterVertex v; - if (temporarySplit) { - v = new TemporarySplitterVertex("split from " + edge.getId(), splitPoint.x, splitPoint.y, edge, endVertex); - } else { - v = new SplitterVertex(graph, "split from " + edge.getId(), splitPoint.x, splitPoint.y, edge); - } - - // Split the 'edge' at 'v' in 2 new edges and connect these 2 edges to the - // existing vertices - P2 edges = edge.split(v, !temporarySplit); - - if (destructiveSplitting) { - // update indices of new edges - synchronized (this) { - // Note: Write operations are not synchronized in HashGridSpatialIndex, hence the lock. - idx.insert(edges.first.getGeometry(), edges.first); - idx.insert(edges.second.getGeometry(), edges.second); - } - // (no need to remove original edge, we filter it when it comes out of the index) - - // remove original edge from the graph - edge.getToVertex().removeIncoming(edge); - edge.getFromVertex().removeOutgoing(edge); - } - - return v; - } - - /** Make the appropriate type of link edges from a vertex */ - private void makeLinkEdges(Vertex from, StreetVertex to, final boolean destructiveSplitting) { - if (from instanceof TemporaryStreetLocation) { - makeTemporaryEdges((TemporaryStreetLocation) from, to, destructiveSplitting); - } else if (from instanceof TransitStop) { - makeTransitLinkEdges((TransitStop) from, to, destructiveSplitting); - } else if (from instanceof BikeRentalStationVertex) { - makeBikeRentalLinkEdges((BikeRentalStationVertex) from, to, destructiveSplitting); - } else if (from instanceof BikeParkVertex) { - makeBikeParkEdges((BikeParkVertex) from, to, destructiveSplitting); - } - } - - /** Make temporary edges to origin/destination vertex in origin/destination search **/ - private void makeTemporaryEdges(TemporaryStreetLocation from, Vertex to, final boolean destructiveSplitting) { - if (destructiveSplitting) { - throw new RuntimeException("Destructive splitting is used on temporary edges. Something is wrong!"); - } - if (to instanceof TemporarySplitterVertex) { - from.setWheelchairAccessible(((TemporarySplitterVertex) to).isWheelchairAccessible()); - } - if (from.isEndVertex()) { - LOG.debug("Linking end vertex to {} -> {}", to, from); - new TemporaryFreeEdge(to, from); - } else { - LOG.debug("Linking start vertex to {} -> {}", from, to); - new TemporaryFreeEdge(from, to); - } - } - - /** Make bike park edges */ - private void makeBikeParkEdges(BikeParkVertex from, StreetVertex to, final boolean destructiveSplitting) { - if (!destructiveSplitting) { - throw new RuntimeException("Bike park edges are created with non destructive splitting!"); - } - for (StreetBikeParkLink sbpl : Iterables.filter(from.getOutgoing(), StreetBikeParkLink.class)) { - if (sbpl.getToVertex() == to) - return; - } - - new StreetBikeParkLink(from, to); - new StreetBikeParkLink(to, from); - } - - /** - * Make street transit link edges, unless they already exist. - */ - private void makeTransitLinkEdges (TransitStop tstop, StreetVertex v, final boolean destructiveSplitting) { - if (!destructiveSplitting) { - throw new RuntimeException("Transitedges are created with non destructive splitting!"); - } - // ensure that the requisite edges do not already exist - // this can happen if we link to duplicate ways that have the same start/end vertices. - for (StreetTransitLink e : Iterables.filter(tstop.getOutgoing(), StreetTransitLink.class)) { - if (e.getToVertex() == v) - return; - } - - new StreetTransitLink(tstop, v, tstop.hasWheelchairEntrance()); - new StreetTransitLink(v, tstop, tstop.hasWheelchairEntrance()); - } - - /** Make link edges for bike rental */ - private void makeBikeRentalLinkEdges (BikeRentalStationVertex from, StreetVertex to, - final boolean destructiveSplitting) { - if (!destructiveSplitting) { - throw new RuntimeException("Bike rental edges are created with non destructive splitting!"); - } - for (StreetBikeRentalLink sbrl : Iterables.filter(from.getOutgoing(), StreetBikeRentalLink.class)) { - if (sbrl.getToVertex() == to) - return; - } - - new StreetBikeRentalLink(from, to); - new StreetBikeRentalLink(to, from); - } - - /** projected distance from stop to edge, in latitude degrees */ - private static double distance (Vertex tstop, StreetEdge edge, double xscale) { - // Despite the fact that we want to use a fast somewhat inaccurate projection, still use JTS library tools - // for the actual distance calculations. - LineString transformed = equirectangularProject(edge.getGeometry(), xscale); - return transformed.distance(geometryFactory.createPoint(new Coordinate(tstop.getLon() * xscale, tstop.getLat()))); - } - - /** projected distance from stop to another stop, in latitude degrees */ - private static double distance (Vertex tstop, Vertex tstop2, double xscale) { - // use JTS internal tools wherever possible - return new Coordinate(tstop.getLon() * xscale, tstop.getLat()).distance(new Coordinate(tstop2.getLon() * xscale, tstop2.getLat())); - } - - /** project this linestring to an equirectangular projection */ - private static LineString equirectangularProject(LineString geometry, double xscale) { - Coordinate[] coords = new Coordinate[geometry.getNumPoints()]; - - for (int i = 0; i < coords.length; i++) { - Coordinate c = geometry.getCoordinateN(i); - c = (Coordinate) c.clone(); - c.x *= xscale; - coords[i] = c; - } - - return geometryFactory.createLineString(coords); - } - - /** - * Used to link origin and destination points to graph non destructively. - * - * Split edges don't replace existing ones and only temporary edges and vertices are created. - * - * Will throw ThrivialPathException if origin and destination Location are on the same edge - * - * @param location - * @param options - * @param endVertex true if this is destination vertex - * @return - */ - public Vertex linkOriginDestination(GenericLocation location, RoutingRequest options, boolean endVertex) { - - if (endVertex) { - LOG.debug("Finding end vertex for {}", location); - } else { - LOG.debug("Finding start vertex for {}", location); - } - Coordinate coord = location.getCoordinate(); - //TODO: add nice name - String name; - - if (location.name == null || location.name.isEmpty()) { - if (endVertex) { - name = "Destination"; - } else { - name = "Origin"; - } - } else { - name = location.name; - } - TemporaryStreetLocation closest = new TemporaryStreetLocation(UUID.randomUUID().toString(), - coord, new NonLocalizedString(name), endVertex); - - TraverseMode nonTransitMode = TraverseMode.WALK; - //It can be null in tests - if (options != null) { - TraverseModeSet modes = options.modes; - if (modes.getCar()) - // for park and ride we will start in car mode and walk to the end vertex - if (endVertex && (options.parkAndRide || options.kissAndRide)) { - nonTransitMode = TraverseMode.WALK; - } else { - nonTransitMode = TraverseMode.CAR; - } - else if (modes.getWalk()) - nonTransitMode = TraverseMode.WALK; - else if (modes.getBicycle()) - nonTransitMode = TraverseMode.BICYCLE; - } - - if(!linkToGraph(closest, nonTransitMode, options, NON_DESTRUCTIVE_SPLIT)) { - LOG.warn("Couldn't link {}", location); - } - return closest; - - } - - public Boolean getAddExtraEdgesToAreas() { - return addExtraEdgesToAreas; - } - - public void setAddExtraEdgesToAreas(Boolean addExtraEdgesToAreas) { - this.addExtraEdgesToAreas = addExtraEdgesToAreas; - } -} diff --git a/src/main/java/org/opentripplanner/graph_builder/linking/StreetSplitter.java b/src/main/java/org/opentripplanner/graph_builder/linking/StreetSplitter.java index 1e6a3ca5550..2938a2069c2 100644 --- a/src/main/java/org/opentripplanner/graph_builder/linking/StreetSplitter.java +++ b/src/main/java/org/opentripplanner/graph_builder/linking/StreetSplitter.java @@ -1,11 +1,640 @@ package org.opentripplanner.graph_builder.linking; +import com.google.common.collect.Iterables; +import gnu.trove.map.TIntDoubleMap; +import gnu.trove.map.hash.TIntDoubleHashMap; +import jersey.repackaged.com.google.common.collect.Lists; +import org.locationtech.jts.geom.Coordinate; +import org.locationtech.jts.geom.Envelope; +import org.locationtech.jts.geom.GeometryFactory; +import org.locationtech.jts.geom.LineString; +import org.locationtech.jts.index.SpatialIndex; +import org.locationtech.jts.linearref.LinearLocation; +import org.locationtech.jts.linearref.LocationIndexedLine; +import org.opentripplanner.common.geometry.GeometryUtils; +import org.opentripplanner.common.geometry.HashGridSpatialIndex; +import org.opentripplanner.common.geometry.SphericalDistanceLibrary; +import org.opentripplanner.common.model.GenericLocation; +import org.opentripplanner.common.model.P2; +import org.opentripplanner.graph_builder.annotation.BikeParkUnlinked; +import org.opentripplanner.graph_builder.annotation.BikeRentalStationUnlinked; +import org.opentripplanner.graph_builder.annotation.StopLinkedTooFar; +import org.opentripplanner.graph_builder.annotation.StopUnlinked; +import org.opentripplanner.graph_builder.services.DefaultStreetEdgeFactory; +import org.opentripplanner.graph_builder.services.StreetEdgeFactory; +import org.opentripplanner.openstreetmap.model.OSMWithTags; +import org.opentripplanner.routing.core.RoutingRequest; +import org.opentripplanner.routing.core.TraverseMode; +import org.opentripplanner.routing.core.TraverseModeSet; +import org.opentripplanner.routing.edgetype.AreaEdge; +import org.opentripplanner.routing.edgetype.AreaEdgeList; +import org.opentripplanner.routing.edgetype.StreetBikeParkLink; +import org.opentripplanner.routing.edgetype.StreetBikeRentalLink; +import org.opentripplanner.routing.edgetype.StreetEdge; +import org.opentripplanner.routing.edgetype.StreetTransitLink; +import org.opentripplanner.routing.edgetype.StreetTraversalPermission; +import org.opentripplanner.routing.edgetype.TemporaryFreeEdge; +import org.opentripplanner.routing.graph.Edge; +import org.opentripplanner.routing.graph.Graph; import org.opentripplanner.routing.graph.Vertex; +import org.opentripplanner.routing.location.TemporaryStreetLocation; +import org.opentripplanner.routing.vertextype.BikeParkVertex; +import org.opentripplanner.routing.vertextype.BikeRentalStationVertex; +import org.opentripplanner.routing.vertextype.IntersectionVertex; +import org.opentripplanner.routing.vertextype.SplitterVertex; +import org.opentripplanner.routing.vertextype.StreetVertex; +import org.opentripplanner.routing.vertextype.TemporarySplitterVertex; +import org.opentripplanner.routing.vertextype.TemporaryVertex; +import org.opentripplanner.routing.vertextype.TransitStop; +import org.opentripplanner.util.I18NString; +import org.opentripplanner.util.LocalizedString; +import org.opentripplanner.util.NonLocalizedString; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.UUID; +import java.util.stream.Collectors; /** - * This interface allows passing a SimpleStreetSplitter object to the Graph without making Graph depend on - * SimpleStreetSplitter (avoiding circular dependency). + * This class links transit stops to streets by splitting the streets (unless the stop is extremely close to the street + * intersection). + * + * It is intended to eventually completely replace the existing stop linking code, which had been through so many + * revisions and adaptations to different street and turn representations that it was very glitchy. This new code is + * also intended to be deterministic in linking to streets, independent of the order in which the JVM decides to + * iterate over Maps and even in the presence of points that are exactly halfway between multiple candidate linking + * points. + * + * See discussion in pull request #1922, follow up issue #1934, and the original issue calling for replacement of + * the stop linker, #1305. */ -public interface StreetSplitter { - boolean linkToClosestWalkableEdge (Vertex vertex, boolean destructiveSplitting); +public class StreetSplitter { + + private static final Logger LOG = LoggerFactory.getLogger(StreetSplitter.class); + + public static final int MAX_SEARCH_RADIUS_METERS = 1000; + + private Boolean addExtraEdgesToAreas = false; + + private StreetEdgeFactory edgeFactory = new DefaultStreetEdgeFactory(); + + public static final int WARNING_DISTANCE_METERS = 20; + + // DESTRUCTIVE_SPLIT means edges are split and new edges are created (used when linking transit stops etc. during graph building) + // NON_DESTRUCTIVE_SPLIT means new temporary edges are created and no edges are deleted (Used when searching for origin/destination) + public static final boolean DESTRUCTIVE_SPLIT = true; + public static final boolean NON_DESTRUCTIVE_SPLIT = false; + + /** if there are two ways and the distances to them differ by less than this value, we link to both of them */ + public static final double DUPLICATE_WAY_EPSILON_METERS = 0.001; + + private Graph graph; + + private HashGridSpatialIndex idx; + + private SpatialIndex transitStopIndex; + + private static GeometryFactory geometryFactory = GeometryUtils.getGeometryFactory(); + + // used to keep track of splitters associated with graphs to make sure that + public static final HashSet graphsWithSplitters = new HashSet<>(); + + /** + * Construct a new StreetSplitter. + * NOTE: Only one StreetSplitter should be active on a graph at any given time and this is enforced by an error + * being thrown if more than an attempt is made to instantiate a 2nd instance of a StreetSplitter on the same graph. + * + * @param hashGridSpatialIndex If not null this index is used instead of creating new one + * @param transitStopIndex Index of all transitStops which is generated in {@link org.opentripplanner.routing.impl.StreetVertexIndexServiceImpl} + */ + public StreetSplitter( + Graph graph, + HashGridSpatialIndex hashGridSpatialIndex, + SpatialIndex transitStopIndex + ) { + this.graph = graph; + if (graphsWithSplitters.contains(graph)) { + throw new UnsupportedOperationException("Only one street splitter should be active on a graph at any given time"); + } + LOG.info("New StreetSpiltter created successfully!"); + graphsWithSplitters.add(graph); + this.transitStopIndex = transitStopIndex; + + //We build a spatial index if it isn't provided + if (hashGridSpatialIndex == null) { + // build a nice private spatial index, since we're adding and removing edges + // place a lock while doing this to ensure thread-safe writes to the index + synchronized (this) { + idx = new HashGridSpatialIndex<>(); + for (StreetEdge se : Iterables.filter(graph.getEdges(), StreetEdge.class)) { + idx.insert(se.getGeometry(), se); + } + } + } else { + idx = hashGridSpatialIndex; + } + + } + + /** + * Construct a new StreetSplitter. Be aware that only one StreetSplitter should be + * active on a graph at any given time. + * + * StreetSplitter generates index on graph and splits destructively (used in transit splitter) + * @param graph + */ + public StreetSplitter(Graph graph) { + this(graph, null, null); + } + + /** Link all relevant vertices to the street network */ + public void linkAllStationsToGraph() { + for (Vertex v : graph.getVertices()) { + if (v instanceof TransitStop || v instanceof BikeRentalStationVertex || v instanceof BikeParkVertex) + if (!linkToClosestWalkableEdge(v, DESTRUCTIVE_SPLIT)) { + if (v instanceof TransitStop) + LOG.warn(graph.addBuilderAnnotation(new StopUnlinked((TransitStop) v))); + else if (v instanceof BikeRentalStationVertex) + LOG.warn(graph.addBuilderAnnotation(new BikeRentalStationUnlinked((BikeRentalStationVertex) v))); + else if (v instanceof BikeParkVertex) + LOG.warn(graph.addBuilderAnnotation(new BikeParkUnlinked((BikeParkVertex) v))); + }; + } + } + + /** + * Link this vertex into the graph to the closest walkable edge + * @param vertex The vertex to be linked. + * @param destructiveSplitting If true splitting is permanent (Used when linking transit stops etc.) when + * false Splitting is only for duration of a request. Since they are made from + * temporary vertices and edges. + * @return + */ + public boolean linkToClosestWalkableEdge(Vertex vertex, final boolean destructiveSplitting) { + return linkToGraph(vertex, TraverseMode.WALK, null, destructiveSplitting); + } + + public boolean linkToGraph(Vertex vertex, TraverseMode traverseMode, RoutingRequest options, + final boolean destructiveSplitting) { + final TraverseModeSet traverseModeSet = new TraverseModeSet(traverseMode); + if (traverseMode == TraverseMode.BICYCLE) { + traverseModeSet.setWalk(true); + } + return linkToGraph(vertex, traverseModeSet, options, destructiveSplitting); + } + + /** + * Link this vertex into the graph + * @param vertex The vertex to be linked. + * @param traverseModeSet The traverse modes. + * @param options The routing options. + * @param destructiveSplitting If true splitting is permanent (Used when linking transit stops etc.) when + * false Splitting is only for duration of a request. Since they are made from + * temporary vertices and edges. + * @return + */ + public boolean linkToGraph(Vertex vertex, TraverseModeSet traverseModeSet, RoutingRequest options, + final boolean destructiveSplitting) { + // find nearby street edges + // TODO: we used to use an expanding-envelope search, which is more efficient in + // dense areas. but first let's see how inefficient this is. I suspect it's not too + // bad and the gains in simplicity are considerable. + final double radiusDeg = SphericalDistanceLibrary.metersToDegrees(MAX_SEARCH_RADIUS_METERS); + + Envelope env = new Envelope(vertex.getCoordinate()); + + // Perform a simple local equirectangular projection, so distances are expressed in degrees latitude. + final double xscale = Math.cos(vertex.getLat() * Math.PI / 180); + + // Expand more in the longitude direction than the latitude direction to account for converging meridians. + env.expandBy(radiusDeg / xscale, radiusDeg); + + final double DUPLICATE_WAY_EPSILON_DEGREES = SphericalDistanceLibrary.metersToDegrees(DUPLICATE_WAY_EPSILON_METERS); + + // We sort the list of candidate edges by distance to the stop + // This should remove any issues with things coming out of the spatial index in different orders + // Then we link to everything that is within DUPLICATE_WAY_EPSILON_METERS of of the best distance + // so that we capture back edges and duplicate ways. + List candidateEdges = idx.query(env).stream() + .filter(streetEdge -> streetEdge instanceof StreetEdge) + .map(edge -> (StreetEdge) edge) + // note: not filtering by radius here as distance calculation is expensive + // we do that below. + .filter(edge -> edge.canTraverse(traverseModeSet) && + // only link to edges still in the graph. + edge.getToVertex().getIncoming().contains(edge)) + .collect(Collectors.toList()); + + // Make a map of distances to all edges. + final TIntDoubleMap distances = new TIntDoubleHashMap(); + for (StreetEdge e : candidateEdges) { + distances.put(e.getId(), distance(vertex, e, xscale)); + } + + // Sort the list. + Collections.sort(candidateEdges, (o1, o2) -> { + double diff = distances.get(o1.getId()) - distances.get(o2.getId()); + // A Comparator must return an integer but our distances are doubles. + if (diff < 0) + return -1; + if (diff > 0) + return 1; + return 0; + }); + + // find the closest candidate edges + if (candidateEdges.isEmpty() || distances.get(candidateEdges.get(0).getId()) > radiusDeg) { + // We only link to stops if we are searching for origin/destination and for that we need transitStopIndex. + if (destructiveSplitting || transitStopIndex == null) { + return false; + } + LOG.debug("No street edge was found for {}", vertex); + // We search for closest stops (since this is only used in origin/destination linking if no edges were found) + // in the same way the closest edges are found. + List candidateStops = new ArrayList<>(); + transitStopIndex.query(env).forEach(candidateStop -> + candidateStops.add((TransitStop) candidateStop) + ); + + final TIntDoubleMap stopDistances = new TIntDoubleHashMap(); + + for (TransitStop t : candidateStops) { + stopDistances.put(t.getIndex(), distance(vertex, t, xscale)); + } + + Collections.sort(candidateStops, (o1, o2) -> { + double diff = stopDistances.get(o1.getIndex()) - stopDistances.get(o2.getIndex()); + if (diff < 0) { + return -1; + } + if (diff > 0) { + return 1; + } + return 0; + }); + if (candidateStops.isEmpty() || stopDistances.get(candidateStops.get(0).getIndex()) > radiusDeg) { + LOG.debug("Stops aren't close either!"); + return false; + } else { + List bestStops = Lists.newArrayList(); + // Add stops until there is a break of epsilon meters. + // we do this to enforce determinism. if there are a lot of stops that are all extremely close to each other, + // we want to be sure that we deterministically link to the same ones every time. Any hard cutoff means things can + // fall just inside or beyond the cutoff depending on floating-point operations. + int i = 0; + do { + bestStops.add(candidateStops.get(i++)); + } while (i < candidateStops.size() && + stopDistances.get(candidateStops.get(i).getIndex()) - stopDistances + .get(candidateStops.get(i - 1).getIndex()) < DUPLICATE_WAY_EPSILON_DEGREES); + + for (TransitStop stop: bestStops) { + LOG.debug("Linking vertex to stop: {}", stop.getName()); + makeTemporaryEdges((TemporaryStreetLocation)vertex, stop, destructiveSplitting); + } + return true; + } + } else { + + // find the best edges + List bestEdges = Lists.newArrayList(); + + // add edges until there is a break of epsilon meters. + // we do this to enforce determinism. if there are a lot of edges that are all extremely close to each other, + // we want to be sure that we deterministically link to the same ones every time. Any hard cutoff means things can + // fall just inside or beyond the cutoff depending on floating-point operations. + int i = 0; + do { + bestEdges.add(candidateEdges.get(i++)); + } while (i < candidateEdges.size() && + distances.get(candidateEdges.get(i).getId()) - distances + .get(candidateEdges.get(i - 1).getId()) < DUPLICATE_WAY_EPSILON_DEGREES); + + for (StreetEdge edge : bestEdges) { + linkToEdge(vertex, edge, xscale, options, destructiveSplitting); + } + + // Warn if a linkage was made, but the linkage was suspiciously long. + if (vertex instanceof TransitStop) { + double distanceDegreesLatitude = distances.get(candidateEdges.get(0).getId()); + int distanceMeters = (int)SphericalDistanceLibrary.degreesLatitudeToMeters(distanceDegreesLatitude); + if (distanceMeters > WARNING_DISTANCE_METERS) { + // Registering an annotation but not logging because tests produce thousands of these warnings. + graph.addBuilderAnnotation(new StopLinkedTooFar((TransitStop)vertex, distanceMeters)); + } + } + + return true; + } + } + + // Link to all vertices in area/platform + private void linkTransitToAreaVertices(Vertex splitterVertex, AreaEdgeList area) { + List vertices = new ArrayList<>(); + + for (AreaEdge areaEdge : area.getEdges()) { + if (!vertices.contains(areaEdge.getToVertex())) vertices.add(areaEdge.getToVertex()); + if (!vertices.contains(areaEdge.getFromVertex())) vertices.add(areaEdge.getFromVertex()); + } + + for (Vertex vertex : vertices) { + if (vertex instanceof StreetVertex && !vertex.equals(splitterVertex)) { + LineString line = geometryFactory.createLineString(new Coordinate[] { splitterVertex.getCoordinate(), vertex.getCoordinate()}); + double length = SphericalDistanceLibrary.distance(splitterVertex.getCoordinate(), + vertex.getCoordinate()); + I18NString name = new LocalizedString("", new OSMWithTags()); + + edgeFactory.createAreaEdge((IntersectionVertex) splitterVertex, (IntersectionVertex) vertex, line, name, length,StreetTraversalPermission.PEDESTRIAN_AND_BICYCLE, false, area); + edgeFactory.createAreaEdge((IntersectionVertex) vertex, (IntersectionVertex) splitterVertex, line, name, length,StreetTraversalPermission.PEDESTRIAN_AND_BICYCLE, false, area); + } + } + } + + /** + * Split the edge and link in the transit stop. + * @param vertex An object of Vertex to be linked to an edge. + * @param edge An object of StreetEdge to be linked to. + * @param xscale The longitude scale factor in Equirectangular projection. + * @param options An object of RoutingRequest + * @param destructiveSplitting If true splitting is permanent (Used when linking transit stops etc.) when + * false Splitting is only for duration of a request. Since they are made from + * temporary vertices and edges. + */ + private void linkToEdge(Vertex vertex, StreetEdge edge, double xscale, RoutingRequest options, + final boolean destructiveSplitting) { + // TODO: we've already built this line string, we should save it + LineString orig = edge.getGeometry(); + LineString transformed = equirectangularProject(orig, xscale); + LocationIndexedLine il = new LocationIndexedLine(transformed); + LinearLocation ll = il.project(new Coordinate(vertex.getLon() * xscale, vertex.getLat())); + + // if we're very close to one end of the line or the other, or endwise, don't bother to split, + // cut to the chase and link directly + // We use a really tiny epsilon here because we only want points that actually snap to exactly the same location on the + // street to use the same vertices. Otherwise the order the stops are loaded in will affect where they are snapped. + if (ll.getSegmentIndex() == 0 && ll.getSegmentFraction() < 1e-8) { + makeLinkEdges(vertex, (StreetVertex) edge.getFromVertex(), destructiveSplitting); + } + // -1 converts from count to index. Because of the fencepost problem, npoints - 1 is the "segment" + // past the last point + else if (ll.getSegmentIndex() == orig.getNumPoints() - 1) { + makeLinkEdges(vertex, (StreetVertex) edge.getToVertex(), destructiveSplitting); + } + + // nPoints - 2: -1 to correct for index vs count, -1 to account for fencepost problem + else if (ll.getSegmentIndex() == orig.getNumPoints() - 2 && ll.getSegmentFraction() > 1 - 1e-8) { + makeLinkEdges(vertex, (StreetVertex) edge.getToVertex(), destructiveSplitting); + } + + else { + + TemporaryVertex temporaryVertex = null; + boolean endVertex = false; + if (vertex instanceof TemporaryVertex) { + temporaryVertex = (TemporaryVertex) vertex; + endVertex = temporaryVertex.isEndVertex(); + + } + //This throws runtime TrivialPathException if same edge is split in origin and destination link + //It is only used in origin/destination linking since otherwise options is null + if (options != null) { + options.canSplitEdge(edge); + } + // split the edge, get the split vertex + SplitterVertex v0 = split(edge, ll, temporaryVertex != null, endVertex, destructiveSplitting); + makeLinkEdges(vertex, v0, destructiveSplitting); + + // If splitter vertex is part of area; link splittervertex to all other vertexes in area, this creates + // edges that were missed by WalkableAreaBuilder + if (edge instanceof AreaEdge && vertex instanceof TransitStop && this.addExtraEdgesToAreas) { + linkTransitToAreaVertices(v0, ((AreaEdge) edge).getArea()); + } + } + } + + + /** + * Split the street edge at the given fraction + * + * @param edge to be split + * @param ll fraction at which to split the edge + * @param temporarySplit if true this is temporary split at origin/destinations search and only temporary edges vertices are created + * @param endVertex if this is temporary edge this is true if this is end vertex otherwise it doesn't matter + * @param destructiveSplitting If true splitting is permanent (Used when linking transit stops etc.) when + * false Splitting is only for duration of a request. Since they are made from + * temporary vertices and edges. + * @return Splitter vertex with added new edges + */ + private SplitterVertex split (StreetEdge edge, LinearLocation ll, boolean temporarySplit, boolean endVertex, + final boolean destructiveSplitting) { + + LineString geometry = edge.getGeometry(); + + // create the geometries + Coordinate splitPoint = ll.getCoordinate(geometry); + + // every edge can be split exactly once, so this is a valid label + SplitterVertex v; + if (temporarySplit) { + v = new TemporarySplitterVertex("split from " + edge.getId(), splitPoint.x, splitPoint.y, edge, endVertex); + } else { + v = new SplitterVertex(graph, "split from " + edge.getId(), splitPoint.x, splitPoint.y, edge); + } + + // Split the 'edge' at 'v' in 2 new edges and connect these 2 edges to the + // existing vertices + P2 edges = edge.split(v, !temporarySplit); + + if (destructiveSplitting) { + // update indices of new edges + synchronized (this) { + // Note: Write operations are not synchronized in HashGridSpatialIndex, hence the lock. + idx.insert(edges.first.getGeometry(), edges.first); + idx.insert(edges.second.getGeometry(), edges.second); + } + // (no need to remove original edge, we filter it when it comes out of the index) + + // remove original edge from the graph + edge.getToVertex().removeIncoming(edge); + edge.getFromVertex().removeOutgoing(edge); + } + + return v; + } + + /** Make the appropriate type of link edges from a vertex */ + private void makeLinkEdges(Vertex from, StreetVertex to, final boolean destructiveSplitting) { + if (from instanceof TemporaryStreetLocation) { + makeTemporaryEdges((TemporaryStreetLocation) from, to, destructiveSplitting); + } else if (from instanceof TransitStop) { + makeTransitLinkEdges((TransitStop) from, to, destructiveSplitting); + } else if (from instanceof BikeRentalStationVertex) { + makeBikeRentalLinkEdges((BikeRentalStationVertex) from, to, destructiveSplitting); + } else if (from instanceof BikeParkVertex) { + makeBikeParkEdges((BikeParkVertex) from, to, destructiveSplitting); + } + } + + /** Make temporary edges to origin/destination vertex in origin/destination search **/ + private void makeTemporaryEdges(TemporaryStreetLocation from, Vertex to, final boolean destructiveSplitting) { + if (destructiveSplitting) { + throw new RuntimeException("Destructive splitting is used on temporary edges. Something is wrong!"); + } + if (to instanceof TemporarySplitterVertex) { + from.setWheelchairAccessible(((TemporarySplitterVertex) to).isWheelchairAccessible()); + } + if (from.isEndVertex()) { + LOG.debug("Linking end vertex to {} -> {}", to, from); + new TemporaryFreeEdge(to, from); + } else { + LOG.debug("Linking start vertex to {} -> {}", from, to); + new TemporaryFreeEdge(from, to); + } + } + + /** Make bike park edges */ + private void makeBikeParkEdges(BikeParkVertex from, StreetVertex to, final boolean destructiveSplitting) { + if (!destructiveSplitting) { + throw new RuntimeException("Bike park edges are created with non destructive splitting!"); + } + for (StreetBikeParkLink sbpl : Iterables.filter(from.getOutgoing(), StreetBikeParkLink.class)) { + if (sbpl.getToVertex() == to) + return; + } + + new StreetBikeParkLink(from, to); + new StreetBikeParkLink(to, from); + } + + /** + * Make street transit link edges, unless they already exist. + */ + private void makeTransitLinkEdges (TransitStop tstop, StreetVertex v, final boolean destructiveSplitting) { + if (!destructiveSplitting) { + throw new RuntimeException("Transitedges are created with non destructive splitting!"); + } + // ensure that the requisite edges do not already exist + // this can happen if we link to duplicate ways that have the same start/end vertices. + for (StreetTransitLink e : Iterables.filter(tstop.getOutgoing(), StreetTransitLink.class)) { + if (e.getToVertex() == v) + return; + } + + new StreetTransitLink(tstop, v, tstop.hasWheelchairEntrance()); + new StreetTransitLink(v, tstop, tstop.hasWheelchairEntrance()); + } + + /** Make link edges for bike rental */ + private void makeBikeRentalLinkEdges (BikeRentalStationVertex from, StreetVertex to, + final boolean destructiveSplitting) { + if (!destructiveSplitting) { + throw new RuntimeException("Bike rental edges are created with non destructive splitting!"); + } + for (StreetBikeRentalLink sbrl : Iterables.filter(from.getOutgoing(), StreetBikeRentalLink.class)) { + if (sbrl.getToVertex() == to) + return; + } + + new StreetBikeRentalLink(from, to); + new StreetBikeRentalLink(to, from); + } + + /** projected distance from stop to edge, in latitude degrees */ + private static double distance (Vertex tstop, StreetEdge edge, double xscale) { + // Despite the fact that we want to use a fast somewhat inaccurate projection, still use JTS library tools + // for the actual distance calculations. + LineString transformed = equirectangularProject(edge.getGeometry(), xscale); + return transformed.distance(geometryFactory.createPoint(new Coordinate(tstop.getLon() * xscale, tstop.getLat()))); + } + + /** projected distance from stop to another stop, in latitude degrees */ + private static double distance (Vertex tstop, Vertex tstop2, double xscale) { + // use JTS internal tools wherever possible + return new Coordinate(tstop.getLon() * xscale, tstop.getLat()).distance(new Coordinate(tstop2.getLon() * xscale, tstop2.getLat())); + } + + /** project this linestring to an equirectangular projection */ + private static LineString equirectangularProject(LineString geometry, double xscale) { + Coordinate[] coords = new Coordinate[geometry.getNumPoints()]; + + for (int i = 0; i < coords.length; i++) { + Coordinate c = geometry.getCoordinateN(i); + c = (Coordinate) c.clone(); + c.x *= xscale; + coords[i] = c; + } + + return geometryFactory.createLineString(coords); + } + + /** + * Used to link origin and destination points to graph non destructively. + * + * Split edges don't replace existing ones and only temporary edges and vertices are created. + * + * Will throw ThrivialPathException if origin and destination Location are on the same edge + * + * @param location + * @param options + * @param endVertex true if this is destination vertex + * @return + */ + public Vertex linkOriginDestination(GenericLocation location, RoutingRequest options, boolean endVertex) { + + if (endVertex) { + LOG.debug("Finding end vertex for {}", location); + } else { + LOG.debug("Finding start vertex for {}", location); + } + Coordinate coord = location.getCoordinate(); + //TODO: add nice name + String name; + + if (location.name == null || location.name.isEmpty()) { + if (endVertex) { + name = "Destination"; + } else { + name = "Origin"; + } + } else { + name = location.name; + } + TemporaryStreetLocation closest = new TemporaryStreetLocation(UUID.randomUUID().toString(), + coord, new NonLocalizedString(name), endVertex); + + TraverseMode nonTransitMode = TraverseMode.WALK; + //It can be null in tests + if (options != null) { + TraverseModeSet modes = options.modes; + if (modes.getCar()) + // for park and ride we will start in car mode and walk to the end vertex + if (endVertex && (options.parkAndRide || options.kissAndRide)) { + nonTransitMode = TraverseMode.WALK; + } else { + nonTransitMode = TraverseMode.CAR; + } + else if (modes.getWalk()) + nonTransitMode = TraverseMode.WALK; + else if (modes.getBicycle()) + nonTransitMode = TraverseMode.BICYCLE; + } + + if(!linkToGraph(closest, nonTransitMode, options, NON_DESTRUCTIVE_SPLIT)) { + LOG.warn("Couldn't link {}", location); + } + return closest; + + } + + public Boolean getAddExtraEdgesToAreas() { + return addExtraEdgesToAreas; + } + + public void setAddExtraEdgesToAreas(Boolean addExtraEdgesToAreas) { + this.addExtraEdgesToAreas = addExtraEdgesToAreas; + } } diff --git a/src/main/java/org/opentripplanner/graph_builder/linking/TransitToStreetNetworkModule.java b/src/main/java/org/opentripplanner/graph_builder/linking/TransitToStreetNetworkModule.java deleted file mode 100644 index c5e7bcbdc4c..00000000000 --- a/src/main/java/org/opentripplanner/graph_builder/linking/TransitToStreetNetworkModule.java +++ /dev/null @@ -1,47 +0,0 @@ -package org.opentripplanner.graph_builder.linking; - -import java.util.Arrays; -import java.util.HashMap; -import java.util.List; - -import org.opentripplanner.graph_builder.services.GraphBuilderModule; -import org.opentripplanner.routing.graph.Graph; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -/** - * {@link org.opentripplanner.graph_builder.services.GraphBuilderModule} plugin that links up the stops of a transit network to a street network. - * Should be called after both the transit network and street network are loaded. - */ -public class TransitToStreetNetworkModule implements GraphBuilderModule { - - private static final Logger LOG = LoggerFactory.getLogger(TransitToStreetNetworkModule.class); - - public List provides() { - return Arrays.asList("street to transit", "linking"); - } - - public List getPrerequisites() { - return Arrays.asList("streets"); // why not "transit" ? - } - - @Override - public void buildGraph(Graph graph, HashMap, Object> extra) { - LOG.info("Linking transit stops to streets..."); - // split streets - //NetworkLinker linker = new NetworkLinker(graph, extra); - //linker.createLinkage(); - - SimpleStreetSplitter splitter = new SimpleStreetSplitter(graph); - splitter.linkAllStationsToGraph(); - - // don't split streets - //SampleStopLinker linker = new SampleStopLinker(graph); - //linker.link(true); - } - - @Override - public void checkInputs() { - //no inputs - } -} diff --git a/src/main/java/org/opentripplanner/graph_builder/module/PruneFloatingIslands.java b/src/main/java/org/opentripplanner/graph_builder/module/PruneFloatingIslands.java index 4b48308755e..691edf1a05d 100644 --- a/src/main/java/org/opentripplanner/graph_builder/module/PruneFloatingIslands.java +++ b/src/main/java/org/opentripplanner/graph_builder/module/PruneFloatingIslands.java @@ -1,16 +1,15 @@ package org.opentripplanner.graph_builder.module; +import org.opentripplanner.common.StreetUtils; +import org.opentripplanner.graph_builder.services.GraphBuilderModule; +import org.opentripplanner.routing.graph.Graph; +import org.slf4j.LoggerFactory; + import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.List; -import org.opentripplanner.common.StreetUtils; -import org.opentripplanner.graph_builder.linking.TransitToStreetNetworkModule; -import org.opentripplanner.graph_builder.services.GraphBuilderModule; -import org.opentripplanner.routing.graph.Graph; -import org.slf4j.*; - /** * this module is part of the {@link org.opentripplanner.graph_builder.services.GraphBuilderModule} process. it design to remove small isolated * islands form the graph. Islands are created when there is no connectivity in the map, island @@ -40,8 +39,6 @@ public class PruneFloatingIslands implements GraphBuilderModule { */ private String islandLogFile; - private StreetLinkerModule transitToStreetNetwork; - public List provides() { return Collections.emptyList(); } @@ -59,14 +56,12 @@ public List getPrerequisites() { public void buildGraph(Graph graph, HashMap, Object> extra) { LOG.info("Pruning isolated islands in street network"); - StreetUtils.pruneFloatingIslands(graph, pruningThresholdIslandWithoutStops, - pruningThresholdIslandWithStops, islandLogFile); - if (transitToStreetNetwork == null) { - LOG.debug("TransitToStreetNetworkGraphBuilder was not provided to PruneFloatingIslands. Not attempting to reconnect stops."); - } else { - //reconnect stops on small islands (that removed) - transitToStreetNetwork.buildGraph(graph,extra); - } + StreetUtils.pruneFloatingIslands( + graph, + pruningThresholdIslandWithoutStops, + pruningThresholdIslandWithStops, + islandLogFile + ); LOG.debug("Done pruning isolated islands"); } diff --git a/src/main/java/org/opentripplanner/graph_builder/module/StreetLinkerModule.java b/src/main/java/org/opentripplanner/graph_builder/module/StreetLinkerModule.java index f3cf1061a95..4b891de1130 100644 --- a/src/main/java/org/opentripplanner/graph_builder/module/StreetLinkerModule.java +++ b/src/main/java/org/opentripplanner/graph_builder/module/StreetLinkerModule.java @@ -1,15 +1,16 @@ package org.opentripplanner.graph_builder.module; -import java.util.Arrays; -import java.util.HashMap; -import java.util.List; - -import org.opentripplanner.graph_builder.linking.SimpleStreetSplitter; +import org.opentripplanner.graph_builder.linking.StreetSplitter; import org.opentripplanner.graph_builder.services.GraphBuilderModule; import org.opentripplanner.routing.graph.Graph; +import org.opentripplanner.routing.impl.DefaultStreetVertexIndexFactory; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.util.Arrays; +import java.util.HashMap; +import java.util.List; + /** * {@link org.opentripplanner.graph_builder.services.GraphBuilderModule} plugin that links various objects * in the graph to the street network. It should be run after both the transit network and street network are loaded. @@ -44,7 +45,10 @@ public List getPrerequisites() { public void buildGraph(Graph graph, HashMap, Object> extra) { if(graph.hasStreets) { LOG.info("Linking transit stops, bike rental stations, bike parking areas, and park-and-rides to graph . . ."); - SimpleStreetSplitter linker = new SimpleStreetSplitter(graph); + if (graph.streetIndex == null) { + graph.index(new DefaultStreetVertexIndexFactory()); + } + StreetSplitter linker = graph.streetIndex.getStreetSplitter(); linker.setAddExtraEdgesToAreas(this.addExtraEdgesToAreas); linker.linkAllStationsToGraph(); } diff --git a/src/main/java/org/opentripplanner/routing/core/RoutingRequest.java b/src/main/java/org/opentripplanner/routing/core/RoutingRequest.java index d790b0377e2..9e4692188fe 100644 --- a/src/main/java/org/opentripplanner/routing/core/RoutingRequest.java +++ b/src/main/java/org/opentripplanner/routing/core/RoutingRequest.java @@ -1,6 +1,7 @@ package org.opentripplanner.routing.core; import com.google.common.base.Objects; +import org.opentripplanner.graph_builder.linking.StreetSplitter; import org.opentripplanner.model.FeedScopedId; import org.opentripplanner.model.Route; import org.opentripplanner.api.parameter.QualifiedModeSet; @@ -24,7 +25,6 @@ import java.io.Serializable; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.Comparator; @@ -1469,7 +1469,7 @@ public ShortestPathTree getNewShortestPathTree() { * * But throws TrivialPathException if same edge is split in origin/destination search. * - * used in {@link org.opentripplanner.graph_builder.linking.SimpleStreetSplitter} in {@link org.opentripplanner.graph_builder.linking.SimpleStreetSplitter#link(Vertex, StreetEdge, double, RoutingRequest)} + * used in {@link StreetSplitter} in {@link StreetSplitter#link(Vertex, StreetEdge, double, RoutingRequest)} * @param edge */ public void canSplitEdge(StreetEdge edge) { diff --git a/src/main/java/org/opentripplanner/routing/graph/Graph.java b/src/main/java/org/opentripplanner/routing/graph/Graph.java index 6cf51156215..4ad911a6cc9 100644 --- a/src/main/java/org/opentripplanner/routing/graph/Graph.java +++ b/src/main/java/org/opentripplanner/routing/graph/Graph.java @@ -682,6 +682,11 @@ public static Graph load(File file) throws IOException { * TODO: do we really need a factory for different street vertex indexes? */ public void index (StreetVertexIndexFactory indexFactory) { + if (streetIndex != null) { + throw new UnsupportedOperationException( + "A streetIndex has already been defined. Can't you just reuse the existing one?" + ); + } streetIndex = indexFactory.newIndex(this); LOG.debug("street index built."); LOG.debug("Rebuilding edge and vertex indices."); diff --git a/src/main/java/org/opentripplanner/routing/impl/StreetVertexIndexServiceImpl.java b/src/main/java/org/opentripplanner/routing/impl/StreetVertexIndexServiceImpl.java index 3301246ce65..5685cf6bbfa 100644 --- a/src/main/java/org/opentripplanner/routing/impl/StreetVertexIndexServiceImpl.java +++ b/src/main/java/org/opentripplanner/routing/impl/StreetVertexIndexServiceImpl.java @@ -4,8 +4,6 @@ import org.locationtech.jts.geom.Coordinate; import org.locationtech.jts.geom.Envelope; import org.locationtech.jts.geom.LineString; -import org.locationtech.jts.index.SpatialIndex; -import org.locationtech.jts.index.strtree.STRtree; import org.opentripplanner.analyst.core.Sample; import org.opentripplanner.analyst.request.SampleFactory; import org.opentripplanner.common.geometry.GeometryUtils; @@ -13,16 +11,19 @@ import org.opentripplanner.common.geometry.SphericalDistanceLibrary; import org.opentripplanner.common.model.GenericLocation; import org.opentripplanner.common.model.P2; -import org.opentripplanner.graph_builder.linking.SimpleStreetSplitter; import org.opentripplanner.graph_builder.linking.StreetSplitter; import org.opentripplanner.routing.core.RoutingRequest; -import org.opentripplanner.routing.edgetype.*; +import org.opentripplanner.routing.edgetype.PatternEdge; +import org.opentripplanner.routing.edgetype.SampleEdge; +import org.opentripplanner.routing.edgetype.SimpleTransfer; +import org.opentripplanner.routing.edgetype.StreetEdge; +import org.opentripplanner.routing.edgetype.TemporaryFreeEdge; +import org.opentripplanner.routing.edgetype.TemporaryPartialStreetEdge; import org.opentripplanner.routing.graph.Edge; import org.opentripplanner.routing.graph.Graph; import org.opentripplanner.routing.graph.Vertex; import org.opentripplanner.routing.location.TemporaryStreetLocation; import org.opentripplanner.routing.services.StreetVertexIndexService; -import org.opentripplanner.routing.util.ElevationUtils; import org.opentripplanner.routing.vertextype.SampleVertex; import org.opentripplanner.routing.vertextype.StreetVertex; import org.opentripplanner.routing.vertextype.TransitStop; @@ -30,7 +31,10 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.util.*; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Iterator; +import java.util.List; /** * Indexes all edges and transit vertices of the graph spatially. Has a variety of query methods @@ -47,9 +51,9 @@ public class StreetVertexIndexServiceImpl implements StreetVertexIndexService { /** * Contains only instances of {@link StreetEdge} */ - private SpatialIndex edgeTree; - private SpatialIndex transitStopTree; - private SpatialIndex verticesTree; + private HashGridSpatialIndex edgeTree; + private HashGridSpatialIndex transitStopTree; + private HashGridSpatialIndex verticesTree; // private static final double SEARCH_RADIUS_M = 100; // meters // private static final double SEARCH_RADIUS_DEG = DistanceLibrary.metersToDegrees(SEARCH_RADIUS_M); @@ -70,33 +74,15 @@ public class StreetVertexIndexServiceImpl implements StreetVertexIndexService { static final Logger LOG = LoggerFactory.getLogger(StreetVertexIndexServiceImpl.class); - private SimpleStreetSplitter simpleStreetSplitter; + private StreetSplitter streetSplitter; public StreetVertexIndexServiceImpl(Graph graph) { - this(graph, true); - } - - public StreetVertexIndexServiceImpl(Graph graph, boolean hashGrid) { this.graph = graph; - if (hashGrid) { - edgeTree = new HashGridSpatialIndex<>(); - transitStopTree = new HashGridSpatialIndex<>(); - verticesTree = new HashGridSpatialIndex<>(); - } else { - edgeTree = new STRtree(); - transitStopTree = new STRtree(); - verticesTree = new STRtree(); - } + edgeTree = new HashGridSpatialIndex<>(); + transitStopTree = new HashGridSpatialIndex<>(); + verticesTree = new HashGridSpatialIndex<>(); postSetup(); - if (!hashGrid) { - ((STRtree) edgeTree).build(); - ((STRtree) transitStopTree).build(); - simpleStreetSplitter = new SimpleStreetSplitter(this.graph, null, null); - } else { - simpleStreetSplitter = new SimpleStreetSplitter(this.graph, - (HashGridSpatialIndex) edgeTree, transitStopTree); - } - + streetSplitter = new StreetSplitter(graph, edgeTree, transitStopTree); } /** @@ -212,11 +198,7 @@ private void postSetup() { if (geometry == null) { continue; } - Envelope env = geometry.getEnvelopeInternal(); - if (edgeTree instanceof HashGridSpatialIndex) - ((HashGridSpatialIndex)edgeTree).insert(geometry, e); - else - edgeTree.insert(env, e); + edgeTree.insert(geometry, e); } if (v instanceof TransitStop) { Envelope env = new Envelope(v.getCoordinate()); @@ -317,7 +299,7 @@ public Vertex getVertexForLocation(GenericLocation loc, RoutingRequest options, boolean endVertex) { Coordinate c = loc.getCoordinate(); if (c != null) { - return simpleStreetSplitter.linkOriginDestination(loc, options, endVertex); + return streetSplitter.linkOriginDestination(loc, options, endVertex); } // No Coordinate available. @@ -370,6 +352,6 @@ public Vertex getSampleVertexAt(Coordinate coordinate, boolean dest) { @Override public StreetSplitter getStreetSplitter() { - return simpleStreetSplitter; + return streetSplitter; } } diff --git a/src/main/java/org/opentripplanner/updater/bike_park/BikeParkUpdater.java b/src/main/java/org/opentripplanner/updater/bike_park/BikeParkUpdater.java index 058732e8cb4..39783aade14 100644 --- a/src/main/java/org/opentripplanner/updater/bike_park/BikeParkUpdater.java +++ b/src/main/java/org/opentripplanner/updater/bike_park/BikeParkUpdater.java @@ -7,7 +7,6 @@ import java.util.Map; import java.util.Map.Entry; import java.util.Set; -import java.util.concurrent.ExecutionException; import com.fasterxml.jackson.databind.JsonNode; import org.opentripplanner.graph_builder.linking.StreetSplitter; @@ -23,7 +22,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import static org.opentripplanner.graph_builder.linking.SimpleStreetSplitter.*; +import static org.opentripplanner.graph_builder.linking.StreetSplitter.*; /** * Graph updater that dynamically sets availability information on bike parking lots. diff --git a/src/main/java/org/opentripplanner/updater/bike_rental/BikeRentalUpdater.java b/src/main/java/org/opentripplanner/updater/bike_rental/BikeRentalUpdater.java index 8ccf22778f6..e33a6cf91a7 100644 --- a/src/main/java/org/opentripplanner/updater/bike_rental/BikeRentalUpdater.java +++ b/src/main/java/org/opentripplanner/updater/bike_rental/BikeRentalUpdater.java @@ -25,7 +25,7 @@ import java.util.Set; import java.util.concurrent.ExecutionException; -import static org.opentripplanner.graph_builder.linking.SimpleStreetSplitter.DESTRUCTIVE_SPLIT; +import static org.opentripplanner.graph_builder.linking.StreetSplitter.DESTRUCTIVE_SPLIT; /** * Dynamic bike-rental station updater which updates the Graph with bike rental stations from one BikeRentalDataSource. diff --git a/src/test/java/org/opentripplanner/ConstantsForTests.java b/src/test/java/org/opentripplanner/ConstantsForTests.java index 64667c28c03..67bd1983ac7 100644 --- a/src/test/java/org/opentripplanner/ConstantsForTests.java +++ b/src/test/java/org/opentripplanner/ConstantsForTests.java @@ -1,22 +1,21 @@ package org.opentripplanner; -import java.io.File; -import java.io.IOException; -import java.net.URLDecoder; -import java.util.HashMap; - -import org.opentripplanner.model.calendar.CalendarServiceData; import org.opentripplanner.graph_builder.module.DirectTransferGenerator; import org.opentripplanner.graph_builder.module.StreetLinkerModule; import org.opentripplanner.graph_builder.module.osm.DefaultWayPropertySetSource; import org.opentripplanner.graph_builder.module.osm.OpenStreetMapModule; import org.opentripplanner.gtfs.GtfsContext; import org.opentripplanner.gtfs.GtfsLibrary; +import org.opentripplanner.model.calendar.CalendarServiceData; import org.opentripplanner.openstreetmap.impl.AnyFileBasedOpenStreetMapProviderImpl; import org.opentripplanner.routing.edgetype.factory.PatternHopFactory; import org.opentripplanner.routing.edgetype.factory.TransferGraphLinker; import org.opentripplanner.routing.graph.Graph; -import org.opentripplanner.routing.impl.DefaultStreetVertexIndexFactory; + +import java.io.File; +import java.io.IOException; +import java.net.URLDecoder; +import java.util.HashMap; import static org.opentripplanner.calendar.impl.CalendarServiceDataFactoryImpl.createCalendarServiceData; @@ -131,8 +130,6 @@ private Graph getGraph(String osmFile, String gtfsFile) { new StreetLinkerModule().buildGraph(g, new HashMap<>()); - g.index(new DefaultStreetVertexIndexFactory()); - return g; } catch(Exception ex) { ex.printStackTrace(); diff --git a/src/test/java/org/opentripplanner/analyst/InitialStopsTest.java b/src/test/java/org/opentripplanner/analyst/InitialStopsTest.java index 8b4d6465c12..6bd9ee3700c 100644 --- a/src/test/java/org/opentripplanner/analyst/InitialStopsTest.java +++ b/src/test/java/org/opentripplanner/analyst/InitialStopsTest.java @@ -12,9 +12,11 @@ import org.opentripplanner.profile.RepeatedRaptorProfileRouter; import org.opentripplanner.routing.core.TraverseModeSet; import org.opentripplanner.routing.graph.Graph; -import org.opentripplanner.routing.impl.DefaultStreetVertexIndexFactory; -import static org.opentripplanner.graph_builder.module.FakeGraph.*; +import static org.opentripplanner.graph_builder.module.FakeGraph.addRegularStopGrid; +import static org.opentripplanner.graph_builder.module.FakeGraph.addTransitMultipleLines; +import static org.opentripplanner.graph_builder.module.FakeGraph.buildGraphNoTransit; +import static org.opentripplanner.graph_builder.module.FakeGraph.indexGraphAndLinkStations; /** * Test the code that finds initial transit stops. @@ -35,8 +37,7 @@ public void testInitialStopBikeSpeedIncrease () throws Exception { Graph g = buildGraphNoTransit(); addRegularStopGrid(g); addTransitMultipleLines(g); - link(g); - g.index(new DefaultStreetVertexIndexFactory()); + indexGraphAndLinkStations(g); ProfileRequest req = new ProfileRequest(); req.fromLon = req.toLon = -83.0118; @@ -94,8 +95,7 @@ public void testInitialStopWalkSpeedIncrease () throws Exception { Graph g = buildGraphNoTransit(); addRegularStopGrid(g); addTransitMultipleLines(g); - link(g); - g.index(new DefaultStreetVertexIndexFactory()); + indexGraphAndLinkStations(g); ProfileRequest req = new ProfileRequest(); req.fromLon = req.toLon = -83.0118; diff --git a/src/test/java/org/opentripplanner/graph_builder/linking/LinkStopToPlatformTest.java b/src/test/java/org/opentripplanner/graph_builder/linking/LinkStopToPlatformTest.java index 25f863269e0..a7f2dd7d6ad 100644 --- a/src/test/java/org/opentripplanner/graph_builder/linking/LinkStopToPlatformTest.java +++ b/src/test/java/org/opentripplanner/graph_builder/linking/LinkStopToPlatformTest.java @@ -72,7 +72,7 @@ public void before() { */ @Test public void testLinkStopWithoutExtraEdges() { - SimpleStreetSplitter splitter = new SimpleStreetSplitter(graph); + StreetSplitter splitter = new StreetSplitter(graph); splitter.linkAllStationsToGraph(); assertEquals(16, graph.getEdges().size()); @@ -80,7 +80,7 @@ public void testLinkStopWithoutExtraEdges() { @Test public void testLinkStopWithExtraEdges() { - SimpleStreetSplitter splitter = new SimpleStreetSplitter(graph); + StreetSplitter splitter = new StreetSplitter(graph); splitter.setAddExtraEdgesToAreas(true); splitter.linkAllStationsToGraph(); diff --git a/src/test/java/org/opentripplanner/graph_builder/linking/SimpleStreetSplitterTest.java b/src/test/java/org/opentripplanner/graph_builder/linking/StreetSplitterTest.java similarity index 89% rename from src/test/java/org/opentripplanner/graph_builder/linking/SimpleStreetSplitterTest.java rename to src/test/java/org/opentripplanner/graph_builder/linking/StreetSplitterTest.java index ff732f08e4e..7b7768fa861 100644 --- a/src/test/java/org/opentripplanner/graph_builder/linking/SimpleStreetSplitterTest.java +++ b/src/test/java/org/opentripplanner/graph_builder/linking/StreetSplitterTest.java @@ -28,18 +28,18 @@ import static org.mockito.Matchers.eq; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; -import static org.opentripplanner.graph_builder.linking.SimpleStreetSplitter.DESTRUCTIVE_SPLIT; +import static org.opentripplanner.graph_builder.linking.StreetSplitter.DESTRUCTIVE_SPLIT; -public class SimpleStreetSplitterTest { +public class StreetSplitterTest { private final GeometryFactory gf = GeometryUtils.getGeometryFactory(); - private SimpleStreetSplitter spySimpleStreetSplitter; + private StreetSplitter spyStreetSplitter; @Before public void buildSpy(){ Graph graph = new Graph(); - SimpleStreetSplitter simpleStreetSplitter = new SimpleStreetSplitter(graph, null, null); - spySimpleStreetSplitter = spy(simpleStreetSplitter); + StreetSplitter streetSplitter = new StreetSplitter(graph, null, null); + spyStreetSplitter = spy(streetSplitter); } /** @@ -53,8 +53,8 @@ public void testFindEndVertexForParkAndRide(){ routingRequest.setMode(TraverseMode.CAR); routingRequest.parkAndRide = true; - spySimpleStreetSplitter.linkOriginDestination(genericLocation, routingRequest, true); - verify(spySimpleStreetSplitter).linkToGraph(any(Vertex.class), eq(TraverseMode.WALK), eq(routingRequest), eq(false)); + spyStreetSplitter.linkOriginDestination(genericLocation, routingRequest, true); + verify(spyStreetSplitter).linkToGraph(any(Vertex.class), eq(TraverseMode.WALK), eq(routingRequest), eq(false)); } /** diff --git a/src/test/java/org/opentripplanner/graph_builder/module/FakeGraph.java b/src/test/java/org/opentripplanner/graph_builder/module/FakeGraph.java index 8568b8c9d8e..a65fb593230 100644 --- a/src/test/java/org/opentripplanner/graph_builder/module/FakeGraph.java +++ b/src/test/java/org/opentripplanner/graph_builder/module/FakeGraph.java @@ -1,13 +1,14 @@ package org.opentripplanner.graph_builder.module; -import org.opentripplanner.model.FeedScopedId; -import org.opentripplanner.model.Stop; -import org.opentripplanner.graph_builder.linking.SimpleStreetSplitter; +import org.opentripplanner.graph_builder.linking.StreetSplitter; import org.opentripplanner.graph_builder.model.GtfsBundle; import org.opentripplanner.graph_builder.module.osm.DefaultWayPropertySetSource; import org.opentripplanner.graph_builder.module.osm.OpenStreetMapModule; +import org.opentripplanner.model.FeedScopedId; +import org.opentripplanner.model.Stop; import org.opentripplanner.openstreetmap.impl.AnyFileBasedOpenStreetMapProviderImpl; import org.opentripplanner.routing.graph.Graph; +import org.opentripplanner.routing.impl.DefaultStreetVertexIndexFactory; import org.opentripplanner.routing.vertextype.TransitStop; import java.io.File; @@ -130,9 +131,13 @@ public static void addExtraStops (Graph g) { } } - /** link the stops in the graph */ - public static void link (Graph g) { - SimpleStreetSplitter linker = new SimpleStreetSplitter(g); + /** + * Index the graph and then link all stations in the graph. + */ + public static void indexGraphAndLinkStations (Graph g) { + // creating a new DefaultStreetVertexIndexFactory results in setting the streetIndex on the graph. + g.index(new DefaultStreetVertexIndexFactory()); + StreetSplitter linker = (StreetSplitter) g.streetIndex.getStreetSplitter(); linker.linkAllStationsToGraph(); } diff --git a/src/test/java/org/opentripplanner/graph_builder/module/linking/LinkingTest.java b/src/test/java/org/opentripplanner/graph_builder/module/linking/LinkingTest.java index 9fe98b4c202..fe13145bd79 100644 --- a/src/test/java/org/opentripplanner/graph_builder/module/linking/LinkingTest.java +++ b/src/test/java/org/opentripplanner/graph_builder/module/linking/LinkingTest.java @@ -3,14 +3,14 @@ import com.google.common.base.Predicate; import com.google.common.collect.Collections2; import com.google.common.collect.Iterables; -import org.locationtech.jts.geom.Coordinate; -import org.locationtech.jts.geom.GeometryFactory; -import org.locationtech.jts.geom.LineString; import gnu.trove.iterator.TObjectIntIterator; import gnu.trove.map.TObjectIntMap; import gnu.trove.map.hash.TObjectIntHashMap; import jersey.repackaged.com.google.common.collect.Maps; import org.junit.Test; +import org.locationtech.jts.geom.Coordinate; +import org.locationtech.jts.geom.GeometryFactory; +import org.locationtech.jts.geom.LineString; import org.opentripplanner.common.geometry.GeometryUtils; import org.opentripplanner.common.geometry.SphericalDistanceLibrary; import org.opentripplanner.common.model.P2; @@ -21,7 +21,6 @@ import org.opentripplanner.routing.graph.Edge; import org.opentripplanner.routing.graph.Graph; import org.opentripplanner.routing.graph.Vertex; -import org.opentripplanner.routing.impl.DefaultStreetVertexIndexFactory; import org.opentripplanner.routing.vertextype.IntersectionVertex; import org.opentripplanner.routing.vertextype.SplitterVertex; import org.opentripplanner.routing.vertextype.StreetVertex; @@ -32,8 +31,13 @@ import java.util.Map; import java.util.Map.Entry; -import static org.junit.Assert.*; -import static org.opentripplanner.graph_builder.module.FakeGraph.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.opentripplanner.graph_builder.module.FakeGraph.addExtraStops; +import static org.opentripplanner.graph_builder.module.FakeGraph.addRegularStopGrid; +import static org.opentripplanner.graph_builder.module.FakeGraph.buildGraphNoTransit; +import static org.opentripplanner.graph_builder.module.FakeGraph.indexGraphAndLinkStations; public class LinkingTest { /** maximum difference in walk distance, in meters, that is acceptable between the graphs */ @@ -90,12 +94,12 @@ public void testStopsLinkedIdentically () throws UnsupportedEncodingException { // build the graph without the added stops Graph g1 = buildGraphNoTransit(); addRegularStopGrid(g1); - link(g1); + indexGraphAndLinkStations(g1); Graph g2 = buildGraphNoTransit(); addExtraStops(g2); addRegularStopGrid(g2); - link(g2); + indexGraphAndLinkStations(g2); // compare the linkages for (TransitStop ts : Iterables.filter(g1.getVertices(), TransitStop.class)) { @@ -117,12 +121,6 @@ public void testStopsLinkedIdentically () throws UnsupportedEncodingException { } // compare the stop tree caches - g1.index(new DefaultStreetVertexIndexFactory()); - g2.index(new DefaultStreetVertexIndexFactory()); - - g1.rebuildVertexAndEdgeIndices(); - g2.rebuildVertexAndEdgeIndices(); - StopTreeCache s1 = g1.index.getStopTreeCache(); StopTreeCache s2 = g2.index.getStopTreeCache(); diff --git a/src/test/java/org/opentripplanner/graph_builder/module/osm/TestIntermediatePlaces.java b/src/test/java/org/opentripplanner/graph_builder/module/osm/TestIntermediatePlaces.java index fc117254e58..41f0514baef 100644 --- a/src/test/java/org/opentripplanner/graph_builder/module/osm/TestIntermediatePlaces.java +++ b/src/test/java/org/opentripplanner/graph_builder/module/osm/TestIntermediatePlaces.java @@ -12,7 +12,6 @@ import org.opentripplanner.graph_builder.module.FakeGraph; import org.opentripplanner.routing.core.RoutingRequest; import org.opentripplanner.routing.graph.Graph; -import org.opentripplanner.routing.impl.DefaultStreetVertexIndexFactory; import org.opentripplanner.routing.impl.GraphPathFinder; import org.opentripplanner.routing.impl.MemoryGraphSource; import org.opentripplanner.routing.services.GraphService; @@ -21,12 +20,14 @@ import org.opentripplanner.standalone.OTPServer; import org.opentripplanner.standalone.Router; -import java.io.UnsupportedEncodingException; import java.util.Calendar; import java.util.List; import java.util.TimeZone; -import static org.junit.Assert.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; /** * Tests for planning with intermediate places @@ -46,8 +47,7 @@ public class TestIntermediatePlaces { try { Graph graph = FakeGraph.buildGraphNoTransit(); FakeGraph.addPerpendicularRoutes(graph); - FakeGraph.link(graph); - graph.index(new DefaultStreetVertexIndexFactory()); + FakeGraph.indexGraphAndLinkStations(graph); OTPServer otpServer = new OTPServer(new CommandLineParameters(), new GraphService()); otpServer.getGraphService().registerGraph("A", new MemoryGraphSource("A", graph)); diff --git a/src/test/java/org/opentripplanner/routing/graph/GraphSerializationTest.java b/src/test/java/org/opentripplanner/routing/graph/GraphSerializationTest.java index 8b1f929b347..23ce9bfe0a0 100644 --- a/src/test/java/org/opentripplanner/routing/graph/GraphSerializationTest.java +++ b/src/test/java/org/opentripplanner/routing/graph/GraphSerializationTest.java @@ -44,7 +44,6 @@ public void testRoundTrip () throws Exception { List transitVertices = originalGraph.getVertices().stream() .filter(v -> v instanceof TransitStation).collect(Collectors.toList()); transitVertices.forEach(originalGraph::remove); - originalGraph.index(new DefaultStreetVertexIndexFactory()); // The cached timezone in the graph is transient and lazy-initialized. // Previous tests may have caused a timezone to be cached. originalGraph.clearTimeZone(); From cf74d8132a221812ce2ed114a13c3faffd7cd282 Mon Sep 17 00:00:00 2001 From: evansiroky Date: Fri, 10 May 2019 17:07:04 -0700 Subject: [PATCH 08/12] Add more Javadoc to clarify splitting types --- .../graph_builder/linking/StreetSplitter.java | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/opentripplanner/graph_builder/linking/StreetSplitter.java b/src/main/java/org/opentripplanner/graph_builder/linking/StreetSplitter.java index 2938a2069c2..ffd72efa103 100644 --- a/src/main/java/org/opentripplanner/graph_builder/linking/StreetSplitter.java +++ b/src/main/java/org/opentripplanner/graph_builder/linking/StreetSplitter.java @@ -84,9 +84,19 @@ public class StreetSplitter { public static final int WARNING_DISTANCE_METERS = 20; - // DESTRUCTIVE_SPLIT means edges are split and new edges are created (used when linking transit stops etc. during graph building) - // NON_DESTRUCTIVE_SPLIT means new temporary edges are created and no edges are deleted (Used when searching for origin/destination) + /** + * DESTRUCTIVE_SPLIT means edges are split and new edges and vertices are created. This should occur when linking + * transit stops etc. during graph building or inserting floating bicycles in a bikeshare system in realtime into + * the graph. When these splits occur, a spatial index of all edges should be kept updated, otherwise there will + * appear to be snapping issues as observed in https://github.com/opentripplanner/OpenTripPlanner/issues/2758. + */ public static final boolean DESTRUCTIVE_SPLIT = true; + + /** + * NON_DESTRUCTIVE_SPLIT means new temporary edges and vertices are created and no edges are deleted. This is used + * in individual requests when temporarily inserting the origin/destination into the graph. The spatial index of + * street edges should not be updated to include these temporary edges. + */ public static final boolean NON_DESTRUCTIVE_SPLIT = false; /** if there are two ways and the distances to them differ by less than this value, we link to both of them */ @@ -188,7 +198,9 @@ public boolean linkToGraph(Vertex vertex, TraverseMode traverseMode, RoutingRequ } /** - * Link this vertex into the graph + * Link this vertex into the graph (ie, connect the vertex to other vertices in the graph by creating one or more + * new edges/vertexes which are then connected to the other vertices/edges of the graph. + * * @param vertex The vertex to be linked. * @param traverseModeSet The traverse modes. * @param options The routing options. From 43ad34adbd1ea5b3b21de9fa4d5d70b186e4022b Mon Sep 17 00:00:00 2001 From: evansiroky Date: Fri, 10 May 2019 22:19:47 -0700 Subject: [PATCH 09/12] Make sure all StreetVertexIndexServices are created during graph indexing --- .../module/NearbyStopFinder.java | 23 ++++++++----------- .../module/TransitToTaggedStopsModule.java | 10 +++++--- .../impl/StreetVertexIndexServiceImpl.java | 17 ++++++++------ .../osm/TestOpenStreetMapGraphBuilder.java | 21 ++++++++--------- .../routing/core/StateEditorTest.java | 4 ++-- 5 files changed, 38 insertions(+), 37 deletions(-) diff --git a/src/main/java/org/opentripplanner/graph_builder/module/NearbyStopFinder.java b/src/main/java/org/opentripplanner/graph_builder/module/NearbyStopFinder.java index f6b87a1393a..203e1b366bd 100644 --- a/src/main/java/org/opentripplanner/graph_builder/module/NearbyStopFinder.java +++ b/src/main/java/org/opentripplanner/graph_builder/module/NearbyStopFinder.java @@ -5,7 +5,6 @@ import org.locationtech.jts.geom.Coordinate; import org.locationtech.jts.geom.GeometryFactory; import org.locationtech.jts.geom.LineString; - import org.opentripplanner.api.resource.CoordinateArrayListSequence; import org.opentripplanner.api.resource.SimpleIsochrone; import org.opentripplanner.common.geometry.GeometryUtils; @@ -20,7 +19,7 @@ import org.opentripplanner.routing.graph.Edge; import org.opentripplanner.routing.graph.Graph; import org.opentripplanner.routing.graph.Vertex; -import org.opentripplanner.routing.impl.StreetVertexIndexServiceImpl; +import org.opentripplanner.routing.impl.DefaultStreetVertexIndexFactory; import org.opentripplanner.routing.services.StreetVertexIndexService; import org.opentripplanner.routing.spt.GraphPath; import org.opentripplanner.routing.spt.ShortestPathTree; @@ -28,7 +27,9 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.util.*; +import java.util.ArrayList; +import java.util.List; +import java.util.Set; /** * These library functions are used by the streetless and streetful stop linkers, and in profile transfer generation. @@ -56,16 +57,8 @@ public class NearbyStopFinder { * network or straight line distance based on the presence of OSM street data in the graph. */ public NearbyStopFinder(Graph graph, double radiusMeters) { - this (graph, radiusMeters, graph.hasStreets); - } - - /** - * Construct a NearbyStopFinder for the given graph and search radius. - * @param useStreets if true, search via the street network instead of using straight-line distance. - */ - public NearbyStopFinder(Graph graph, double radiusMeters, boolean useStreets) { this.graph = graph; - this.useStreets = useStreets; + this.useStreets = graph.hasStreets; this.radiusMeters = radiusMeters; if (useStreets) { earliestArrivalSearch = new EarliestArrivalSearch(); @@ -74,8 +67,10 @@ public NearbyStopFinder(Graph graph, double radiusMeters, boolean useStreets) { // but we don't have much of a choice here. Use the default walking speed to convert. earliestArrivalSearch.maxDuration = (int) (radiusMeters / new RoutingRequest().walkSpeed); } else { - // FIXME use the vertex index already in the graph if it exists. - streetIndex = new StreetVertexIndexServiceImpl(graph); + if (graph.streetIndex == null) { + graph.index(new DefaultStreetVertexIndexFactory()); + } + streetIndex = graph.streetIndex; } } diff --git a/src/main/java/org/opentripplanner/graph_builder/module/TransitToTaggedStopsModule.java b/src/main/java/org/opentripplanner/graph_builder/module/TransitToTaggedStopsModule.java index 3d0633f2fad..6b86e7390d8 100644 --- a/src/main/java/org/opentripplanner/graph_builder/module/TransitToTaggedStopsModule.java +++ b/src/main/java/org/opentripplanner/graph_builder/module/TransitToTaggedStopsModule.java @@ -8,7 +8,8 @@ import org.opentripplanner.routing.graph.Edge; import org.opentripplanner.routing.graph.Graph; import org.opentripplanner.routing.graph.Vertex; -import org.opentripplanner.routing.impl.StreetVertexIndexServiceImpl; +import org.opentripplanner.routing.impl.DefaultStreetVertexIndexFactory; +import org.opentripplanner.routing.services.StreetVertexIndexService; import org.opentripplanner.routing.vertextype.TransitStop; import org.opentripplanner.routing.vertextype.TransitStopStreetVertex; import org.slf4j.Logger; @@ -36,7 +37,7 @@ public class TransitToTaggedStopsModule implements GraphBuilderModule { private static final Logger LOG = LoggerFactory.getLogger(TransitToTaggedStopsModule.class); - StreetVertexIndexServiceImpl index; + StreetVertexIndexService index; private double searchRadiusM = 250; private double searchRadiusLat = SphericalDistanceLibrary.metersToDegrees(searchRadiusM); @@ -52,7 +53,10 @@ public List getPrerequisites() { public void buildGraph(Graph graph, HashMap, Object> extra) { LOG.info("Linking transit stops to tagged bus stops..."); - index = new StreetVertexIndexServiceImpl(graph); + if (graph.streetIndex == null) { + graph.index(new DefaultStreetVertexIndexFactory()); + } + index = graph.streetIndex; // iterate over a copy of vertex list because it will be modified ArrayList vertices = new ArrayList<>(); diff --git a/src/main/java/org/opentripplanner/routing/impl/StreetVertexIndexServiceImpl.java b/src/main/java/org/opentripplanner/routing/impl/StreetVertexIndexServiceImpl.java index 5685cf6bbfa..00722b67b31 100644 --- a/src/main/java/org/opentripplanner/routing/impl/StreetVertexIndexServiceImpl.java +++ b/src/main/java/org/opentripplanner/routing/impl/StreetVertexIndexServiceImpl.java @@ -191,14 +191,17 @@ private void postSetup() { * If one need to store transit edges in the index, we could improve the hash grid * rasterizing splitting long segments. */ - for (Edge e : gv.getOutgoing()) { - if (e instanceof PatternEdge || e instanceof SimpleTransfer) - continue; - LineString geometry = e.getGeometry(); - if (geometry == null) { - continue; + synchronized (this) { + // place a lock here in order to ensure writes are thread-safe to the edgeTree + for (Edge e : gv.getOutgoing()) { + if (e instanceof PatternEdge || e instanceof SimpleTransfer) + continue; + LineString geometry = e.getGeometry(); + if (geometry == null) { + continue; + } + edgeTree.insert(geometry, e); } - edgeTree.insert(geometry, e); } if (v instanceof TransitStop) { Envelope env = new Envelope(v.getCoordinate()); diff --git a/src/test/java/org/opentripplanner/graph_builder/module/osm/TestOpenStreetMapGraphBuilder.java b/src/test/java/org/opentripplanner/graph_builder/module/osm/TestOpenStreetMapGraphBuilder.java index 94fc8b5813f..7f4e8cebf3f 100644 --- a/src/test/java/org/opentripplanner/graph_builder/module/osm/TestOpenStreetMapGraphBuilder.java +++ b/src/test/java/org/opentripplanner/graph_builder/module/osm/TestOpenStreetMapGraphBuilder.java @@ -1,21 +1,13 @@ package org.opentripplanner.graph_builder.module.osm; -import java.io.File; -import java.io.UnsupportedEncodingException; -import java.util.*; -import java.net.URLDecoder; - import junit.framework.TestCase; - import org.junit.Before; import org.junit.Test; -import org.opentripplanner.common.TurnRestriction; import org.opentripplanner.common.model.P2; +import org.opentripplanner.openstreetmap.impl.FileBasedOpenStreetMapProviderImpl; import org.opentripplanner.openstreetmap.model.OSMWay; import org.opentripplanner.openstreetmap.model.OSMWithTags; -import org.opentripplanner.openstreetmap.impl.FileBasedOpenStreetMapProviderImpl; import org.opentripplanner.routing.core.RoutingRequest; -import org.opentripplanner.routing.core.TraverseModeSet; import org.opentripplanner.routing.edgetype.StreetEdge; import org.opentripplanner.routing.edgetype.StreetTraversalPermission; import org.opentripplanner.routing.graph.Edge; @@ -23,7 +15,6 @@ import org.opentripplanner.routing.graph.Vertex; import org.opentripplanner.routing.impl.GraphPathFinder; import org.opentripplanner.routing.impl.MemoryGraphSource; -import org.opentripplanner.routing.impl.StreetVertexIndexServiceImpl; import org.opentripplanner.routing.services.GraphService; import org.opentripplanner.routing.spt.GraphPath; import org.opentripplanner.routing.vertextype.IntersectionVertex; @@ -32,6 +23,15 @@ import org.opentripplanner.standalone.Router; import org.opentripplanner.util.LocalizedString; +import java.io.File; +import java.io.UnsupportedEncodingException; +import java.net.URLDecoder; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Locale; +import java.util.Set; + public class TestOpenStreetMapGraphBuilder extends TestCase { private HashMap, Object> extra; @@ -171,7 +171,6 @@ private void testBuildingAreas(boolean skipVisibility) throws UnsupportedEncodin loader.setProvider(provider); loader.buildGraph(gg, extra); - new StreetVertexIndexServiceImpl(gg); OTPServer otpServer = new OTPServer(new CommandLineParameters(), new GraphService()); otpServer.getGraphService().registerGraph("A", new MemoryGraphSource("A", gg)); diff --git a/src/test/java/org/opentripplanner/routing/core/StateEditorTest.java b/src/test/java/org/opentripplanner/routing/core/StateEditorTest.java index d4de4e773a7..caff2414d3b 100644 --- a/src/test/java/org/opentripplanner/routing/core/StateEditorTest.java +++ b/src/test/java/org/opentripplanner/routing/core/StateEditorTest.java @@ -2,7 +2,7 @@ import org.junit.Test; import org.opentripplanner.routing.graph.Graph; -import org.opentripplanner.routing.impl.StreetVertexIndexServiceImpl; +import org.opentripplanner.routing.impl.DefaultStreetVertexIndexFactory; import static org.junit.Assert.assertEquals; @@ -28,7 +28,7 @@ public final void testSetNonTransitOptionsFromState(){ request.setMode(TraverseMode.CAR); request.parkAndRide = true; Graph graph = new Graph(); - graph.streetIndex = new StreetVertexIndexServiceImpl(graph); + graph.index(new DefaultStreetVertexIndexFactory()); request.rctx = new RoutingContext(request, graph); State state = new State(request); From ee448eac84cd22d04a0bdcb7180bc69f792b8f69 Mon Sep 17 00:00:00 2001 From: Evan Siroky Date: Thu, 29 Aug 2019 11:18:31 -0700 Subject: [PATCH 10/12] Refactor graph indexing to allow streetIndex recreation sometimes --- .../opentripplanner/api/resource/Routers.java | 4 +- .../graph_builder/linking/StreetSplitter.java | 11 +- .../module/DirectTransferGenerator.java | 6 +- .../module/NearbyStopFinder.java | 5 +- .../module/StreetLinkerModule.java | 5 +- .../module/TransitToTaggedStopsModule.java | 5 +- .../module/map/BusRouteStreetMatcher.java | 3 +- .../opentripplanner/routing/graph/Graph.java | 52 ++- .../impl/DefaultStreetVertexIndexFactory.java | 17 - .../routing/impl/InputStreamGraphSource.java | 6 +- .../impl/StreetVertexIndexServiceImpl.java | 360 ----------------- .../services/StreetVertexIndexFactory.java | 19 - .../services/StreetVertexIndexService.java | 368 +++++++++++++++++- .../opentripplanner/standalone/OTPMain.java | 3 +- .../java/org/opentripplanner/GtfsTest.java | 3 +- .../linking/StreetSplitterTest.java | 3 +- .../graph_builder/module/FakeGraph.java | 3 +- .../module/GtfsGraphBuilderModuleTest.java | 5 +- .../routing/TestHalfEdges.java | 28 +- .../routing/alertpatch/AlertPatchTest.java | 3 +- .../core/RoutingContextDestroyTest.java | 3 +- .../routing/core/StateEditorTest.java | 3 +- .../routing/core/TestTransfers.java | 3 +- .../TemporaryPartialStreetEdgeTest.java | 6 +- .../stoptime/TimetableSnapshotSourceTest.java | 3 +- 25 files changed, 429 insertions(+), 498 deletions(-) delete mode 100644 src/main/java/org/opentripplanner/routing/impl/DefaultStreetVertexIndexFactory.java delete mode 100644 src/main/java/org/opentripplanner/routing/impl/StreetVertexIndexServiceImpl.java delete mode 100644 src/main/java/org/opentripplanner/routing/services/StreetVertexIndexFactory.java diff --git a/src/main/java/org/opentripplanner/api/resource/Routers.java b/src/main/java/org/opentripplanner/api/resource/Routers.java index a330dbe3d14..a01c322de5a 100644 --- a/src/main/java/org/opentripplanner/api/resource/Routers.java +++ b/src/main/java/org/opentripplanner/api/resource/Routers.java @@ -32,7 +32,7 @@ import org.opentripplanner.graph_builder.GraphBuilder; import org.opentripplanner.routing.error.GraphNotFoundException; import org.opentripplanner.routing.graph.Graph; -import org.opentripplanner.routing.impl.DefaultStreetVertexIndexFactory; +import org.opentripplanner.routing.graph.Graph.LoadLevel; import org.opentripplanner.routing.impl.MemoryGraphSource; import org.opentripplanner.routing.services.GraphService; import org.opentripplanner.standalone.CommandLineParameters; @@ -270,7 +270,7 @@ public Response buildGraphOverWire ( tempDir.delete(); Graph graph = graphBuilder.getGraph(); - graph.index(new DefaultStreetVertexIndexFactory()); + graph.index(true); GraphService graphService = otpServer.getGraphService(); graphService.registerGraph(routerId, new MemoryGraphSource(routerId, graph)); diff --git a/src/main/java/org/opentripplanner/graph_builder/linking/StreetSplitter.java b/src/main/java/org/opentripplanner/graph_builder/linking/StreetSplitter.java index ffd72efa103..d70ebd6ec97 100644 --- a/src/main/java/org/opentripplanner/graph_builder/linking/StreetSplitter.java +++ b/src/main/java/org/opentripplanner/graph_builder/linking/StreetSplitter.java @@ -115,8 +115,10 @@ public class StreetSplitter { /** * Construct a new StreetSplitter. - * NOTE: Only one StreetSplitter should be active on a graph at any given time and this is enforced by an error - * being thrown if more than an attempt is made to instantiate a 2nd instance of a StreetSplitter on the same graph. + * + * IMPORTANT NOTE: Only one StreetSplitter should be active on a graph at any given time. This is because any + * newly-created split edges in this street splitter won't show up in the index of the other street splitters + * (and potentially StreetVertexIndexServices). * * @param hashGridSpatialIndex If not null this index is used instead of creating new one * @param transitStopIndex Index of all transitStops which is generated in {@link org.opentripplanner.routing.impl.StreetVertexIndexServiceImpl} @@ -128,7 +130,7 @@ public StreetSplitter( ) { this.graph = graph; if (graphsWithSplitters.contains(graph)) { - throw new UnsupportedOperationException("Only one street splitter should be active on a graph at any given time"); + LOG.warn("Multiple StreetSplitters detected on the same graph! Make sure only the most recently created one is used going forward!"); } LOG.info("New StreetSpiltter created successfully!"); graphsWithSplitters.add(graph); @@ -530,7 +532,8 @@ private void makeTransitLinkEdges (TransitStop tstop, StreetVertex v, final bool throw new RuntimeException("Transitedges are created with non destructive splitting!"); } // ensure that the requisite edges do not already exist - // this can happen if we link to duplicate ways that have the same start/end vertices. + // this can happen if we link to duplicate ways that have the same start/end vertices or if + // certain transit stops were already linked to the network in the TransitToTaggedStopsModule. for (StreetTransitLink e : Iterables.filter(tstop.getOutgoing(), StreetTransitLink.class)) { if (e.getToVertex() == v) return; diff --git a/src/main/java/org/opentripplanner/graph_builder/module/DirectTransferGenerator.java b/src/main/java/org/opentripplanner/graph_builder/module/DirectTransferGenerator.java index 23aace1a0a4..ca9d04704ef 100644 --- a/src/main/java/org/opentripplanner/graph_builder/module/DirectTransferGenerator.java +++ b/src/main/java/org/opentripplanner/graph_builder/module/DirectTransferGenerator.java @@ -47,10 +47,8 @@ public DirectTransferGenerator (double radiusMeters) { @Override public void buildGraph(Graph graph, HashMap, Object> extra) { - /* Initialize graph index which is needed by the nearby stop finder. */ - if (graph.index == null) { - graph.index = new GraphIndex(graph); - } + /* Initialize or recalculate the graph index which is needed by the nearby stop finder. */ + graph.index(false); /* The linker will use streets if they are available, or straight-line distance otherwise. */ NearbyStopFinder nearbyStopFinder = new NearbyStopFinder(graph, radiusMeters); diff --git a/src/main/java/org/opentripplanner/graph_builder/module/NearbyStopFinder.java b/src/main/java/org/opentripplanner/graph_builder/module/NearbyStopFinder.java index 203e1b366bd..64d957adc72 100644 --- a/src/main/java/org/opentripplanner/graph_builder/module/NearbyStopFinder.java +++ b/src/main/java/org/opentripplanner/graph_builder/module/NearbyStopFinder.java @@ -19,7 +19,6 @@ import org.opentripplanner.routing.graph.Edge; import org.opentripplanner.routing.graph.Graph; import org.opentripplanner.routing.graph.Vertex; -import org.opentripplanner.routing.impl.DefaultStreetVertexIndexFactory; import org.opentripplanner.routing.services.StreetVertexIndexService; import org.opentripplanner.routing.spt.GraphPath; import org.opentripplanner.routing.spt.ShortestPathTree; @@ -67,9 +66,7 @@ public NearbyStopFinder(Graph graph, double radiusMeters) { // but we don't have much of a choice here. Use the default walking speed to convert. earliestArrivalSearch.maxDuration = (int) (radiusMeters / new RoutingRequest().walkSpeed); } else { - if (graph.streetIndex == null) { - graph.index(new DefaultStreetVertexIndexFactory()); - } + graph.index(false); streetIndex = graph.streetIndex; } } diff --git a/src/main/java/org/opentripplanner/graph_builder/module/StreetLinkerModule.java b/src/main/java/org/opentripplanner/graph_builder/module/StreetLinkerModule.java index 4b891de1130..4c6af90a6ed 100644 --- a/src/main/java/org/opentripplanner/graph_builder/module/StreetLinkerModule.java +++ b/src/main/java/org/opentripplanner/graph_builder/module/StreetLinkerModule.java @@ -3,7 +3,6 @@ import org.opentripplanner.graph_builder.linking.StreetSplitter; import org.opentripplanner.graph_builder.services.GraphBuilderModule; import org.opentripplanner.routing.graph.Graph; -import org.opentripplanner.routing.impl.DefaultStreetVertexIndexFactory; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -45,9 +44,7 @@ public List getPrerequisites() { public void buildGraph(Graph graph, HashMap, Object> extra) { if(graph.hasStreets) { LOG.info("Linking transit stops, bike rental stations, bike parking areas, and park-and-rides to graph . . ."); - if (graph.streetIndex == null) { - graph.index(new DefaultStreetVertexIndexFactory()); - } + graph.index(false); StreetSplitter linker = graph.streetIndex.getStreetSplitter(); linker.setAddExtraEdgesToAreas(this.addExtraEdgesToAreas); linker.linkAllStationsToGraph(); diff --git a/src/main/java/org/opentripplanner/graph_builder/module/TransitToTaggedStopsModule.java b/src/main/java/org/opentripplanner/graph_builder/module/TransitToTaggedStopsModule.java index 6b86e7390d8..5da44223ccf 100644 --- a/src/main/java/org/opentripplanner/graph_builder/module/TransitToTaggedStopsModule.java +++ b/src/main/java/org/opentripplanner/graph_builder/module/TransitToTaggedStopsModule.java @@ -8,7 +8,6 @@ import org.opentripplanner.routing.graph.Edge; import org.opentripplanner.routing.graph.Graph; import org.opentripplanner.routing.graph.Vertex; -import org.opentripplanner.routing.impl.DefaultStreetVertexIndexFactory; import org.opentripplanner.routing.services.StreetVertexIndexService; import org.opentripplanner.routing.vertextype.TransitStop; import org.opentripplanner.routing.vertextype.TransitStopStreetVertex; @@ -53,9 +52,7 @@ public List getPrerequisites() { public void buildGraph(Graph graph, HashMap, Object> extra) { LOG.info("Linking transit stops to tagged bus stops..."); - if (graph.streetIndex == null) { - graph.index(new DefaultStreetVertexIndexFactory()); - } + graph.index(false); index = graph.streetIndex; // iterate over a copy of vertex list because it will be modified diff --git a/src/main/java/org/opentripplanner/graph_builder/module/map/BusRouteStreetMatcher.java b/src/main/java/org/opentripplanner/graph_builder/module/map/BusRouteStreetMatcher.java index 2db8a8637d0..e24e0881267 100644 --- a/src/main/java/org/opentripplanner/graph_builder/module/map/BusRouteStreetMatcher.java +++ b/src/main/java/org/opentripplanner/graph_builder/module/map/BusRouteStreetMatcher.java @@ -17,7 +17,6 @@ import java.util.Arrays; import java.util.HashMap; import java.util.List; -import org.opentripplanner.routing.impl.DefaultStreetVertexIndexFactory; /** * Uses the shapes from GTFS to determine which streets buses drive on. This is used to improve the quality of @@ -47,7 +46,7 @@ NetworkLinkerLibrary later (actually in LinkRequests). public void buildGraph(Graph graph, HashMap, Object> extra) { //Mapbuilder needs transit index - graph.index(new DefaultStreetVertexIndexFactory()); + graph.index(false); StreetMatcher matcher = new StreetMatcher(graph); EdgesForRoute edgesForRoute = new EdgesForRoute(); diff --git a/src/main/java/org/opentripplanner/routing/graph/Graph.java b/src/main/java/org/opentripplanner/routing/graph/Graph.java index 4ad911a6cc9..ec0b5afda88 100644 --- a/src/main/java/org/opentripplanner/routing/graph/Graph.java +++ b/src/main/java/org/opentripplanner/routing/graph/Graph.java @@ -48,8 +48,6 @@ import org.opentripplanner.routing.edgetype.StreetEdge; import org.opentripplanner.routing.edgetype.TripPattern; import org.opentripplanner.routing.flex.FlexIndex; -import org.opentripplanner.routing.impl.DefaultStreetVertexIndexFactory; -import org.opentripplanner.routing.services.StreetVertexIndexFactory; import org.opentripplanner.routing.services.StreetVertexIndexService; import org.opentripplanner.routing.services.notes.StreetNotesService; import org.opentripplanner.routing.trippattern.Deduplicator; @@ -673,23 +671,41 @@ public static Graph load(File file) throws IOException { LOG.info("Reading graph " + file.getAbsolutePath() + " ..."); return load(new FileInputStream(file)); } - - /** - * Perform indexing on vertices, edges, and timetables, and create transient data structures. - * This used to be done in readObject methods upon deserialization, but stand-alone mode now - * allows passing graphs from graphbuilder to server in memory, without a round trip through - * serialization. - * TODO: do we really need a factory for different street vertex indexes? + + /** + * Perform indexing on vertices, edges, and timetables, and create transient data structures. This used to be done + * in readObject methods upon deserialization, but stand-alone mode now allows passing graphs from graphbuilder to + * server in memory, without a round trip through serialization. + * + * After a refactor in the year 2019, the method signature was changed to no longer take in an argument of a + * StreetVertexIndexFactory as a single factory was always being used to create the same StreetVertexIndexService. + * Instead the management of instances of StreetVertexIndexServices is encapsulated within this method only. Also, + * the parameter of `recreateStreetIndex` was added. + * + * In a lot of cases, especially when this method is called from certain build phases, the streetIndex does not need + * to be recreated. However, in other use cases such as the "build graph over wire" and in-memory graph builds where + * a newly created graph is used immediately instead of it being saved and the program exiting, a reindex is needed + * in order to make sure that every non-transit edge with geographic data is available in the respective indexes. + * In those use cases, the streetIndex was created during the graph building phase for the purposes of splitting + * various edges to insert certain items like transit stops. However, the graph does need to be reindexed after all + * items are inserted in order to have full knowledge of all items with geography such as SimpleTransfers and + * StreetTransitLinks. From looking at the code paths of where a new index is requested, it seems that the indexing + * takes place in a manner that would not result in two different StreetSplitters being used at once as the old one + * is typically used only during graph building. + * + * However, due to the potential for problems to arise with two StreetVertexIndexServices and StreetSplitters being + * active at once, a warning is logged just in case. */ - public void index (StreetVertexIndexFactory indexFactory) { - if (streetIndex != null) { - throw new UnsupportedOperationException( - "A streetIndex has already been defined. Can't you just reuse the existing one?" - ); + public void index (boolean recreateStreetIndex) { + if (streetIndex == null || recreateStreetIndex) { + if (streetIndex != null && recreateStreetIndex) { + LOG.warn("Overwritting an existing streetIndex! This could lead to problems if both the old and new streetIndex are used. Make sure only the new streetIndex is used going forward."); + } + LOG.info("building street index"); + streetIndex = new StreetVertexIndexService(this); + LOG.info("street index built"); } - streetIndex = indexFactory.newIndex(this); - LOG.debug("street index built."); - LOG.debug("Rebuilding edge and vertex indices."); + LOG.info("Rebuilding edge and vertex indices"); rebuildVertexAndEdgeIndices(); Set tableTripPatterns = Sets.newHashSet(); for (PatternArriveVertex pav : Iterables.filter(this.getVertices(), PatternArriveVertex.class)) { @@ -734,7 +750,7 @@ public static Graph load(InputStream in) { } LOG.info("Main graph read. |V|={} |E|={}", graph.countVertices(), graph.countEdges()); - graph.index(new DefaultStreetVertexIndexFactory()); + graph.index(false); return graph; } diff --git a/src/main/java/org/opentripplanner/routing/impl/DefaultStreetVertexIndexFactory.java b/src/main/java/org/opentripplanner/routing/impl/DefaultStreetVertexIndexFactory.java deleted file mode 100644 index d491a56e0a8..00000000000 --- a/src/main/java/org/opentripplanner/routing/impl/DefaultStreetVertexIndexFactory.java +++ /dev/null @@ -1,17 +0,0 @@ -package org.opentripplanner.routing.impl; - -import org.opentripplanner.routing.graph.Graph; -import org.opentripplanner.routing.services.StreetVertexIndexFactory; -import org.opentripplanner.routing.services.StreetVertexIndexService; - -/** - * Default implementation. Simply returns an instance of StreetVertexIndexServiceImpl. - * @author avi - */ -public class DefaultStreetVertexIndexFactory implements StreetVertexIndexFactory { - - @Override - public StreetVertexIndexService newIndex(Graph g) { - return new StreetVertexIndexServiceImpl(g); - } -} diff --git a/src/main/java/org/opentripplanner/routing/impl/InputStreamGraphSource.java b/src/main/java/org/opentripplanner/routing/impl/InputStreamGraphSource.java index 7a3489e3df8..55bc5cc57eb 100644 --- a/src/main/java/org/opentripplanner/routing/impl/InputStreamGraphSource.java +++ b/src/main/java/org/opentripplanner/routing/impl/InputStreamGraphSource.java @@ -7,7 +7,6 @@ import com.google.common.io.ByteStreams; import org.opentripplanner.routing.graph.Graph; import org.opentripplanner.routing.services.GraphSource; -import org.opentripplanner.routing.services.StreetVertexIndexFactory; import org.opentripplanner.standalone.Router; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -45,9 +44,6 @@ public class InputStreamGraphSource implements GraphSource { */ private Streams streams; - // TODO Why do we need a factory? There is a single one implementation. - private StreetVertexIndexFactory streetVertexIndexFactory = new DefaultStreetVertexIndexFactory(); - /** * @return A GraphSource loading graph from the file system under a base path. */ @@ -180,7 +176,7 @@ private Router loadGraph() { try (InputStream is = streams.getGraphInputStream()) { LOG.info("Loading graph..."); try { - newGraph = Graph.load(is); + newGraph = Graph.load(new ObjectInputStream(is), loadLevel); } catch (Exception ex) { LOG.error("Exception while loading graph '{}'.", routerId, ex); return null; diff --git a/src/main/java/org/opentripplanner/routing/impl/StreetVertexIndexServiceImpl.java b/src/main/java/org/opentripplanner/routing/impl/StreetVertexIndexServiceImpl.java deleted file mode 100644 index 00722b67b31..00000000000 --- a/src/main/java/org/opentripplanner/routing/impl/StreetVertexIndexServiceImpl.java +++ /dev/null @@ -1,360 +0,0 @@ -package org.opentripplanner.routing.impl; - - -import org.locationtech.jts.geom.Coordinate; -import org.locationtech.jts.geom.Envelope; -import org.locationtech.jts.geom.LineString; -import org.opentripplanner.analyst.core.Sample; -import org.opentripplanner.analyst.request.SampleFactory; -import org.opentripplanner.common.geometry.GeometryUtils; -import org.opentripplanner.common.geometry.HashGridSpatialIndex; -import org.opentripplanner.common.geometry.SphericalDistanceLibrary; -import org.opentripplanner.common.model.GenericLocation; -import org.opentripplanner.common.model.P2; -import org.opentripplanner.graph_builder.linking.StreetSplitter; -import org.opentripplanner.routing.core.RoutingRequest; -import org.opentripplanner.routing.edgetype.PatternEdge; -import org.opentripplanner.routing.edgetype.SampleEdge; -import org.opentripplanner.routing.edgetype.SimpleTransfer; -import org.opentripplanner.routing.edgetype.StreetEdge; -import org.opentripplanner.routing.edgetype.TemporaryFreeEdge; -import org.opentripplanner.routing.edgetype.TemporaryPartialStreetEdge; -import org.opentripplanner.routing.graph.Edge; -import org.opentripplanner.routing.graph.Graph; -import org.opentripplanner.routing.graph.Vertex; -import org.opentripplanner.routing.location.TemporaryStreetLocation; -import org.opentripplanner.routing.services.StreetVertexIndexService; -import org.opentripplanner.routing.vertextype.SampleVertex; -import org.opentripplanner.routing.vertextype.StreetVertex; -import org.opentripplanner.routing.vertextype.TransitStop; -import org.opentripplanner.util.I18NString; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -import java.util.ArrayList; -import java.util.Collection; -import java.util.Iterator; -import java.util.List; - -/** - * Indexes all edges and transit vertices of the graph spatially. Has a variety of query methods - * used during network linking and trip planning. - * - * Creates a TemporaryStreetLocation representing a location on a street that's not at an - * intersection, based on input latitude and longitude. Instantiating this class is expensive, - * because it creates a spatial index of all of the intersections in the graph. - */ -public class StreetVertexIndexServiceImpl implements StreetVertexIndexService { - - private Graph graph; - - /** - * Contains only instances of {@link StreetEdge} - */ - private HashGridSpatialIndex edgeTree; - private HashGridSpatialIndex transitStopTree; - private HashGridSpatialIndex verticesTree; - - // private static final double SEARCH_RADIUS_M = 100; // meters - // private static final double SEARCH_RADIUS_DEG = DistanceLibrary.metersToDegrees(SEARCH_RADIUS_M); - - // Maximum difference in distance for two geometries to be considered coincident, plate-carée Euclidean - // 0.001 ~= 100m at equator - public static final double DISTANCE_ERROR = 0.000001; - - // If a point is within MAX_CORNER_DISTANCE, it is treated as at the corner. - private static final double MAX_CORNER_DISTANCE_METERS = 10; - - // Edges will only be found if they are closer than this distance - // TODO: this default may be too large? - public static final double MAX_DISTANCE_FROM_STREET_METERS = 1000; - - private static final double MAX_DISTANCE_FROM_STREET_DEGREES = - MAX_DISTANCE_FROM_STREET_METERS * 180 / Math.PI / SphericalDistanceLibrary.RADIUS_OF_EARTH_IN_M; - - static final Logger LOG = LoggerFactory.getLogger(StreetVertexIndexServiceImpl.class); - - private StreetSplitter streetSplitter; - - public StreetVertexIndexServiceImpl(Graph graph) { - this.graph = graph; - edgeTree = new HashGridSpatialIndex<>(); - transitStopTree = new HashGridSpatialIndex<>(); - verticesTree = new HashGridSpatialIndex<>(); - postSetup(); - streetSplitter = new StreetSplitter(graph, edgeTree, transitStopTree); - } - - /** - * Creates a TemporaryStreetLocation on the given street (set of PlainStreetEdges). How far - * along is controlled by the location parameter, which represents a distance along the edge - * between 0 (the from vertex) and 1 (the to vertex). - * - * @param graph - * - * @param label - * @param name - * @param edges A collection of nearby edges, which represent one street. - * @param nearestPoint - * - * @return the new TemporaryStreetLocation - */ - public static TemporaryStreetLocation createTemporaryStreetLocation(Graph graph, String label, - I18NString name, Iterable edges, Coordinate nearestPoint, boolean endVertex) { - boolean wheelchairAccessible = false; - - TemporaryStreetLocation location = new TemporaryStreetLocation(label, nearestPoint, name, endVertex); - - for (StreetEdge street : edges) { - Vertex fromv = street.getFromVertex(); - Vertex tov = street.getToVertex(); - wheelchairAccessible |= street.isWheelchairAccessible(); - - /* forward edges and vertices */ - Vertex edgeLocation; - if (SphericalDistanceLibrary.distance(nearestPoint, fromv.getCoordinate()) < 1) { - // no need to link to area edges caught on-end - edgeLocation = fromv; - - if (endVertex) { - new TemporaryFreeEdge(edgeLocation, location); - } else { - new TemporaryFreeEdge(location, edgeLocation); - } - } else if (SphericalDistanceLibrary.distance(nearestPoint, tov.getCoordinate()) < 1) { - // no need to link to area edges caught on-end - edgeLocation = tov; - - if (endVertex) { - new TemporaryFreeEdge(edgeLocation, location); - } else { - new TemporaryFreeEdge(location, edgeLocation); - } - } else { - // location is somewhere in the middle of the edge. - edgeLocation = location; - - // creates links from street head -> location -> street tail. - createHalfLocation(location, name, - nearestPoint, street, endVertex); - } - } - location.setWheelchairAccessible(wheelchairAccessible); - return location; - - } - - private static void createHalfLocation(TemporaryStreetLocation base, I18NString name, - Coordinate nearestPoint, StreetEdge street, boolean endVertex) { - StreetVertex tov = (StreetVertex) street.getToVertex(); - StreetVertex fromv = (StreetVertex) street.getFromVertex(); - LineString geometry = street.getGeometry(); - - P2 geometries = getGeometry(street, nearestPoint); - - double totalGeomLength = geometry.getLength(); - double lengthRatioIn = geometries.first.getLength() / totalGeomLength; - - double lengthIn = street.getDistance() * lengthRatioIn; - double lengthOut = street.getDistance() * (1 - lengthRatioIn); - - if (endVertex) { - TemporaryPartialStreetEdge temporaryPartialStreetEdge = new TemporaryPartialStreetEdge( - street, fromv, base, geometries.first, name, lengthIn); - - temporaryPartialStreetEdge.setNoThruTraffic(street.isNoThruTraffic()); - temporaryPartialStreetEdge.setStreetClass(street.getStreetClass()); - } else { - TemporaryPartialStreetEdge temporaryPartialStreetEdge = new TemporaryPartialStreetEdge( - street, base, tov, geometries.second, name, lengthOut); - - temporaryPartialStreetEdge.setStreetClass(street.getStreetClass()); - temporaryPartialStreetEdge.setNoThruTraffic(street.isNoThruTraffic()); - } - } - - private static P2 getGeometry(StreetEdge e, Coordinate nearestPoint) { - LineString geometry = e.getGeometry(); - return GeometryUtils.splitGeometryAtPoint(geometry, nearestPoint); - } - - @SuppressWarnings("rawtypes") - private void postSetup() { - for (Vertex gv : graph.getVertices()) { - Vertex v = gv; - /* - * We add all edges with geometry, skipping transit, filtering them out after. We do not - * index transit edges as we do not need them and some GTFS do not have shape data, so - * long straight lines between 2 faraway stations will wreck performance on a hash grid - * spatial index. - * - * If one need to store transit edges in the index, we could improve the hash grid - * rasterizing splitting long segments. - */ - synchronized (this) { - // place a lock here in order to ensure writes are thread-safe to the edgeTree - for (Edge e : gv.getOutgoing()) { - if (e instanceof PatternEdge || e instanceof SimpleTransfer) - continue; - LineString geometry = e.getGeometry(); - if (geometry == null) { - continue; - } - edgeTree.insert(geometry, e); - } - } - if (v instanceof TransitStop) { - Envelope env = new Envelope(v.getCoordinate()); - transitStopTree.insert(env, v); - } - Envelope env = new Envelope(v.getCoordinate()); - verticesTree.insert(env, v); - } - } - - /** - * Get all transit stops within a given distance of a coordinate - */ - @Override - public List getNearbyTransitStops(Coordinate coordinate, double radius) { - Envelope env = new Envelope(coordinate); - env.expandBy(SphericalDistanceLibrary.metersToLonDegrees(radius, coordinate.y), - SphericalDistanceLibrary.metersToDegrees(radius)); - List nearby = getTransitStopForEnvelope(env); - List results = new ArrayList(); - for (TransitStop v : nearby) { - if (SphericalDistanceLibrary.distance(v.getCoordinate(), coordinate) <= radius) { - results.add(v); - } - } - return results; - } - - @SuppressWarnings("unchecked") - @Override - public List getVerticesForEnvelope(Envelope envelope) { - List vertices = verticesTree.query(envelope); - // Here we assume vertices list modifiable - for (Iterator iv = vertices.iterator(); iv.hasNext();) { - Vertex v = iv.next(); - if (!envelope.contains(new Coordinate(v.getLon(), v.getLat()))) - iv.remove(); - } - return vertices; - } - - @SuppressWarnings("unchecked") - @Override - public Collection getEdgesForEnvelope(Envelope envelope) { - List edges = edgeTree.query(envelope); - for (Iterator ie = edges.iterator(); ie.hasNext();) { - Edge e = ie.next(); - Envelope eenv = e.getGeometry().getEnvelopeInternal(); - //Envelope eenv = e.getEnvelope(); - if (!envelope.intersects(eenv)) - ie.remove(); - } - return edges; - } - - @SuppressWarnings("unchecked") - @Override - public List getTransitStopForEnvelope(Envelope envelope) { - List transitStops = transitStopTree.query(envelope); - for (Iterator its = transitStops.iterator(); its.hasNext();) { - TransitStop ts = its.next(); - if (!envelope.intersects(new Coordinate(ts.getLon(), ts.getLat()))) - its.remove(); - } - return transitStops; - } - - /** - * @param coordinate Location to search intersection at. Look in a MAX_CORNER_DISTANCE_METERS radius. - * @return The nearest intersection, null if none found. - */ - public StreetVertex getIntersectionAt(Coordinate coordinate) { - double dLon = SphericalDistanceLibrary.metersToLonDegrees(MAX_CORNER_DISTANCE_METERS, - coordinate.y); - double dLat = SphericalDistanceLibrary.metersToDegrees(MAX_CORNER_DISTANCE_METERS); - Envelope envelope = new Envelope(coordinate); - envelope.expandBy(dLon, dLat); - List nearby = getVerticesForEnvelope(envelope); - StreetVertex nearest = null; - double bestDistanceMeter = Double.POSITIVE_INFINITY; - for (Vertex v : nearby) { - if (v instanceof StreetVertex) { - v.getLabel().startsWith("osm:"); - double distanceMeter = SphericalDistanceLibrary.fastDistance(coordinate, v.getCoordinate()); - if (distanceMeter < MAX_CORNER_DISTANCE_METERS) { - if (distanceMeter < bestDistanceMeter) { - bestDistanceMeter = distanceMeter; - nearest = (StreetVertex) v; - } - } - } - } - return nearest; - } - - @Override - public Vertex getVertexForLocation(GenericLocation loc, RoutingRequest options, - boolean endVertex) { - Coordinate c = loc.getCoordinate(); - if (c != null) { - return streetSplitter.linkOriginDestination(loc, options, endVertex); - } - - // No Coordinate available. - String place = loc.place; - if (place == null) { - return null; - } - - // did not match lat/lon, interpret place as a vertex label. - // this should probably only be used in tests, - // though it does allow routing from stop to stop. - return graph.getVertex(place); - } - - @Override - public String toString() { - return getClass().getName() + " -- edgeTree: " + edgeTree.toString() + " -- verticesTree: " + verticesTree.toString(); - } - - @Override - public Vertex getSampleVertexAt(Coordinate coordinate, boolean dest) { - SampleFactory sfac = graph.getSampleFactory(); - - Sample s = sfac.getSample(coordinate.x, coordinate.y); - - if (s == null) - return null; - - // create temp vertex - SampleVertex v = new SampleVertex(graph, coordinate); - - // create edges - if (dest) { - if (s.v0 != null) - new SampleEdge(s.v0, v, s.d0); - - if (s.v1 != null) - new SampleEdge(s.v1, v, s.d1); - } - else { - if (s.v0 != null) - new SampleEdge(v, s.v0, s.d0); - - if (s.v1 != null) - new SampleEdge(v, s.v1, s.d1); - } - - return v; - } - - @Override - public StreetSplitter getStreetSplitter() { - return streetSplitter; - } -} diff --git a/src/main/java/org/opentripplanner/routing/services/StreetVertexIndexFactory.java b/src/main/java/org/opentripplanner/routing/services/StreetVertexIndexFactory.java deleted file mode 100644 index f2240246fed..00000000000 --- a/src/main/java/org/opentripplanner/routing/services/StreetVertexIndexFactory.java +++ /dev/null @@ -1,19 +0,0 @@ -package org.opentripplanner.routing.services; - -import org.opentripplanner.routing.graph.Graph; - - -/** - * Factory for StreetVertexIndexServices. - * - * @author avi - */ -public interface StreetVertexIndexFactory { - - /** - * Returns a new StreetVertexIndexService for this graph. - * @param g - * @return - */ - public StreetVertexIndexService newIndex(Graph g); -} diff --git a/src/main/java/org/opentripplanner/routing/services/StreetVertexIndexService.java b/src/main/java/org/opentripplanner/routing/services/StreetVertexIndexService.java index 5de3e74e7f3..63980fa2871 100644 --- a/src/main/java/org/opentripplanner/routing/services/StreetVertexIndexService.java +++ b/src/main/java/org/opentripplanner/routing/services/StreetVertexIndexService.java @@ -2,68 +2,402 @@ import org.locationtech.jts.geom.Coordinate; import org.locationtech.jts.geom.Envelope; +import org.locationtech.jts.geom.LineString; +import org.opentripplanner.analyst.core.Sample; +import org.opentripplanner.analyst.request.SampleFactory; +import org.opentripplanner.common.geometry.GeometryUtils; +import org.opentripplanner.common.geometry.HashGridSpatialIndex; +import org.opentripplanner.common.geometry.SphericalDistanceLibrary; import org.opentripplanner.common.model.GenericLocation; +import org.opentripplanner.common.model.P2; import org.opentripplanner.graph_builder.linking.StreetSplitter; import org.opentripplanner.routing.core.RoutingRequest; +import org.opentripplanner.routing.edgetype.PatternEdge; +import org.opentripplanner.routing.edgetype.SampleEdge; +import org.opentripplanner.routing.edgetype.SimpleTransfer; +import org.opentripplanner.routing.edgetype.StreetEdge; +import org.opentripplanner.routing.edgetype.TemporaryFreeEdge; +import org.opentripplanner.routing.edgetype.TemporaryPartialStreetEdge; import org.opentripplanner.routing.graph.Edge; +import org.opentripplanner.routing.graph.Graph; import org.opentripplanner.routing.graph.Vertex; +import org.opentripplanner.routing.location.TemporaryStreetLocation; +import org.opentripplanner.routing.vertextype.SampleVertex; +import org.opentripplanner.routing.vertextype.StreetVertex; import org.opentripplanner.routing.vertextype.TransitStop; +import org.opentripplanner.util.I18NString; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import java.util.ArrayList; import java.util.Collection; +import java.util.HashSet; +import java.util.Iterator; import java.util.List; -public interface StreetVertexIndexService { +/** + * Indexes all edges and transit vertices of the graph spatially. Has a variety of query methods + * used during network linking and trip planning. + * + * Creates a TemporaryStreetLocation representing a location on a street that's not at an + * intersection, based on input latitude and longitude. Instantiating this class is expensive, + * because it creates a spatial index of all of the intersections in the graph. + */ +public class StreetVertexIndexService { + + private Graph graph; + + /** + * Contains only instances of {@link StreetEdge} + */ + private HashGridSpatialIndex edgeTree; + private HashGridSpatialIndex transitStopTree; + private HashGridSpatialIndex verticesTree; + + // private static final double SEARCH_RADIUS_M = 100; // meters + // private static final double SEARCH_RADIUS_DEG = DistanceLibrary.metersToDegrees(SEARCH_RADIUS_M); + + // Maximum difference in distance for two geometries to be considered coincident, plate-carée Euclidean + // 0.001 ~= 100m at equator + public static final double DISTANCE_ERROR = 0.000001; + + // If a point is within MAX_CORNER_DISTANCE, it is treated as at the corner. + private static final double MAX_CORNER_DISTANCE_METERS = 10; + + // Edges will only be found if they are closer than this distance + // TODO: this default may be too large? + public static final double MAX_DISTANCE_FROM_STREET_METERS = 1000; + + private static final double MAX_DISTANCE_FROM_STREET_DEGREES = + MAX_DISTANCE_FROM_STREET_METERS * 180 / Math.PI / SphericalDistanceLibrary.RADIUS_OF_EARTH_IN_M; + + static final Logger LOG = LoggerFactory.getLogger(StreetVertexIndexService.class); + + private StreetSplitter streetSplitter; + + // used to keep track of street vertex index services associated with graphs to make sure that + public static final HashSet graphsWithStreetVertexIndexServices = new HashSet<>(); + + /** + * Construct a new StreetVertexIndexService using the given graph. Upon instantiation, a number of spatial indexes + * will be created to be used for the quick lookup of data with spatial attributes. + * + * IMPORTANT NOTE: Only one StreetVertexIndexService should be active on a graph at any given time. This is because + * any newly-created (or removed) vertices or edges in this StreetVertexIndexService won't show up in the indices of + * the other StreetVertexIndexServices. + * + * @param graph + */ + public StreetVertexIndexService(Graph graph) { + this.graph = graph; + if (graphsWithStreetVertexIndexServices.contains(graph)) { + LOG.warn("Multiple StreetVertexIndexServices detected on the same graph! Make sure only the most recently created one is used going forward!"); + } + graphsWithStreetVertexIndexServices.add(graph); + edgeTree = new HashGridSpatialIndex<>(); + transitStopTree = new HashGridSpatialIndex<>(); + verticesTree = new HashGridSpatialIndex<>(); + postSetup(); + streetSplitter = new StreetSplitter(graph, edgeTree, transitStopTree); + } + + /** + * Creates a TemporaryStreetLocation on the given street (set of PlainStreetEdges). How far + * along is controlled by the location parameter, which represents a distance along the edge + * between 0 (the from vertex) and 1 (the to vertex). + * + * @param graph + * + * @param label + * @param name + * @param edges A collection of nearby edges, which represent one street. + * @param nearestPoint + * + * @return the new TemporaryStreetLocation + */ + public static TemporaryStreetLocation createTemporaryStreetLocation(Graph graph, String label, + I18NString name, Iterable edges, Coordinate nearestPoint, boolean endVertex) { + boolean wheelchairAccessible = false; + + TemporaryStreetLocation location = new TemporaryStreetLocation(label, nearestPoint, name, endVertex); + + for (StreetEdge street : edges) { + Vertex fromv = street.getFromVertex(); + Vertex tov = street.getToVertex(); + wheelchairAccessible |= street.isWheelchairAccessible(); + + /* forward edges and vertices */ + Vertex edgeLocation; + if (SphericalDistanceLibrary.distance(nearestPoint, fromv.getCoordinate()) < 1) { + // no need to link to area edges caught on-end + edgeLocation = fromv; + + if (endVertex) { + new TemporaryFreeEdge(edgeLocation, location); + } else { + new TemporaryFreeEdge(location, edgeLocation); + } + } else if (SphericalDistanceLibrary.distance(nearestPoint, tov.getCoordinate()) < 1) { + // no need to link to area edges caught on-end + edgeLocation = tov; + + if (endVertex) { + new TemporaryFreeEdge(edgeLocation, location); + } else { + new TemporaryFreeEdge(location, edgeLocation); + } + } else { + // location is somewhere in the middle of the edge. + edgeLocation = location; + + // creates links from street head -> location -> street tail. + createHalfLocation(location, name, + nearestPoint, street, endVertex); + } + } + location.setWheelchairAccessible(wheelchairAccessible); + return location; + + } + + private static void createHalfLocation(TemporaryStreetLocation base, I18NString name, + Coordinate nearestPoint, StreetEdge street, boolean endVertex) { + StreetVertex tov = (StreetVertex) street.getToVertex(); + StreetVertex fromv = (StreetVertex) street.getFromVertex(); + LineString geometry = street.getGeometry(); + + P2 geometries = getGeometry(street, nearestPoint); + + double totalGeomLength = geometry.getLength(); + double lengthRatioIn = geometries.first.getLength() / totalGeomLength; + + double lengthIn = street.getDistance() * lengthRatioIn; + double lengthOut = street.getDistance() * (1 - lengthRatioIn); + + if (endVertex) { + TemporaryPartialStreetEdge temporaryPartialStreetEdge = new TemporaryPartialStreetEdge( + street, fromv, base, geometries.first, name, lengthIn); + + temporaryPartialStreetEdge.setNoThruTraffic(street.isNoThruTraffic()); + temporaryPartialStreetEdge.setStreetClass(street.getStreetClass()); + } else { + TemporaryPartialStreetEdge temporaryPartialStreetEdge = new TemporaryPartialStreetEdge( + street, base, tov, geometries.second, name, lengthOut); + + temporaryPartialStreetEdge.setStreetClass(street.getStreetClass()); + temporaryPartialStreetEdge.setNoThruTraffic(street.isNoThruTraffic()); + } + } + + private static P2 getGeometry(StreetEdge e, Coordinate nearestPoint) { + LineString geometry = e.getGeometry(); + return GeometryUtils.splitGeometryAtPoint(geometry, nearestPoint); + } + + @SuppressWarnings("rawtypes") + private void postSetup() { + for (Vertex gv : graph.getVertices()) { + Vertex v = gv; + /* + * We add all edges with geometry, skipping transit, filtering them out after. We do not + * index transit edges as we do not need them and some GTFS do not have shape data, so + * long straight lines between 2 faraway stations will wreck performance on a hash grid + * spatial index. + * + * If one need to store transit edges in the index, we could improve the hash grid + * rasterizing splitting long segments. + */ + synchronized (this) { + // place a lock here in order to ensure writes are thread-safe to the edgeTree + for (Edge e : gv.getOutgoing()) { + if (e instanceof PatternEdge || e instanceof SimpleTransfer) + continue; + LineString geometry = e.getGeometry(); + if (geometry == null) { + continue; + } + edgeTree.insert(geometry, e); + } + } + if (v instanceof TransitStop) { + Envelope env = new Envelope(v.getCoordinate()); + transitStopTree.insert(env, v); + } + Envelope env = new Envelope(v.getCoordinate()); + verticesTree.insert(env, v); + } + } + + /** + * Get all transit stops within a given distance of a coordinate + */ + public List getNearbyTransitStops(Coordinate coordinate, double radius) { + Envelope env = new Envelope(coordinate); + env.expandBy(SphericalDistanceLibrary.metersToLonDegrees(radius, coordinate.y), + SphericalDistanceLibrary.metersToDegrees(radius)); + List nearby = getTransitStopForEnvelope(env); + List results = new ArrayList(); + for (TransitStop v : nearby) { + if (SphericalDistanceLibrary.distance(v.getCoordinate(), coordinate) <= radius) { + results.add(v); + } + } + return results; + } /** * Returns the vertices intersecting with the specified envelope. - * + * * @param envelope * @return */ - Collection getVerticesForEnvelope(Envelope envelope); + @SuppressWarnings("unchecked") + public List getVerticesForEnvelope(Envelope envelope) { + List vertices = verticesTree.query(envelope); + // Here we assume vertices list modifiable + for (Iterator iv = vertices.iterator(); iv.hasNext();) { + Vertex v = iv.next(); + if (!envelope.contains(new Coordinate(v.getLon(), v.getLat()))) + iv.remove(); + } + return vertices; + } /** * Return the edges whose geometry intersect with the specified envelope. Warning: edges w/o * geometry will not be indexed. - * + * * @param envelope * @return */ - Collection getEdgesForEnvelope(Envelope envelope); + @SuppressWarnings("unchecked") + public Collection getEdgesForEnvelope(Envelope envelope) { + List edges = edgeTree.query(envelope); + for (Iterator ie = edges.iterator(); ie.hasNext();) { + Edge e = ie.next(); + Envelope eenv = e.getGeometry().getEnvelopeInternal(); + //Envelope eenv = e.getEnvelope(); + if (!envelope.intersects(eenv)) + ie.remove(); + } + return edges; + } /** - * @param coordinate - * @param radiusMeters - * @return The transit stops within a certain radius of the given location. + * @param envelope + * @return The transit stops within an envelope. */ - List getNearbyTransitStops(Coordinate coordinate, double radiusMeters); + @SuppressWarnings("unchecked") + public List getTransitStopForEnvelope(Envelope envelope) { + List transitStops = transitStopTree.query(envelope); + for (Iterator its = transitStops.iterator(); its.hasNext();) { + TransitStop ts = its.next(); + if (!envelope.intersects(new Coordinate(ts.getLon(), ts.getLat()))) + its.remove(); + } + return transitStops; + } /** - * @param envelope - * @return The transit stops within an envelope. + * @param coordinate Location to search intersection at. Look in a MAX_CORNER_DISTANCE_METERS radius. + * @return The nearest intersection, null if none found. */ - List getTransitStopForEnvelope(Envelope envelope); + public StreetVertex getIntersectionAt(Coordinate coordinate) { + double dLon = SphericalDistanceLibrary.metersToLonDegrees(MAX_CORNER_DISTANCE_METERS, + coordinate.y); + double dLat = SphericalDistanceLibrary.metersToDegrees(MAX_CORNER_DISTANCE_METERS); + Envelope envelope = new Envelope(coordinate); + envelope.expandBy(dLon, dLat); + List nearby = getVerticesForEnvelope(envelope); + StreetVertex nearest = null; + double bestDistanceMeter = Double.POSITIVE_INFINITY; + for (Vertex v : nearby) { + if (v instanceof StreetVertex) { + v.getLabel().startsWith("osm:"); + double distanceMeter = SphericalDistanceLibrary.fastDistance(coordinate, v.getCoordinate()); + if (distanceMeter < MAX_CORNER_DISTANCE_METERS) { + if (distanceMeter < bestDistanceMeter) { + bestDistanceMeter = distanceMeter; + nearest = (StreetVertex) v; + } + } + } + } + return nearest; + } /** * Finds the appropriate vertex for this location. - * - * @param place + * + * @param loc * @param options * @param endVertex: whether this is a start vertex (if it's false) or end vertex (if it's true) * @return */ - Vertex getVertexForLocation(GenericLocation place, RoutingRequest options, boolean endVertex); + public Vertex getVertexForLocation(GenericLocation loc, RoutingRequest options, + boolean endVertex) { + Coordinate c = loc.getCoordinate(); + if (c != null) { + return streetSplitter.linkOriginDestination(loc, options, endVertex); + } + + // No Coordinate available. + String place = loc.place; + if (place == null) { + return null; + } + + // did not match lat/lon, interpret place as a vertex label. + // this should probably only be used in tests, + // though it does allow routing from stop to stop. + return graph.getVertex(place); + } + + @Override + public String toString() { + return getClass().getName() + " -- edgeTree: " + edgeTree.toString() + " -- verticesTree: " + verticesTree.toString(); + } /** * Get a vertex at a given coordinate, using the same logic as in Samples. Used in Analyst * so that origins and destinations are linked the same way. */ - Vertex getSampleVertexAt(Coordinate coordinate, boolean dest); + public Vertex getSampleVertexAt(Coordinate coordinate, boolean dest) { + SampleFactory sfac = graph.getSampleFactory(); + + Sample s = sfac.getSample(coordinate.x, coordinate.y); + + if (s == null) + return null; + + // create temp vertex + SampleVertex v = new SampleVertex(graph, coordinate); + + // create edges + if (dest) { + if (s.v0 != null) + new SampleEdge(s.v0, v, s.d0); + + if (s.v1 != null) + new SampleEdge(s.v1, v, s.d1); + } + else { + if (s.v0 != null) + new SampleEdge(v, s.v0, s.d0); + + if (s.v1 != null) + new SampleEdge(v, s.v1, s.d1); + } + + return v; + } /** * Getter method for StreetSplitter * * @return StreetSplitter */ - StreetSplitter getStreetSplitter(); + public StreetSplitter getStreetSplitter() { + return streetSplitter; + } } diff --git a/src/main/java/org/opentripplanner/standalone/OTPMain.java b/src/main/java/org/opentripplanner/standalone/OTPMain.java index 23e6dee93c3..5f2eb3f3db9 100644 --- a/src/main/java/org/opentripplanner/standalone/OTPMain.java +++ b/src/main/java/org/opentripplanner/standalone/OTPMain.java @@ -9,7 +9,6 @@ import org.opentripplanner.common.MavenVersion; import org.opentripplanner.graph_builder.GraphBuilder; import org.opentripplanner.routing.graph.Graph; -import org.opentripplanner.routing.impl.DefaultStreetVertexIndexFactory; import org.opentripplanner.routing.impl.GraphScanner; import org.opentripplanner.routing.impl.InputStreamGraphSource; import org.opentripplanner.routing.impl.MemoryGraphSource; @@ -104,7 +103,7 @@ public boolean run() { /* If requested, hand off the graph to the server as the default graph using an in-memory GraphSource. */ if (params.inMemory || params.preFlight) { Graph graph = graphBuilder.getGraph(); - graph.index(new DefaultStreetVertexIndexFactory()); + graph.index(true); // FIXME set true router IDs graphService.registerGraph("", new MemoryGraphSource("", graph)); } diff --git a/src/test/java/org/opentripplanner/GtfsTest.java b/src/test/java/org/opentripplanner/GtfsTest.java index 34a802c6fa0..c35f7d7ab8f 100644 --- a/src/test/java/org/opentripplanner/GtfsTest.java +++ b/src/test/java/org/opentripplanner/GtfsTest.java @@ -23,7 +23,6 @@ import org.opentripplanner.routing.core.TraverseModeSet; import org.opentripplanner.routing.graph.Graph; import org.opentripplanner.routing.impl.AlertPatchServiceImpl; -import org.opentripplanner.routing.impl.DefaultStreetVertexIndexFactory; import org.opentripplanner.routing.impl.GraphPathFinder; import org.opentripplanner.routing.spt.GraphPath; import org.opentripplanner.standalone.Router; @@ -71,7 +70,7 @@ protected void setUp() { // Set the agency ID to be used for tests to the first one in the feed. agencyId = graph.getAgencies(feedId.getId()).iterator().next().getId(); System.out.printf("Set the agency ID for this test to %s\n", agencyId); - graph.index(new DefaultStreetVertexIndexFactory()); + graph.index(false); timetableSnapshotSource = new TimetableSnapshotSource(graph); timetableSnapshotSource.purgeExpiredData = (false); graph.timetableSnapshotSource = (timetableSnapshotSource); diff --git a/src/test/java/org/opentripplanner/graph_builder/linking/StreetSplitterTest.java b/src/test/java/org/opentripplanner/graph_builder/linking/StreetSplitterTest.java index 7b7768fa861..2aa60201e71 100644 --- a/src/test/java/org/opentripplanner/graph_builder/linking/StreetSplitterTest.java +++ b/src/test/java/org/opentripplanner/graph_builder/linking/StreetSplitterTest.java @@ -16,7 +16,6 @@ import org.opentripplanner.routing.edgetype.StreetTraversalPermission; import org.opentripplanner.routing.graph.Graph; import org.opentripplanner.routing.graph.Vertex; -import org.opentripplanner.routing.impl.DefaultStreetVertexIndexFactory; import org.opentripplanner.routing.vertextype.BikeRentalStationVertex; import org.opentripplanner.routing.vertextype.IntersectionVertex; import org.opentripplanner.routing.vertextype.StreetVertex; @@ -76,7 +75,7 @@ public void canLinkToEdgeSplitForBikeRental() { createStreetEdge(a, b, "a -> b"); createStreetEdge(b, a, "b -> a"); createStreetEdge(a, c, "a -> c"); - g.index(new DefaultStreetVertexIndexFactory()); + g.index(false); // When - a bike rental station between A and B that has been inserted into the graph BikeRentalStation brstation = new BikeRentalStation(); diff --git a/src/test/java/org/opentripplanner/graph_builder/module/FakeGraph.java b/src/test/java/org/opentripplanner/graph_builder/module/FakeGraph.java index a65fb593230..f6da9b83776 100644 --- a/src/test/java/org/opentripplanner/graph_builder/module/FakeGraph.java +++ b/src/test/java/org/opentripplanner/graph_builder/module/FakeGraph.java @@ -8,7 +8,6 @@ import org.opentripplanner.model.Stop; import org.opentripplanner.openstreetmap.impl.AnyFileBasedOpenStreetMapProviderImpl; import org.opentripplanner.routing.graph.Graph; -import org.opentripplanner.routing.impl.DefaultStreetVertexIndexFactory; import org.opentripplanner.routing.vertextype.TransitStop; import java.io.File; @@ -136,7 +135,7 @@ public static void addExtraStops (Graph g) { */ public static void indexGraphAndLinkStations (Graph g) { // creating a new DefaultStreetVertexIndexFactory results in setting the streetIndex on the graph. - g.index(new DefaultStreetVertexIndexFactory()); + g.index(false); StreetSplitter linker = (StreetSplitter) g.streetIndex.getStreetSplitter(); linker.linkAllStationsToGraph(); } diff --git a/src/test/java/org/opentripplanner/graph_builder/module/GtfsGraphBuilderModuleTest.java b/src/test/java/org/opentripplanner/graph_builder/module/GtfsGraphBuilderModuleTest.java index 213936592b0..e27197c7c26 100644 --- a/src/test/java/org/opentripplanner/graph_builder/module/GtfsGraphBuilderModuleTest.java +++ b/src/test/java/org/opentripplanner/graph_builder/module/GtfsGraphBuilderModuleTest.java @@ -17,7 +17,6 @@ import org.opentripplanner.gtfs.BikeAccess; import org.opentripplanner.routing.edgetype.TripPattern; import org.opentripplanner.routing.graph.Graph; -import org.opentripplanner.routing.impl.DefaultStreetVertexIndexFactory; public class GtfsGraphBuilderModuleTest { @@ -39,7 +38,7 @@ public void testNoBikesByDefault() throws IOException { Graph graph = new Graph(); builder.buildGraph(graph, _extra); - graph.index(new DefaultStreetVertexIndexFactory()); + graph.index(true); // Feed id is used instead of the agency id for OBA entities. GtfsBundle gtfsBundle = bundleList.get(0); @@ -68,7 +67,7 @@ public void testBikesByDefault() throws IOException { Graph graph = new Graph(); builder.buildGraph(graph, _extra); - graph.index(new DefaultStreetVertexIndexFactory()); + graph.index(true); // Feed id is used instead of the agency id for OBA entities. GtfsBundle gtfsBundle = bundleList.get(0); diff --git a/src/test/java/org/opentripplanner/routing/TestHalfEdges.java b/src/test/java/org/opentripplanner/routing/TestHalfEdges.java index bbaf73cc3bc..5a615dd4ea6 100644 --- a/src/test/java/org/opentripplanner/routing/TestHalfEdges.java +++ b/src/test/java/org/opentripplanner/routing/TestHalfEdges.java @@ -25,8 +25,8 @@ import org.opentripplanner.routing.graph.Edge; import org.opentripplanner.routing.graph.Graph; import org.opentripplanner.routing.graph.Vertex; -import org.opentripplanner.routing.impl.StreetVertexIndexServiceImpl; import org.opentripplanner.routing.location.TemporaryStreetLocation; +import org.opentripplanner.routing.services.StreetVertexIndexService; import org.opentripplanner.routing.services.notes.StreetNotesService; import org.opentripplanner.routing.spt.GraphPath; import org.opentripplanner.routing.spt.ShortestPathTree; @@ -140,7 +140,7 @@ public void testHalfEdges() { turns.add(left); turns.add(leftBack); - TemporaryStreetLocation start = StreetVertexIndexServiceImpl.createTemporaryStreetLocation(graph, "start", new NonLocalizedString("start"), + TemporaryStreetLocation start = StreetVertexIndexService.createTemporaryStreetLocation(graph, "start", new NonLocalizedString("start"), filter(turns, StreetEdge.class), new LinearLocation(0, 0.4).getCoordinate(left.getGeometry()), false); @@ -148,7 +148,7 @@ public void testHalfEdges() { endTurns.add(right); endTurns.add(rightBack); - TemporaryStreetLocation end = StreetVertexIndexServiceImpl.createTemporaryStreetLocation(graph, "end", new NonLocalizedString("end"), + TemporaryStreetLocation end = StreetVertexIndexService.createTemporaryStreetLocation(graph, "end", new NonLocalizedString("end"), filter(endTurns, StreetEdge.class), new LinearLocation(0, 0.8).getCoordinate(right.getGeometry()), true); @@ -212,10 +212,10 @@ public void testHalfEdges() { */ options = new RoutingRequest(new TraverseModeSet(TraverseMode.BICYCLE)); - start = StreetVertexIndexServiceImpl.createTemporaryStreetLocation(graph, "start1", new NonLocalizedString("start1"), + start = StreetVertexIndexService.createTemporaryStreetLocation(graph, "start1", new NonLocalizedString("start1"), filter(turns, StreetEdge.class), new LinearLocation(0, 0.95).getCoordinate(top.getGeometry()), false); - end = StreetVertexIndexServiceImpl.createTemporaryStreetLocation(graph, "end1", new NonLocalizedString("end1"), + end = StreetVertexIndexService.createTemporaryStreetLocation(graph, "end1", new NonLocalizedString("end1"), filter(turns, StreetEdge.class), new LinearLocation(0, 0.95).getCoordinate(bottom.getGeometry()), true); @@ -236,10 +236,10 @@ public void testHalfEdges() { assertEquals(nVertices, graph.getVertices().size()); assertEquals(nEdges, graph.getEdges().size()); - start = StreetVertexIndexServiceImpl.createTemporaryStreetLocation(graph, "start2", new NonLocalizedString("start2"), + start = StreetVertexIndexService.createTemporaryStreetLocation(graph, "start2", new NonLocalizedString("start2"), filter(turns, StreetEdge.class), new LinearLocation(0, 0.55).getCoordinate(top.getGeometry()), false); - end = StreetVertexIndexServiceImpl.createTemporaryStreetLocation(graph, "end2", new NonLocalizedString("end2"), + end = StreetVertexIndexService.createTemporaryStreetLocation(graph, "end2", new NonLocalizedString("end2"), filter(turns, StreetEdge.class), new LinearLocation(0, 0.55).getCoordinate(bottom.getGeometry()), true); @@ -269,11 +269,11 @@ public void testRouteToSameEdge() { turns.add(left); turns.add(leftBack); - TemporaryStreetLocation start = StreetVertexIndexServiceImpl.createTemporaryStreetLocation(graph, "start", new NonLocalizedString("start"), + TemporaryStreetLocation start = StreetVertexIndexService.createTemporaryStreetLocation(graph, "start", new NonLocalizedString("start"), filter(turns, StreetEdge.class), new LinearLocation(0, 0.4).getCoordinate(left.getGeometry()), false); - TemporaryStreetLocation end = StreetVertexIndexServiceImpl.createTemporaryStreetLocation(graph, "end", new NonLocalizedString("end"), + TemporaryStreetLocation end = StreetVertexIndexService.createTemporaryStreetLocation(graph, "end", new NonLocalizedString("end"), filter(turns, StreetEdge.class), new LinearLocation(0, 0.8).getCoordinate(left.getGeometry()), true); @@ -304,11 +304,11 @@ public void testRouteToSameEdgeBackwards() { HashSet turns = new HashSet(); turns.add(left); - TemporaryStreetLocation start = StreetVertexIndexServiceImpl.createTemporaryStreetLocation(graph, "start", new NonLocalizedString("start"), + TemporaryStreetLocation start = StreetVertexIndexService.createTemporaryStreetLocation(graph, "start", new NonLocalizedString("start"), filter(turns, StreetEdge.class), new LinearLocation(0, 0.8).getCoordinate(left.getGeometry()), false); - TemporaryStreetLocation end = StreetVertexIndexServiceImpl.createTemporaryStreetLocation(graph, "end", new NonLocalizedString("end"), + TemporaryStreetLocation end = StreetVertexIndexService.createTemporaryStreetLocation(graph, "end", new NonLocalizedString("end"), filter(turns, StreetEdge.class), new LinearLocation(0, 0.4).getCoordinate(left.getGeometry()), true); @@ -347,7 +347,7 @@ public void testStreetSplittingAlerts() { graph.streetNotesService.addStaticNote(left, alert, StreetNotesService.ALWAYS_MATCHER); graph.streetNotesService.addStaticNote(leftBack, alert, StreetNotesService.ALWAYS_MATCHER); - TemporaryStreetLocation start = StreetVertexIndexServiceImpl.createTemporaryStreetLocation(graph, "start", new NonLocalizedString("start"), + TemporaryStreetLocation start = StreetVertexIndexService.createTemporaryStreetLocation(graph, "start", new NonLocalizedString("start"), filter(turns, StreetEdge.class), new LinearLocation(0, 0.4).getCoordinate(left.getGeometry()), false); @@ -383,7 +383,7 @@ public void testStreetSplittingAlerts() { req.setWheelchairAccessible(true); - start = StreetVertexIndexServiceImpl.createTemporaryStreetLocation(graph, "start", new NonLocalizedString("start"), + start = StreetVertexIndexService.createTemporaryStreetLocation(graph, "start", new NonLocalizedString("start"), filter(turns, StreetEdge.class), new LinearLocation(0, 0.4).getCoordinate(left.getGeometry()), false); @@ -403,7 +403,7 @@ public void testStreetSplittingAlerts() { @Test public void testStreetLocationFinder() { - StreetVertexIndexServiceImpl finder = new StreetVertexIndexServiceImpl(graph); + StreetVertexIndexService finder = new StreetVertexIndexService(graph); // test that the local stop finder finds stops GenericLocation loc = new GenericLocation(40.01, -74.005000001); assertTrue(finder.getNearbyTransitStops(loc.getCoordinate(), 100).size() > 0); diff --git a/src/test/java/org/opentripplanner/routing/alertpatch/AlertPatchTest.java b/src/test/java/org/opentripplanner/routing/alertpatch/AlertPatchTest.java index 32f7ed59e38..ca0aaaa5fb5 100644 --- a/src/test/java/org/opentripplanner/routing/alertpatch/AlertPatchTest.java +++ b/src/test/java/org/opentripplanner/routing/alertpatch/AlertPatchTest.java @@ -19,7 +19,6 @@ import org.opentripplanner.routing.graph.Edge; import org.opentripplanner.routing.graph.Graph; import org.opentripplanner.routing.graph.Vertex; -import org.opentripplanner.routing.impl.DefaultStreetVertexIndexFactory; import org.opentripplanner.routing.spt.GraphPath; import org.opentripplanner.routing.spt.ShortestPathTree; import org.opentripplanner.util.TestUtils; @@ -46,7 +45,7 @@ public void setUp() throws Exception { CalendarServiceData.class, createCalendarServiceData(context.getOtpTransitService()) ); - graph.index(new DefaultStreetVertexIndexFactory()); + graph.index(false); feedId = context.getFeedId().getId(); } diff --git a/src/test/java/org/opentripplanner/routing/core/RoutingContextDestroyTest.java b/src/test/java/org/opentripplanner/routing/core/RoutingContextDestroyTest.java index 2388f4ed153..c7e8e189fcf 100644 --- a/src/test/java/org/opentripplanner/routing/core/RoutingContextDestroyTest.java +++ b/src/test/java/org/opentripplanner/routing/core/RoutingContextDestroyTest.java @@ -14,7 +14,6 @@ import org.opentripplanner.routing.graph.Edge; import org.opentripplanner.routing.graph.Graph; import org.opentripplanner.routing.graph.Vertex; -import org.opentripplanner.routing.impl.DefaultStreetVertexIndexFactory; import org.opentripplanner.routing.vertextype.IntersectionVertex; import org.opentripplanner.routing.vertextype.StreetVertex; import org.opentripplanner.routing.vertextype.TemporaryVertex; @@ -55,7 +54,7 @@ public class RoutingContextDestroyTest { createStreetEdge(a, b, "a -> b"); createStreetEdge(b, a, "b -> a"); createStreetEdge(a, c, "a -> c"); - g.index(new DefaultStreetVertexIndexFactory()); + g.index(false); } @Test public void temporaryChangesRemovedOnContextDestroy() { diff --git a/src/test/java/org/opentripplanner/routing/core/StateEditorTest.java b/src/test/java/org/opentripplanner/routing/core/StateEditorTest.java index caff2414d3b..39c2f3792bf 100644 --- a/src/test/java/org/opentripplanner/routing/core/StateEditorTest.java +++ b/src/test/java/org/opentripplanner/routing/core/StateEditorTest.java @@ -2,7 +2,6 @@ import org.junit.Test; import org.opentripplanner.routing.graph.Graph; -import org.opentripplanner.routing.impl.DefaultStreetVertexIndexFactory; import static org.junit.Assert.assertEquals; @@ -28,7 +27,7 @@ public final void testSetNonTransitOptionsFromState(){ request.setMode(TraverseMode.CAR); request.parkAndRide = true; Graph graph = new Graph(); - graph.index(new DefaultStreetVertexIndexFactory()); + graph.index(false); request.rctx = new RoutingContext(request, graph); State state = new State(request); diff --git a/src/test/java/org/opentripplanner/routing/core/TestTransfers.java b/src/test/java/org/opentripplanner/routing/core/TestTransfers.java index 6ccb55fca0f..ba18734f141 100644 --- a/src/test/java/org/opentripplanner/routing/core/TestTransfers.java +++ b/src/test/java/org/opentripplanner/routing/core/TestTransfers.java @@ -34,7 +34,6 @@ import org.opentripplanner.routing.edgetype.factory.PatternHopFactory; import org.opentripplanner.routing.graph.Graph; import org.opentripplanner.routing.graph.Vertex; -import org.opentripplanner.routing.impl.DefaultStreetVertexIndexFactory; import org.opentripplanner.routing.spt.GraphPath; import org.opentripplanner.routing.spt.ShortestPathTree; import org.opentripplanner.routing.trippattern.TripTimes; @@ -76,7 +75,7 @@ public Context() throws IOException { graph = spy(new Graph()); PatternHopFactory factory = new PatternHopFactory(context); factory.run(graph); - graph.index(new DefaultStreetVertexIndexFactory()); + graph.index(false); graph.putService( CalendarServiceData.class, createCalendarServiceData(context.getOtpTransitService()) diff --git a/src/test/java/org/opentripplanner/routing/edgetype/TemporaryPartialStreetEdgeTest.java b/src/test/java/org/opentripplanner/routing/edgetype/TemporaryPartialStreetEdgeTest.java index af6dcaacfef..023d5860db3 100644 --- a/src/test/java/org/opentripplanner/routing/edgetype/TemporaryPartialStreetEdgeTest.java +++ b/src/test/java/org/opentripplanner/routing/edgetype/TemporaryPartialStreetEdgeTest.java @@ -17,8 +17,8 @@ import org.opentripplanner.routing.core.TraverseMode; import org.opentripplanner.routing.graph.Edge; import org.opentripplanner.routing.graph.Graph; -import org.opentripplanner.routing.impl.StreetVertexIndexServiceImpl; import org.opentripplanner.routing.location.TemporaryStreetLocation; +import org.opentripplanner.routing.services.StreetVertexIndexService; import org.opentripplanner.routing.vertextype.IntersectionVertex; import org.opentripplanner.routing.vertextype.StreetVertex; @@ -102,9 +102,9 @@ public void testTraversalOfSubdividedEdge() { Coordinate nearestPoint = new Coordinate(0.5, 2.0); List edges = new ArrayList(); edges.add(e2); - TemporaryStreetLocation end = StreetVertexIndexServiceImpl.createTemporaryStreetLocation( + TemporaryStreetLocation end = StreetVertexIndexService.createTemporaryStreetLocation( graph, "middle of e2", new NonLocalizedString("foo"), edges, nearestPoint, true); - TemporaryStreetLocation start = StreetVertexIndexServiceImpl.createTemporaryStreetLocation( + TemporaryStreetLocation start = StreetVertexIndexService.createTemporaryStreetLocation( graph, "middle of e2", new NonLocalizedString("foo"), edges, nearestPoint, false); RoutingRequest options = new RoutingRequest(); diff --git a/src/test/java/org/opentripplanner/updater/stoptime/TimetableSnapshotSourceTest.java b/src/test/java/org/opentripplanner/updater/stoptime/TimetableSnapshotSourceTest.java index 617b6218f43..e8ab67e87d1 100644 --- a/src/test/java/org/opentripplanner/updater/stoptime/TimetableSnapshotSourceTest.java +++ b/src/test/java/org/opentripplanner/updater/stoptime/TimetableSnapshotSourceTest.java @@ -39,7 +39,6 @@ import org.opentripplanner.routing.edgetype.factory.PatternHopFactory; import org.opentripplanner.routing.graph.Edge; import org.opentripplanner.routing.graph.Graph; -import org.opentripplanner.routing.impl.DefaultStreetVertexIndexFactory; import org.opentripplanner.routing.trippattern.RealTimeState; import org.opentripplanner.routing.trippattern.TripTimes; import org.opentripplanner.routing.vertextype.TransitStopDepart; @@ -96,7 +95,7 @@ public static void setUpClass() throws Exception { PatternHopFactory factory = new PatternHopFactory(context); factory.run(graph); - graph.index(new DefaultStreetVertexIndexFactory()); + graph.index(false); final TripDescriptor.Builder tripDescriptorBuilder = TripDescriptor.newBuilder(); From 104af87ca46d5c3210ef8e37188c33a3cc499c71 Mon Sep 17 00:00:00 2001 From: Evan Siroky Date: Thu, 29 Aug 2019 00:25:38 -0700 Subject: [PATCH 11/12] Add more comments about calling of graph.index method. --- .../java/org/opentripplanner/api/resource/Routers.java | 3 +++ .../graph_builder/module/DirectTransferGenerator.java | 4 +++- .../graph_builder/module/NearbyStopFinder.java | 1 - .../graph_builder/module/StreetLinkerModule.java | 3 +++ .../graph_builder/module/TransitToTaggedStopsModule.java | 3 +++ .../graph_builder/module/map/BusRouteStreetMatcher.java | 4 +++- .../java/org/opentripplanner/routing/graph/Graph.java | 9 +++++++-- .../routing/services/StreetVertexIndexService.java | 4 ++++ .../java/org/opentripplanner/standalone/OTPMain.java | 3 +++ .../opentripplanner/graph_builder/module/FakeGraph.java | 5 +++-- 10 files changed, 32 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/opentripplanner/api/resource/Routers.java b/src/main/java/org/opentripplanner/api/resource/Routers.java index a01c322de5a..c3b318a5044 100644 --- a/src/main/java/org/opentripplanner/api/resource/Routers.java +++ b/src/main/java/org/opentripplanner/api/resource/Routers.java @@ -270,6 +270,9 @@ public Response buildGraphOverWire ( tempDir.delete(); Graph graph = graphBuilder.getGraph(); + // re-index the graph to ensure all data is added and recreate a new streetIndex. It's ok to recreate the + // streetIndex because the previous one created during graph build is not needed anymore and isn't able to be + // used outside of the graphBuilder. graph.index(true); GraphService graphService = otpServer.getGraphService(); diff --git a/src/main/java/org/opentripplanner/graph_builder/module/DirectTransferGenerator.java b/src/main/java/org/opentripplanner/graph_builder/module/DirectTransferGenerator.java index ca9d04704ef..36951c82a9a 100644 --- a/src/main/java/org/opentripplanner/graph_builder/module/DirectTransferGenerator.java +++ b/src/main/java/org/opentripplanner/graph_builder/module/DirectTransferGenerator.java @@ -47,7 +47,9 @@ public DirectTransferGenerator (double radiusMeters) { @Override public void buildGraph(Graph graph, HashMap, Object> extra) { - /* Initialize or recalculate the graph index which is needed by the nearby stop finder. */ + // Make sure the graph index has been initialized because it is needed by the nearby stop finder. Don't + // recalculate the street index because it is only used for finding nearby transit stops which should already + // have been indexed properly in a previous build module. graph.index(false); /* The linker will use streets if they are available, or straight-line distance otherwise. */ diff --git a/src/main/java/org/opentripplanner/graph_builder/module/NearbyStopFinder.java b/src/main/java/org/opentripplanner/graph_builder/module/NearbyStopFinder.java index 64d957adc72..7aec5bf1c3a 100644 --- a/src/main/java/org/opentripplanner/graph_builder/module/NearbyStopFinder.java +++ b/src/main/java/org/opentripplanner/graph_builder/module/NearbyStopFinder.java @@ -66,7 +66,6 @@ public NearbyStopFinder(Graph graph, double radiusMeters) { // but we don't have much of a choice here. Use the default walking speed to convert. earliestArrivalSearch.maxDuration = (int) (radiusMeters / new RoutingRequest().walkSpeed); } else { - graph.index(false); streetIndex = graph.streetIndex; } } diff --git a/src/main/java/org/opentripplanner/graph_builder/module/StreetLinkerModule.java b/src/main/java/org/opentripplanner/graph_builder/module/StreetLinkerModule.java index 4c6af90a6ed..d952b108a44 100644 --- a/src/main/java/org/opentripplanner/graph_builder/module/StreetLinkerModule.java +++ b/src/main/java/org/opentripplanner/graph_builder/module/StreetLinkerModule.java @@ -44,6 +44,9 @@ public List getPrerequisites() { public void buildGraph(Graph graph, HashMap, Object> extra) { if(graph.hasStreets) { LOG.info("Linking transit stops, bike rental stations, bike parking areas, and park-and-rides to graph . . ."); + // Make sure the graph index has been initialized. Don't recalculate the street index because it should + // already have been initialized in a previous build module and should have all needed street data for + // splitting StreetEdges. graph.index(false); StreetSplitter linker = graph.streetIndex.getStreetSplitter(); linker.setAddExtraEdgesToAreas(this.addExtraEdgesToAreas); diff --git a/src/main/java/org/opentripplanner/graph_builder/module/TransitToTaggedStopsModule.java b/src/main/java/org/opentripplanner/graph_builder/module/TransitToTaggedStopsModule.java index 5da44223ccf..8a6d5fdbef8 100644 --- a/src/main/java/org/opentripplanner/graph_builder/module/TransitToTaggedStopsModule.java +++ b/src/main/java/org/opentripplanner/graph_builder/module/TransitToTaggedStopsModule.java @@ -52,6 +52,9 @@ public List getPrerequisites() { public void buildGraph(Graph graph, HashMap, Object> extra) { LOG.info("Linking transit stops to tagged bus stops..."); + // Make sure the graph index has been initialized. Don't recalculate the street index because it should + // already have been initialized in a previous build module and should have all needed vertex data for + // spatially querying vertices. graph.index(false); index = graph.streetIndex; diff --git a/src/main/java/org/opentripplanner/graph_builder/module/map/BusRouteStreetMatcher.java b/src/main/java/org/opentripplanner/graph_builder/module/map/BusRouteStreetMatcher.java index e24e0881267..324193e29e0 100644 --- a/src/main/java/org/opentripplanner/graph_builder/module/map/BusRouteStreetMatcher.java +++ b/src/main/java/org/opentripplanner/graph_builder/module/map/BusRouteStreetMatcher.java @@ -45,7 +45,9 @@ NetworkLinkerLibrary later (actually in LinkRequests). */ public void buildGraph(Graph graph, HashMap, Object> extra) { - //Mapbuilder needs transit index + // Make sure the graph index has been initialized. As of a refactor in 2019, this is the first place in the + // build process that an index is created, but just in case future build modules are added later, send over + // false for indexing a graph just in case. graph.index(false); StreetMatcher matcher = new StreetMatcher(graph); diff --git a/src/main/java/org/opentripplanner/routing/graph/Graph.java b/src/main/java/org/opentripplanner/routing/graph/Graph.java index ec0b5afda88..d16298ba212 100644 --- a/src/main/java/org/opentripplanner/routing/graph/Graph.java +++ b/src/main/java/org/opentripplanner/routing/graph/Graph.java @@ -695,11 +695,14 @@ public static Graph load(File file) throws IOException { * * However, due to the potential for problems to arise with two StreetVertexIndexServices and StreetSplitters being * active at once, a warning is logged just in case. + * + * @param recreateStreetIndex if true, recreate the streetIndex even if it already exists. Make sure you know what + * you're doing when setting this to true! See above method documentation. */ public void index (boolean recreateStreetIndex) { if (streetIndex == null || recreateStreetIndex) { if (streetIndex != null && recreateStreetIndex) { - LOG.warn("Overwritting an existing streetIndex! This could lead to problems if both the old and new streetIndex are used. Make sure only the new streetIndex is used going forward."); + LOG.warn("Overwriting an existing streetIndex! This could lead to problems if both the old and new streetIndex are used. Make sure only the new streetIndex is used going forward."); } LOG.info("building street index"); streetIndex = new StreetVertexIndexService(this); @@ -750,7 +753,9 @@ public static Graph load(InputStream in) { } LOG.info("Main graph read. |V|={} |E|={}", graph.countVertices(), graph.countEdges()); - graph.index(false); + // Make sure the graph index has been initialized. The streetIndex should be able to be safely recreated + // because the streetIndexes used during build will not be visible outside of this method. + graph.index(true); return graph; } diff --git a/src/main/java/org/opentripplanner/routing/services/StreetVertexIndexService.java b/src/main/java/org/opentripplanner/routing/services/StreetVertexIndexService.java index 63980fa2871..b8b1c51cadd 100644 --- a/src/main/java/org/opentripplanner/routing/services/StreetVertexIndexService.java +++ b/src/main/java/org/opentripplanner/routing/services/StreetVertexIndexService.java @@ -82,6 +82,10 @@ public class StreetVertexIndexService { * Construct a new StreetVertexIndexService using the given graph. Upon instantiation, a number of spatial indexes * will be created to be used for the quick lookup of data with spatial attributes. * + * After a refactor in the year 2019, this class was refactored to be just this class instead of the previous + * structure of having an interface and factory implementation. The StreetVertexIndexFactory was always being used + * to create the same StreetVertexIndexService implementation. + * * IMPORTANT NOTE: Only one StreetVertexIndexService should be active on a graph at any given time. This is because * any newly-created (or removed) vertices or edges in this StreetVertexIndexService won't show up in the indices of * the other StreetVertexIndexServices. diff --git a/src/main/java/org/opentripplanner/standalone/OTPMain.java b/src/main/java/org/opentripplanner/standalone/OTPMain.java index 5f2eb3f3db9..50d768c3ac9 100644 --- a/src/main/java/org/opentripplanner/standalone/OTPMain.java +++ b/src/main/java/org/opentripplanner/standalone/OTPMain.java @@ -103,6 +103,9 @@ public boolean run() { /* If requested, hand off the graph to the server as the default graph using an in-memory GraphSource. */ if (params.inMemory || params.preFlight) { Graph graph = graphBuilder.getGraph(); + // re-index the graph to ensure all data is added and recreate a new streetIndex. It's ok to + // recreate the streetIndex because the previous one created during graph build is not needed + // anymore and isn't able to be used outside of the graphBuilder. graph.index(true); // FIXME set true router IDs graphService.registerGraph("", new MemoryGraphSource("", graph)); diff --git a/src/test/java/org/opentripplanner/graph_builder/module/FakeGraph.java b/src/test/java/org/opentripplanner/graph_builder/module/FakeGraph.java index f6da9b83776..570c26b2acc 100644 --- a/src/test/java/org/opentripplanner/graph_builder/module/FakeGraph.java +++ b/src/test/java/org/opentripplanner/graph_builder/module/FakeGraph.java @@ -134,8 +134,9 @@ public static void addExtraStops (Graph g) { * Index the graph and then link all stations in the graph. */ public static void indexGraphAndLinkStations (Graph g) { - // creating a new DefaultStreetVertexIndexFactory results in setting the streetIndex on the graph. - g.index(false); + // recreate a new streetIndex even if one exists to ensure all indexes are built. It is assumed that all tests + // that use this method do not need any previously street indexes created during building the graph + g.index(true); StreetSplitter linker = (StreetSplitter) g.streetIndex.getStreetSplitter(); linker.linkAllStationsToGraph(); } From 3decd947a5e4b7138a7084c7622f7e0aed0d9445 Mon Sep 17 00:00:00 2001 From: Evan Siroky Date: Thu, 29 Aug 2019 11:38:55 -0700 Subject: [PATCH 12/12] Get build and tests working again --- src/main/java/org/opentripplanner/api/resource/Routers.java | 1 - .../opentripplanner/routing/impl/InputStreamGraphSource.java | 2 +- .../routing/graph/GraphSerializationTest.java | 5 +++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/opentripplanner/api/resource/Routers.java b/src/main/java/org/opentripplanner/api/resource/Routers.java index c3b318a5044..cf9224bc8ed 100644 --- a/src/main/java/org/opentripplanner/api/resource/Routers.java +++ b/src/main/java/org/opentripplanner/api/resource/Routers.java @@ -32,7 +32,6 @@ import org.opentripplanner.graph_builder.GraphBuilder; import org.opentripplanner.routing.error.GraphNotFoundException; import org.opentripplanner.routing.graph.Graph; -import org.opentripplanner.routing.graph.Graph.LoadLevel; import org.opentripplanner.routing.impl.MemoryGraphSource; import org.opentripplanner.routing.services.GraphService; import org.opentripplanner.standalone.CommandLineParameters; diff --git a/src/main/java/org/opentripplanner/routing/impl/InputStreamGraphSource.java b/src/main/java/org/opentripplanner/routing/impl/InputStreamGraphSource.java index 55bc5cc57eb..6739f3e7aa1 100644 --- a/src/main/java/org/opentripplanner/routing/impl/InputStreamGraphSource.java +++ b/src/main/java/org/opentripplanner/routing/impl/InputStreamGraphSource.java @@ -176,7 +176,7 @@ private Router loadGraph() { try (InputStream is = streams.getGraphInputStream()) { LOG.info("Loading graph..."); try { - newGraph = Graph.load(new ObjectInputStream(is), loadLevel); + newGraph = Graph.load(is); } catch (Exception ex) { LOG.error("Exception while loading graph '{}'.", routerId, ex); return null; diff --git a/src/test/java/org/opentripplanner/routing/graph/GraphSerializationTest.java b/src/test/java/org/opentripplanner/routing/graph/GraphSerializationTest.java index 23ce9bfe0a0..9757bd8e755 100644 --- a/src/test/java/org/opentripplanner/routing/graph/GraphSerializationTest.java +++ b/src/test/java/org/opentripplanner/routing/graph/GraphSerializationTest.java @@ -8,7 +8,6 @@ import org.junit.Test; import org.opentripplanner.ConstantsForTests; import org.opentripplanner.common.geometry.HashGridSpatialIndex; -import org.opentripplanner.routing.impl.DefaultStreetVertexIndexFactory; import org.opentripplanner.routing.trippattern.Deduplicator; import org.opentripplanner.routing.vertextype.TransitStation; @@ -78,7 +77,9 @@ public void compareGraphToItself () { // This graph does not make an ideal test because it doesn't have any street data. // TODO switch to another graph that has both GTFS and OSM data Graph originalGraph = ConstantsForTests.getInstance().getPortlandGraph(); - originalGraph.index(new DefaultStreetVertexIndexFactory()); + // Make sure the graph index has been initialized. Do not recreate the streetIndex as this graph is used in + // multiple tests and could have been created and used elsewhere. + originalGraph.index(false); // We can exclude relatively few classes here, because the object trees are of course perfectly identical. // We do skip edge lists - otherwise we trigger a depth-first search of the graph causing a stack overflow. // We also skip some deeply buried weak-value hash maps, which refuse to tell you what their keys are.