Fix bug in Django migration 0023_calc_job_option_attribute_keys

The migration changed the keys of certain node attributes but failed to
properly join the matched attributes on the matched nodes. This could
cause attributes with the target keys to be affected even if the
corresponding node would not have matched the filter. The chances of
this having occurred are minimal to non-existent, as it would require
non calculation job nodes with these particular keys to have existed, so
we simply fix the migration and leave it at that.

aiida/backends/djsite/db/migrations/0023_calc_job_option_attribute_keys.py

@@ -8,7 +8,7 @@
 # For further information please visit http://www.aiida.net               #
 ###########################################################################
 # pylint: disable=invalid-name,too-few-public-methods
-"""Migration of CalcJobNode attributes for metadata options whose key changed."""
+"""Migration of ProcessNode attributes for metadata options whose key changed."""
 from __future__ import division
 from __future__ import print_function
 from __future__ import unicode_literals
@@ -25,14 +25,14 @@
 
 
 class Migration(migrations.Migration):
-    """Migration of CalcJobNode attributes for metadata options whose key changed.
+    """Migration of ProcessNode attributes for metadata options whose key changed.
 
     Renamed attribute keys:
 
-      * `custom_environment_variables` -> `environment_variables`
-      * `jobresource_params` -> `resources`
-      * `_process_label` -> `process_label`
-      * `parser` -> `parser_name`
+      * `custom_environment_variables` -> `environment_variables` (CalcJobNode)
+      * `jobresource_params` -> `resources` (CalcJobNode)
+      * `_process_label` -> `process_label` (ProcessNode)
+      * `parser` -> `parser_name` (CalcJobNode)
 
     Deleted attributes:
       * `linkname_retrieved` (We do not actually delete it just in case some relies on it)
@@ -54,7 +54,8 @@ class Migration(migrations.Migration):
                     attribute.key = 'custom_environment_variables' OR
                     attribute.key LIKE 'custom\_environment\_variables.%'
                 ) AND
-                node.type = 'node.process.calculation.calcjob.CalcJobNode.';
+                node.type = 'node.process.calculation.calcjob.CalcJobNode.' AND
+                node.id = attribute.dbnode_id;
             -- custom_environment_variables -> environment_variables
 
             UPDATE db_dbattribute AS attribute
@@ -65,23 +66,26 @@ class Migration(migrations.Migration):
                     attribute.key = 'jobresource_params' OR
                     attribute.key LIKE 'jobresource\_params.%'
                 ) AND
-                node.type = 'node.process.calculation.calcjob.CalcJobNode.';
+                node.type = 'node.process.calculation.calcjob.CalcJobNode.' AND
+                node.id = attribute.dbnode_id;
             -- jobresource_params -> resources
 
             UPDATE db_dbattribute AS attribute
             SET key =  regexp_replace(attribute.key, '^_process_label', 'process_label')
             FROM db_dbnode AS node
             WHERE
                 attribute.key = '_process_label' AND
-                node.type LIKE 'node.process.%';
+                node.type LIKE 'node.process.%' AND
+                node.id = attribute.dbnode_id;
             -- _process_label -> process_label
 
             UPDATE db_dbattribute AS attribute
             SET key =  regexp_replace(attribute.key, '^parser', 'parser_name')
             FROM db_dbnode AS node
             WHERE
                 attribute.key = 'parser' AND
-                node.type = 'node.process.calculation.calcjob.CalcJobNode.';
+                node.type = 'node.process.calculation.calcjob.CalcJobNode.' AND
+                node.id = attribute.dbnode_id;
             -- parser -> parser_name
             """,
             reverse_sql=r"""
@@ -93,7 +97,8 @@ class Migration(migrations.Migration):
                     attribute.key = 'environment_variables' OR
                     attribute.key LIKE 'environment\_variables.%'
                 ) AND
-                node.type = 'node.process.calculation.calcjob.CalcJobNode.';
+                node.type = 'node.process.calculation.calcjob.CalcJobNode.' AND
+                node.id = attribute.dbnode_id;
             -- environment_variables -> custom_environment_variables
 
             UPDATE db_dbattribute AS attribute
@@ -104,23 +109,26 @@ class Migration(migrations.Migration):
                     attribute.key = 'resources' OR
                     attribute.key LIKE 'resources.%'
                 ) AND
-                node.type = 'node.process.calculation.calcjob.CalcJobNode.';
+                node.type = 'node.process.calculation.calcjob.CalcJobNode.' AND
+                node.id = attribute.dbnode_id;
             -- resources -> jobresource_params
 
             UPDATE db_dbattribute AS attribute
             SET key =  regexp_replace(attribute.key, '^process_label', '_process_label')
             FROM db_dbnode AS node
             WHERE
                 attribute.key = 'process_label' AND
-                node.type LIKE 'node.process.%';
+                node.type LIKE 'node.process.%' AND
+                node.id = attribute.dbnode_id;
             -- process_label -> _process_label
 
             UPDATE db_dbattribute AS attribute
             SET key =  regexp_replace(attribute.key, '^parser_name', 'parser')
             FROM db_dbnode AS node
             WHERE
                 attribute.key = 'parser_name' AND
-                node.type = 'node.process.calculation.calcjob.CalcJobNode.';
+                node.type = 'node.process.calculation.calcjob.CalcJobNode.' AND
+                node.id = attribute.dbnode_id;
             -- parser_name -> parser
             """),
         upgrade_schema_version(REVISION, DOWN_REVISION)

aiida/backends/djsite/db/subtests/test_migrations.py

@@ -293,6 +293,14 @@ def setUpBeforeMigration(self):
         self.set_attribute(self.node_calc, self.KEY_ENVIRONMENT_VARIABLES_OLD, self.environment_variables)
         self.set_attribute(self.node_calc, self.KEY_PARSER_NAME_OLD, self.parser_name)
 
+        # Create a node of a different type to ensure that its attributes are not updated
+        self.node_other = self.DbNode(type='node.othernode.', user_id=self.default_user.id)
+        self.node_other.save()
+        self.set_attribute(self.node_other, self.KEY_PROCESS_LABEL_OLD, self.process_label)
+        self.set_attribute(self.node_other, self.KEY_RESOURCES_OLD, self.resources)
+        self.set_attribute(self.node_other, self.KEY_ENVIRONMENT_VARIABLES_OLD, self.environment_variables)
+        self.set_attribute(self.node_other, self.KEY_PARSER_NAME_OLD, self.parser_name)
+
     def test_attribute_key_changes(self):
         """Verify that the keys are successfully changed of the affected attributes."""
         NOT_FOUND = tuple([0])
@@ -309,6 +317,17 @@ def test_attribute_key_changes(self):
         self.assertEqual(self.get_attribute(self.node_calc, self.KEY_ENVIRONMENT_VARIABLES_OLD, default=NOT_FOUND), NOT_FOUND)
         self.assertEqual(self.get_attribute(self.node_calc, self.KEY_PARSER_NAME_OLD, default=NOT_FOUND), NOT_FOUND)
 
+        # The following node should not be migrated even if its attributes have the matching keys because
+        # the node is not a ProcessNode
+        self.assertEqual(self.get_attribute(self.node_other, self.KEY_PROCESS_LABEL_OLD), self.process_label)
+        self.assertEqual(self.get_attribute(self.node_other, self.KEY_RESOURCES_OLD), self.resources)
+        self.assertEqual(self.get_attribute(self.node_other, self.KEY_ENVIRONMENT_VARIABLES_OLD), self.environment_variables)
+        self.assertEqual(self.get_attribute(self.node_other, self.KEY_PARSER_NAME_OLD), self.parser_name)
+        self.assertEqual(self.get_attribute(self.node_other, self.KEY_PROCESS_LABEL_NEW, default=NOT_FOUND), NOT_FOUND)
+        self.assertEqual(self.get_attribute(self.node_other, self.KEY_RESOURCES_NEW, default=NOT_FOUND), NOT_FOUND)
+        self.assertEqual(self.get_attribute(self.node_other, self.KEY_ENVIRONMENT_VARIABLES_NEW, default=NOT_FOUND), NOT_FOUND)
+        self.assertEqual(self.get_attribute(self.node_other, self.KEY_PARSER_NAME_NEW, default=NOT_FOUND), NOT_FOUND)
+
 
 class TestDbLogMigrationRecordCleaning(TestMigrations):
 

aiida/backends/sqlalchemy/migrations/versions/7ca08c391c49_calc_job_option_attribute_keys.py

@@ -8,7 +8,7 @@
 # For further information please visit http://www.aiida.net               #
 ###########################################################################
 # pylint: disable=invalid-name,no-member
-"""Migration of CalcJobNode attributes for metadata options whose key changed.
+"""Migration of ProcessNode attributes for metadata options whose key changed.
 
 Revision ID: 7ca08c391c49
 Revises: e72ad251bcdb
@@ -32,14 +32,14 @@
 
 
 def upgrade():
-    """Migration of CalcJobNode attributes for metadata options whose key changed.
+    """Migration of ProcessNode attributes for metadata options whose key changed.
 
     Renamed attribute keys:
 
-      * `custom_environment_variables` -> `environment_variables`
-      * `jobresource_params` -> `resources`
-      * `_process_label` -> `process_label`
-      * `parser` -> `parser_name`
+      * `custom_environment_variables` -> `environment_variables` (CalcJobNode)
+      * `jobresource_params` -> `resources` (CalcJobNode)
+      * `_process_label` -> `process_label` (ProcessNode)
+      * `parser` -> `parser_name` (CalcJobNode)
 
     Deleted attributes:
       * `linkname_retrieved` (We do not actually delete it just in case some relies on it)

aiida/backends/sqlalchemy/tests/test_migrations.py

@@ -559,13 +559,17 @@ def setUpBeforeMigration(self):
                 node_work = DbNode(type='node.process.workflow.WorkflowNode.', attributes=attributes, user_id=user.id)
                 node_calc = DbNode(
                     type='node.process.calculation.calcjob.CalcJobNode.', attributes=attributes, user_id=user.id)
+                # Create a node of a different type to ensure that its attributes are not updated
+                node_other = DbNode(type='node.othernode.', attributes=attributes, user_id=user.id)
 
                 session.add(node_work)
                 session.add(node_calc)
+                session.add(node_other)
                 session.commit()
 
                 self.node_work_id = node_work.id
                 self.node_calc_id = node_calc.id
+                self.node_other_id = node_other.id
             finally:
                 session.close()
 
@@ -586,10 +590,9 @@ def test_attribute_key_changes(self):
                 self.assertEqual(node_work.attributes.get(self.KEY_PROCESS_LABEL_NEW), self.process_label)
                 self.assertEqual(node_work.attributes.get(self.KEY_PROCESS_LABEL_OLD, not_found), not_found)
 
-                node_calc = session.query(DbNode).filter(DbNode.id == self.node_calc_id).one()
-
                 # The dictionaries need to be cast with ast.literal_eval, because the `get` will return a string
                 # representation of the dictionary that the attribute contains
+                node_calc = session.query(DbNode).filter(DbNode.id == self.node_calc_id).one()
                 self.assertEqual(node_calc.attributes.get(self.KEY_PROCESS_LABEL_NEW), self.process_label)
                 self.assertEqual(node_calc.attributes.get(self.KEY_PARSER_NAME_NEW), self.parser_name)
                 self.assertEqual(ast.literal_eval(node_calc.attributes.get(self.KEY_RESOURCES_NEW)), self.resources)
@@ -600,6 +603,19 @@ def test_attribute_key_changes(self):
                 self.assertEqual(node_calc.attributes.get(self.KEY_PARSER_NAME_OLD, not_found), not_found)
                 self.assertEqual(node_calc.attributes.get(self.KEY_RESOURCES_OLD, not_found), not_found)
                 self.assertEqual(node_calc.attributes.get(self.KEY_ENVIRONMENT_VARIABLES_OLD, not_found), not_found)
+
+                # The following node should not be migrated even if its attributes have the matching keys because
+                # the node is not a ProcessNode
+                node_other = session.query(DbNode).filter(DbNode.id == self.node_other_id).one()
+                self.assertEqual(node_other.attributes.get(self.KEY_PROCESS_LABEL_OLD), self.process_label)
+                self.assertEqual(node_other.attributes.get(self.KEY_PARSER_NAME_OLD), self.parser_name)
+                self.assertEqual(node_other.attributes.get(self.KEY_RESOURCES_OLD), self.resources)
+                self.assertEqual(
+                    node_other.attributes.get(self.KEY_ENVIRONMENT_VARIABLES_OLD), self.environment_variables)
+                self.assertEqual(node_other.attributes.get(self.KEY_PROCESS_LABEL_NEW, not_found), not_found)
+                self.assertEqual(node_other.attributes.get(self.KEY_PARSER_NAME_NEW, not_found), not_found)
+                self.assertEqual(node_other.attributes.get(self.KEY_RESOURCES_NEW, not_found), not_found)
+                self.assertEqual(node_other.attributes.get(self.KEY_ENVIRONMENT_VARIABLES_NEW, not_found), not_found)
             finally:
                 session.close()