Skip to content

Commit d8e0997

Browse files
injectivesinjectives
authored
Add exceptions to numeric id accessors in nodes and relationships (neo4j#1192)
* 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`. * Update error message to mention server
1 parent a114b6d commit d8e0997

25 files changed

+501
-152
lines changed

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

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

3333
public abstract class InternalEntity implements Entity, AsValue
3434
{
35+
public static final String INVALID_ID_ERROR =
36+
"Numeric id is not available with this server deployment, please use the new string based element id alternative";
37+
3538
private final long id;
3639
private final String elementId;
3740
private final Map<String,Value> properties;
41+
private final boolean numericIdAvailable;
3842

39-
public InternalEntity( long id, String elementId, Map<String,Value> properties )
43+
public InternalEntity( long id, String elementId, Map<String,Value> properties, boolean numericIdAvailable )
4044
{
4145
this.id = id;
4246
this.elementId = elementId;
4347
this.properties = properties;
48+
this.numericIdAvailable = numericIdAvailable;
4449
}
4550

4651
@Override
4752
public long id()
4853
{
54+
assertNumericIdAvailable();
4955
return id;
5056
}
5157

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

‎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)