Merge pull request #2028 from antgonza/fix-2026

fix #2026, fix #1854

Author: josenavas

qiita_db/metadata_template/base_metadata_template.py

@@ -514,21 +514,9 @@ def _clean_validate_template(cls, md_template, study_id,
         md_template.columns = [c.lower() for c in md_template.columns]
         # validating pgsql reserved words not to be column headers
         current_headers = set(md_template.columns.values)
-        reserved_words = qdb.metadata_template.util.get_pgsql_reserved_words()
-        overlap = reserved_words & current_headers
-        if overlap:
-            raise qdb.exceptions.QiitaDBColumnError(
-                "The following column names in the template contain PgSQL "
-                "reserved words: %s. You need to modify them." % ", ".join(
-                    overlap))
-        # validating invalid column names
-        invalid_ids = qdb.metadata_template.util.get_invalid_column_names(
+
+        qdb.metadata_template.util.validate_invalid_column_names(
             current_headers)
-        if invalid_ids:
-            raise qdb.exceptions.QiitaDBColumnError(
-                "The following column names in the template contain invalid "
-                "chars: %s. You need to modify them." % ", ".join(
-                    invalid_ids))
 
         # Prefix the sample names with the study_id
         qdb.metadata_template.util.prefix_sample_names_with_id(md_template,

qiita_db/metadata_template/test/test_util.py

@@ -207,12 +207,48 @@ def test_get_get_invalid_sample_names_mixed(self):
         obs = qdb.metadata_template.util.get_invalid_sample_names(one_invalid)
         self.assertItemsEqual(obs, [' ', ' ', ' '])
 
-    def test_get_invalid_column_names(self):
-        invalid = ['tax on', 'bla.', '.', '{', 'this|is', '4column']
-        valid = ['fine', 'select']
-        obs = qdb.metadata_template.util.get_invalid_column_names(
-            invalid + valid)
-        self.assertEqual(obs, invalid)
+    def test_validate_invalid_column_names(self):
+        # testing just pgsql
+        pgsql = ['select', 'column', 'just_fine1']
+        with self.assertRaises(qdb.exceptions.QiitaDBColumnError) as error:
+            qdb.metadata_template.util.validate_invalid_column_names(pgsql)
+        self.assertEqual(
+            str(error.exception),
+            'The following column names in the template contain PgSQL '
+            'reserved words: column, select.\nYou need to modify them.')
+
+        # testing just wrong chars
+        invalid = ['tax on', 'bla.', '.', '{', 'this|is',
+                   '4column', 'just_fine2']
+        with self.assertRaises(qdb.exceptions.QiitaDBColumnError) as error:
+            qdb.metadata_template.util.validate_invalid_column_names(invalid)
+        self.assertEqual(
+            str(error.exception),
+            'The following column names in the template contain invalid '
+            'chars: bla., ., tax on, this|is, {, 4column.\nYou need to '
+            'modify them.')
+
+        # testing just forbidden
+        forbidden = ['sampleid', 'just_fine3']
+        with self.assertRaises(qdb.exceptions.QiitaDBColumnError) as error:
+            qdb.metadata_template.util.validate_invalid_column_names(forbidden)
+        self.assertEqual(
+            str(error.exception),
+            'The following column names in the template contain invalid '
+            'values: sampleid.\nYou need to modify them.')
+
+        # testing all
+        _all = pgsql + invalid + forbidden
+        with self.assertRaises(qdb.exceptions.QiitaDBColumnError) as error:
+            qdb.metadata_template.util.validate_invalid_column_names(_all)
+        self.assertEqual(
+            str(error.exception),
+            'The following column names in the template contain PgSQL '
+            'reserved words: column, select.\n'
+            'The following column names in the template contain invalid '
+            'chars: this|is, ., tax on, bla., {, 4column.\n'
+            'The following column names in the template contain invalid '
+            'values: sampleid.\nYou need to modify them.')
 
     def test_looks_like_qiime_mapping_file(self):
         obs = qdb.metadata_template.util.looks_like_qiime_mapping_file(

qiita_db/metadata_template/util.py

@@ -238,34 +238,65 @@ def get_invalid_sample_names(sample_names):
     return inv
 
 
-def get_invalid_column_names(column_names):
-    """Get a list of column names that are not SQL compliant
+def validate_invalid_column_names(column_names):
+    """Validate a list of column names that are not SQL compliant
 
     Parameters
     ----------
     column_names : iterable
         Iterable containing the column names to check.
 
-    Returns
-    -------
-    list
-        List of str objects where each object is an invalid column name.
+    Raises
+    ------
+    QiitaDBColumnError
+        If column_name is in get_pgsql_reserved_words or contains invalid
+        chars or is within the forbidden_values
 
     References
     ----------
     .. [1] postgresql SQL-SYNTAX-IDENTIFIERS: https://goo.gl/EF0cUV.
     """
+    column_names = set(column_names)
+
+    # testing for specific column names that are not included in the other
+    # tests.
+    forbidden_values = {
+        # https://github.com/biocore/qiita/issues/2026
+        'sampleid'
+    }
+    forbidden = forbidden_values & column_names
+
+    # pgsql reserved words
+    pgsql_reserved = (
+        qdb.metadata_template.util.get_pgsql_reserved_words() & column_names)
+
+    # invalid letters in headers
     valid_initial_char = letters
     valid_rest = set(letters+digits+'_')
-    inv = []
-
+    invalid = []
     for s in column_names:
         if s[0] not in valid_initial_char:
-            inv.append(s)
+            invalid.append(s)
         elif set(s) - valid_rest:
-            inv.append(s)
-
-    return inv
+            invalid.append(s)
+
+    error = []
+    if pgsql_reserved:
+        error.append(
+            "The following column names in the template contain PgSQL "
+            "reserved words: %s." % ", ".join(pgsql_reserved))
+    if invalid:
+        error.append(
+            "The following column names in the template contain invalid "
+            "chars: %s." % ", ".join(invalid))
+    if forbidden:
+        error.append(
+            "The following column names in the template contain invalid "
+            "values: %s." % ", ".join(forbidden))
+
+    if error:
+        raise qdb.exceptions.QiitaDBColumnError(
+            "%s\nYou need to modify them." % '\n'.join(error))
 
 
 def looks_like_qiime_mapping_file(fp):