Changed to use bolt path encoding

BOLT uses a very particular path encoding, initially the writeTo of PathValue now completely mimics the bolt encoding but at some point when there are more users AnyValueWriter there may be reason to move some of that logic to the implementing class.
neo4j · Jun 19, 2017 · 6797a3e · 6797a3e
1 parent 48f69b6
commit 6797a3e
Show file tree

Hide file tree

Showing 10 changed files with 160 additions and 83 deletions.
diff --git a/community/values/src/main/java/org/neo4j/values/AnyValueWriter.java b/community/values/src/main/java/org/neo4j/values/AnyValueWriter.java
@@ -46,8 +46,6 @@ public interface AnyValueWriter<E extends Exception> extends ValueWriter<E>
 
     void beginMap( int size ) throws E;
 
-    void writeKeyId( int keyId ) throws E;
-
     void endMap() throws E;
 
     void beginList( int size ) throws E;

diff --git a/community/values/src/main/java/org/neo4j/values/virtual/EdgeValue.java b/community/values/src/main/java/org/neo4j/values/virtual/EdgeValue.java
@@ -55,6 +55,16 @@ public String toString()
         return format( "-[%d]-", id );
     }
 
+    public long startNode()
+    {
+        return startNodeId;
+    }
+
+    public long endNode()
+    {
+        return endNodeId;
+    }
+
     @Override
     long id()
     {

diff --git a/community/values/src/main/java/org/neo4j/values/virtual/PathValue.java b/community/values/src/main/java/org/neo4j/values/virtual/PathValue.java
@@ -28,10 +28,10 @@
 
 final class PathValue extends VirtualValue
 {
-    private final NodeReference[] nodes;
+    private final NodeValue[] nodes;
-    private final EdgeReference[] edges;
+    private final EdgeValue[] edges;
 
-    PathValue( NodeReference[] nodes, EdgeReference[] edges )
+    PathValue( NodeValue[] nodes, EdgeValue[] edges )
     {
         assert nodes != null;
         assert edges != null;
@@ -50,8 +50,8 @@ public boolean equals( VirtualValue other )
         }
         PathValue that = (PathValue) other;
         return size() == that.size() &&
-                Arrays.equals( nodes, that.nodes ) &&
+               Arrays.equals( nodes, that.nodes ) &&
-                Arrays.equals( edges, that.edges );
+               Arrays.equals( edges, that.edges );
     }
 
     @Override
@@ -60,24 +60,74 @@ public int hash()
         int result = nodes[0].hashCode();
         for ( int i = 1; i < nodes.length; i++ )
         {
-            result += 31 * ( result + edges[i - 1].hashCode() );
+            result += 31 * (result + edges[i - 1].hashCode());
-            result += 31 * ( result + nodes[i].hashCode() );
+            result += 31 * (result + nodes[i].hashCode());
         }
         return result;
     }
 
     @Override
     public <E extends Exception> void writeTo( AnyValueWriter<E> writer ) throws E
     {
+        //A path is serialized in the following form
+        // Given path: (a {id: 42})-[r1 {id: 10}]->(b {id: 43})<-[r1 {id: 11}]-(c {id: 44})
+        //The seralization will look like:
+        //
+        // {
+        //    [a, b, c]
+        //    [r1, r2]
+        //    [1, 1, -2, 2]
+        // }
+        // The first list contains all nodes where the first node (a) is guaranteed to be the start node of the path
+        // The second list contains all edges of the path
+        // The third list defines the path order, where every other item specifies the offset into the
+        // relationship and node list respectively. Since all paths is guaranteed to start with a 0, meaning that
+        // a is the start node in this case, those are excluded. So the first integer in the array refers to the
+        // position
+        // in the relationship array (1 indexed where sign denotes direction) and the second one refers to the offset
+        // into the
+        // node list (zero indexed) and so on.
         writer.beginPath( edges.length );
-        for ( NodeReference node : nodes )
+        writer.beginList( nodes.length );
+        for ( NodeValue node : nodes )
         {
             node.writeTo( writer );
         }
-        for ( EdgeReference edge : edges )
+        writer.endList();
+        writer.beginList( edges.length );
+        for ( EdgeValue edge : edges )
         {
             edge.writeTo( writer );
         }
+        writer.endList();
+        writer.beginList( 2 * edges.length );
+        long node = -1L;
+        for ( int i = 0; i < 2 * edges.length; i++ )
+        {
+            int index = i / 2;
+            if ( i % 2 == 0 )
+            {
+                node = nodes[index].id();
+                if ( i > 0 )
+                {
+                    writer.writeInteger( index );
+                }
+            }
+            else
+            {
+                EdgeValue edge = edges[index];
+                if ( node >= 0 && node == edge.startNode() )
+                {
+                    writer.writeInteger( index + 1 );
+                }
+                else
+                {
+                    writer.writeInteger( -index - 1 );
+                }
+            }
+
+        }
+        writer.endList();
         writer.endPath();
     }
 

diff --git a/community/values/src/main/java/org/neo4j/values/virtual/VirtualValues.java b/community/values/src/main/java/org/neo4j/values/virtual/VirtualValues.java
@@ -83,7 +83,7 @@ public static EdgeReference edge( long id )
         return new EdgeReference( id );
     }
 
-    public static PathValue path( NodeReference[] nodes, EdgeReference[] edges )
+    public static PathValue path( NodeValue[] nodes, EdgeValue[] edges )
     {
         return new PathValue( nodes, edges );
     }

diff --git a/community/values/src/test/java/org/neo4j/values/AnyValueComparatorTest.java b/community/values/src/test/java/org/neo4j/values/AnyValueComparatorTest.java
@@ -122,17 +122,17 @@ public class AnyValueComparatorTest
 
             // Map
             map(),
-            map( 1, 'a' ),
+            map( "1", 'a' ),
-            map( 1, 'b' ),
+            map( "1", 'b' ),
-            map( 2, 'a' ),
+            map( "2", 'a' ),
-            map( 1, 'a', 2, 'b' ),
+            map( "1", 'a', "2", 'b' ),
-            map( 1, 'b', 2, 'a' ),
+            map( "1", 'b', "2", 'a' ),
-            map( 1, 'b', 2, map() ),
+            map( "1", 'b', "2", map() ),
-            map( 1, 'b', 2, map( 10, 'a' ) ),
+            map( "1", 'b', "2", map( "10", 'a' ) ),
-            map( 1, 'b', 2, map( 10, 'b' ) ),
+            map( "1", 'b', "2", map( "10", 'b' ) ),
-            map( 1, 'b', 2, map( 20, 'a' ) ),
+            map( "1", 'b', "2", map( "20", 'a' ) ),
-            map( 1, map( 1, map( 1, 'a' ) ), 2, 'x' ),
+            map( "1", map( "1", map( "1", 'a' ) ), "2", 'x' ),
-            map( 1, map( 1, map( 1, 'b' ) ), 2, 'x' ),
+            map( "1", map( "1", map( "1", 'b' ) ), "2", 'x' ),
     };
 
     @Test

diff --git a/community/values/src/test/java/org/neo4j/values/virtual/BufferAnyValueWriter.java b/community/values/src/test/java/org/neo4j/values/virtual/BufferAnyValueWriter.java
@@ -143,12 +143,6 @@ public void beginMap( int size )
         buffer.add( Specials.beginMap( size ) );
     }
 
-    @Override
-    public void writeKeyId( int keyId )
-    {
-        buffer.add( Specials.writeKeyId( keyId ) );
-    }
-
     @Override
     public void endMap()
     {

diff --git a/community/values/src/test/java/org/neo4j/values/virtual/GraphElementValueTest.java b/community/values/src/test/java/org/neo4j/values/virtual/GraphElementValueTest.java
@@ -24,11 +24,11 @@
 import static org.junit.Assert.fail;
 import static org.neo4j.values.virtual.VirtualValueTestUtil.assertEqual;
 import static org.neo4j.values.virtual.VirtualValueTestUtil.assertNotEqual;
+import static org.neo4j.values.virtual.VirtualValueTestUtil.edge;
 import static org.neo4j.values.virtual.VirtualValueTestUtil.edges;
+import static org.neo4j.values.virtual.VirtualValueTestUtil.node;
 import static org.neo4j.values.virtual.VirtualValueTestUtil.nodes;
 import static org.neo4j.values.virtual.VirtualValueTestUtil.path;
-import static org.neo4j.values.virtual.VirtualValues.edge;
-import static org.neo4j.values.virtual.VirtualValues.node;
 
 public class GraphElementValueTest
 {
@@ -47,42 +47,42 @@ public void nodeShouldNotEqualOtherNode()
     @Test
     public void edgeShouldEqualItself()
     {
-        assertEqual( edge( 1L ), edge( 1L ) );
+        assertEqual( edge( 1L, 1L, 2L ), edge( 1L ,1L, 2L) );
     }
 
     @Test
     public void edgeShouldNotEqualOtherEdge()
     {
-        assertNotEqual( edge( 1L ), edge( 2L ) );
+        assertNotEqual( edge( 1L, 1L, 2L), edge( 2L, 1L, 2L ) );
     }
 
     @Test
     public void pathShouldEqualItself()
     {
         assertEqual( path( node( 1L ) ), path( node( 1L ) ) );
         assertEqual(
-                path( node( 1L ), edge( 2L ), node( 3L ) ),
+                path( node( 1L ), edge( 2L, 1L, 3L ), node( 3L ) ),
-                path( node( 1L ), edge( 2L ), node( 3L ) ) );
+                path( node( 1L ), edge( 2L, 1L, 3L ), node( 3L ) ) );
 
         assertEqual(
-                path( node( 1L ), edge( 2L ), node( 2L ), edge( 3L ), node( 1L ) ),
+                path( node( 1L ), edge( 2L, 1L, 3L ), node( 2L ), edge( 3L, 2L, 1L ), node( 1L ) ),
-                path( node( 1L ), edge( 2L ), node( 2L ), edge( 3L ), node( 1L ) ) );
+                path( node( 1L ), edge( 2L, 1L, 3L ), node( 2L ), edge( 3L, 2L, 1L ), node( 1L ) ) );
     }
 
     @Test
     public void pathShouldNotEqualOtherPath()
     {
         assertNotEqual( path( node( 1L ) ), path( node( 2L ) ) );
-        assertNotEqual( path( node( 1L ) ), path( node( 1L ), edge( 1L ), node( 2L ) ) );
+        assertNotEqual( path( node( 1L ) ), path( node( 1L ), edge( 1L, 1L, 2L ), node( 2L ) ) );
-        assertNotEqual( path( node( 1L ) ), path( node( 2L ), edge( 1L ), node( 1L ) ) );
+        assertNotEqual( path( node( 1L ) ), path( node( 2L ), edge( 1L, 2L, 1L ), node( 1L ) ) );
 
         assertNotEqual(
-                path( node( 1L ), edge( 2L ), node( 3L ) ),
+                path( node( 1L ), edge( 2L, 1L, 3L ), node( 3L ) ),
-                path( node( 1L ), edge( 3L ), node( 3L ) ) );
+                path( node( 1L ), edge( 3L, 1L, 3L ), node( 3L ) ) );
 
         assertNotEqual(
-                path( node( 1L ), edge( 2L ), node( 2L ) ),
+                path( node( 1L ), edge( 2L, 1L, 2L ), node( 2L ) ),
-                path( node( 1L ), edge( 2L ), node( 3L ) ) );
+                path( node( 1L ), edge( 2L, 1L, 3L ), node( 3L ) ) );
     }
 
     @Test

diff --git a/community/values/src/test/java/org/neo4j/values/virtual/MapTest.java b/community/values/src/test/java/org/neo4j/values/virtual/MapTest.java
@@ -21,12 +21,6 @@
 
 import org.junit.Test;
 
-import org.neo4j.values.AnyValue;
-import org.neo4j.values.Values;
-import org.neo4j.values.VirtualValue;
-
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
 import static org.neo4j.values.virtual.VirtualValueTestUtil.assertEqual;
 import static org.neo4j.values.virtual.VirtualValueTestUtil.map;
 
@@ -36,47 +30,47 @@ public class MapTest
     public void shouldBeEqualToItself()
     {
         assertEqual(
-                map( 1, false, 20, new short[]{4} ),
+                map( "1", false, "20", new short[]{4} ),
-                map( 1, false, 20, new short[]{4} ) );
+                map( "1", false, "20", new short[]{4} ) );
 
         assertEqual(
-                map( 1, 101L, 20, "yo" ),
+                map( "1", 101L, "20", "yo" ),
-                map( 1, 101L, 20, "yo" ) );
+                map( "1", 101L, "20", "yo" ) );
     }
 
     @Test
     public void shouldCoerce()
     {
         assertEqual(
-                map( 1, 1, 20, 'a' ),
+                map( "1", 1, "20", 'a' ),
-                map( 1, 1.0, 20, "a" ) );
+                map( "1", 1.0, "20", "a" ) );
 
         assertEqual(
-                map( 1, new byte[]{1}, 20, new String[]{"x"} ),
+                map( "1", new byte[]{1}, "20", new String[]{"x"} ),
-                map( 1, new short[]{1}, 20, new char[]{'x'} ) );
+                map( "1", new short[]{1}, "20", new char[]{'x'} ) );
 
         assertEqual(
-                map( 1, new int[]{1}, 20, new double[]{2.0} ),
+                map( "1", new int[]{1}, "20", new double[]{2.0} ),
-                map( 1, new float[]{1.0f}, 20, new float[]{2.0f} ) );
+                map( "1", new float[]{1.0f}, "20", new float[]{2.0f} ) );
     }
 
     @Test
     public void shouldRecurse()
     {
         assertEqual(
-                map( 1, map( 2, map( 3, "hi" ) ) ),
+                map( "1", map( "2", map( "3", "hi" ) ) ),
-                map( 1, map( 2, map( 3, "hi" ) ) ) );
+                map( "1", map( "2", map( "3", "hi" ) ) ) );
     }
 
     @Test
     public void shouldRecurseAndCoerce()
     {
         assertEqual(
-                map( 1, map( 2, map( 3, "x" ) ) ),
+                map( "1", map( "2", map( "3", "x" ) ) ),
-                map( 1, map( 2, map( 3, 'x' ) ) ) );
+                map( "1", map( "2", map( "3", 'x' ) ) ) );
 
         assertEqual(
-                map( 1, map( 2, map( 3, 1.0 ) ) ),
+                map( "1", map( "2", map( "3", 1.0 ) ) ),
-                map( 1, map( 2, map( 3, 1 ) ) ) );
+                map( "1", map( "2", map( "3", 1 ) ) ) );
     }
 }