Index incoming link cache of Node on link triple instead of just label

The link triple is defined as the tuple of the source node, link type
and link label of an incoming link into a target node. This concept is
implemented in a named tuple `LinkTriple`. Since the introduction of
link types, the link label of incoming links no longer necessarily needs
to be unique, as long as the triple with the link type and source node
*is* unique.

However, the old implementation of the incoming link cache relied on label
uniqueness, because it used only the label as the key in the cache, which
used a dictionary as its data structure. Here we change that data structure
of the node's internal incoming link cache to be a set of link triples.

aiida/backends/tests/cmdline/commands/test_calculation.py

@@ -239,7 +239,7 @@ def test_calculation_show(self):
         # Giving multiple identifiers should print a non empty string message
         options = [str(calc.pk) for calc in self.calcs]
         result = self.cli_runner.invoke(command.calculation_show, options)
-        self.assertIsNone(result.exception, result.output)
+        self.assertClickResultNoException(result)
         self.assertTrue(len(get_result_lines(result)) > 0)
 
     def test_calculation_logshow(self):

aiida/backends/tests/cmdline/commands/test_work.py

@@ -13,8 +13,6 @@
 from __future__ import print_function
 from __future__ import absolute_import
 
-import traceback
-
 from click.testing import CliRunner
 
 from aiida.backends.testbase import AiidaTestCase
@@ -39,7 +37,7 @@ def test_status(self):
         """Test the status command."""
         calc = WorkChainNode().store()
         result = self.cli_runner.invoke(cmd_work.work_status, [str(calc.pk)])
-        self.assertIsNone(result.exception, ''.join(traceback.format_exception(*result.exc_info)))
+        self.assertClickResultNoException(result)
 
     def test_list(self):
         """Test the list command."""

aiida/backends/tests/nodes.py

@@ -1546,7 +1546,7 @@ def test_cachelink(self):
         endnode.add_incoming(n2, LinkType.CREATE, "N2")
 
         self.assertEqual(
-            set([(i.label, i.node.uuid) for i in endnode.get_incoming()]),
+            set([(i.link_label, i.node.uuid) for i in endnode.get_incoming()]),
             set([("N1", n1.uuid), ("N2", n2.uuid)]))
 
         # Endnode not stored yet, n3 and n4 already stored
@@ -1555,22 +1555,22 @@ def test_cachelink(self):
         endnode.add_incoming(n4, LinkType.CREATE, "N4")
 
         self.assertEqual(
-            set([(i.label, i.node.uuid) for i in endnode.get_incoming()]),
+            set([(i.link_label, i.node.uuid) for i in endnode.get_incoming()]),
             set([("N1", n1.uuid), ("N2", n2.uuid), ("N3", n3.uuid), ("N4", n4.uuid)]))
 
         # Some parent nodes are not stored yet
         with self.assertRaises(ModificationNotAllowed):
             endnode.store()
 
         self.assertEqual(
-            set([(i.label, i.node.uuid) for i in endnode.get_incoming()]),
+            set([(i.link_label, i.node.uuid) for i in endnode.get_incoming()]),
             set([("N1", n1.uuid), ("N2", n2.uuid), ("N3", n3.uuid), ("N4", n4.uuid)]))
 
         # This will also store n1 and n2!
         endnode.store_all()
 
         self.assertEqual(
-            set([(i.label, i.node.uuid) for i in endnode.get_incoming()]),
+            set([(i.link_label, i.node.uuid) for i in endnode.get_incoming()]),
             set([("N1", n1.uuid), ("N2", n2.uuid), ("N3", n3.uuid), ("N4", n4.uuid)]))
 
     def test_store_with_unstored_parents(self):
@@ -1593,7 +1593,7 @@ def test_store_with_unstored_parents(self):
         endnode.store()
 
         self.assertEqual(
-            set([(i.label, i.node.uuid) for i in endnode.get_incoming()]),
+            set([(i.link_label, i.node.uuid) for i in endnode.get_incoming()]),
             set([("N1", n1.uuid), ("N2", n2.uuid)]))
 
     def test_storeall_with_unstored_grandparents(self):
@@ -1616,8 +1616,8 @@ def test_storeall_with_unstored_grandparents(self):
         endnode.store_all()
 
         # Check the parents...
-        self.assertEqual(set([(i.label, i.node.uuid) for i in n2.get_incoming()]), set([("N1", n1.uuid)]))
-        self.assertEqual(set([(i.label, i.node.uuid) for i in endnode.get_incoming()]), set([("N2", n2.uuid)]))
+        self.assertEqual(set([(i.link_label, i.node.uuid) for i in n2.get_incoming()]), set([("N1", n1.uuid)]))
+        self.assertEqual(set([(i.link_label, i.node.uuid) for i in endnode.get_incoming()]), set([("N2", n2.uuid)]))
 
     def test_has_children_has_parents(self):
         """
@@ -1733,9 +1733,17 @@ def test_links_label_autogenerator(self):
         n10.add_incoming(n8, link_type=LinkType.RETURN)
         n10.add_incoming(n9, link_type=LinkType.RETURN)
 
-        all_labels = [_.label for _ in n10.get_incoming()]
+        all_labels = [_.link_label for _ in n10.get_incoming()]
         self.assertEquals(len(set(all_labels)), len(all_labels), "There are duplicate links, that are not expected")
 
+    @unittest.skip('write and activate this test once #2238 is addressed')
+    def test_link_label_autogenerator(self):
+        """
+        When the uniqueness constraints on links are reimplemented on the database level, auto generation of
+        labels that relies directly on those database level constraints should be reinstated and tested for here.
+        """
+        raise NotImplementedError
+
     @unittest.skip('remove this test once #2219 is addressed')
     def test_link_replace(self):
         from aiida.orm.node.process import CalculationNode
@@ -1752,73 +1760,73 @@ def test_link_replace(self):
             n3.add_incoming(n1, link_type=LinkType.CREATE, link_label='the_label')
 
         # I can replace the link and check that it was replaced
-        n3._replace_link_from(n2, LinkType.CREATE, label='the_label')
-        the_parent = [_.node.uuid for _ in n3.get_incoming() if _.label == 'the_label']
+        n3._replace_link_from(n2, LinkType.CREATE, link_label='the_label')
+        the_parent = [_.node.uuid for _ in n3.get_incoming() if _.link_label == 'the_label']
         self.assertEquals(len(the_parent), 1, "There are multiple input links with the same label (the_label)!")
         self.assertEquals(n2.uuid, the_parent[0])
 
         # _replace_link_from should work also if there is no previous link
-        n2._replace_link_from(n1, LinkType.CREATE, label='the_label_2')
-        the_parent_2 = [_.node.uuid for _ in n4.get_incoming() if _.label == 'the_label_2']
+        n2._replace_link_from(n1, LinkType.CREATE, link_label='the_label_2')
+        the_parent_2 = [_.node.uuid for _ in n4.get_incoming() if _.link_label == 'the_label_2']
         self.assertEquals(len(the_parent_2), 1, "There are multiple input links with the same label (the_label_2)!")
         self.assertEquals(n1.uuid, the_parent_2[0])
 
     def test_link_with_unstored(self):
         """
-        It is possible to store links between nodes even if they are unstored;
-        these links are cached. However, if working in the cache, an explicit
-        link name must be provided.
+        It is possible to store links between nodes even if they are unstored these links are cached.
         """
-        n1 = Node()
-        n2 = Node()
-        n3 = Node()
-        n4 = Node()
+        from aiida.orm.node.process import CalculationNode, WorkflowNode
+        from aiida.orm import Data
 
-        # No link names provided
-        with self.assertRaises(ModificationNotAllowed):
-            n4.add_incoming(n1, link_type=LinkType.CREATE)
+        n1 = Data()
+        n2 = WorkflowNode()
+        n3 = CalculationNode()
+        n4 = Data()
 
         # Caching the links
-        n2.add_incoming(n1, link_type=LinkType.CREATE, link_label='l1')
-        n3.add_incoming(n2, link_type=LinkType.CREATE, link_label='l2')
-        n3.add_incoming(n1, link_type=LinkType.CREATE, link_label='l3')
+        n2.add_incoming(n1, link_type=LinkType.INPUT_WORK, link_label='l1')
+        n3.add_incoming(n1, link_type=LinkType.INPUT_CALC, link_label='l3')
+        n3.add_incoming(n2, link_type=LinkType.CALL_CALC, link_label='l2')
 
         # Twice the same link name
-        with self.assertRaises(UniquenessError):
-            n3.add_incoming(n4, link_type=LinkType.CREATE, link_label='l2')
+        with self.assertRaises(ValueError):
+            n3.add_incoming(n4, link_type=LinkType.INPUT_CALC, link_label='l3')
 
         n2.store_all()
         n3.store_all()
 
-        n2_in_links = [(n.label, n.node.uuid) for n in n2.get_incoming()]
+        n2_in_links = [(n.link_label, n.node.uuid) for n in n2.get_incoming()]
         self.assertEquals(sorted(n2_in_links), sorted([
             ('l1', n1.uuid),
         ]))
-        n3_in_links = [(n.label, n.node.uuid) for n in n3.get_incoming()]
+        n3_in_links = [(n.link_label, n.node.uuid) for n in n3.get_incoming()]
         self.assertEquals(sorted(n3_in_links), sorted([
             ('l2', n2.uuid),
             ('l3', n1.uuid),
         ]))
 
-        n1_out_links = [(entry.label, entry.node.pk) for entry in n1.get_outgoing()]
+        n1_out_links = [(entry.link_label, entry.node.pk) for entry in n1.get_outgoing()]
         self.assertEquals(sorted(n1_out_links), sorted([
             ('l1', n2.pk),
             ('l3', n3.pk),
         ]))
-        n2_out_links = [(entry.label, entry.node.pk) for entry in n2.get_outgoing()]
+        n2_out_links = [(entry.link_label, entry.node.pk) for entry in n2.get_outgoing()]
         self.assertEquals(sorted(n2_out_links), sorted([('l2', n3.pk)]))
 
     def test_multiple_create_links(self):
         """
         Cannot have two CREATE links for the same node.
         """
-        n1 = Node()
-        n2 = Node()
-        n3 = Node()
+        from aiida.orm.data import Data
+        from aiida.orm.node.process import CalculationNode
+
+        n1 = CalculationNode()
+        n2 = CalculationNode()
+        n3 = Data()
 
         # Caching the links
         n3.add_incoming(n1, link_type=LinkType.CREATE, link_label='CREATE')
-        with self.assertRaises(UniquenessError):
+        with self.assertRaises(ValueError):
             n3.add_incoming(n2, link_type=LinkType.CREATE, link_label='CREATE')
 
     def test_valid_links(self):
@@ -1916,7 +1924,7 @@ def test_valid_links(self):
 
         # Cannot replace input nodes if the calculation is not in status NEW
         with self.assertRaises(ModificationNotAllowed):
-            calc_a._replace_link_from(d2, LinkType.INPUT_CALC, label='some_label')
+            calc_a._replace_link_from(d2, LinkType.INPUT_CALC, link_label='some_label')
 
         # Cannot (re)set the code if the calculation is not in status NEW
         with self.assertRaises(ModificationNotAllowed):

aiida/backends/tests/orm/node/test_node.py

@@ -54,6 +54,10 @@ def test_add_incoming_create(self):
         with self.assertRaises(ValueError):
             target.validate_incoming(source_two, LinkType.CREATE)
 
+        # Or when the link label is different
+        with self.assertRaises(ValueError):
+            target.validate_incoming(source_one, LinkType.CREATE, 'other_label')
+
     def test_add_incoming_call_calc(self):
         """Nodes can only have a single incoming CALL_CALC link, independent of the source node."""
         source_one = WorkflowNode()
@@ -70,6 +74,10 @@ def test_add_incoming_call_calc(self):
         with self.assertRaises(ValueError):
             target.validate_incoming(source_two, LinkType.CALL_CALC)
 
+        # Or when the link label is different
+        with self.assertRaises(ValueError):
+            target.validate_incoming(source_one, LinkType.CALL_CALC, 'other_label')
+
     def test_add_incoming_call_work(self):
         """Nodes can only have a single incoming CALL_WORK link, independent of the source node."""
         source_one = WorkflowNode()
@@ -86,24 +94,12 @@ def test_add_incoming_call_work(self):
         with self.assertRaises(ValueError):
             target.validate_incoming(source_two, LinkType.CALL_WORK)
 
-    def test_add_incoming_return(self):
-        """Nodes can have an infinite amount of incoming RETURN links, as long as (source, label) pair is unique."""
-        source_one = WorkflowNode()
-        source_two = WorkflowNode()
-        target = Data()
-
-        target.add_incoming(source_one, LinkType.RETURN, 'link_label')
-
-        # Can only have a single incoming RETURN link from each source node if the label is not unique
+        # Or when the link label is different
         with self.assertRaises(ValueError):
-            target.validate_incoming(source_one, LinkType.RETURN, 'link_label')
-
-        # From another source node or using another label is fine
-        target.validate_incoming(source_one, LinkType.RETURN, 'other_label')
-        target.validate_incoming(source_two, LinkType.RETURN, 'link_label')
+            target.validate_incoming(source_one, LinkType.CALL_WORK, 'other_label')
 
     def test_add_incoming_input_calc(self):
-        """Nodes can have an infinite amount of incoming INPUT_CALC links, as long as (source, label) pair is unique."""
+        """Nodes can have an infinite amount of incoming INPUT_CALC links, as long as the link pair is unique."""
         source_one = Data()
         source_two = Data()
         target = CalculationNode()
@@ -114,12 +110,15 @@ def test_add_incoming_input_calc(self):
         with self.assertRaises(ValueError):
             target.validate_incoming(source_one, LinkType.INPUT_CALC, 'link_label')
 
-        # From another source node or using another label is fine
+        # Using another link label is fine
         target.validate_incoming(source_one, LinkType.INPUT_CALC, 'other_label')
-        target.validate_incoming(source_two, LinkType.INPUT_CALC, 'link_label')
+
+        # However, using the same link, even from another node is illegal
+        with self.assertRaises(ValueError):
+            target.validate_incoming(source_two, LinkType.INPUT_CALC, 'link_label')
 
     def test_add_incoming_input_work(self):
-        """Nodes can have an infinite amount of incoming INPUT_WORK links, as long as (source, label) pair is unique."""
+        """Nodes can have an infinite amount of incoming INPUT_WORK links, as long as the link pair is unique."""
         source_one = Data()
         source_two = Data()
         target = WorkflowNode()
@@ -130,9 +129,28 @@ def test_add_incoming_input_work(self):
         with self.assertRaises(ValueError):
             target.validate_incoming(source_one, LinkType.INPUT_WORK, 'link_label')
 
-        # From another source node or using another label is fine
+        # Using another link label is fine
         target.validate_incoming(source_one, LinkType.INPUT_WORK, 'other_label')
-        target.validate_incoming(source_two, LinkType.INPUT_WORK, 'link_label')
+
+        # However, using the same link, even from another node is illegal
+        with self.assertRaises(ValueError):
+            target.validate_incoming(source_two, LinkType.INPUT_WORK, 'link_label')
+
+    def test_add_incoming_return(self):
+        """Nodes can have an infinite amount of incoming RETURN links, as long as the link triple is unique."""
+        source_one = WorkflowNode()
+        source_two = WorkflowNode()
+        target = Data()
+
+        target.add_incoming(source_one, LinkType.RETURN, 'link_label')
+
+        # Can only have a single incoming RETURN link from each source node if the label is not unique
+        with self.assertRaises(ValueError):
+            target.validate_incoming(source_one, LinkType.RETURN, 'link_label')
+
+        # From another source node or using another label is fine
+        target.validate_incoming(source_one, LinkType.RETURN, 'other_label')
+        target.validate_incoming(source_two, LinkType.RETURN, 'link_label')
 
     def test_get_incoming(self):
         """Test that `Node.get_incoming` will return stored and cached input links."""
@@ -157,3 +175,62 @@ def test_get_incoming(self):
         incoming_nodes = target.get_incoming(link_type=(LinkType.INPUT_CALC, LinkType.INPUT_WORK)).all()
         incoming_uuids = sorted([neighbor.node.uuid for neighbor in incoming_nodes])
         self.assertEqual(incoming_uuids, sorted([source_one.uuid, source_two.uuid]))
+
+    def test_node_indegree_unique_pair(self):
+        """Test that the validation of links with indegree `unique_pair` works correctly
+
+        The example here is a `DataNode` that has two incoming links with the same label, but with different types.
+        This is legal and should pass validation.
+        """
+        caller = WorkflowNode().store()
+        data = Data().store()
+        called = CalculationNode()
+
+        # Verify that adding two incoming links with the same link label but different type is allowed
+        called.add_incoming(caller, link_type=LinkType.CALL_CALC, link_label='call')
+        called.add_incoming(data, link_type=LinkType.INPUT_CALC, link_label='call')
+        called.store()
+
+        uuids_incoming = set([node.uuid for node in called.get_incoming().all_nodes()])
+        uuids_expected = set([caller.uuid, data.uuid])
+        self.assertEqual(uuids_incoming, uuids_expected)
+
+    def test_node_indegree_unique_triple(self):
+        """Test that the validation of links with indegree `unique_triple` works correctly
+
+        The example here is a `DataNode` that has two incoming RETURN links with the same label, but from different
+        source nodes. This is legal and should pass validation.
+        """
+        return_one = WorkflowNode().store()
+        return_two = WorkflowNode().store()
+        data = Data()
+
+        # Verify that adding two return links with the same link label but from different source is allowed
+        data.add_incoming(return_one, link_type=LinkType.RETURN, link_label='return')
+        data.add_incoming(return_two, link_type=LinkType.RETURN, link_label='return')
+        data.store()
+
+        uuids_incoming = set([node.uuid for node in data.get_incoming().all_nodes()])
+        uuids_expected = set([return_one.uuid, return_two.uuid])
+        self.assertEqual(uuids_incoming, uuids_expected)
+
+    def test_node_outdegree_unique_triple(self):
+        """Test that the validation of links with outdegree `unique_triple` works correctly
+
+        The example here is a `CalculationNode` that has two outgoing CREATE links with the same label, but to different
+        target nodes. This is legal and should pass validation.
+        """
+        creator = CalculationNode().store()
+        data_one = Data()
+        data_two = Data()
+
+        # Verify that adding two create links with the same link label but to different target is allowed from the
+        # perspective of the source node (the CalculationNode in this case)
+        data_one.add_incoming(creator, link_type=LinkType.CREATE, link_label='create')
+        data_two.add_incoming(creator, link_type=LinkType.CREATE, link_label='create')
+        data_one.store()
+        data_two.store()
+
+        uuids_outgoing = set([node.uuid for node in creator.get_outgoing().all_nodes()])
+        uuids_expected = set([data_one.uuid, data_two.uuid])
+        self.assertEqual(uuids_outgoing, uuids_expected)

aiida/backends/tests/work/process.py

@@ -63,7 +63,7 @@ def test_namespaced_process(self):
         self.assertEquals(input_node.value, 5)
 
         # Check that the link of the process node has the correct link name
-        self.assertTrue('some_name_space_a' in proc.calc.get_incoming().get_labels())
+        self.assertTrue('some_name_space_a' in proc.calc.get_incoming().all_link_labels())
         self.assertEquals(proc.calc.get_incoming().get_node_by_label('some_name_space_a'), 5)
 
 
@@ -110,9 +110,9 @@ def test_input_link_creation(self):
         p = test_utils.DummyProcess(inputs)
 
         for entry in p._calc.get_incoming().all():
-            self.assertTrue(entry.label in inputs)
-            self.assertEqual(int(entry.label), int(entry.node.value))
-            dummy_inputs.remove(entry.label)
+            self.assertTrue(entry.link_label in inputs)
+            self.assertEqual(int(entry.link_label), int(entry.node.value))
+            dummy_inputs.remove(entry.link_label)
 
         # Make sure there are no other inputs
         self.assertFalse(dummy_inputs)