Splitter refactor

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)
 

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);
+}

src/main/java/org/opentripplanner/graph_builder/linking/TransitToStreetNetworkModule.java

@@ -33,7 +33,7 @@ public void buildGraph(Graph graph, HashMap<Class<?>, Object> extra) {
         //linker.createLinkage();
 
         SimpleStreetSplitter splitter = new SimpleStreetSplitter(graph);
-        splitter.link();
+        splitter.linkAllStationsToGraph();
 
         // don't split streets
         //SampleStopLinker linker = new SampleStopLinker(graph);

src/main/java/org/opentripplanner/graph_builder/module/StreetLinkerModule.java

@@ -46,7 +46,7 @@ public void buildGraph(Graph graph, HashMap<Class<?>, 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();

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<Edge>) edgeTree, transitStopTree, false);
+                    (HashGridSpatialIndex<Edge>) 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;
+    }
 }

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<Vertex> getVerticesForEnvelope(Envelope envelope);
+    Collection<Vertex> 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<Edge> getEdgesForEnvelope(Envelope envelope);
+    Collection<Edge> getEdgesForEnvelope(Envelope envelope);
 
     /**
      * @param coordinate
      * @param radiusMeters
      * @return The transit stops within a certain radius of the given location.
      */
-    public List<TransitStop> getNearbyTransitStops(Coordinate coordinate, double radiusMeters);
+    List<TransitStop> getNearbyTransitStops(Coordinate coordinate, double radiusMeters);
 
     /**
      * @param envelope
      * @return The transit stops within an envelope.
      */
-    public List<TransitStop> getTransitStopForEnvelope(Envelope envelope);
+    List<TransitStop> 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();
 }

src/main/java/org/opentripplanner/updater/bike_park/BikeParkUpdater.java

@@ -1,18 +1,16 @@
 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;
 import java.util.Map;
 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);

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<BikeRentalStation, BikeRentalStationVertex> verticesByStation = new HashMap<BikeRentalStation, BikeRentalStationVertex>();
+    Map<BikeRentalStation, BikeRentalStationVertex> 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<BikeRentalStation> stationSet = new HashSet<>();
             Set<String> 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);
                     }

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());
     }