Fix bugs in Node._store_from_cache and Node.repository.erase (#3777)

The `_store_from_cache` needed to erase the content of its sandbox
folder before copying over the content of the cache source. Currently,
the existing contents are mixed with with the contents to be copied in.

In fixing this, we discovered an issue in `Node.repository.erase`, where
an unstored node would try erasing the (non-existent) folder in the
permanent repository, instead of the `SandboxFolder`.

aiida/orm/nodes/node.py

@@ -1090,6 +1090,11 @@ def _store_from_cache(self, cache_node, with_transaction):
             if key != Sealable.SEALED_KEY:
                 self.set_attribute(key, value)
 
+        # The erase() removes the current content of the sandbox folder.
+        # If this was not done, the content of the sandbox folder could
+        # become mangled when copying over the content of the cache
+        # source repository folder.
+        self._repository.erase()
         self.put_object_from_tree(cache_node._repository._get_base_folder().abspath)  # pylint: disable=protected-access
 
         self._store(with_transaction=with_transaction, clean=False)

aiida/orm/utils/repository.py

@@ -262,7 +262,7 @@ def erase(self, force=False):
         if not force:
             self.validate_mutability()
 
-        self._repo_folder.erase()
+        self._get_base_folder().erase()
 
     def store(self):
         """Store the contents of the sandbox folder into the repository folder."""

tests/orm/node/test_node.py

@@ -11,11 +11,13 @@
 """Tests for the Node ORM class."""
 
 import os
+import tempfile
 
 from aiida.backends.testbase import AiidaTestCase
 from aiida.common import exceptions, LinkType
 from aiida.orm import Data, Node, User, CalculationNode, WorkflowNode, load_node
 from aiida.orm.utils.links import LinkTriple
+from aiida.manage.caching import enable_caching
 
 
 class TestNode(AiidaTestCase):
@@ -729,3 +731,25 @@ def test_tab_completable_properties(self):
         self.assertEqual(workflow.outputs.result_b.pk, output2.pk)
         with self.assertRaises(exceptions.NotExistent):
             _ = workflow.outputs.some_label
+
+
+# Clearing the DB is needed because other parts of the tests (not using
+# the fixture infrastructure) delete the User.
+def test_store_from_cache(clear_database_before_test):  # pylint: disable=unused-argument
+    """
+    Regression test for storing a Node with (nested) repository
+    content with caching.
+    """
+    data = Data()
+    with tempfile.TemporaryDirectory() as tmpdir:
+        dir_path = os.path.join(tmpdir, 'directory')
+        os.makedirs(dir_path)
+        with open(os.path.join(dir_path, 'file'), 'w') as file:
+            file.write('content')
+        data.put_object_from_tree(tmpdir)
+
+    data.store()
+    with enable_caching():
+        clone = data.clone().store()
+    assert clone.get_cache_source() == data.uuid
+    assert data.get_hash() == clone.get_hash()

tests/orm/utils/test_repository.py

@@ -14,8 +14,9 @@
 import tempfile
 
 from aiida.backends.testbase import AiidaTestCase
-from aiida.orm import Node
+from aiida.orm import Node, Data
 from aiida.orm.utils.repository import File, FileType
+from aiida.common.exceptions import ModificationNotAllowed
 
 
 class TestRepository(AiidaTestCase):
@@ -147,3 +148,46 @@ def test_put_object_from_tree(self):
         key = os.path.join(basepath, 'subdir', 'a.txt')
         content = self.get_file_content(os.path.join('subdir', 'a.txt'))
         self.assertEqual(node.get_object_content(key), content)
+
+    def test_erase_unstored(self):
+        """
+        Test that _repository.erase removes the content of an unstored
+        node.
+        """
+        node = Node()
+        node.put_object_from_tree(self.tempdir, '')
+
+        self.assertEqual(sorted(node.list_object_names()), ['c.txt', 'subdir'])
+        self.assertEqual(sorted(node.list_object_names('subdir')), ['a.txt', 'b.txt', 'nested'])
+
+        node._repository.erase()  # pylint: disable=protected-access
+        self.assertEqual(node.list_object_names(), [])
+
+    def test_erase_stored_force(self):
+        """
+        Test that _repository.erase removes the content of an stored
+        Data node when passing force=True.
+        """
+        node = Data()
+        node.put_object_from_tree(self.tempdir, '')
+        node.store()
+
+        self.assertEqual(sorted(node.list_object_names()), ['c.txt', 'subdir'])
+        self.assertEqual(sorted(node.list_object_names('subdir')), ['a.txt', 'b.txt', 'nested'])
+
+        node._repository.erase(force=True)  # pylint: disable=protected-access
+        self.assertEqual(node.list_object_names(), [])
+
+    def test_erase_stored_raise(self):
+        """
+        Test that trying to erase the repository content of a stored
+        Data node without the force flag raises.
+        """
+        node = Data()
+        node.put_object_from_tree(self.tempdir, '')
+        node.store()
+
+        self.assertEqual(sorted(node.list_object_names()), ['c.txt', 'subdir'])
+        self.assertEqual(sorted(node.list_object_names('subdir')), ['a.txt', 'b.txt', 'nested'])
+
+        self.assertRaises(ModificationNotAllowed, node._repository.erase)  # pylint: disable=protected-access