Skip to content

Commit 092de0b

Browse files
thelonelyvulpesrobsdedude
thelonelyvulpes
and
robsdedude
authored
Remove null handling for Node/Relationship Ids (neo4j#1240)
* remove null handling for nodes and relationships * Remove unused feature * Code style Co-authored-by: Rouven Bauer <[email protected]>
1 parent 8a436c1 commit 092de0b

File tree

10 files changed

+24
-135
lines changed

10 files changed

+24
-135
lines changed

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

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -36,18 +36,15 @@ public abstract class InternalEntity implements Entity, AsValue {
3636
private final long id;
3737
private final String elementId;
3838
private final Map<String, Value> properties;
39-
private final boolean numericIdAvailable;
4039

41-
public InternalEntity(long id, String elementId, Map<String, Value> properties, boolean numericIdAvailable) {
40+
public InternalEntity(long id, String elementId, Map<String, Value> properties) {
4241
this.id = id;
4342
this.elementId = elementId;
4443
this.properties = properties;
45-
this.numericIdAvailable = numericIdAvailable;
4644
}
4745

4846
@Override
4947
public long id() {
50-
assertNumericIdAvailable();
5148
return id;
5249
}
5350

@@ -125,14 +122,4 @@ public Iterable<Value> values() {
125122
public <T> Iterable<T> values(Function<Value, T> mapFunction) {
126123
return Iterables.map(properties.values(), mapFunction);
127124
}
128-
129-
protected void assertNumericIdAvailable() {
130-
if (!numericIdAvailable) {
131-
throw new IllegalStateException(INVALID_ID_ERROR);
132-
}
133-
}
134-
135-
public boolean isNumericIdAvailable() {
136-
return numericIdAvailable;
137-
}
138125
}

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

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -36,16 +36,11 @@ public InternalNode(long id) {
3636
}
3737

3838
public InternalNode(long id, Collection<String> labels, Map<String, Value> properties) {
39-
this(id, String.valueOf(id), labels, properties, true);
39+
this(id, String.valueOf(id), labels, properties);
4040
}
4141

42-
public InternalNode(
43-
long id,
44-
String elementId,
45-
Collection<String> labels,
46-
Map<String, Value> properties,
47-
boolean numericIdAvailable) {
48-
super(id, elementId, properties, numericIdAvailable);
42+
public InternalNode(long id, String elementId, Collection<String> labels, Map<String, Value> properties) {
43+
super(id, elementId, properties);
4944
this.labels = labels;
5045
}
5146

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

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ public InternalRelationship(long id, long start, long end, String type) {
3939
}
4040

4141
public InternalRelationship(long id, long start, long end, String type, Map<String, Value> properties) {
42-
this(id, String.valueOf(id), start, String.valueOf(start), end, String.valueOf(end), type, properties, true);
42+
this(id, String.valueOf(id), start, String.valueOf(start), end, String.valueOf(end), type, properties);
4343
}
4444

4545
public InternalRelationship(
@@ -50,9 +50,8 @@ public InternalRelationship(
5050
long end,
5151
String endElementId,
5252
String type,
53-
Map<String, Value> properties,
54-
boolean numericIdAvailable) {
55-
super(id, elementId, properties, numericIdAvailable);
53+
Map<String, Value> properties) {
54+
super(id, elementId, properties);
5655
this.start = start;
5756
this.startElementId = startElementId;
5857
this.end = end;
@@ -77,7 +76,6 @@ public void setStartAndEnd(long start, String startElementId, long end, String e
7776

7877
@Override
7978
public long startNodeId() {
80-
assertNumericIdAvailable();
8179
return start;
8280
}
8381

@@ -88,7 +86,6 @@ public String startNodeElementId() {
8886

8987
@Override
9088
public long endNodeId() {
91-
assertNumericIdAvailable();
9289
return end;
9390
}
9491

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -244,8 +244,7 @@ protected Value unpackRelationship() throws IOException {
244244
endUrn,
245245
String.valueOf(endUrn),
246246
relType,
247-
props,
248-
true);
247+
props);
249248
return new RelationshipValue(adapted);
250249
}
251250

@@ -264,7 +263,7 @@ protected InternalNode unpackNode() throws IOException {
264263
props.put(key, unpack());
265264
}
266265

267-
return new InternalNode(urn, String.valueOf(urn), labels, props, true);
266+
return new InternalNode(urn, String.valueOf(urn), labels, props);
268267
}
269268

270269
protected Value unpackPath() throws IOException {
@@ -286,7 +285,7 @@ protected Value unpackPath() throws IOException {
286285
String relType = unpacker.unpackString();
287286
Map<String, Value> props = unpackMap();
288287
uniqRels[i] = new InternalRelationship(
289-
id, String.valueOf(id), -1, String.valueOf(-1), -1, String.valueOf(-1), relType, props, true);
288+
id, String.valueOf(id), -1, String.valueOf(-1), -1, String.valueOf(-1), relType, props);
290289
}
291290

292291
// Path sequence

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

Lines changed: 11 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ protected int getRelationshipFields() {
5757

5858
@Override
5959
protected InternalNode unpackNode() throws IOException {
60-
Long urn = unpacker.unpackLongOrNull();
60+
long urn = unpacker.unpackLong();
6161

6262
int numLabels = (int) unpacker.unpackListHeader();
6363
List<String> labels = new ArrayList<>(numLabels);
@@ -73,9 +73,7 @@ protected InternalNode unpackNode() throws IOException {
7373

7474
String elementId = unpacker.unpackString();
7575

76-
return urn == null
77-
? new InternalNode(-1, elementId, labels, props, false)
78-
: new InternalNode(urn, elementId, labels, props, true);
76+
return new InternalNode(urn, elementId, labels, props);
7977
}
8078

8179
@Override
@@ -94,15 +92,12 @@ protected Value unpackPath() throws IOException {
9492
ensureCorrectStructSize(TypeConstructor.RELATIONSHIP, 4, unpacker.unpackStructHeader());
9593
ensureCorrectStructSignature(
9694
"UNBOUND_RELATIONSHIP", UNBOUND_RELATIONSHIP, unpacker.unpackStructSignature());
97-
Long id = unpacker.unpackLongOrNull();
95+
long id = unpacker.unpackLong();
9896
String relType = unpacker.unpackString();
9997
Map<String, Value> props = unpackMap();
10098
String elementId = unpacker.unpackString();
101-
uniqRels[i] = id == null
102-
? new InternalRelationship(
103-
-1, elementId, -1, String.valueOf(-1), -1, String.valueOf(-1), relType, props, false)
104-
: new InternalRelationship(
105-
id, elementId, -1, String.valueOf(-1), -1, String.valueOf(-1), relType, props, true);
99+
uniqRels[i] = new InternalRelationship(
100+
id, elementId, -1, String.valueOf(-1), -1, String.valueOf(-1), relType, props);
106101
}
107102

108103
// Path sequence
@@ -138,28 +133,22 @@ protected Value unpackPath() throws IOException {
138133
}
139134

140135
private void setStartAndEnd(InternalRelationship rel, InternalNode start, InternalNode end) {
141-
if (rel.isNumericIdAvailable() && start.isNumericIdAvailable() && end.isNumericIdAvailable()) {
142-
rel.setStartAndEnd(start.id(), start.elementId(), end.id(), end.elementId());
143-
} else {
144-
rel.setStartAndEnd(-1, start.elementId(), -1, end.elementId());
145-
}
136+
rel.setStartAndEnd(start.id(), start.elementId(), end.id(), end.elementId());
146137
}
147138

148139
@Override
149140
protected Value unpackRelationship() throws IOException {
150-
Long urn = unpacker.unpackLongOrNull();
151-
Long startUrn = unpacker.unpackLongOrNull();
152-
Long endUrn = unpacker.unpackLongOrNull();
141+
long urn = unpacker.unpackLong();
142+
long startUrn = unpacker.unpackLong();
143+
long endUrn = unpacker.unpackLong();
153144
String relType = unpacker.unpackString();
154145
Map<String, Value> props = unpackMap();
155146
String elementId = unpacker.unpackString();
156147
String startElementId = unpacker.unpackString();
157148
String endElementId = unpacker.unpackString();
158149

159-
InternalRelationship adapted = urn == null
160-
? new InternalRelationship(-1, elementId, -1, startElementId, -1, endElementId, relType, props, false)
161-
: new InternalRelationship(
162-
urn, elementId, startUrn, startElementId, endUrn, endElementId, relType, props, true);
150+
InternalRelationship adapted = new InternalRelationship(
151+
urn, elementId, startUrn, startElementId, endUrn, endElementId, relType, props);
163152
return new RelationshipValue(adapted);
164153
}
165154
}

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

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -404,27 +404,6 @@ public long unpackMapHeader() throws IOException {
404404
}
405405
}
406406

407-
public Long unpackLongOrNull() throws IOException {
408-
final byte markerByte = in.readByte();
409-
if (markerByte >= MINUS_2_TO_THE_4) {
410-
return (long) markerByte;
411-
}
412-
switch (markerByte) {
413-
case INT_8:
414-
return (long) in.readByte();
415-
case INT_16:
416-
return (long) in.readShort();
417-
case INT_32:
418-
return (long) in.readInt();
419-
case INT_64:
420-
return in.readLong();
421-
case NULL:
422-
return null;
423-
default:
424-
throw new Unexpected("Expected an integer, but got: " + toHexString(markerByte));
425-
}
426-
}
427-
428407
public long unpackLong() throws IOException {
429408
final byte markerByte = in.readByte();
430409
if (markerByte >= MINUS_2_TO_THE_4) {

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

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,7 @@
2020

2121
import static org.hamcrest.CoreMatchers.equalTo;
2222
import static org.hamcrest.junit.MatcherAssert.assertThat;
23-
import static org.junit.jupiter.api.Assertions.assertEquals;
2423
import static org.junit.jupiter.api.Assertions.assertFalse;
25-
import static org.junit.jupiter.api.Assertions.assertThrows;
2624
import static org.neo4j.driver.Values.NULL;
2725
import static org.neo4j.driver.Values.value;
2826

@@ -60,20 +58,10 @@ void accessUnknownKeyShouldBeNull() {
6058
assertThat(node.get("k3"), equalTo(NULL));
6159
}
6260

63-
@Test
64-
void shouldThrowOnIdWhenNumericIdUnavailable() {
65-
// GIVEN
66-
InternalNode node = new InternalNode(-1, "value", Collections.emptyList(), Collections.emptyMap(), false);
67-
68-
// WHEN & THEN
69-
IllegalStateException e = assertThrows(IllegalStateException.class, node::id);
70-
assertEquals(InternalEntity.INVALID_ID_ERROR, e.getMessage());
71-
}
72-
7361
private InternalNode createNode() {
7462
Map<String, Value> props = new HashMap<>();
7563
props.put("k1", value(1));
7664
props.put("k2", value(2));
77-
return new InternalNode(42L, String.valueOf(42L), Collections.singletonList("L"), props, true);
65+
return new InternalNode(42L, String.valueOf(42L), Collections.singletonList("L"), props);
7866
}
7967
}

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

Lines changed: 0 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,10 @@
2020

2121
import static org.hamcrest.CoreMatchers.equalTo;
2222
import static org.hamcrest.junit.MatcherAssert.assertThat;
23-
import static org.junit.jupiter.api.Assertions.assertEquals;
2423
import static org.junit.jupiter.api.Assertions.assertFalse;
25-
import static org.junit.jupiter.api.Assertions.assertThrows;
2624
import static org.neo4j.driver.Values.NULL;
2725
import static org.neo4j.driver.Values.value;
2826

29-
import java.util.Collections;
3027
import java.util.HashMap;
3128
import java.util.Iterator;
3229
import java.util.Map;
@@ -60,39 +57,6 @@ void accessUnknownKeyShouldBeNull() {
6057
assertThat(relationship.get("k3"), equalTo(NULL));
6158
}
6259

63-
@Test
64-
void shouldThrowOnIdWhenNumericIdUnavailable() {
65-
// GIVEN
66-
InternalRelationship relationship = new InternalRelationship(
67-
-1, "value", 1, String.valueOf(1), 2, String.valueOf(2), "T", Collections.emptyMap(), false);
68-
69-
// WHEN & THEN
70-
IllegalStateException e = assertThrows(IllegalStateException.class, relationship::id);
71-
assertEquals(InternalEntity.INVALID_ID_ERROR, e.getMessage());
72-
}
73-
74-
@Test
75-
void shouldThrowOnStartNodeIdWhenNumericIdUnavailable() {
76-
// GIVEN
77-
InternalRelationship relationship = new InternalRelationship(
78-
-1, "value", 1, String.valueOf(1), 2, String.valueOf(2), "T", Collections.emptyMap(), false);
79-
80-
// WHEN & THEN
81-
IllegalStateException e = assertThrows(IllegalStateException.class, relationship::startNodeId);
82-
assertEquals(InternalEntity.INVALID_ID_ERROR, e.getMessage());
83-
}
84-
85-
@Test
86-
void shouldThrowOnEndNodeIdWhenNumericIdUnavailable() {
87-
// GIVEN
88-
InternalRelationship relationship = new InternalRelationship(
89-
-1, "value", 1, String.valueOf(1), 2, String.valueOf(2), "T", Collections.emptyMap(), false);
90-
91-
// WHEN & THEN
92-
IllegalStateException e = assertThrows(IllegalStateException.class, relationship::endNodeId);
93-
assertEquals(InternalEntity.INVALID_ID_ERROR, e.getMessage());
94-
}
95-
9660
private InternalRelationship createRelationship() {
9761
Map<String, Value> props = new HashMap<>();
9862
props.put("k1", value(1));

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

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -43,17 +43,9 @@
4343

4444
class TypeSystemTest {
4545
private final InternalNode node =
46-
new InternalNode(42L, String.valueOf(42L), Collections.emptyList(), Collections.emptyMap(), true);
46+
new InternalNode(42L, String.valueOf(42L), Collections.emptyList(), Collections.emptyMap());
4747
private final InternalRelationship relationship = new InternalRelationship(
48-
42L,
49-
String.valueOf(42L),
50-
42L,
51-
String.valueOf(42L),
52-
43L,
53-
String.valueOf(43L),
54-
"T",
55-
Collections.emptyMap(),
56-
true);
48+
42L, String.valueOf(42L), 42L, String.valueOf(42L), 43L, String.valueOf(43L), "T", Collections.emptyMap());
5749

5850
private Value integerValue = value(13);
5951
private Value floatValue = value(13.1);

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,6 @@ public class GetFeatures implements TestkitRequest {
5555
"Feature:API:Driver.IsEncrypted",
5656
"Feature:API:SSLConfig",
5757
"Detail:DefaultSecurityConfigValueEquality",
58-
"Detail:ThrowOnMissingId",
5958
"Optimization:ImplicitDefaultArguments",
6059
"Feature:Bolt:Patch:UTC",
6160
"Feature:API:Type.Temporal"));

0 commit comments

Comments
 (0)