Fix inconsistency on failure updating

qiita_db/metadata_template/base_metadata_template.py

@@ -36,9 +36,10 @@
 # -----------------------------------------------------------------------------
 
 from __future__ import division
-from future.utils import viewitems
+from future.utils import viewitems, viewvalues
 from os.path import join
 from functools import partial
+from collections import defaultdict
 from copy import deepcopy
 
 import pandas as pd
@@ -48,6 +49,7 @@
 
 from qiita_db.exceptions import (QiitaDBUnknownIDError, QiitaDBColumnError,
                                  QiitaDBNotImplementedError,
+                                 QiitaDBExecutionError,
                                  QiitaDBDuplicateHeaderError)
 from qiita_db.base import QiitaObject
 from qiita_db.sql_connection import SQLConnectionHandler
@@ -313,8 +315,8 @@ def handler(x):
                            " in template %d" %
                            (key, self._id, self._md_template.id))
 
-    def __setitem__(self, column, value):
-        r"""Sets the metadata value for the category `column`
+    def add_setitem_queries(self, column, value, conn_handler, queue):
+        """Adds the SQL queries needed to set a value to the provided queue
 
         Parameters
         ----------
@@ -323,16 +325,16 @@ def __setitem__(self, column, value):
         value : str
             The value to set. This is expected to be a str on the assumption
             that psycopg2 will cast as necessary when updating.
+        conn_handler : SQLConnectionHandler
+            The connection handler object connected to the DB
+        queue : str
+            The queue where the SQL statements will be added
 
         Raises
         ------
         QiitaDBColumnError
             If the column does not exist in the table
-        ValueError
-            If the value type does not match the one in the DB
         """
-        conn_handler = SQLConnectionHandler()
-
         # try dynamic tables
         sql = """SELECT EXISTS (
                     SELECT column_name
@@ -367,27 +369,58 @@ def __setitem__(self, column, value):
             raise QiitaDBColumnError("Column %s does not exist in %s" %
                                      (column, self._dynamic_table))
 
+        conn_handler.add_to_queue(queue, sql, (value, self._id))
+
+    def __setitem__(self, column, value):
+        r"""Sets the metadata value for the category `column`
+
+        Parameters
+        ----------
+        column : str
+            The column to update
+        value : str
+            The value to set. This is expected to be a str on the assumption
+            that psycopg2 will cast as necessary when updating.
+
+        Raises
+        ------
+        ValueError
+            If the value type does not match the one in the DB
+        """
+        conn_handler = SQLConnectionHandler()
+        queue_name = "set_item_%s" % self._id
+        conn_handler.create_queue(queue_name)
+
+        self.add_setitem_queries(column, value, conn_handler, queue_name)
+
         try:
-            conn_handler.execute(sql, (value, self._id))
-        except Exception as e:
+            conn_handler.execute_queue(queue_name)
+        except QiitaDBExecutionError as e:
             # catching error so we can check if the error is due to different
             # column type or something else
+            type_lookup = defaultdict(lambda: 'varchar')
+            type_lookup[int] = 'integer'
+            type_lookup[float] = 'float8'
+            type_lookup[str] = 'varchar'
+            value_type = type_lookup[type(value)]
+
+            sql = """SELECT udt_name
+                     FROM information_schema.columns
+                     WHERE column_name = %s
+                        AND table_schema = 'qiita'
+                        AND (table_name = %s OR table_name = %s)"""
             column_type = conn_handler.execute_fetchone(
-                """SELECT data_type
-                   FROM information_schema.columns
-                   WHERE column_name=%s AND table_schema='qiita'
-                """, (column,))[0]
-            value_type = type(value).__name__
+                sql, (column, self._table, self._dynamic_table))
 
             if column_type != value_type:
                 raise ValueError(
                     'The new value being added to column: "{0}" is "{1}" '
                     '(type: "{2}"). However, this column in the DB is of '
                     'type "{3}". Please change the value in your updated '
-                    'template or reprocess your sample template.'.format(
+                    'template or reprocess your template.'.format(
                         column, value, value_type, column_type))
-            else:
-                raise e
+
+            raise e
 
     def __delitem__(self, key):
         r"""Removes the sample with sample id `key` from the database
@@ -1180,12 +1213,53 @@ def update_category(self, category, samples_and_values):
         QiitaDBColumnError
             If the column does not exist in the table. This is implicit, and
             can be thrown by the contained Samples.
+        ValueError
+            If one of the new values cannot be inserted in the DB due to
+            different types
         """
         if not set(self.keys()).issuperset(samples_and_values):
             missing = set(self.keys()) - set(samples_and_values)
             table_name = self._table_name(self.study_id)
             raise QiitaDBUnknownIDError(missing, table_name)
 
+        conn_handler = SQLConnectionHandler()
+        queue_name = "update_category_%s_%s" % (self._id, category)
+        conn_handler.create_queue(queue_name)
+
         for k, v in viewitems(samples_and_values):
             sample = self[k]
-            sample[category] = v
+            sample.add_setitem_queries(category, v, conn_handler, queue_name)
+
+        try:
+            conn_handler.execute_queue(queue_name)
+        except QiitaDBExecutionError as e:
+            # catching error so we can check if the error is due to different
+            # column type or something else
+            type_lookup = defaultdict(lambda: 'varchar')
+            type_lookup[int] = 'integer'
+            type_lookup[float] = 'float8'
+            type_lookup[str] = 'varchar'
+            value_types = set(type_lookup[type(value)]
+                              for value in viewvalues(samples_and_values))
+
+            sql = """SELECT udt_name
+                     FROM information_schema.columns
+                     WHERE column_name = %s
+                        AND table_schema = 'qiita'
+                        AND (table_name = %s OR table_name = %s)"""
+            column_type = conn_handler.execute_fetchone(
+                sql, (category, self._table, self._table_name(self._id)))
+
+            if any([column_type != vt for vt in value_types]):
+                value_str = ', '.join(
+                    [str(value) for value in viewvalues(samples_and_values)])
+                value_types_str = ', '.join(value_types)
+
+                raise ValueError(
+                    'The new values being added to column: "%s" are "%s" '
+                    '(types: "%s"). However, this column in the DB is of '
+                    'type "%s". Please change the values in your updated '
+                    'template or reprocess your template.'
+                    % (category, value_str, value_types_str, column_type))
+
+            raise e

qiita_db/metadata_template/test/test_prep_template.py

@@ -53,6 +53,45 @@ def setUp(self):
 
 
 class TestPrepSampleReadOnly(BaseTestPrepSample):
+    def test_add_setitem_queries_error(self):
+        conn_handler = SQLConnectionHandler()
+        queue = "test_queue"
+        conn_handler.create_queue(queue)
+
+        with self.assertRaises(QiitaDBColumnError):
+            self.tester.add_setitem_queries(
+                'COL_DOES_NOT_EXIST', 'Foo', conn_handler, queue)
+
+    def test_add_setitem_queries_required(self):
+        conn_handler = SQLConnectionHandler()
+        queue = "test_queue"
+        conn_handler.create_queue(queue)
+
+        self.tester.add_setitem_queries(
+            'center_name', 'FOO', conn_handler, queue)
+
+        obs = conn_handler.queues[queue]
+        sql = """UPDATE qiita.common_prep_info
+                     SET center_name=%s
+                     WHERE sample_id=%s"""
+        exp = [(sql, ('FOO', '1.SKB8.640193'))]
+        self.assertEqual(obs, exp)
+
+    def test_add_setitem_queries_dynamic(self):
+        conn_handler = SQLConnectionHandler()
+        queue = "test_queue"
+        conn_handler.create_queue(queue)
+
+        self.tester.add_setitem_queries(
+            'barcodesequence', 'AAAAAAAAAAAA', conn_handler, queue)
+
+        obs = conn_handler.queues[queue]
+        sql = """UPDATE qiita.prep_1
+                         SET barcodesequence=%s
+                         WHERE sample_id=%s"""
+        exp = [(sql, ('AAAAAAAAAAAA', '1.SKB8.640193'))]
+        self.assertEqual(obs, exp)
+
     def test_init_unknown_error(self):
         """Init errors if the PrepSample id is not found in the template"""
         with self.assertRaises(QiitaDBUnknownIDError):

qiita_db/metadata_template/test/test_sample_template.py

@@ -53,6 +53,45 @@ def setUp(self):
 
 
 class TestSampleReadOnly(BaseTestSample):
+    def test_add_setitem_queries_error(self):
+        conn_handler = SQLConnectionHandler()
+        queue = "test_queue"
+        conn_handler.create_queue(queue)
+
+        with self.assertRaises(QiitaDBColumnError):
+            self.tester.add_setitem_queries(
+                'COL_DOES_NOT_EXIST', 0.30, conn_handler, queue)
+
+    def test_add_setitem_queries_required(self):
+        conn_handler = SQLConnectionHandler()
+        queue = "test_queue"
+        conn_handler.create_queue(queue)
+
+        self.tester.add_setitem_queries(
+            'has_physical_specimen', True, conn_handler, queue)
+
+        obs = conn_handler.queues[queue]
+        sql = """UPDATE qiita.required_sample_info
+                     SET has_physical_specimen=%s
+                     WHERE sample_id=%s"""
+        exp = [(sql, (True, '1.SKB8.640193'))]
+        self.assertEqual(obs, exp)
+
+    def test_add_setitem_queries_dynamic(self):
+        conn_handler = SQLConnectionHandler()
+        queue = "test_queue"
+        conn_handler.create_queue(queue)
+
+        self.tester.add_setitem_queries(
+            'tot_nitro', '1234.5', conn_handler, queue)
+
+        obs = conn_handler.queues[queue]
+        sql = """UPDATE qiita.sample_1
+                         SET tot_nitro=%s
+                         WHERE sample_id=%s"""
+        exp = [(sql, ('1234.5', '1.SKB8.640193'))]
+        self.assertEqual(obs, exp)
+
     def test_init_unknown_error(self):
         """Init raises an error if the sample id is not found in the template
         """
@@ -1214,7 +1253,6 @@ def test_exists_false(self):
         self.assertFalse(SampleTemplate.exists(self.new_study.id))
 
     def test_update_category(self):
-        """setitem raises an error (currently not allowed)"""
         with self.assertRaises(QiitaDBUnknownIDError):
             self.tester.update_category('country', {"foo": "bar"})
 
@@ -1256,10 +1294,18 @@ def test_update_category(self):
         # testing that if fails when trying to change an int column value
         # to str
         st = SampleTemplate.create(self.metadata, self.new_study)
-        mapping = {'2.Sample1': "no_value"}
+
+        sql = """SELECT * FROM qiita.sample_2 ORDER BY sample_id"""
+        before = self.conn_handler.execute_fetchall(sql)
+        mapping = {'2.Sample1': 1, '2.Sample2': "no_value"}
+
         with self.assertRaises(ValueError):
             st.update_category('int_column', mapping)
 
+        after = self.conn_handler.execute_fetchall(sql)
+
+        self.assertEqual(before, after)
+
     def test_update(self):
         """Updates values in existing mapping file"""
         # creating a new sample template