Skip to content

Commit 776a1f0

Browse files
injectivesinjectives
committed
Add exceptions to numeric id accessors in nodes and relationships
Depending on server configuration numeric id might not be available and accessing it will result in `IllegalStateException`.
1 parent df8b781 commit 776a1f0

25 files changed

+500
-152
lines changed

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

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,20 +32,25 @@
3232

3333
public abstract class InternalEntity implements Entity, AsValue
3434
{
35+
public static final String INVALID_ID_ERROR = "Numeric id is not available, please use string based element id alternative";
36+
3537
private final long id;
3638
private final String elementId;
3739
private final Map<String,Value> properties;
40+
private final boolean numericIdAvailable;
3841

39-
public InternalEntity( long id, String elementId, Map<String,Value> properties )
42+
public InternalEntity( long id, String elementId, Map<String,Value> properties, boolean numericIdAvailable )
4043
{
4144
this.id = id;
4245
this.elementId = elementId;
4346
this.properties = properties;
47+
this.numericIdAvailable = numericIdAvailable;
4448
}
4549

4650
@Override
4751
public long id()
4852
{
53+
assertNumericIdAvailable();
4954
return id;
5055
}
5156

@@ -142,4 +147,17 @@ public <T> Iterable<T> values( Function<Value,T> mapFunction )
142147
{
143148
return Iterables.map( properties.values(), mapFunction );
144149
}
150+
151+
protected void assertNumericIdAvailable()
152+
{
153+
if ( !numericIdAvailable )
154+
{
155+
throw new IllegalStateException( INVALID_ID_ERROR );
156+
}
157+
}
158+
159+
public boolean isNumericIdAvailable()
160+
{
161+
return numericIdAvailable;
162+
}
145163
}

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

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,12 +35,17 @@ public class InternalNode extends InternalEntity implements Node
3535

3636
public InternalNode( long id )
3737
{
38-
this( id, String.valueOf( id ), Collections.emptyList(), Collections.emptyMap() );
38+
this( id, Collections.emptyList(), Collections.emptyMap() );
3939
}
4040

41-
public InternalNode( long id, String elementId, Collection<String> labels, Map<String,Value> properties )
41+
public InternalNode( long id, Collection<String> labels, Map<String,Value> properties )
4242
{
43-
super( id, elementId, properties );
43+
this( id, String.valueOf( id ), labels, properties, true );
44+
}
45+
46+
public InternalNode( long id, String elementId, Collection<String> labels, Map<String,Value> properties, boolean numericIdAvailable )
47+
{
48+
super( id, elementId, properties, numericIdAvailable );
4449
this.labels = labels;
4550
}
4651

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

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,15 +36,20 @@ public class InternalRelationship extends InternalEntity implements Relationship
3636
private String endElementId;
3737
private final String type;
3838

39-
public InternalRelationship( long id, String elementId, long start, String startElementId, long end, String endElementId, String type )
39+
public InternalRelationship( long id, long start, long end, String type )
4040
{
41-
this( id, elementId, start, startElementId, end, endElementId, type, Collections.emptyMap() );
41+
this( id, start, end, type, Collections.emptyMap() );
42+
}
43+
44+
public InternalRelationship( long id, long start, long end, String type, Map<String,Value> properties )
45+
{
46+
this( id, String.valueOf( id ), start, String.valueOf( start ), end, String.valueOf( end ), type, properties, true );
4247
}
4348

4449
public InternalRelationship( long id, String elementId, long start, String startElementId, long end, String endElementId, String type,
45-
Map<String,Value> properties )
50+
Map<String,Value> properties, boolean numericIdAvailable )
4651
{
47-
super( id, elementId, properties );
52+
super( id, elementId, properties, numericIdAvailable );
4853
this.start = start;
4954
this.startElementId = startElementId;
5055
this.end = end;
@@ -72,6 +77,7 @@ public void setStartAndEnd( long start, String startElementId, long end, String
7277
@Override
7378
public long startNodeId()
7479
{
80+
assertNumericIdAvailable();
7581
return start;
7682
}
7783

@@ -84,6 +90,7 @@ public String startNodeElementId()
8490
@Override
8591
public long endNodeId()
8692
{
93+
assertNumericIdAvailable();
8794
return end;
8895
}
8996

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -237,8 +237,8 @@ protected Value unpackRelationship() throws IOException
237237
String relType = unpacker.unpackString();
238238
Map<String,Value> props = unpackMap();
239239

240-
InternalRelationship adapted =
241-
new InternalRelationship( urn, String.valueOf( urn ), startUrn, String.valueOf( startUrn ), endUrn, String.valueOf( endUrn ), relType, props );
240+
InternalRelationship adapted = new InternalRelationship( urn, String.valueOf( urn ), startUrn, String.valueOf( startUrn ), endUrn,
241+
String.valueOf( endUrn ), relType, props, true );
242242
return new RelationshipValue( adapted );
243243
}
244244

@@ -260,7 +260,7 @@ protected InternalNode unpackNode() throws IOException
260260
props.put( key, unpack() );
261261
}
262262

263-
return new InternalNode( urn, String.valueOf( urn ), labels, props );
263+
return new InternalNode( urn, String.valueOf( urn ), labels, props, true );
264264
}
265265

266266
protected Value unpackPath() throws IOException
@@ -283,7 +283,7 @@ protected Value unpackPath() throws IOException
283283
long id = unpacker.unpackLong();
284284
String relType = unpacker.unpackString();
285285
Map<String,Value> props = unpackMap();
286-
uniqRels[i] = new InternalRelationship( id, String.valueOf( id ), -1, String.valueOf( -1 ), -1, String.valueOf( -1 ), relType, props );
286+
uniqRels[i] = new InternalRelationship( id, String.valueOf( id ), -1, String.valueOf( -1 ), -1, String.valueOf( -1 ), relType, props, true );
287287
}
288288

289289
// Path sequence

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

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ protected int getRelationshipFields()
6363
@Override
6464
protected InternalNode unpackNode() throws IOException
6565
{
66-
long urn = unpacker.unpackLongOrDefaultOnNull( -1 );
66+
Long urn = unpacker.unpackLongOrNull();
6767

6868
int numLabels = (int) unpacker.unpackListHeader();
6969
List<String> labels = new ArrayList<>( numLabels );
@@ -81,14 +81,14 @@ protected InternalNode unpackNode() throws IOException
8181

8282
String elementId = unpacker.unpackString();
8383

84-
return new InternalNode( urn, elementId, labels, props );
84+
return urn == null ? new InternalNode( -1, elementId, labels, props, false ) : new InternalNode( urn, elementId, labels, props, true );
8585
}
8686

8787
@Override
8888
protected Value unpackPath() throws IOException
8989
{
9090
// List of unique nodes
91-
Node[] uniqNodes = new Node[(int) unpacker.unpackListHeader()];
91+
InternalNode[] uniqNodes = new InternalNode[(int) unpacker.unpackListHeader()];
9292
for ( int i = 0; i < uniqNodes.length; i++ )
9393
{
9494
ensureCorrectStructSize( TypeConstructor.NODE, getNodeFields(), unpacker.unpackStructHeader() );
@@ -102,11 +102,13 @@ protected Value unpackPath() throws IOException
102102
{
103103
ensureCorrectStructSize( TypeConstructor.RELATIONSHIP, 4, unpacker.unpackStructHeader() );
104104
ensureCorrectStructSignature( "UNBOUND_RELATIONSHIP", UNBOUND_RELATIONSHIP, unpacker.unpackStructSignature() );
105-
Long id = unpacker.unpackLongOrDefaultOnNull( -1 );
105+
Long id = unpacker.unpackLongOrNull();
106106
String relType = unpacker.unpackString();
107107
Map<String,Value> props = unpackMap();
108108
String elementId = unpacker.unpackString();
109-
uniqRels[i] = new InternalRelationship( id, elementId, -1, String.valueOf( -1 ), -1, String.valueOf( -1 ), relType, props );
109+
uniqRels[i] = id == null
110+
? new InternalRelationship( -1, elementId, -1, String.valueOf( -1 ), -1, String.valueOf( -1 ), relType, props, false )
111+
: new InternalRelationship( id, elementId, -1, String.valueOf( -1 ), -1, String.valueOf( -1 ), relType, props, true );
110112
}
111113

112114
// Path sequence
@@ -117,7 +119,7 @@ protected Value unpackPath() throws IOException
117119
Node[] nodes = new Node[segments.length + 1];
118120
Relationship[] rels = new Relationship[segments.length];
119121

120-
Node prevNode = uniqNodes[0], nextNode; // Start node is always 0, and isn't encoded in the sequence
122+
InternalNode prevNode = uniqNodes[0], nextNode; // Start node is always 0, and isn't encoded in the sequence
121123
nodes[0] = prevNode;
122124
InternalRelationship rel;
123125
for ( int i = 0; i < segments.length; i++ )
@@ -128,12 +130,12 @@ protected Value unpackPath() throws IOException
128130
if ( relIdx < 0 )
129131
{
130132
rel = uniqRels[(-relIdx) - 1]; // -1 because rel idx are 1-indexed
131-
rel.setStartAndEnd( nextNode.id(), nextNode.elementId(), prevNode.id(), prevNode.elementId() );
133+
setStartAndEnd( rel, nextNode, prevNode );
132134
}
133135
else
134136
{
135137
rel = uniqRels[relIdx - 1];
136-
rel.setStartAndEnd( prevNode.id(), prevNode.elementId(), nextNode.id(), nextNode.elementId() );
138+
setStartAndEnd( rel, prevNode, nextNode );
137139
}
138140

139141
nodes[i + 1] = nextNode;
@@ -144,19 +146,33 @@ protected Value unpackPath() throws IOException
144146
return new PathValue( new InternalPath( Arrays.asList( segments ), Arrays.asList( nodes ), Arrays.asList( rels ) ) );
145147
}
146148

149+
private void setStartAndEnd( InternalRelationship rel, InternalNode start, InternalNode end )
150+
{
151+
if ( rel.isNumericIdAvailable() && start.isNumericIdAvailable() && end.isNumericIdAvailable() )
152+
{
153+
rel.setStartAndEnd( start.id(), start.elementId(), end.id(), end.elementId() );
154+
}
155+
else
156+
{
157+
rel.setStartAndEnd( -1, start.elementId(), -1, end.elementId() );
158+
}
159+
}
160+
147161
@Override
148162
protected Value unpackRelationship() throws IOException
149163
{
150-
long urn = unpacker.unpackLongOrDefaultOnNull( -1 );
151-
long startUrn = unpacker.unpackLongOrDefaultOnNull( -1 );
152-
long endUrn = unpacker.unpackLongOrDefaultOnNull( -1 );
164+
Long urn = unpacker.unpackLongOrNull();
165+
Long startUrn = unpacker.unpackLongOrNull();
166+
Long endUrn = unpacker.unpackLongOrNull();
153167
String relType = unpacker.unpackString();
154168
Map<String,Value> props = unpackMap();
155169
String elementId = unpacker.unpackString();
156170
String startElementId = unpacker.unpackString();
157171
String endElementId = unpacker.unpackString();
158172

159-
InternalRelationship adapted = new InternalRelationship( urn, elementId, startUrn, startElementId, endUrn, endElementId, relType, props );
173+
InternalRelationship adapted = urn == null
174+
? new InternalRelationship( -1, elementId, -1, startElementId, -1, endElementId, relType, props, false )
175+
: new InternalRelationship( urn, elementId, startUrn, startElementId, endUrn, endElementId, relType, props, true );
160176
return new RelationshipValue( adapted );
161177
}
162178
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -467,7 +467,7 @@ public long unpackMapHeader() throws IOException
467467
}
468468
}
469469

470-
public Long unpackLongOrDefaultOnNull( long defaultValue ) throws IOException
470+
public Long unpackLongOrNull() throws IOException
471471
{
472472
final byte markerByte = in.readByte();
473473
if ( markerByte >= MINUS_2_TO_THE_4 )
@@ -485,7 +485,7 @@ public Long unpackLongOrDefaultOnNull( long defaultValue ) throws IOException
485485
case INT_64:
486486
return in.readLong();
487487
case NULL:
488-
return defaultValue;
488+
return null;
489489
default:
490490
throw new Unexpected( "Expected an integer, but got: " + toHexString( markerByte ) );
491491
}

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,10 @@ public interface Entity extends MapAccessor
3131
* 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
3232
* 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
3333
* unique identifier is a better choice.
34+
* <p>
35+
* Please note that depending on server configuration numeric id might not be available and accessing it will result in {@link IllegalStateException}.
3436
*
35-
* @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
36-
* {@link #elementId()}
37+
* @return the id of this entity
3738
* @deprecated superseded by {@link #elementId()}
3839
*/
3940
@Deprecated

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ public interface Relationship extends Entity
2626
{
2727
/**
2828
* Id of the node where this relationship starts.
29+
* <p>
30+
* Please note that depending on server configuration numeric id might not be available and accessing it will result in {@link IllegalStateException}.
2931
*
3032
* @return the node id
3133
* @deprecated superseded by {@link #startNodeElementId()}
@@ -42,6 +44,8 @@ public interface Relationship extends Entity
4244

4345
/**
4446
* Id of the node where this relationship ends.
47+
* <p>
48+
* Please note that depending on server configuration numeric id might not be available and accessing it will result in {@link IllegalStateException}.
4549
*
4650
* @return the node id
4751
* @deprecated superseded by {@link #endNodeElementId()}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ void shouldDisallowMetadataWithIllegalValues()
7171

7272
assertThrows( ClientException.class,
7373
() -> TransactionConfig.builder().withMetadata(
74-
singletonMap( "key", new InternalRelationship( 1, String.valueOf( 1 ), 1, String.valueOf( 1 ), 1, String.valueOf( 1 ), "" ) ) ) );
74+
singletonMap( "key", new InternalRelationship( 1, 1, 1, "" ) ) ) );
7575

7676
assertThrows( ClientException.class,
7777
() -> TransactionConfig.builder().withMetadata( singletonMap( "key", new InternalPath( new InternalNode( 1 ) ) ) ) );

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

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ void testProperties()
127127
Map<String,Value> props = new HashMap<>();
128128
props.put( "k1", value( 43 ) );
129129
props.put( "k2", value( 42 ) );
130-
InternalNode node = new InternalNode( 42L, String.valueOf( 42L ), Collections.singletonList( "L" ), props );
130+
InternalNode node = new InternalNode( 42L, Collections.singletonList( "L" ), props );
131131

132132
// WHEN
133133
Iterable<Pair<String,Integer>> properties = Extract.properties( node, Value::asInt );
@@ -143,7 +143,7 @@ void testProperties()
143143
void testFields()
144144
{
145145
// GIVEN
146-
InternalRecord record = new InternalRecord( Arrays.asList( "k1" ), new Value[]{value( 42 )} );
146+
InternalRecord record = new InternalRecord( singletonList( "k1" ), new Value[]{value( 42 )} );
147147
// WHEN
148148
List<Pair<String,Integer>> fields = Extract.fields( record, Value::asInt );
149149

@@ -181,11 +181,7 @@ void shouldExtractMapOfValues()
181181
void shouldFailToExtractMapOfValuesFromUnsupportedValues()
182182
{
183183
assertThrows( ClientException.class, () -> Extract.mapOfValues( singletonMap( "key", new InternalNode( 1 ) ) ) );
184-
assertThrows( ClientException.class,
185-
() -> Extract.mapOfValues(
186-
singletonMap( "key",
187-
new InternalRelationship( 1, String.valueOf( 1 ), 1, String.valueOf( 1 ),
188-
1, String.valueOf( 1 ), "HI" ) ) ) );
184+
assertThrows( ClientException.class, () -> Extract.mapOfValues( singletonMap( "key", new InternalRelationship( 1, 1, 1, "HI" ) ) ) );
189185
assertThrows( ClientException.class, () -> Extract.mapOfValues( singletonMap( "key", new InternalPath( new InternalNode( 1 ) ) ) ) );
190186
}
191187
}

0 commit comments

Comments
 (0)