Merge pull request #903 from lassewesth/shortestpathissue

Fixed problem with shortest path algorithm terminating too soon
systay · Oct 8, 2012 · 6513098 · 6513098
2 parents ed08ddd + ef73e63
commit 6513098
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 0 deletions.
diff --git a/graph-algo/src/main/java/org/neo4j/graphalgo/impl/path/ShortestPath.java b/graph-algo/src/main/java/org/neo4j/graphalgo/impl/path/ShortestPath.java
@@ -253,6 +253,10 @@ private void goOneStep( DirectionData directionData, DirectionData otherSide, Hi
                 else if ( stopAsap )
                 {   // This side found a hit, but wait for the other side to complete its current depth
                     // to see if it finds a shorter path. (i.e. stop this side and freeze the depth).
+
+                    // but only if the other side has not stopped, otherwise we might miss shorter paths
+                    if (otherSide.stop == true) return;
+
                     directionData.stop = true;
                 }
             }

diff --git a/graph-algo/src/test/java/org/neo4j/graphalgo/path/TestShortestPath.java b/graph-algo/src/test/java/org/neo4j/graphalgo/path/TestShortestPath.java
@@ -22,13 +22,16 @@
 import static common.Neo4jAlgoTestCase.MyRelTypes.R1;
 import static common.SimpleGraphBuilder.KEY_ID;
 import static java.util.Arrays.asList;
+import static org.hamcrest.CoreMatchers.is;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
 import static org.junit.Assert.fail;
 import static org.neo4j.graphalgo.GraphAlgoFactory.shortestPath;
 import static org.neo4j.graphdb.Direction.BOTH;
 import static org.neo4j.graphdb.Direction.INCOMING;
 import static org.neo4j.graphdb.Direction.OUTGOING;
 import static org.neo4j.helpers.collection.IteratorUtil.count;
+import static org.neo4j.kernel.Traversal.expanderForAllTypes;
 import static org.neo4j.kernel.Traversal.expanderForTypes;
 
 import java.util.ArrayList;
@@ -518,6 +521,29 @@ public void pathsWithLengthProblem() throws Exception
         assertPaths( new ShortestPath( 3, expanderForTypes( R1 ), 10, true ).findAllPaths( a, c ), "a,d,b,c" );
     }
 
+    @Test
+    public void shouldFindShortestPathWhenOneSideFindsLongerPathFirst() throws Exception
+    {
+        /*
+        The order in which nodes are created matters when reproducing the original problem
+         */
+        graph.makeEdge( "start", "c" );
+        graph.makeEdge( "start", "a" );
+        graph.makeEdge( "b", "end" );
+        graph.makeEdge( "d", "end" );
+        graph.makeEdge( "c", "e" );
+        graph.makeEdge( "f", "end" );
+        graph.makeEdge( "c", "b" );
+        graph.makeEdge( "e", "end" );
+        graph.makeEdge( "a", "end" );
+
+        Node start = graph.getNode( "start" );
+        Node end = graph.getNode( "end" );
+
+        assertThat( new ShortestPath( 2, expanderForAllTypes(), 42 ).findSinglePath( start, end ).length(), is( 2 ) );
+        assertThat( new ShortestPath( 3, expanderForAllTypes(), 42 ).findSinglePath( start, end ).length(), is( 2 ) );
+    }
+
     private void testShortestPathFinder( PathFinderTester tester, RelationshipExpander expander, int maxDepth )
     {
         testShortestPathFinder( tester, expander, maxDepth, null );