Unverified Commit c47cf2fa authored by MishaDemianenko's avatar MishaDemianenko Committed by GitHub
Browse files

Merge pull request #10815 from MishaDemianenko/3.3-make-path-not-a-resource

Cleanup accidental Path exposure as a Resource.
parents 1725d634 941875c7
Showing with 60 additions and 113 deletions
+60 -113
...@@ -172,7 +172,6 @@ public abstract class Neo4jAlgoTestCase ...@@ -172,7 +172,6 @@ public abstract class Neo4jAlgoTestCase
{ {
unexpectedDefs.add( getPathDef( path ) ); unexpectedDefs.add( getPathDef( path ) );
} }
path.close();
} }
} }
assertTrue( "These unexpected paths were found: " + unexpectedDefs + assertTrue( "These unexpected paths were found: " + unexpectedDefs +
......
...@@ -65,15 +65,12 @@ public class DijkstraTest extends Neo4jAlgoTestCase ...@@ -65,15 +65,12 @@ public class DijkstraTest extends Neo4jAlgoTestCase
// WHEN // WHEN
PathFinder<WeightedPath> finder = factory.dijkstra( PathExpanders.allTypesAndDirections() ); PathFinder<WeightedPath> finder = factory.dijkstra( PathExpanders.allTypesAndDirections() );
try ( WeightedPath path = finder.findSinglePath( start, start ) ) WeightedPath path = finder.findSinglePath( start, start );
{ // THEN
assertNotNull( path );
// THEN assertEquals( start, path.startNode() );
assertNotNull( path ); assertEquals( start, path.endNode() );
assertEquals( start, path.startNode() ); assertEquals( 0, path.length() );
assertEquals( start, path.endNode() );
assertEquals( 0, path.length() );
}
} }
@Test @Test
......
...@@ -63,14 +63,12 @@ public class TestAStar extends Neo4jAlgoTestCase ...@@ -63,14 +63,12 @@ public class TestAStar extends Neo4jAlgoTestCase
Node start = graph.makeNode( "start", "x", 0d, "y", 0d ); Node start = graph.makeNode( "start", "x", 0d, "y", 0d );
// WHEN // WHEN
try ( WeightedPath path = finder.findSinglePath( start, start ) ) WeightedPath path = finder.findSinglePath( start, start );
{ // THEN
// THEN assertNotNull( path );
assertNotNull( path ); assertEquals( start, path.startNode() );
assertEquals( start, path.startNode() ); assertEquals( start, path.endNode() );
assertEquals( start, path.endNode() ); assertEquals( 0, path.length() );
assertEquals( 0, path.length() );
}
} }
@Test @Test
...@@ -90,8 +88,6 @@ public class TestAStar extends Neo4jAlgoTestCase ...@@ -90,8 +88,6 @@ public class TestAStar extends Neo4jAlgoTestCase
assertEquals( start, path.endNode() ); assertEquals( start, path.endNode() );
assertEquals( 0, path.length() ); assertEquals( 0, path.length() );
} }
paths.forEach( Path::close );
} }
@Test @Test
...@@ -128,12 +124,9 @@ public class TestAStar extends Neo4jAlgoTestCase ...@@ -128,12 +124,9 @@ public class TestAStar extends Neo4jAlgoTestCase
graph.makeEdge( "e", "end", "length", 2 ); graph.makeEdge( "e", "end", "length", 2 );
// WHEN // WHEN
try ( WeightedPath path = finder.findSinglePath( start, end ) ) WeightedPath path = finder.findSinglePath( start, end );
{ // THEN
assertPathDef( path, "start", "d", "e", "end" );
// THEN
assertPathDef( path, "start", "d", "e", "end" );
}
} }
/** /**
...@@ -165,7 +158,6 @@ public class TestAStar extends Neo4jAlgoTestCase ...@@ -165,7 +158,6 @@ public class TestAStar extends Neo4jAlgoTestCase
assertPath( path, nodeA, nodeB, nodeC ); assertPath( path, nodeA, nodeB, nodeC );
counter++; counter++;
} }
allPaths.forEach( Path::close );
assertEquals( 1, counter ); assertEquals( 1, counter );
} }
...@@ -228,12 +220,10 @@ public class TestAStar extends Neo4jAlgoTestCase ...@@ -228,12 +220,10 @@ public class TestAStar extends Neo4jAlgoTestCase
PathFinder<WeightedPath> traversalFinder = new TraversalAStar( expander, PathFinder<WeightedPath> traversalFinder = new TraversalAStar( expander,
new InitialBranchState.State( initialStateValue, initialStateValue ), new InitialBranchState.State( initialStateValue, initialStateValue ),
doubleCostEvaluator( "length" ), ESTIMATE_EVALUATOR ); doubleCostEvaluator( "length" ), ESTIMATE_EVALUATOR );
try ( WeightedPath path = traversalFinder.findSinglePath( nodeA, nodeC ) ) WeightedPath path = traversalFinder.findSinglePath( nodeA, nodeC );
{ assertEquals( (Double) 5.0D, (Double) path.weight() );
assertEquals( (Double) 5.0D, (Double) path.weight() ); assertPathDef( path, "A", "B", "C" );
assertPathDef( path, "A", "B", "C" ); assertEquals( MapUtil.<Node,Double>genericMap( nodeA, 0D, nodeB, 2D ), seenBranchStates );
assertEquals( MapUtil.<Node,Double>genericMap( nodeA, 0D, nodeB, 2D ), seenBranchStates );
}
} }
@Test @Test
...@@ -257,11 +247,9 @@ public class TestAStar extends Neo4jAlgoTestCase ...@@ -257,11 +247,9 @@ public class TestAStar extends Neo4jAlgoTestCase
graph.makeEdge( "3", "4", "weight", 0.013d ); graph.makeEdge( "3", "4", "weight", 0.013d );
// WHEN // WHEN
try ( WeightedPath best1_4 = finder.findSinglePath( node1, node4 ) ) WeightedPath best1_4 = finder.findSinglePath( node1, node4 );
{ // THEN
// THEN assertPath( best1_4, node1, node2, node3, node4 );
assertPath( best1_4, node1, node2, node3, node4 );
}
} }
static EstimateEvaluator<Double> ESTIMATE_EVALUATOR = ( node, goal ) -> static EstimateEvaluator<Double> ESTIMATE_EVALUATOR = ( node, goal ) ->
......
...@@ -65,11 +65,9 @@ public class TestExactDepthPathFinder extends Neo4jAlgoTestCase ...@@ -65,11 +65,9 @@ public class TestExactDepthPathFinder extends Neo4jAlgoTestCase
{ {
createGraph(); createGraph();
PathFinder<Path> finder = newFinder(); PathFinder<Path> finder = newFinder();
try ( Path path = finder.findSinglePath( graph.getNode( "SOURCE" ), graph.getNode( "TARGET" ) ) ) Path path = finder.findSinglePath( graph.getNode( "SOURCE" ), graph.getNode( "TARGET" ) );
{ assertNotNull( path );
assertNotNull( path ); assertPathDef( path, "SOURCE", "z", "9", "0", "TARGET" );
assertPathDef( path, "SOURCE", "z", "9", "0", "TARGET" );
}
} }
@Test @Test
......
...@@ -30,7 +30,7 @@ import java.util.Iterator; ...@@ -30,7 +30,7 @@ import java.util.Iterator;
* position of the traverser is represented by each such path. The current * position of the traverser is represented by each such path. The current
* node in such a traversal is reached via {@link Path#endNode()}. * node in such a traversal is reached via {@link Path#endNode()}.
*/ */
public interface Path extends Iterable<PropertyContainer>, Resource public interface Path extends Iterable<PropertyContainer>
{ {
/** /**
* Returns the start node of this path. It's also the first node returned * Returns the start node of this path. It's also the first node returned
...@@ -130,10 +130,4 @@ public interface Path extends Iterable<PropertyContainer>, Resource ...@@ -130,10 +130,4 @@ public interface Path extends Iterable<PropertyContainer>, Resource
* @see Iterable#iterator() * @see Iterable#iterator()
*/ */
Iterator<PropertyContainer> iterator(); Iterator<PropertyContainer> iterator();
@Override
default void close()
{
// empty
}
} }
...@@ -73,9 +73,9 @@ public class TestPath extends TraversalTestBase ...@@ -73,9 +73,9 @@ public class TestPath extends TraversalTestBase
public void testPathIterator() public void testPathIterator()
{ {
Traverser traverse = getGraphDb().traversalDescription().evaluator( atDepth( 4 ) ).traverse( node( "A" ) ); Traverser traverse = getGraphDb().traversalDescription().evaluator( atDepth( 4 ) ).traverse( node( "A" ) );
try ( ResourceIterator<Path> resourceIterator = traverse.iterator(); try ( ResourceIterator<Path> resourceIterator = traverse.iterator() )
Path path = resourceIterator.next() )
{ {
Path path = resourceIterator.next();
assertPathIsCorrect( path ); assertPathIsCorrect( path );
} }
} }
...@@ -84,38 +84,30 @@ public class TestPath extends TraversalTestBase ...@@ -84,38 +84,30 @@ public class TestPath extends TraversalTestBase
public void reverseNodes() throws Exception public void reverseNodes() throws Exception
{ {
Traverser traverse = getGraphDb().traversalDescription().evaluator( atDepth( 0 ) ).traverse( a ); Traverser traverse = getGraphDb().traversalDescription().evaluator( atDepth( 0 ) ).traverse( a );
try ( Path path = getFirstPath( traverse ) ) Path path = getFirstPath( traverse );
{ assertContains( path.reverseNodes(), a );
assertContains( path.reverseNodes(), a );
}
Traverser traverse2 = getGraphDb().traversalDescription().evaluator( atDepth( 4 ) ).traverse( a ); Traverser traverse2 = getGraphDb().traversalDescription().evaluator( atDepth( 4 ) ).traverse( a );
try ( Path path = getFirstPath( traverse2 ) ) Path path2 = getFirstPath( traverse2 );
{ assertContainsInOrder( path2.reverseNodes(), e, d, c, b, a );
assertContainsInOrder( path.reverseNodes(), e, d, c, b, a );
}
} }
@Test @Test
public void reverseRelationships() throws Exception public void reverseRelationships() throws Exception
{ {
Traverser traverser = getGraphDb().traversalDescription().evaluator( atDepth( 0 ) ).traverse( a ); Traverser traverser = getGraphDb().traversalDescription().evaluator( atDepth( 0 ) ).traverse( a );
try ( Path path = getFirstPath( traverser ) ) Path path = getFirstPath( traverser );
{ assertFalse( path.reverseRelationships().iterator().hasNext() );
assertFalse( path.reverseRelationships().iterator().hasNext() );
}
Traverser traverser2 = getGraphDb().traversalDescription().evaluator( atDepth( 4 ) ).traverse( a ); Traverser traverser2 = getGraphDb().traversalDescription().evaluator( atDepth( 4 ) ).traverse( a );
try ( Path path2 = getFirstPath( traverser2 ) ) Path path2 = getFirstPath( traverser2 );
Node[] expectedNodes = new Node[]{e, d, c, b, a};
int index = 0;
for ( Relationship rel : path2.reverseRelationships() )
{ {
Node[] expectedNodes = new Node[]{e, d, c, b, a}; assertEquals( "For index " + index, expectedNodes[index++], rel.getEndNode() );
int index = 0;
for ( Relationship rel : path2.reverseRelationships() )
{
assertEquals( "For index " + index, expectedNodes[index++], rel.getEndNode() );
}
assertEquals( 4, index );
} }
assertEquals( 4, index );
} }
@Test @Test
...@@ -124,48 +116,34 @@ public class TestPath extends TraversalTestBase ...@@ -124,48 +116,34 @@ public class TestPath extends TraversalTestBase
TraversalDescription side = getGraphDb().traversalDescription().uniqueness( Uniqueness.NODE_PATH ); TraversalDescription side = getGraphDb().traversalDescription().uniqueness( Uniqueness.NODE_PATH );
BidirectionalTraversalDescription bidirectional = BidirectionalTraversalDescription bidirectional =
getGraphDb().bidirectionalTraversalDescription().mirroredSides( side ); getGraphDb().bidirectionalTraversalDescription().mirroredSides( side );
try ( Path bidirectionalPath = getFirstPath( bidirectional.traverse( a, e ) ) ) Path bidirectionalPath = getFirstPath( bidirectional.traverse( a, e ) );
{ assertPathIsCorrect( bidirectionalPath );
assertPathIsCorrect( bidirectionalPath );
try ( Path path = getFirstPath( bidirectional.traverse( a, e ) ) ) Path path = getFirstPath( bidirectional.traverse( a, e ) );
{ Node node = path.startNode();
Node node = path.startNode(); assertEquals( a, node );
assertEquals( a, node );
}
}
// White box testing below: relationships(), nodes(), reverseRelationships(), reverseNodes() // White box testing below: relationships(), nodes(), reverseRelationships(), reverseNodes()
// does cache the start node if not already cached, so just make sure they to it properly. // does cache the start node if not already cached, so just make sure they to it properly.
try ( Path bidirectionalPath = getFirstPath( bidirectional.traverse( a, e ) ) ) bidirectionalPath = getFirstPath( bidirectional.traverse( a, e ) );
{ bidirectionalPath.relationships();
bidirectionalPath.relationships(); assertEquals( a, bidirectionalPath.startNode() );
assertEquals( a, bidirectionalPath.startNode() );
}
try ( Path bidirectionalPath = getFirstPath(bidirectional.traverse(a,e ) ) ) bidirectionalPath = getFirstPath(bidirectional.traverse(a,e ) );
{ bidirectionalPath.nodes();
bidirectionalPath.nodes(); assertEquals( a, bidirectionalPath.startNode() );
assertEquals( a, bidirectionalPath.startNode() );
}
try ( Path bidirectionalPath = getFirstPath( bidirectional.traverse( a, e ) ) ) bidirectionalPath = getFirstPath( bidirectional.traverse( a, e ) );
{ bidirectionalPath.reverseRelationships();
bidirectionalPath.reverseRelationships(); assertEquals( a, bidirectionalPath.startNode() );
assertEquals( a, bidirectionalPath.startNode() );
}
try ( Path bidirectionalPath = getFirstPath( bidirectional.traverse( a, e ) ) ) bidirectionalPath = getFirstPath( bidirectional.traverse( a, e ) );
{ bidirectionalPath.reverseNodes();
bidirectionalPath.reverseNodes(); assertEquals( a, bidirectionalPath.startNode() );
assertEquals( a, bidirectionalPath.startNode() );
}
try ( Path bidirectionalPath = getFirstPath( bidirectional.traverse( a, e ) ) ) bidirectionalPath = getFirstPath( bidirectional.traverse( a, e ) );
{ bidirectionalPath.iterator();
bidirectionalPath.iterator(); assertEquals( a, bidirectionalPath.startNode() );
assertEquals( a, bidirectionalPath.startNode() );
}
} }
private Path getFirstPath( Traverser traverse ) private Path getFirstPath( Traverser traverse )
......
...@@ -107,7 +107,6 @@ import static org.neo4j.graphdb.Label.label; ...@@ -107,7 +107,6 @@ import static org.neo4j.graphdb.Label.label;
import static org.neo4j.helpers.collection.Iterables.filter; import static org.neo4j.helpers.collection.Iterables.filter;
import static org.neo4j.helpers.collection.Iterables.map; import static org.neo4j.helpers.collection.Iterables.map;
import static org.neo4j.helpers.collection.Iterators.asList; import static org.neo4j.helpers.collection.Iterators.asList;
import static org.neo4j.helpers.collection.ResourceClosingIterator.newResourceIterator;
import static org.neo4j.server.rest.repr.RepresentationType.CONSTRAINT_DEFINITION; import static org.neo4j.server.rest.repr.RepresentationType.CONSTRAINT_DEFINITION;
public class DatabaseActions public class DatabaseActions
...@@ -1102,12 +1101,6 @@ public class DatabaseActions ...@@ -1102,12 +1101,6 @@ public class DatabaseActions
{ {
return returnType.toRepresentation( position ); return returnType.toRepresentation( position );
} }
@Override
public Iterator<Representation> iterator()
{
return newResourceIterator( () -> paths.forEach( Path::close ), super.iterator() );
}
}; };
return new ListRepresentation( returnType.repType, result ); return new ListRepresentation( returnType.repType, result );
} }
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment