Add removeEdge() method to Graph class and cleanup

opentripplanner · Jan 16, 2015 · fbb0f5a · fbb0f5a
1 parent 98510d4
commit fbb0f5a
Show file tree

Hide file tree

Showing 12 changed files with 82 additions and 92 deletions.
diff --git a/src/main/java/org/opentripplanner/common/StreetUtils.java b/src/main/java/org/opentripplanner/common/StreetUtils.java
@@ -163,9 +163,7 @@ private static void depedestrianizeOrRemove(Graph graph, Subgraph island) {
                     permission = permission.remove(StreetTraversalPermission.PEDESTRIAN);
                     permission = permission.remove(StreetTraversalPermission.BICYCLE);
                     if (permission == StreetTraversalPermission.NONE) {
-                        // TODO Shouldn't we have a graph.removeEdge()?
-                        graph.streetNotesService.removeStaticNotes(pse);
-                        pse.detach(graph);
+                        graph.removeEdge(pse);
                     } else {
                         pse.setPermission(permission);
                     }
@@ -186,7 +184,7 @@ private static void depedestrianizeOrRemove(Graph graph, Subgraph island) {
             edges.addAll(v.getIncoming());
             for (Edge e : edges) {
                 if (e instanceof StreetTransitLink) {
-                    e.detach(graph);
+                    graph.removeEdge(e);
                 }
             }
         }

diff --git a/src/main/java/org/opentripplanner/graph_builder/impl/osm/WalkableAreaBuilder.java b/src/main/java/org/opentripplanner/graph_builder/impl/osm/WalkableAreaBuilder.java
@@ -293,8 +293,7 @@ private void pruneAreaEdges(Collection<Vertex> startingVertices, Set<Edge> edges
         }
         for (Edge edge : edges) {
             if (!usedEdges.contains(edge)) {
-                graph.streetNotesService.removeStaticNotes(edge);
-                edge.detach(graph);
+                graph.removeEdge(edge);
             }
         }
     }

diff --git a/src/main/java/org/opentripplanner/routing/edgetype/AreaEdge.java b/src/main/java/org/opentripplanner/routing/edgetype/AreaEdge.java
@@ -13,12 +13,10 @@ the License, or (at your option) any later version.
 
 package org.opentripplanner.routing.edgetype;
 
-import org.opentripplanner.routing.graph.Graph;
-import org.opentripplanner.routing.vertextype.IntersectionVertex;
-
 import com.vividsolutions.jts.geom.LineString;
+import org.opentripplanner.routing.vertextype.IntersectionVertex;
 
-public class AreaEdge extends StreetWithElevationEdge {
+public class AreaEdge extends StreetWithElevationEdge implements EdgeWithCleanup {
     private static final long serialVersionUID = 6761687673982054612L;
     private AreaEdgeList area;
 
@@ -33,10 +31,10 @@ public AreaEdge(IntersectionVertex startEndpoint, IntersectionVertex endEndpoint
     public AreaEdgeList getArea() {
         return area;
     }
-
-    public int detach(Graph graph) {
+
+    @Override
+    public void detach() {
         area.removeEdge(this);
-        return super.detach(graph);
     }
 
     public void setArea(AreaEdgeList area) {

diff --git a/src/main/java/org/opentripplanner/routing/edgetype/EdgeWithCleanup.java b/src/main/java/org/opentripplanner/routing/edgetype/EdgeWithCleanup.java
@@ -0,0 +1,19 @@
+/* This program is free software: you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public License
+ as published by the Free Software Foundation, either version 3 of
+ the License, or (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program.  If not, see <http://www.gnu.org/licenses/>. */
+
+package org.opentripplanner.routing.edgetype;
+
+/** An interface to be implemented by edges that need to perform some cleanup upon being detached */
+public interface EdgeWithCleanup {
+    public void detach();
+}
diff --git a/src/main/java/org/opentripplanner/routing/edgetype/LegSwitchingEdge.java b/src/main/java/org/opentripplanner/routing/edgetype/LegSwitchingEdge.java
@@ -26,11 +26,8 @@ the License, or (props, at your option) any later version.
 public class LegSwitchingEdge extends Edge {
 	private static final long serialVersionUID = 1L;
 
-    private static final Vertex dummy = new Vertex(null, null, 0.0, 0.0) {};
-
 	public LegSwitchingEdge(Vertex v1, Vertex v2) {
-		super(dummy, dummy);
-        dummy.removeAllEdges();
+        super(new Vertex(null, null, 0.0, 0.0) {}, new Vertex(null, null, 0.0, 0.0) {});
         fromv = v1;
         tov = v2;
         // Why is this code so dirty? Because we don't want this edge to be added to the edge lists.

diff --git a/src/main/java/org/opentripplanner/routing/edgetype/StreetEdge.java b/src/main/java/org/opentripplanner/routing/edgetype/StreetEdge.java
@@ -560,21 +560,6 @@ public boolean canTurnOnto(Edge e, State state, TraverseMode mode) {
         return true;
     }
 
-    protected boolean detachFrom(Graph graph) {
-        if (fromv != null) {
-            for (Edge e : fromv.getIncoming()) {
-                if (e instanceof StreetEdge) {
-                    for (TurnRestriction restriction : graph.getTurnRestrictions(e)) {
-                        if (restriction.to == this) {
-                            graph.removeTurnRestriction(e, restriction);
-                        }
-                    }
-                }
-            }
-        }
-        return super.detachFrom(graph);
-    }
-
 	@Override
 	public String getName() {
 		return this.name;

diff --git a/src/main/java/org/opentripplanner/routing/graph/Edge.java b/src/main/java/org/opentripplanner/routing/graph/Edge.java
@@ -110,40 +110,6 @@ public String getDirection() {
         return null;
     }
 
-    protected boolean detachFrom(Graph graph) {
-        boolean detached = false;
-        if (fromv != null) {
-            detached = fromv.removeOutgoing(this);
-            fromv = null;
-        }
-        return detached;
-    }
-
-    protected boolean detachTo(Graph graph) {
-        boolean detached = false;
-        if (tov != null) {
-            detached = tov.removeIncoming(this);
-            tov = null;
-        }
-        return detached;
-    }
-
-    /**
-     * Disconnect this edge from its endpoint vertices, keeping edgelists coherent
-     * 
-     * @return
-     */
-    public int detach(Graph graph) {
-        int nDetached = 0;
-        if (detachFrom(graph)) {
-            ++nDetached;
-        }
-        if (detachTo(graph)) {
-            ++nDetached;
-        }
-        return nDetached;
-    }
-
     /**
      * This should only be called inside State; other methods should call
      * org.opentripplanner.routing.core.State.getBackTrip()

diff --git a/src/main/java/org/opentripplanner/routing/graph/Graph.java b/src/main/java/org/opentripplanner/routing/graph/Graph.java
@@ -61,6 +61,7 @@ the License, or (at your option) any later version.
 import org.opentripplanner.routing.alertpatch.AlertPatch;
 import org.opentripplanner.routing.core.MortonVertexComparatorFactory;
 import org.opentripplanner.routing.core.TransferTable;
+import org.opentripplanner.routing.edgetype.EdgeWithCleanup;
 import org.opentripplanner.routing.edgetype.StreetEdge;
 import org.opentripplanner.routing.edgetype.TripPattern;
 import org.opentripplanner.routing.impl.DefaultStreetVertexIndexFactory;
@@ -227,6 +228,43 @@ public void removeVertex(Vertex v) {
         }
     }
 
+    /**
+     * Removes an edge from the graph. This method is not thread-safe.
+     * @param e The edge to be removed
+     */
+    public void removeEdge(Edge e) {
+        if (e != null) {
+            synchronized (alertPatches) {   // This synchronization is somewhat silly because this
+                alertPatches.remove(e);     // method isn't thread-safe anyway, but it is consistent
+            }
+
+            turnRestrictions.remove(e);
+            streetNotesService.removeStaticNotes(e);
+            edgeById.remove(e.getId());
+
+            if (e instanceof EdgeWithCleanup) ((EdgeWithCleanup) e).detach();
+
+            if (e.fromv != null) {
+                e.fromv.removeOutgoing(e);
+
+                for (Edge otherEdge : e.fromv.getIncoming()) {
+                    for (TurnRestriction turnRestriction : getTurnRestrictions(otherEdge)) {
+                        if (turnRestriction.to == e) {
+                            removeTurnRestriction(otherEdge, turnRestriction);
+                        }
+                    }
+                }
+
+                e.fromv = null;
+            }
+
+            if (e.tov != null) {
+                e.tov.removeIncoming(e);
+                e.tov = null;
+            }
+        }
+    }
+
     /* Fetching vertices by label is convenient in tests and such, but avoid using in general. */
     @VisibleForTesting
     public Vertex getVertex(String label) {
@@ -440,7 +478,15 @@ public void removeVertexAndEdges(Vertex vertex) {
         if (!containsVertex(vertex)) {
             throw new IllegalStateException("attempting to remove vertex that is not in graph.");
         }
-        vertex.removeAllEdges();
+
+        List<Edge> edges = new ArrayList<Edge>(vertex.getDegreeIn() + vertex.getDegreeOut());
+        edges.addAll(vertex.getIncoming());
+        edges.addAll(vertex.getOutgoing());
+
+        for (Edge edge : edges) {
+            removeEdge(edge);
+        }
+
         this.remove(vertex);
     }
 

diff --git a/src/main/java/org/opentripplanner/routing/graph/Vertex.java b/src/main/java/org/opentripplanner/routing/graph/Vertex.java
@@ -285,25 +285,4 @@ public List<Edge> getOutgoingStreetEdges() {
         }
         return result;
     }
-
-    /**
-     * Clear this vertex's outgoing and incoming edge lists, and remove all the edges
-     * they contained from this vertex's neighbors.
-     */
-    public void removeAllEdges() {
-        for (Edge e : outgoing) {
-            Vertex target = e.getToVertex();
-            if (target != null && target != this) {
-                target.removeIncoming(e);
-            }
-        }
-        for (Edge e : incoming) {
-            Vertex source = e.getFromVertex();
-            if (source != null && source != this) {
-                source.removeOutgoing(e);
-            }
-        }
-        incoming = new Edge[0];
-        outgoing = new Edge[0];
-    }
 }
diff --git a/src/test/java/org/opentripplanner/api/resource/TestRequest.java b/src/test/java/org/opentripplanner/api/resource/TestRequest.java
@@ -707,7 +707,7 @@ public void testTimedTripToTripTransfer() throws ParseException {
         applyUpdateToTripPattern(pattern, "120W1320", "9756", 22, 41820, 41820,
                 ScheduleRelationship.SCHEDULED, 0, serviceDate);
         // Remove the timed transfer from the graph
-        timedTransferEdge.detach(graph);
+        graph.removeEdge(timedTransferEdge);
         // Revert the graph, thus using the original transfer table again
         reset(graph);
     }
@@ -779,7 +779,7 @@ public void testTimedStopToStopTransfer() throws ParseException {
         applyUpdateToTripPattern(pattern, "120W1320", "9756", 22, 41820, 41820,
                 ScheduleRelationship.SCHEDULED, 0, serviceDate);
         // Remove the timed transfer from the graph
-        timedTransferEdge.detach(graph);
+        graph.removeEdge(timedTransferEdge);
         // Revert the graph, thus using the original transfer table again
         reset(graph);
     }

diff --git a/src/test/java/org/opentripplanner/routing/core/TestOnBoardRouting.java b/src/test/java/org/opentripplanner/routing/core/TestOnBoardRouting.java
@@ -28,6 +28,7 @@ the License, or (at your option) any later version.
 import org.opentripplanner.routing.edgetype.OnBoardDepartPatternHop;
 import org.opentripplanner.routing.edgetype.PatternHop;
 import org.opentripplanner.routing.edgetype.factory.GTFSPatternHopFactory;
+import org.opentripplanner.routing.graph.Edge;
 import org.opentripplanner.routing.graph.Graph;
 import org.opentripplanner.routing.graph.Vertex;
 import org.opentripplanner.routing.spt.GraphPath;
@@ -204,7 +205,9 @@ public void testOnBoardRouting() throws Exception {
             assertTrue(numBoardings2 < numBoardings1);
 
             /* Cleanup edges */
-            onboardOrigin.removeAllEdges();
+            for (Edge edge : onboardOrigin.getOutgoing()) {
+                graph.removeEdge(edge);
+            }
 
             n++;
             if (n > NTESTS)

diff --git a/src/test/java/org/opentripplanner/routing/core/TestTransfers.java b/src/test/java/org/opentripplanner/routing/core/TestTransfers.java
@@ -581,7 +581,7 @@ public void testTimedStopToStopTransfer() throws Exception {
         applyUpdateToTripPattern(pattern, "4.2", "F", 1, 82800, 82800,
                 ScheduleRelationship.SCHEDULED, 0, serviceDate);
         // Remove the timed transfer from the graph
-        timedTransferEdge.detach(graph);
+        graph.removeEdge(timedTransferEdge);
         // Revert the graph, thus using the original transfer table again
         reset(graph);
     }