Merge pull request #737 from neo4j/feature/remove-null-tests-for-ids

Remove null handling for Node/Relationship Ids

Author: thelonelyvulpes

neo4j/graph/__init__.py

@@ -44,10 +44,13 @@ class Graph:
 
     def __init__(self):
         self._nodes = {}
+        self._legacy_nodes = {}  # TODO: 6.0 - remove
         self._relationships = {}
+        self._legacy_relationships = {}  # TODO: 6.0 - remove
         self._relationship_types = {}
-        self._node_set_view = EntitySetView(self._nodes)
-        self._relationship_set_view = EntitySetView(self._relationships)
+        self._node_set_view = EntitySetView(self._nodes, self._legacy_nodes)
+        self._relationship_set_view = EntitySetView(self._relationships,
+                                                    self._legacy_relationships)
 
     @property
     def nodes(self):
@@ -86,9 +89,9 @@ def hydrate_node(self, id_, labels=None,
             try:
                 inst = self.graph._nodes[element_id]
             except KeyError:
-                inst = self.graph._nodes[element_id] = Node(
-                    self.graph, element_id, id_, labels, properties
-                )
+                inst = Node(self.graph, element_id, id_, labels, properties)
+                self.graph._nodes[element_id] = inst
+                self.graph._legacy_nodes[id_] = inst
             else:
                 # If we have already hydrated this node as the endpoint of
                 # a relationship, it won't have any labels or properties.
@@ -128,9 +131,11 @@ def hydrate_unbound_relationship(self, id_, type_, properties=None,
                 inst = self.graph._relationships[element_id]
             except KeyError:
                 r = self.graph.relationship_type(type_)
-                inst = self.graph._relationships[element_id] = r(
+                inst = r(
                     self.graph, element_id, id_, properties
                 )
+                self.graph._relationships[element_id] = inst
+                self.graph._legacy_relationships[id_] = inst
             return inst
 
         def hydrate_path(self, nodes, relationships, sequence):
@@ -260,8 +265,9 @@ class EntitySetView(Mapping):
     """ View of a set of :class:`.Entity` instances within a :class:`.Graph`.
     """
 
-    def __init__(self, entity_dict):
+    def __init__(self, entity_dict, legacy_entity_dict):
         self._entity_dict = entity_dict
+        self._legacy_entity_dict = legacy_entity_dict  # TODO: 6.0 - remove
 
     def __getitem__(self, e_id):
         # TODO: 6.0 - remove this compatibility shim
@@ -270,14 +276,7 @@ def __getitem__(self, e_id):
                 "Accessing entities by an integer id is deprecated, "
                 "use the new style element_id (str) instead"
             )
-            if isinstance(e_id, float) and int(e_id) == e_id:
-                # Non-int floats would always fail for legacy IDs
-                e_id = int(e_id)
-            elif isinstance(e_id, complex) and int(e_id.real) == e_id:
-                # complex numbers with imaginary parts or non-integer real
-                # parts would always fail for legacy IDs
-                e_id = int(e_id.real)
-            e_id = str(e_id)
+            return self._legacy_entity_dict[e_id]
         return self._entity_dict[e_id]
 
     def __len__(self):

testkitbackend/test_config.json

@@ -53,8 +53,6 @@
     "Optimization:PullPipelining": true,
     "Optimization:ResultListFetchAll": "The idiomatic way to cast to list is indistinguishable from iterating over the result.",
 
-    "Detail:NullOnMissingId": true,
-
     "ConfHint:connection.recv_timeout_seconds": true,
 
     "Backend:RTFetch": true,

tests/unit/common/test_api.py

@@ -377,7 +377,8 @@ def test_serverinfo_initialization():
     assert server_info.address is address
     assert server_info.protocol_version is version
     assert server_info.agent is None
-    assert server_info.connection_id is None
+    with pytest.warns(DeprecationWarning):
+        assert server_info.connection_id is None
 
 
 @pytest.mark.parametrize(

tests/unit/common/test_data.py

@@ -33,24 +33,21 @@ def test_can_hydrate_v1_node_structure():
 
     with pytest.warns(DeprecationWarning, match="element_id"):
         assert alice.id == 123
-    # for backwards compatibility, the driver should compy the element_id
+    # for backwards compatibility, the driver should compute the element_id
     assert alice.element_id == "123"
     assert alice.labels == {"Person"}
     assert set(alice.keys()) == {"name"}
     assert alice.get("name") == "Alice"
 
 
-@pytest.mark.parametrize("with_id", (True, False))
-def test_can_hydrate_v2_node_structure(with_id):
+def test_can_hydrate_v2_node_structure():
     hydrant = DataHydrator()
 
-    id_ = 123 if with_id else None
-
-    struct = Structure(b'N', id_, ["Person"], {"name": "Alice"}, "abc")
+    struct = Structure(b'N', 123, ["Person"], {"name": "Alice"}, "abc")
     alice, = hydrant.hydrate([struct])
 
     with pytest.warns(DeprecationWarning, match="element_id"):
-        assert alice.id == id_
+        assert alice.id == 123
     assert alice.element_id == "abc"
     assert alice.labels == {"Person"}
     assert set(alice.keys()) == {"name"}
@@ -78,25 +75,20 @@ def test_can_hydrate_v1_relationship_structure():
     assert rel.get("since") == 1999
 
 
-@pytest.mark.parametrize("with_ids", (True, False))
-def test_can_hydrate_v2_relationship_structure(with_ids):
+def test_can_hydrate_v2_relationship_structure():
     hydrant = DataHydrator()
 
-    id_ = 123 if with_ids else None
-    start_id = 456 if with_ids else None
-    end_id = 789 if with_ids else None
-
-    struct = Structure(b'R', id_, start_id, end_id, "KNOWS", {"since": 1999},
+    struct = Structure(b'R', 123, 456, 789, "KNOWS", {"since": 1999},
                        "abc", "def", "ghi")
 
     rel, = hydrant.hydrate([struct])
 
     with pytest.warns(DeprecationWarning, match="element_id"):
-        assert rel.id == id_
+        assert rel.id == 123
     with pytest.warns(DeprecationWarning, match="element_id"):
-        assert rel.start_node.id == start_id
+        assert rel.start_node.id == 456
     with pytest.warns(DeprecationWarning, match="element_id"):
-        assert rel.end_node.id == end_id
+        assert rel.end_node.id == 789
     # for backwards compatibility, the driver should compy the element_id
     assert rel.element_id == "abc"
     assert rel.start_node.element_id == "def"
@@ -125,7 +117,8 @@ def test_can_hydrate_in_list():
 
     alice, = alice_in_list
 
-    assert alice.id == 123
+    with pytest.warns(DeprecationWarning, match="element_id"):
+        assert alice.id == 123
     assert alice.labels == {"Person"}
     assert set(alice.keys()) == {"name"}
     assert alice.get("name") == "Alice"
@@ -141,7 +134,8 @@ def test_can_hydrate_in_dict():
 
     alice = alice_in_dict["foo"]
 
-    assert alice.id == 123
+    with pytest.warns(DeprecationWarning, match="element_id"):
+        assert alice.id == 123
     assert alice.labels == {"Person"}
     assert set(alice.keys()) == {"name"}
     assert alice.get("name") == "Alice"

tests/unit/common/test_record.py

@@ -287,7 +287,7 @@ def test_data_relationship():
     gh = Graph.Hydrator(g)
     alice = gh.hydrate_node(1, {"Person"}, {"name": "Alice", "age": 33})
     bob = gh.hydrate_node(2, {"Person"}, {"name": "Bob", "age": 44})
-    alice_knows_bob = gh.hydrate_relationship(1, alice.id, bob.id, "KNOWS",
+    alice_knows_bob = gh.hydrate_relationship(1, 1, 2, "KNOWS",
                                               {"since": 1999})
     record = Record(zip(["a", "b", "r"], [alice, bob, alice_knows_bob]))
     assert record.data() == {