- Code cleanup, move dispose class to the same as Temp and add JavaDoc.

- Replace recursion with a loop to avoid stack overflow.

src/main/java/org/opentripplanner/routing/core/RoutingContext.java

@@ -141,7 +141,7 @@ private Set<StreetEdge> overlappingStreetEdges(Vertex u, Vertex v) {
         for (Edge e : Iterables.concat(u.getIncoming(), u.getOutgoing())) {
             uIds.add(e.getId());
         }
-        
+
         // Intesection of edge IDs between u and v.
         uIds.retainAll(vIds);
         Set<Integer> overlappingIds = uIds;
@@ -229,7 +229,6 @@ private RoutingContext(RoutingRequest routingRequest, Graph graph, Vertex from,
         else
             this.streetSpeedSnapshot = null;
 
-
         Edge fromBackEdge = null;
         Edge toBackEdge = null;
         if (findPlaces) {
@@ -290,7 +289,7 @@ private RoutingContext(RoutingRequest routingRequest, Graph graph, Vertex from,
                 makePartialEdgeAlong(pse, fromStreetVertex, toStreetVertex);
             }
         }
-        
+
         if (opt.startingTransitStopId != null) {
             Stop stop = graph.index.stopForId.get(opt.startingTransitStopId);
             TransitStop tstop = graph.index.stopVertexForStop.get(stop);
@@ -404,27 +403,7 @@ public boolean isWheelchairAccessible(Vertex v) {
      * for garbage collection.
      */
     public void destroy() {
-        disposeTemporaryStart(fromVertex, null);
-        disposeTemporaryEnd(toVertex, null);
-    }
-
-    private static void disposeTemporaryStart(Vertex v, Edge incoming) {
-        if (v instanceof TemporaryVertex) {
-            for (Edge edge : v.getOutgoing()) {
-                disposeTemporaryStart(edge.getToVertex(), edge);
-            }
-        } else if (incoming != null) {
-            v.removeIncoming(incoming);
-        }
-    }
-
-    private static void disposeTemporaryEnd(Vertex v, Edge outgoing) {
-        if (v instanceof TemporaryVertex) {
-            for (Edge edge : v.getIncoming()) {
-                disposeTemporaryEnd(edge.getFromVertex(), edge);
-            }
-        } else if (outgoing != null) {
-            v.removeOutgoing(outgoing);
-        }
+        TemporaryVertex.dispose(fromVertex);
+        TemporaryVertex.dispose(toVertex);
     }
-}
+}

src/main/java/org/opentripplanner/routing/vertextype/TemporaryVertex.java

@@ -1,8 +1,26 @@
 package org.opentripplanner.routing.vertextype;
 
+import org.opentripplanner.routing.graph.Vertex;
+
 /**
- * Marker interface for temporary vertices
+ * Marker interface for temporary vertices.
+ * <p/>
+ * Remember to use the {@link #dispose(Vertex)} to delete the temporary vertex
+ * from the main graph after use.
  */
 public interface TemporaryVertex {
     boolean isEndVertex();
+
+    /**
+     * This method traverse the subgraph of temporary vertices, and cuts that subgraph off from the
+     * main graph at each point it encounters a non-temporary vertexes. OTP then holds no
+     * references to the temporary subgraph and it is garbage collected.
+     * <p/>
+     * Note! If the {@code vertex} is NOT a TemporaryVertex the method returns. No action taken.
+     *
+     * @param vertex Vertex part of the temporary part of the graph.
+     */
+    static void dispose(Vertex vertex) {
+        TemporaryVertexDispose.dispose(vertex);
+    }
 }

src/main/java/org/opentripplanner/routing/vertextype/TemporaryVertexDispose.java

@@ -0,0 +1,117 @@
+package org.opentripplanner.routing.vertextype;
+
+import org.opentripplanner.routing.graph.Edge;
+import org.opentripplanner.routing.graph.Vertex;
+
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+
+/**
+ * This is a utility class used to remove a temporary subgraph from the manin graph.
+ * It traverse the subgraph of temporary vertices, and cuts that subgraph off from the
+ * main graph at each point it encounters a non-temporary vertexes.
+ * <p/>
+ * OTP then holds no references to the temporary subgraph and it is garbage collected.
+ * <p/>
+ * The static {@link #dispose(Vertex)} utility method is the only way to access the logic,
+ * hence preventing this class from reuse. This make the class thread safe, and simplify
+ * the implementation.
+ */
+class TemporaryVertexDispose {
+    /**
+     * A list of all Vertexes not jet processed.
+     */
+    private List<Vertex> todo = new ArrayList<>();
+
+    /**
+     * Processed vertexes. To prevent looping and processing the same vertex twice we
+     * keep all processed vertexes in the 'done' set.
+     */
+    private Set<Vertex> done = new HashSet<>();
+
+    /** Intentionally private constructor to prevent instantiation outside of the class. */
+    private TemporaryVertexDispose(Vertex tempVertex) {
+        todo.add(tempVertex);
+    }
+
+    /**
+     * Create an instance and dispose temporary subgraph.
+     * @param tempVertex any temporary vertex part of the temporary subgrap.
+     */
+    static void dispose(Vertex tempVertex) {
+        if(tempVertex instanceof TemporaryVertex) {
+            new TemporaryVertexDispose(tempVertex).dispose();
+        }
+    }
+
+
+    /* private methods */
+
+    private void dispose() {
+        // Add all connected vertexes to the TODO_list and disconnect all
+        // main graph vertexes. We use a loop and not recursion to avoid
+        // stack overflow in the case of deep temporary graphs.
+        while (!todo.isEmpty()) {
+            Vertex current = next();
+            if(isNotAlreadyProcessed(current)) {
+                for (Edge edge : current.getOutgoing()) {
+                    disposeVertex(edge.getToVertex(), edge, true);
+                }
+                for (Edge edge : current.getIncoming()) {
+                    disposeVertex(edge.getFromVertex(), edge, false);
+                }
+                done.add(current);
+            }
+        }
+    }
+
+    /**
+     * Add the temporary vertex to processing queue OR disconnect edge from vertex if
+     * vertex is part of the main graph.
+     *
+     * @param v the vertex to dispose
+     * @param connectedEdge the connected temporary edge
+     * @param incoming true if the edge is an incoming edge, false if it is an outgoing edge
+     */
+    private void disposeVertex(Vertex v, Edge connectedEdge, boolean incoming) {
+        if(v instanceof TemporaryVertex) {
+            addVertexToProcessTodoList(v);
+        }
+        else {
+            removeEdgeFromMainGraphVertex(v, connectedEdge, incoming);
+        }
+    }
+
+    /**
+     * We have reached a NONE temporary Vertex and need to remove the temporary `connectedEdge`
+     * from the Vertex part of the main graph.
+     *
+     * @param v the vertex part of the main graph
+     * @param connectedEdge the connected temporary edge to be removed
+     * @param incoming true if the edge is an incoming edge, false if it is an outgoing edge
+     */
+    private void removeEdgeFromMainGraphVertex(Vertex v, Edge connectedEdge, boolean incoming) {
+        if(incoming) {
+            v.removeIncoming(connectedEdge);
+        }
+        else {
+            v.removeOutgoing(connectedEdge);
+        }
+    }
+
+    private void addVertexToProcessTodoList(Vertex v) {
+        if(isNotAlreadyProcessed(v)) {
+            todo.add(v);
+        }
+    }
+
+    private boolean isNotAlreadyProcessed(Vertex v) {
+        return !done.contains(v);
+    }
+
+    private Vertex next() {
+        return todo.remove(todo.size()-1);
+    }
+}

src/test/java/org/opentripplanner/routing/core/RoutingContext_Destroy_Test.java

@@ -29,11 +29,6 @@
 import static org.junit.Assert.assertTrue;
 
 public class RoutingContext_Destroy_Test {
-
-    private static final boolean ARRIVE_BY_TRUE = true;
-
-    private static final boolean ARRIVE_BY_FALSE = false;
-
     private final GeometryFactory gf = GeometryUtils.getGeometryFactory();
     private RoutingRequest request;
     private RoutingContext subject;
@@ -65,17 +60,10 @@ public class RoutingContext_Destroy_Test {
     }
 
     @Test public void temporary_changes_is_removed_from_graph_on_context_destroy() {
-        // try arriveBy set to both true and false
-        temporary_changes_is_removed_from_graph_on_context_destroy(ARRIVE_BY_TRUE);
-        temporary_changes_is_removed_from_graph_on_context_destroy(ARRIVE_BY_FALSE);
-    }
-
-    private void temporary_changes_is_removed_from_graph_on_context_destroy(boolean arriveBy) {
         // Given - A request
         request = new RoutingRequest();
         request.from = from;
         request.to = to;
-        request.arriveBy = arriveBy;
 
         // When - the context is created
         subject = new RoutingContext(request, g);
@@ -151,9 +139,8 @@ private static <T extends Collection<String>> T findAllReachableVertexes(Vertex
         return list;
     }
 
-    private void assertVertexEdgeIsNotReferencingTemporaryElements(Vertex source, Edge e,
-            Vertex v) {
-        String sourceName = source.getName();
+    private void assertVertexEdgeIsNotReferencingTemporaryElements(Vertex src, Edge e, Vertex v) {
+        String sourceName = src.getName();
         assertFalse(sourceName + " -> " + e.getName(), e instanceof TemporaryEdge);
         assertFalse(sourceName + " -> " + v.getName(), v instanceof TemporaryVertex);
     }