diff --git a/src/main/java/org/opentripplanner/common/StreetUtils.java b/src/main/java/org/opentripplanner/common/StreetUtils.java index acaaa5a8a86..10cc57d5efe 100644 --- 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 index 7717ada40a8..c14ace5c049 100644 --- 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 startingVertices, Set 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 index 7797b8fc77f..154a156a71e 100644 --- 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 new file mode 100644 index 00000000000..e722c16a69f --- /dev/null +++ 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 . */ + +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 index 73b32e95318..8ea8a09a41e 100644 --- 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 index 03f5532d70f..095f4cf37f4 100644 --- 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 index 07e7dd55a6e..5f81d8a73d8 100644 --- 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 index d88c05cfe2d..78990bc472e 100644 --- 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 edges = new ArrayList(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 index 7de39bef290..418677b08ea 100644 --- 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 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 index 2e381cbf290..0a4ce688e38 100644 --- 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 index 9e8f0bb27dc..da0276c2286 100644 --- 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 index 6d55989f8a7..036067a9cda 100644 --- 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); }