Merge pull request graphhopper#1226 from ammagamma/allchedgeiterator_skippededges_fix

Makes AllCHEdgesIteratorImpl read/write skipped edges to/from shortcuts DataAccess

Author: karussell

core/src/main/java/com/graphhopper/routing/util/AbstractFlagEncoder.java

@@ -674,8 +674,8 @@ public double getDouble(long flags, int key) {
     }
 
     /**
-     * @param way:   needed to retrieve tags
-     * @param speed: speed guessed e.g. from the road type or other tags
+     * @param way   needed to retrieve tags
+     * @param speed speed guessed e.g. from the road type or other tags
      * @return The assumed speed.
      */
     protected double applyMaxSpeed(ReaderWay way, double speed) {

core/src/main/java/com/graphhopper/routing/util/BikeCommonFlagEncoder.java

@@ -325,8 +325,8 @@ public long handleRelationTags(ReaderRelation relation, long oldRelationFlags) {
      * just only 90%.
      * <p>
      *
-     * @param way:   needed to retrieve tags
-     * @param speed: speed guessed e.g. from the road type or other tags
+     * @param way   needed to retrieve tags
+     * @param speed speed guessed e.g. from the road type or other tags
      * @return The assumed average speed.
      */
     @Override

core/src/main/java/com/graphhopper/routing/util/CarFlagEncoder.java

@@ -335,8 +335,8 @@ public String getWayInfo(ReaderWay way) {
     }
 
     /**
-     * @param way:   needed to retrieve tags
-     * @param speed: speed guessed e.g. from the road type or other tags
+     * @param way   needed to retrieve tags
+     * @param speed speed guessed e.g. from the road type or other tags
      * @return The assumed speed
      */
     protected double applyBadSurfaceSpeed(ReaderWay way, double speed) {

core/src/main/java/com/graphhopper/storage/CHGraphImpl.java

@@ -599,18 +599,25 @@ public int getMaxId() {
 
         @Override
         public final void setSkippedEdges(int edge1, int edge2) {
-            baseGraph.edges.setInt(edgePointer + S_SKIP_EDGE1, edge1);
-            baseGraph.edges.setInt(edgePointer + S_SKIP_EDGE2, edge2);
+            checkShortcut(true, "setSkippedEdges");
+            if (EdgeIterator.Edge.isValid(edge1) != EdgeIterator.Edge.isValid(edge2)) {
+                throw new IllegalStateException("Skipped edges of a shortcut needs "
+                        + "to be both valid or invalid but they were not " + edge1 + ", " + edge2);
+            }
+            shortcuts.setInt(edgePointer + S_SKIP_EDGE1, edge1);
+            shortcuts.setInt(edgePointer + S_SKIP_EDGE2, edge2);
         }
 
         @Override
         public final int getSkippedEdge1() {
-            return baseGraph.edges.getInt(edgePointer + S_SKIP_EDGE1);
+            checkShortcut(true, "getSkippedEdge1");
+            return shortcuts.getInt(edgePointer + S_SKIP_EDGE1);
         }
 
         @Override
         public final int getSkippedEdge2() {
-            return baseGraph.edges.getInt(edgePointer + S_SKIP_EDGE2);
+            checkShortcut(true, "getSkippedEdge2");
+            return shortcuts.getInt(edgePointer + S_SKIP_EDGE2);
         }
 
         @Override
@@ -634,5 +641,13 @@ public final double getWeight() {
         public int getMergeStatus(long flags) {
             return PrepareEncoder.getScMergeStatus(getDirectFlags(), flags);
         }
+
+        void checkShortcut(boolean shouldBeShortcut, String methodName) {
+            if (isShortcut()) {
+                if (!shouldBeShortcut)
+                    throw new IllegalStateException("Cannot call " + methodName + " on shortcut " + getEdge());
+            } else if (shouldBeShortcut)
+                throw new IllegalStateException("Method " + methodName + " only for shortcuts " + getEdge());
+        }
     }
 }

core/src/test/java/com/graphhopper/storage/GraphHopperStorageCHTest.java

@@ -344,6 +344,46 @@ public void testSimpleShortcutCreationAndTraversal() {
         assertEquals(GHUtility.asSet(3, 4), GHUtility.getNeighbors(vehicleOutExplorer.setBaseNode(1)));
     }
 
+    @Test
+    public void testAddShortcutSkippedEdgesWriteRead() {
+        graph = createGHStorage();
+        final EdgeIteratorState edge1 = graph.edge(1, 3, 10, true);
+        final EdgeIteratorState edge2 = graph.edge(3, 4, 10, true);
+        graph.freeze();
+
+        CHGraph lg = graph.getGraph(CHGraph.class);
+        lg.shortcut(1, 4);
+
+        AllCHEdgesIterator iter = lg.getAllEdges();
+        iter.next();
+        iter.next();
+        iter.next();
+        assertTrue(iter.isShortcut());
+        iter.setSkippedEdges(edge1.getEdge(), edge2.getEdge());
+        assertEquals(edge1.getEdge(), iter.getSkippedEdge1());
+        assertEquals(edge2.getEdge(), iter.getSkippedEdge2());
+    }
+
+    @Test
+    public void testAddShortcutSkippedEdgesWriteRead_writeWithCHEdgeIterator() {
+        graph = createGHStorage();
+        final EdgeIteratorState edge1 = graph.edge(1, 3, 10, true);
+        final EdgeIteratorState edge2 = graph.edge(3, 4, 10, true);
+        graph.freeze();
+
+        CHGraph lg = graph.getGraph(CHGraph.class);
+        CHEdgeIteratorState shortcut = lg.shortcut(1, 4);
+        shortcut.setSkippedEdges(edge1.getEdge(), edge2.getEdge());
+
+        AllCHEdgesIterator iter = lg.getAllEdges();
+        iter.next();
+        iter.next();
+        iter.next();
+        assertTrue(iter.isShortcut());
+        assertEquals(edge1.getEdge(), iter.getSkippedEdge1());
+        assertEquals(edge2.getEdge(), iter.getSkippedEdge2());
+    }
+
     @Test
     public void testShortcutCreationAndAccessForManyVehicles() {
         FlagEncoder tmpCar = new CarFlagEncoder();