Skip to content

Commit

Permalink
Remove null handling for Node/Relationship Ids (#618)
Browse files Browse the repository at this point in the history 
* Remove null being sent for new storage engine

* remove unneeded feature

* remove tests.
  • Loading branch information
@thelonelyvulpes
thelonelyvulpes authored Jul 6, 2022
1 parent bbb6c92 commit b107c9c
Show file tree
Hide file tree
Showing 11 changed files with 14 additions and 285 deletions.
1 change: 0 additions & 1 deletion Neo4j.Driver/Neo4j.Driver.Tests.TestBackend/SupportedFeatures.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ static SupportedFeatures()
"AuthorizationExpiredTreatment",
"Detail:ClosedDriverIsEncrypted",
"Detail:DefaultSecurityConfigValueEquality",
"Detail:ThrowOnMissingId",
"Feature:API:ConnectionAcquisitionTimeout",
"Feature:API:Driver:GetServerInfo",
"Feature:API:Driver.IsEncrypted",
Expand Down
37 changes: 0 additions & 37 deletions Neo4j.Driver/Neo4j.Driver.Tests/Internal/IO/ValueSerializers/ElementNodeSerializerTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,42 +58,5 @@ public void ShouldDeserialize()
});
value.Should().BeOfType<Node>().Which.ElementId.Should().Be("1");
}

[Fact]
public void ShouldDeserializeWithNulls()
{
var writerMachine = CreateWriterMachine();
var writer = writerMachine.Writer();

writer.WriteStructHeader(3, ElementNodeSerializer.Node);
writer.WriteNull();
writer.Write(new List<string> { "Label1", "Label2" });
writer.Write(new Dictionary<string, object>
{
{"prop1", "something"},
{"prop2", 15},
{"prop3", true}
});
writer.Write("1");

var readerMachine = CreateReaderMachine(writerMachine.GetOutput());
var value = readerMachine.Reader().Read();

value.Should().NotBeNull();
value.Should().BeOfType<Node>();
var node = value.As<Node>();

node.ElementId.Should().Be("1");
var exception = Record.Exception(() => node.Id);
exception.Should().BeOfType<InvalidOperationException>();

node.Labels.Should().Equal(new[] { "Label1", "Label2" });
node.Properties.Should().HaveCount(3).And.Contain(new[]
{
new KeyValuePair<string, object>("prop1", "something"),
new KeyValuePair<string, object>("prop2", 15L),
new KeyValuePair<string, object>("prop3", true),
});
}
}
}
55 changes: 0 additions & 55 deletions Neo4j.Driver/Neo4j.Driver.Tests/Internal/IO/ValueSerializers/ElementPathSerializerTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,61 +122,6 @@ public void ShouldDeserializeWithElementIds()
relationships[0].EndNodeElementId.Should().Be("n2");
}

[Fact]
public void ShouldDeserializeWithOnlyElementIds()
{
var writerMachine = CreateWriterMachine();
var writer = writerMachine.Writer();

SerializeElementPath(writer,
new List<Node>
{
new Node(-1, "n1", new List<string>{"a"}, new Dictionary<string, object>()),
new Node(-1, "n2",new List<string>{"a"}, new Dictionary<string, object>()),
},
new List<Relationship>
{
new Relationship(-1, "r1", -1, -1, "-1", "-1", "LIKES", new Dictionary<string, object>())
},
new List<int>
{
1, 1
});

var readerMachine = CreateReaderMachine(writerMachine.GetOutput());
var value = readerMachine.Reader().Read();

var path = value.Should().BeOfType<Path>();

path.Which.Nodes.Should().AllBeOfType<Node>();
path.Which.Relationships.Should().AllBeOfType<Relationship>();

var nodes = path.Which.Nodes;
var relationships = path.Which.Relationships;

var nodeZeroException = Record.Exception(() => nodes[0].Id);
nodeZeroException.Should().BeOfType<InvalidOperationException>();

var nodeOneException = Record.Exception(() => nodes[1].Id);
nodeOneException.Should().BeOfType<InvalidOperationException>();

nodes[0].ElementId.Should().Be("n1");
nodes[1].ElementId.Should().Be("n2");

var idException = Record.Exception(() => relationships[0].Id);
idException.Should().BeOfType<InvalidOperationException>();

var startException = Record.Exception(() => relationships[0].StartNodeId);
startException.Should().BeOfType<InvalidOperationException>();

var endException = Record.Exception(() => relationships[0].EndNodeId);
endException.Should().BeOfType<InvalidOperationException>();

relationships[0].ElementId.Should().Be("r1");
relationships[0].StartNodeElementId.Should().Be("n1");
relationships[0].EndNodeElementId.Should().Be("n2");
}

private static void SerializeElementPath(IPackStreamWriter writer, List<Node> nodes, List<Relationship> rels,
List<int> indicies)
{
Expand Down
47 changes: 0 additions & 47 deletions ...ver/Neo4j.Driver.Tests/Internal/IO/ValueSerializers/ElementRelationshipSerializerTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,52 +64,5 @@ public void ShouldDeserialize()
value.Should().BeOfType<Relationship>().Which.StartNodeElementId.Should().Be("n1");
value.Should().BeOfType<Relationship>().Which.EndNodeElementId.Should().Be("n2");
}

[Fact]
public void ShouldDeserializeWithNulls()
{
var writerMachine = CreateWriterMachine();
var writer = writerMachine.Writer();

writer.WriteStructHeader(3, ElementRelationshipSerializer.Relationship);
writer.WriteNull();
writer.WriteNull();
writer.WriteNull();
writer.Write("RELATES_TO");
writer.Write(new Dictionary<string, object>
{
{"prop3", true}
});
writer.Write("r1");
writer.Write("n1");
writer.Write("n2");


var readerMachine = CreateReaderMachine(writerMachine.GetOutput());
var value = readerMachine.Reader().Read();

value.Should().NotBeNull();
value.Should().BeOfType<Relationship>();
var relationship = value.As<Relationship>();

var idException = Record.Exception(() => relationship.Id);
idException.Should().BeOfType<InvalidOperationException>();

var startException = Record.Exception(() => relationship.StartNodeId);
startException.Should().BeOfType<InvalidOperationException>();

var endException = Record.Exception(() => relationship.EndNodeId);
endException.Should().BeOfType<InvalidOperationException>();

relationship.Type.Should().Be("RELATES_TO");
relationship.Properties.Should()
.HaveCount(1).And.Contain(new[]
{
new KeyValuePair<string, object>("prop3", true),
});
relationship.ElementId.Should().Be("r1");
relationship.StartNodeElementId.Should().Be("n1");
relationship.EndNodeElementId.Should().Be("n2");
}
}
}
43 changes: 0 additions & 43 deletions ...4j.Driver.Tests/Internal/IO/ValueSerializers/ElementUnboundRelationshipSerializerTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,48 +60,5 @@ public void ShouldDeserialize()
value.Should().BeOfType<Relationship>().Which.StartNodeElementId.Should().Be("-1");
value.Should().BeOfType<Relationship>().Which.EndNodeElementId.Should().Be("-1");
}

[Fact]
public void ShouldDeserializeWithNulls()
{
var writerMachine = CreateWriterMachine();
var writer = writerMachine.Writer();

writer.WriteStructHeader(3, UnboundRelationshipSerializer.UnboundRelationship);
writer.WriteNull();
writer.Write("RELATES_TO");
writer.Write(new Dictionary<string, object>
{
{"prop3", true}
});
writer.Write("r1");


var readerMachine = CreateReaderMachine(writerMachine.GetOutput());
var value = readerMachine.Reader().Read();

value.Should().NotBeNull();
value.Should().BeOfType<Relationship>();
var relationship = value.As<Relationship>();

var idException = Record.Exception(() => relationship.Id);
idException.Should().BeOfType<InvalidOperationException>();

var startException = Record.Exception(() => relationship.StartNodeId);
startException.Should().BeOfType<InvalidOperationException>();

var endException = Record.Exception(() => relationship.EndNodeId);
endException.Should().BeOfType<InvalidOperationException>();

relationship.Type.Should().Be("RELATES_TO");
relationship.Properties.Should()
.HaveCount(1).And.Contain(new[]
{
new KeyValuePair<string, object>("prop3", true),
});
relationship.ElementId.Should().Be("r1");
relationship.StartNodeElementId.Should().Be("-1");
relationship.EndNodeElementId.Should().Be("-1");
}
}
}
6 changes: 0 additions & 6 deletions Neo4j.Driver/Neo4j.Driver/Internal/IO/ReadOnlySerializer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,5 @@ public void Serialize(IPackStreamWriter writer, object value)
public abstract IEnumerable<byte> ReadableStructs { get; }

public abstract object Deserialize(IPackStreamReader reader, byte signature, long size);

protected static T? ReadNullAndReturnNull<T>(IPackStreamReader reader) where T : struct
{
reader.ReadNull();
return null;
}
}
}
8 changes: 2 additions & 6 deletions Neo4j.Driver/Neo4j.Driver/Internal/IO/ValueSerializers/ElementNodeSerializer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,7 @@ internal class ElementNodeSerializer : ReadOnlySerializer

public override object Deserialize(IPackStreamReader reader, byte signature, long size)
{
var includingLongs = reader.PeekNextType() != PackStream.PackType.Null;

var nodeId = includingLongs ? reader.ReadLong() : ReadNullAndReturnNull<long>(reader);
var nodeId = reader.ReadLong();

var numLabels = (int) reader.ReadListHeader();
var labels = new List<string>(numLabels);
Expand All @@ -48,9 +46,7 @@ public override object Deserialize(IPackStreamReader reader, byte signature, lon

var stringId = reader.ReadString();

return includingLongs
? new Node(nodeId.Value, stringId, labels, props)
: new Node(stringId, labels, props);
return new Node(nodeId, stringId, labels, props);
}
}
}
11 changes: 4 additions & 7 deletions Neo4j.Driver/Neo4j.Driver/Internal/IO/ValueSerializers/ElementRelationshipSerializer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,9 @@ internal class ElementRelationshipSerializer : ReadOnlySerializer

public override object Deserialize(IPackStreamReader reader, byte signature, long size)
{
var includingLongs = reader.PeekNextType() != PackStream.PackType.Null;
var relId = includingLongs ? reader.ReadLong() : ReadNullAndReturnNull<long>(reader);
var relStartId = includingLongs ? reader.ReadLong() : ReadNullAndReturnNull<long>(reader);
var relEndId = includingLongs ? reader.ReadLong() : ReadNullAndReturnNull<long>(reader);
var relId = reader.ReadLong();
var relStartId = reader.ReadLong();
var relEndId = reader.ReadLong();

var relType = reader.ReadString();
var props = reader.ReadMap();
Expand All @@ -39,9 +38,7 @@ public override object Deserialize(IPackStreamReader reader, byte signature, lon
var startUrn = reader.ReadString();
var endUrn = reader.ReadString();

return includingLongs
? new Relationship(relId.Value, urn, relStartId.Value, relEndId.Value, startUrn, endUrn, relType, props)
: new Relationship(urn, startUrn, endUrn, relType, props);
return new Relationship(relId, urn, relStartId, relEndId, startUrn, endUrn, relType, props);
}
}
}
8 changes: 2 additions & 6 deletions ....Driver/Neo4j.Driver/Internal/IO/ValueSerializers/ElementUnboundRelationshipSerializer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,18 +27,14 @@ internal class ElementUnboundRelationshipSerializer : ReadOnlySerializer

public override object Deserialize(IPackStreamReader reader, byte signature, long size)
{
var includingLongs = reader.PeekNextType() != PackStream.PackType.Null;

var relId = includingLongs ? reader.ReadLong() : ReadNullAndReturnNull<long>(reader);
var relId = reader.ReadLong();

var relType = reader.ReadString();
var props = reader.ReadMap();

var urn = reader.ReadString();

return includingLongs
? new Relationship(relId.Value, urn, -1, -1, "-1", "-1", relType, props)
: new Relationship(urn, "-1", "-1", relType, props);
return new Relationship(relId, urn, -1, -1, "-1", "-1", relType, props);
}
}
}
22 changes: 1 addition & 21 deletions Neo4j.Driver/Neo4j.Driver/Internal/Types/Node.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,20 +22,8 @@ namespace Neo4j.Driver.Internal.Types
{
internal class Node : INode
{
private readonly bool _throwOnIdRead = false;
private long _id;

[Obsolete("Replaced with ElementId, Will be removed in 6.0")]
public long Id
{
get
{
if (_throwOnIdRead)
throw new InvalidOperationException("Id is not compatible with server. use ElementId");
return _id;
}
private set =>_id = value;
}
public long Id { get; set; }

public string ElementId { get; }
public IReadOnlyList<string> Labels { get; }
Expand All @@ -50,14 +38,6 @@ public Node(long id, IReadOnlyList<string> labels, IReadOnlyDictionary<string, o
Properties = prop;
}

public Node(string elementId, IReadOnlyList<string> labels, IReadOnlyDictionary<string, object> prop)
{
_throwOnIdRead = true;
ElementId = elementId;
Labels = labels;
Properties = prop;
}

public Node(long id, string elementId, IReadOnlyList<string> labels, IReadOnlyDictionary<string, object> prop)
{
Id = id;
Expand Down
Loading

0 comments on commit b107c9c

Please sign in to comment.