Remove null handling for Node/Relationship Ids

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

@@ -36,18 +36,15 @@ public abstract class InternalEntity implements Entity, AsValue {
     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, boolean numericIdAvailable) {
+    public InternalEntity(long id, String elementId, Map<String, Value> properties) {
         this.id = id;
         this.elementId = elementId;
         this.properties = properties;
-        this.numericIdAvailable = numericIdAvailable;
     }
 
     @Override
     public long id() {
-        assertNumericIdAvailable();
         return id;
     }
 
@@ -125,14 +122,4 @@ public Iterable<Value> values() {
     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

@@ -36,16 +36,11 @@ public InternalNode(long id) {
     }
 
     public InternalNode(long id, Collection<String> labels, Map<String, Value> properties) {
-        this(id, String.valueOf(id), labels, properties, true);
+        this(id, String.valueOf(id), labels, properties);
     }
 
-    public InternalNode(
-            long id,
-            String elementId,
-            Collection<String> labels,
-            Map<String, Value> properties,
-            boolean numericIdAvailable) {
-        super(id, elementId, properties, numericIdAvailable);
+    public InternalNode(long id, String elementId, Collection<String> labels, Map<String, Value> properties) {
+        super(id, elementId, properties);
         this.labels = labels;
     }
 

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

@@ -39,7 +39,7 @@ public InternalRelationship(long id, long start, long end, String type) {
     }
 
     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);
+        this(id, String.valueOf(id), start, String.valueOf(start), end, String.valueOf(end), type, properties);
     }
 
     public InternalRelationship(
@@ -50,9 +50,8 @@ public InternalRelationship(
             long end,
             String endElementId,
             String type,
-            Map<String, Value> properties,
-            boolean numericIdAvailable) {
-        super(id, elementId, properties, numericIdAvailable);
+            Map<String, Value> properties) {
+        super(id, elementId, properties);
         this.start = start;
         this.startElementId = startElementId;
         this.end = end;
@@ -77,7 +76,6 @@ public void setStartAndEnd(long start, String startElementId, long end, String e
 
     @Override
     public long startNodeId() {
-        assertNumericIdAvailable();
         return start;
     }
 
@@ -88,7 +86,6 @@ public String startNodeElementId() {
 
     @Override
     public long endNodeId() {
-        assertNumericIdAvailable();
         return end;
     }
 

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

@@ -244,8 +244,7 @@ protected Value unpackRelationship() throws IOException {
                 endUrn,
                 String.valueOf(endUrn),
                 relType,
-                props,
-                true);
+                props);
         return new RelationshipValue(adapted);
     }
 
@@ -264,7 +263,7 @@ protected InternalNode unpackNode() throws IOException {
             props.put(key, unpack());
         }
 
-        return new InternalNode(urn, String.valueOf(urn), labels, props, true);
+        return new InternalNode(urn, String.valueOf(urn), labels, props);
     }
 
     protected Value unpackPath() throws IOException {
@@ -286,7 +285,7 @@ protected Value unpackPath() throws IOException {
             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, true);
+                    id, String.valueOf(id), -1, String.valueOf(-1), -1, String.valueOf(-1), relType, props);
         }
 
         // Path sequence

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

@@ -57,7 +57,7 @@ protected int getRelationshipFields() {
 
     @Override
     protected InternalNode unpackNode() throws IOException {
-        Long urn = unpacker.unpackLongOrNull();
+        long urn = unpacker.unpackLong();
 
         int numLabels = (int) unpacker.unpackListHeader();
         List<String> labels = new ArrayList<>(numLabels);
@@ -73,9 +73,7 @@ protected InternalNode unpackNode() throws IOException {
 
         String elementId = unpacker.unpackString();
 
-        return urn == null
-                ? new InternalNode(-1, elementId, labels, props, false)
-                : new InternalNode(urn, elementId, labels, props, true);
+        return new InternalNode(urn, elementId, labels, props);
     }
 
     @Override
@@ -94,15 +92,12 @@ protected Value unpackPath() throws IOException {
             ensureCorrectStructSize(TypeConstructor.RELATIONSHIP, 4, unpacker.unpackStructHeader());
             ensureCorrectStructSignature(
                     "UNBOUND_RELATIONSHIP", UNBOUND_RELATIONSHIP, unpacker.unpackStructSignature());
-            Long id = unpacker.unpackLongOrNull();
+            long id = unpacker.unpackLong();
             String relType = unpacker.unpackString();
             Map<String, Value> props = unpackMap();
             String elementId = unpacker.unpackString();
-            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);
+            uniqRels[i] = new InternalRelationship(
+                    id, elementId, -1, String.valueOf(-1), -1, String.valueOf(-1), relType, props);
         }
 
         // Path sequence
@@ -138,28 +133,22 @@ protected Value unpackPath() throws IOException {
     }
 
     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());
-        }
+        rel.setStartAndEnd(start.id(), start.elementId(), end.id(), end.elementId());
     }
 
     @Override
     protected Value unpackRelationship() throws IOException {
-        Long urn = unpacker.unpackLongOrNull();
-        Long startUrn = unpacker.unpackLongOrNull();
-        Long endUrn = unpacker.unpackLongOrNull();
+        long urn = unpacker.unpackLong();
+        long startUrn = unpacker.unpackLong();
+        long endUrn = unpacker.unpackLong();
         String relType = unpacker.unpackString();
         Map<String, Value> props = unpackMap();
         String elementId = unpacker.unpackString();
         String startElementId = unpacker.unpackString();
         String endElementId = unpacker.unpackString();
 
-        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);
+        InternalRelationship adapted = new InternalRelationship(
+                urn, elementId, startUrn, startElementId, endUrn, endElementId, relType, props);
         return new RelationshipValue(adapted);
     }
 }

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

@@ -404,27 +404,6 @@ public long unpackMapHeader() throws IOException {
             }
         }
 
-        public Long unpackLongOrNull() throws IOException {
-            final byte markerByte = in.readByte();
-            if (markerByte >= MINUS_2_TO_THE_4) {
-                return (long) markerByte;
-            }
-            switch (markerByte) {
-                case INT_8:
-                    return (long) in.readByte();
-                case INT_16:
-                    return (long) in.readShort();
-                case INT_32:
-                    return (long) in.readInt();
-                case INT_64:
-                    return in.readLong();
-                case NULL:
-                    return null;
-                default:
-                    throw new Unexpected("Expected an integer, but got: " + toHexString(markerByte));
-            }
-        }
-
         public long unpackLong() throws IOException {
             final byte markerByte = in.readByte();
             if (markerByte >= MINUS_2_TO_THE_4) {

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

@@ -20,9 +20,7 @@
 
 import static org.hamcrest.CoreMatchers.equalTo;
 import static org.hamcrest.junit.MatcherAssert.assertThat;
-import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
-import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.neo4j.driver.Values.NULL;
 import static org.neo4j.driver.Values.value;
 
@@ -60,20 +58,10 @@ void accessUnknownKeyShouldBeNull() {
         assertThat(node.get("k3"), equalTo(NULL));
     }
 
-    @Test
-    void shouldThrowOnIdWhenNumericIdUnavailable() {
-        // GIVEN
-        InternalNode node = new InternalNode(-1, "value", Collections.emptyList(), Collections.emptyMap(), false);
-
-        // WHEN & THEN
-        IllegalStateException e = assertThrows(IllegalStateException.class, node::id);
-        assertEquals(InternalEntity.INVALID_ID_ERROR, e.getMessage());
-    }
-
     private InternalNode createNode() {
         Map<String, Value> props = new HashMap<>();
         props.put("k1", value(1));
         props.put("k2", value(2));
-        return new InternalNode(42L, String.valueOf(42L), Collections.singletonList("L"), props, true);
+        return new InternalNode(42L, String.valueOf(42L), Collections.singletonList("L"), props);
     }
 }

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

@@ -20,13 +20,10 @@
 
 import static org.hamcrest.CoreMatchers.equalTo;
 import static org.hamcrest.junit.MatcherAssert.assertThat;
-import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
-import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.neo4j.driver.Values.NULL;
 import static org.neo4j.driver.Values.value;
 
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.Map;
@@ -60,39 +57,6 @@ void accessUnknownKeyShouldBeNull() {
         assertThat(relationship.get("k3"), equalTo(NULL));
     }
 
-    @Test
-    void shouldThrowOnIdWhenNumericIdUnavailable() {
-        // GIVEN
-        InternalRelationship relationship = new InternalRelationship(
-                -1, "value", 1, String.valueOf(1), 2, String.valueOf(2), "T", Collections.emptyMap(), false);
-
-        // WHEN & THEN
-        IllegalStateException e = assertThrows(IllegalStateException.class, relationship::id);
-        assertEquals(InternalEntity.INVALID_ID_ERROR, e.getMessage());
-    }
-
-    @Test
-    void shouldThrowOnStartNodeIdWhenNumericIdUnavailable() {
-        // GIVEN
-        InternalRelationship relationship = new InternalRelationship(
-                -1, "value", 1, String.valueOf(1), 2, String.valueOf(2), "T", Collections.emptyMap(), false);
-
-        // WHEN & THEN
-        IllegalStateException e = assertThrows(IllegalStateException.class, relationship::startNodeId);
-        assertEquals(InternalEntity.INVALID_ID_ERROR, e.getMessage());
-    }
-
-    @Test
-    void shouldThrowOnEndNodeIdWhenNumericIdUnavailable() {
-        // GIVEN
-        InternalRelationship relationship = new InternalRelationship(
-                -1, "value", 1, String.valueOf(1), 2, String.valueOf(2), "T", Collections.emptyMap(), false);
-
-        // WHEN & THEN
-        IllegalStateException e = assertThrows(IllegalStateException.class, relationship::endNodeId);
-        assertEquals(InternalEntity.INVALID_ID_ERROR, e.getMessage());
-    }
-
     private InternalRelationship createRelationship() {
         Map<String, Value> props = new HashMap<>();
         props.put("k1", value(1));

driver/src/test/java/org/neo4j/driver/types/TypeSystemTest.java

@@ -43,17 +43,9 @@
 
 class TypeSystemTest {
     private final InternalNode node =
-            new InternalNode(42L, String.valueOf(42L), Collections.emptyList(), Collections.emptyMap(), true);
+            new InternalNode(42L, String.valueOf(42L), Collections.emptyList(), Collections.emptyMap());
     private final InternalRelationship relationship = new InternalRelationship(
-            42L,
-            String.valueOf(42L),
-            42L,
-            String.valueOf(42L),
-            43L,
-            String.valueOf(43L),
-            "T",
-            Collections.emptyMap(),
-            true);
+            42L, String.valueOf(42L), 42L, String.valueOf(42L), 43L, String.valueOf(43L), "T", Collections.emptyMap());
 
     private Value integerValue = value(13);
     private Value floatValue = value(13.1);

testkit-backend/src/main/java/neo4j/org/testkit/backend/messages/requests/GetFeatures.java

@@ -55,7 +55,6 @@ public class GetFeatures implements TestkitRequest {
             "Feature:API:Driver.IsEncrypted",
             "Feature:API:SSLConfig",
             "Detail:DefaultSecurityConfigValueEquality",
-            "Detail:ThrowOnMissingId",
             "Optimization:ImplicitDefaultArguments",
             "Feature:Bolt:Patch:UTC",
             "Feature:API:Type.Temporal"));