Add exceptions to numeric id accessors in nodes and relationships

driver/src/main/java/org/neo4j/driver/internal/InternalEntity.java

@@ -32,20 +32,25 @@
 
 public abstract class InternalEntity implements Entity, AsValue
 {
+    public static final String INVALID_ID_ERROR = "Numeric id is not available, please use string based element id alternative";
+
     private final long id;
     private final String elementId;
     private final Map<String,Value> properties;
+    private final boolean numericIdAvailable;
 
-    public InternalEntity( long id, String elementId, Map<String,Value> properties )
+    public InternalEntity( long id, String elementId, Map<String,Value> properties, boolean numericIdAvailable )
     {
         this.id = id;
         this.elementId = elementId;
         this.properties = properties;
+        this.numericIdAvailable = numericIdAvailable;
     }
 
     @Override
     public long id()
     {
+        assertNumericIdAvailable();
         return id;
     }
 
@@ -142,4 +147,17 @@ public <T> Iterable<T> values( Function<Value,T> mapFunction )
     {
         return Iterables.map( properties.values(), mapFunction );
     }
+
+    protected void assertNumericIdAvailable()
+    {
+        if ( !numericIdAvailable )
+        {
+            throw new IllegalStateException( INVALID_ID_ERROR );
+        }
+    }
+
+    public boolean isNumericIdAvailable()
+    {
+        return numericIdAvailable;
+    }
 }

driver/src/main/java/org/neo4j/driver/internal/InternalNode.java

@@ -35,12 +35,17 @@ public class InternalNode extends InternalEntity implements Node
 
     public InternalNode( long id )
     {
-        this( id, String.valueOf( id ), Collections.emptyList(), Collections.emptyMap() );
+        this( id, Collections.emptyList(), Collections.emptyMap() );
     }
 
-    public InternalNode( long id, String elementId, Collection<String> labels, Map<String,Value> properties )
+    public InternalNode( long id, Collection<String> labels, Map<String,Value> properties )
     {
-        super( id, elementId, properties );
+        this( id, String.valueOf( id ), labels, properties, true );
+    }
+
+    public InternalNode( long id, String elementId, Collection<String> labels, Map<String,Value> properties, boolean numericIdAvailable )
+    {
+        super( id, elementId, properties, numericIdAvailable );
         this.labels = labels;
     }
 

driver/src/main/java/org/neo4j/driver/internal/InternalRelationship.java

@@ -36,15 +36,20 @@ public class InternalRelationship extends InternalEntity implements Relationship
     private String endElementId;
     private final String type;
 
-    public InternalRelationship( long id, String elementId, long start, String startElementId, long end, String endElementId, String type )
+    public InternalRelationship( long id, long start, long end, String type )
     {
-        this( id, elementId, start, startElementId, end, endElementId, type, Collections.emptyMap() );
+        this( id, start, end, type, Collections.emptyMap() );
+    }
+
+    public InternalRelationship( long id, long start, long end, String type, Map<String,Value> properties )
+    {
+        this( id, String.valueOf( id ), start, String.valueOf( start ), end, String.valueOf( end ), type, properties, true );
     }
 
     public InternalRelationship( long id, String elementId, long start, String startElementId, long end, String endElementId, String type,
-                                 Map<String,Value> properties )
+                                 Map<String,Value> properties, boolean numericIdAvailable )
     {
-        super( id, elementId, properties );
+        super( id, elementId, properties, numericIdAvailable );
         this.start = start;
         this.startElementId = startElementId;
         this.end = end;
@@ -72,6 +77,7 @@ public void setStartAndEnd( long start, String startElementId, long end, String
     @Override
     public long startNodeId()
     {
+        assertNumericIdAvailable();
         return start;
     }
 
@@ -84,6 +90,7 @@ public String startNodeElementId()
     @Override
     public long endNodeId()
     {
+        assertNumericIdAvailable();
         return end;
     }
 

driver/src/main/java/org/neo4j/driver/internal/messaging/common/CommonValueUnpacker.java

@@ -237,8 +237,8 @@ protected Value unpackRelationship() throws IOException
         String relType = unpacker.unpackString();
         Map<String,Value> props = unpackMap();
 
-        InternalRelationship adapted =
-                new InternalRelationship( urn, String.valueOf( urn ), startUrn, String.valueOf( startUrn ), endUrn, String.valueOf( endUrn ), relType, props );
+        InternalRelationship adapted = new InternalRelationship( urn, String.valueOf( urn ), startUrn, String.valueOf( startUrn ), endUrn,
+                                                                 String.valueOf( endUrn ), relType, props, true );
         return new RelationshipValue( adapted );
     }
 
@@ -260,7 +260,7 @@ protected InternalNode unpackNode() throws IOException
             props.put( key, unpack() );
         }
 
-        return new InternalNode( urn, String.valueOf( urn ), labels, props );
+        return new InternalNode( urn, String.valueOf( urn ), labels, props, true );
     }
 
     protected Value unpackPath() throws IOException
@@ -283,7 +283,7 @@ protected Value unpackPath() throws IOException
             long id = unpacker.unpackLong();
             String relType = unpacker.unpackString();
             Map<String,Value> props = unpackMap();
-            uniqRels[i] = new InternalRelationship( id, String.valueOf( id ), -1, String.valueOf( -1 ), -1, String.valueOf( -1 ), relType, props );
+            uniqRels[i] = new InternalRelationship( id, String.valueOf( id ), -1, String.valueOf( -1 ), -1, String.valueOf( -1 ), relType, props, true );
         }
 
         // Path sequence

driver/src/main/java/org/neo4j/driver/internal/messaging/v5/ValueUnpackerV5.java

@@ -63,7 +63,7 @@ protected int getRelationshipFields()
     @Override
     protected InternalNode unpackNode() throws IOException
     {
-        long urn = unpacker.unpackLongOrDefaultOnNull( -1 );
+        Long urn = unpacker.unpackLongOrNull();
 
         int numLabels = (int) unpacker.unpackListHeader();
         List<String> labels = new ArrayList<>( numLabels );
@@ -81,14 +81,14 @@ protected InternalNode unpackNode() throws IOException
 
         String elementId = unpacker.unpackString();
 
-        return new InternalNode( urn, elementId, labels, props );
+        return urn == null ? new InternalNode( -1, elementId, labels, props, false ) : new InternalNode( urn, elementId, labels, props, true );
     }
 
     @Override
     protected Value unpackPath() throws IOException
     {
         // List of unique nodes
-        Node[] uniqNodes = new Node[(int) unpacker.unpackListHeader()];
+        InternalNode[] uniqNodes = new InternalNode[(int) unpacker.unpackListHeader()];
         for ( int i = 0; i < uniqNodes.length; i++ )
         {
             ensureCorrectStructSize( TypeConstructor.NODE, getNodeFields(), unpacker.unpackStructHeader() );
@@ -102,11 +102,13 @@ protected Value unpackPath() throws IOException
         {
             ensureCorrectStructSize( TypeConstructor.RELATIONSHIP, 4, unpacker.unpackStructHeader() );
             ensureCorrectStructSignature( "UNBOUND_RELATIONSHIP", UNBOUND_RELATIONSHIP, unpacker.unpackStructSignature() );
-            Long id = unpacker.unpackLongOrDefaultOnNull( -1 );
+            Long id = unpacker.unpackLongOrNull();
             String relType = unpacker.unpackString();
             Map<String,Value> props = unpackMap();
             String elementId = unpacker.unpackString();
-            uniqRels[i] = new InternalRelationship( id, elementId, -1, String.valueOf( -1 ), -1, String.valueOf( -1 ), relType, props );
+            uniqRels[i] = id == null
+                          ? new InternalRelationship( -1, elementId, -1, String.valueOf( -1 ), -1, String.valueOf( -1 ), relType, props, false )
+                          : new InternalRelationship( id, elementId, -1, String.valueOf( -1 ), -1, String.valueOf( -1 ), relType, props, true );
         }
 
         // Path sequence
@@ -117,7 +119,7 @@ protected Value unpackPath() throws IOException
         Node[] nodes = new Node[segments.length + 1];
         Relationship[] rels = new Relationship[segments.length];
 
-        Node prevNode = uniqNodes[0], nextNode; // Start node is always 0, and isn't encoded in the sequence
+        InternalNode prevNode = uniqNodes[0], nextNode; // Start node is always 0, and isn't encoded in the sequence
         nodes[0] = prevNode;
         InternalRelationship rel;
         for ( int i = 0; i < segments.length; i++ )
@@ -128,12 +130,12 @@ protected Value unpackPath() throws IOException
             if ( relIdx < 0 )
             {
                 rel = uniqRels[(-relIdx) - 1]; // -1 because rel idx are 1-indexed
-                rel.setStartAndEnd( nextNode.id(), nextNode.elementId(), prevNode.id(), prevNode.elementId() );
+                setStartAndEnd( rel, nextNode, prevNode );
             }
             else
             {
                 rel = uniqRels[relIdx - 1];
-                rel.setStartAndEnd( prevNode.id(), prevNode.elementId(), nextNode.id(), nextNode.elementId() );
+                setStartAndEnd( rel, prevNode, nextNode );
             }
 
             nodes[i + 1] = nextNode;
@@ -144,19 +146,33 @@ protected Value unpackPath() throws IOException
         return new PathValue( new InternalPath( Arrays.asList( segments ), Arrays.asList( nodes ), Arrays.asList( rels ) ) );
     }
 
+    private void setStartAndEnd( InternalRelationship rel, InternalNode start, InternalNode end )
+    {
+        if ( rel.isNumericIdAvailable() && start.isNumericIdAvailable() && end.isNumericIdAvailable() )
+        {
+            rel.setStartAndEnd( start.id(), start.elementId(), end.id(), end.elementId() );
+        }
+        else
+        {
+            rel.setStartAndEnd( -1, start.elementId(), -1, end.elementId() );
+        }
+    }
+
     @Override
     protected Value unpackRelationship() throws IOException
     {
-        long urn = unpacker.unpackLongOrDefaultOnNull( -1 );
-        long startUrn = unpacker.unpackLongOrDefaultOnNull( -1 );
-        long endUrn = unpacker.unpackLongOrDefaultOnNull( -1 );
+        Long urn = unpacker.unpackLongOrNull();
+        Long startUrn = unpacker.unpackLongOrNull();
+        Long endUrn = unpacker.unpackLongOrNull();
         String relType = unpacker.unpackString();
         Map<String,Value> props = unpackMap();
         String elementId = unpacker.unpackString();
         String startElementId = unpacker.unpackString();
         String endElementId = unpacker.unpackString();
 
-        InternalRelationship adapted = new InternalRelationship( urn, elementId, startUrn, startElementId, endUrn, endElementId, relType, props );
+        InternalRelationship adapted = urn == null
+                                       ? new InternalRelationship( -1, elementId, -1, startElementId, -1, endElementId, relType, props, false )
+                                       : new InternalRelationship( urn, elementId, startUrn, startElementId, endUrn, endElementId, relType, props, true );
         return new RelationshipValue( adapted );
     }
 }

driver/src/main/java/org/neo4j/driver/internal/packstream/PackStream.java

@@ -467,7 +467,7 @@ public long unpackMapHeader() throws IOException
             }
         }
 
-        public Long unpackLongOrDefaultOnNull( long defaultValue ) throws IOException
+        public Long unpackLongOrNull() throws IOException
         {
             final byte markerByte = in.readByte();
             if ( markerByte >= MINUS_2_TO_THE_4 )
@@ -485,7 +485,7 @@ public Long unpackLongOrDefaultOnNull( long defaultValue ) throws IOException
             case INT_64:
                 return in.readLong();
             case NULL:
-                return defaultValue;
+                return null;
             default:
                 throw new Unexpected( "Expected an integer, but got: " + toHexString( markerByte ) );
             }

driver/src/main/java/org/neo4j/driver/types/Entity.java

@@ -31,9 +31,10 @@ public interface Entity extends MapAccessor
      * A unique id for this Entity. Ids are guaranteed to remain stable for the duration of the session they were found in, but may be re-used for other
      * entities after that. As such, if you want a public identity to use for your entities, attaching an explicit 'id' property or similar persistent and
      * unique identifier is a better choice.
+     * <p>
+     * Please note that depending on server configuration numeric id might not be available and accessing it will result in {@link IllegalStateException}.
      *
-     * @return the id of this entity, please note that negative values are considered to be invalid and may be returned when storage engine can only provide the
-     * {@link #elementId()}
+     * @return the id of this entity
      * @deprecated superseded by {@link #elementId()}
      */
     @Deprecated

driver/src/main/java/org/neo4j/driver/types/Relationship.java

@@ -26,6 +26,8 @@ public interface Relationship extends Entity
 {
     /**
      * Id of the node where this relationship starts.
+     * <p>
+     * Please note that depending on server configuration numeric id might not be available and accessing it will result in {@link IllegalStateException}.
      *
      * @return the node id
      * @deprecated superseded by {@link #startNodeElementId()}
@@ -42,6 +44,8 @@ public interface Relationship extends Entity
 
     /**
      * Id of the node where this relationship ends.
+     * <p>
+     * Please note that depending on server configuration numeric id might not be available and accessing it will result in {@link IllegalStateException}.
      *
      * @return the node id
      * @deprecated superseded by {@link #endNodeElementId()}

driver/src/test/java/org/neo4j/driver/TransactionConfigTest.java

@@ -71,7 +71,7 @@ void shouldDisallowMetadataWithIllegalValues()
 
         assertThrows( ClientException.class,
                       () -> TransactionConfig.builder().withMetadata(
-                              singletonMap( "key", new InternalRelationship( 1, String.valueOf( 1 ), 1, String.valueOf( 1 ), 1, String.valueOf( 1 ), "" ) ) ) );
+                              singletonMap( "key", new InternalRelationship( 1, 1, 1, "" ) ) ) );
 
         assertThrows( ClientException.class,
                 () -> TransactionConfig.builder().withMetadata( singletonMap( "key", new InternalPath( new InternalNode( 1 ) ) ) ) );

driver/src/test/java/org/neo4j/driver/internal/ExtractTest.java

@@ -127,7 +127,7 @@ void testProperties()
         Map<String,Value> props = new HashMap<>();
         props.put( "k1", value( 43 ) );
         props.put( "k2", value( 42 ) );
-        InternalNode node = new InternalNode( 42L, String.valueOf( 42L ), Collections.singletonList( "L" ), props );
+        InternalNode node = new InternalNode( 42L, Collections.singletonList( "L" ), props );
 
         // WHEN
         Iterable<Pair<String,Integer>> properties = Extract.properties( node, Value::asInt );
@@ -143,7 +143,7 @@ void testProperties()
     void testFields()
     {
         // GIVEN
-        InternalRecord record = new InternalRecord( Arrays.asList( "k1" ), new Value[]{value( 42 )} );
+        InternalRecord record = new InternalRecord( singletonList( "k1" ), new Value[]{value( 42 )} );
         // WHEN
         List<Pair<String,Integer>> fields = Extract.fields( record, Value::asInt );
 
@@ -181,11 +181,7 @@ void shouldExtractMapOfValues()
     void shouldFailToExtractMapOfValuesFromUnsupportedValues()
     {
         assertThrows( ClientException.class, () -> Extract.mapOfValues( singletonMap( "key", new InternalNode( 1 ) ) ) );
-        assertThrows( ClientException.class,
-                      () -> Extract.mapOfValues(
-                              singletonMap( "key",
-                                            new InternalRelationship( 1, String.valueOf( 1 ), 1, String.valueOf( 1 ),
-                                                                      1, String.valueOf( 1 ), "HI" ) ) ) );
+        assertThrows( ClientException.class, () -> Extract.mapOfValues( singletonMap( "key", new InternalRelationship( 1, 1, 1, "HI" ) ) ) );
         assertThrows( ClientException.class, () -> Extract.mapOfValues( singletonMap( "key", new InternalPath( new InternalNode( 1 ) ) ) ) );
     }
 }