Fixes Qiita db (#2046)

* New DB structure

* Adding python patch

* Adding a biom so we can actually execute the patch

* Fixing the patch to correctly transfer the information from the old structure to the new one

* Fixing patch

* Fixing patch and a few other bits to make the patch run successfully

* These files are no longer needed

* Removing unused code

* Droping analysis status table

* Linking the analysis with all the artifacts

* Fixing typo

* Fixing HTML and dbschema files

* Adding analyisis jobs

* Extending the artifact to work with the analysis

* Allowing multiomics datatype

* Adding private_job_submitter and modifying proc job handler to use it

* Adding logging column to the analysis

* Adding datatype to the analysis-processing job table

* Adding REST endpoint to access the analysis metadata

* Adding private jobs to plugin

* Fixing typo

* Fixing the processing jobs complete

* Removing the old job code

* Oops removed the wrong file

* Removing QiitaStatusObject because it is not used

* fixing metautil

* Fixing porntal, setup and sql tests

* Fixing user and util

* Fixing qiita_db

* Addressing @antgonza's comments

* Taking into account non-phylogenetic metrics in beta diversity

* Addressing @antgonza's comments

Author: josenavas

qiita_db/base.py

@@ -14,7 +14,6 @@
     :toctree: generated/
 
     QiitaObject
-    QiitaStatusObject
 """
 
 # -----------------------------------------------------------------------------
@@ -220,106 +219,3 @@ def __hash__(self):
     def id(self):
         r"""The object id on the storage system"""
         return self._id
-
-
-class QiitaStatusObject(QiitaObject):
-    r"""Base class for any qiita_db object with a status property
-
-    Attributes
-    ----------
-    status
-
-    Methods
-    -------
-    check_status
-    _status_setter_checks
-    """
-
-    @property
-    def status(self):
-        r"""String with the current status of the analysis"""
-        # Get the DB status of the object
-        with qdb.sql_connection.TRN:
-            sql = """SELECT status FROM qiita.{0}_status
-                     WHERE {0}_status_id = (
-                        SELECT {0}_status_id FROM qiita.{0}
-                        WHERE {0}_id = %s)""".format(self._table)
-            qdb.sql_connection.TRN.add(sql, [self._id])
-            return qdb.sql_connection.TRN.execute_fetchlast()
-
-    def _status_setter_checks(self):
-        r"""Perform any extra checks that needed to be done before setting the
-        object status on the database. Should be overwritten by the subclasses
-        """
-        raise qdb.exceptions.QiitaDBNotImplementedError()
-
-    @status.setter
-    def status(self, status):
-        r"""Change the status of the analysis
-
-        Parameters
-        ----------
-        status: str
-            The new object status
-        """
-        with qdb.sql_connection.TRN:
-            # Perform any extra checks needed before
-            # we update the status in the DB
-            self._status_setter_checks()
-
-            # Update the status of the object
-            sql = """UPDATE qiita.{0} SET {0}_status_id = (
-                        SELECT {0}_status_id FROM qiita.{0}_status
-                        WHERE status = %s)
-                     WHERE {0}_id = %s""".format(self._table)
-            qdb.sql_connection.TRN.add(sql, [status, self._id])
-            qdb.sql_connection.TRN.execute()
-
-    def check_status(self, status, exclude=False):
-        r"""Checks status of object.
-
-        Parameters
-        ----------
-        status: iterable
-            Iterable of statuses to check against.
-        exclude: bool, optional
-            If True, will check that database status is NOT one of the statuses
-            passed. Default False.
-
-        Returns
-        -------
-        bool
-            True if the object status is in the desired set of statuses. False
-            otherwise.
-
-        Notes
-        -----
-        This assumes the following database setup is in place: For a given
-        cls._table setting, such as "table", there is a corresponding table
-        with the name "table_status" holding the status entries allowed. This
-        table has a column called "status" that holds the values corresponding
-        to what is passed as status in this function and a column
-        "table_status_id" corresponding to the column of the same name in
-        "table".
-
-        Table setup:
-        foo: foo_status_id  ----> foo_status: foo_status_id, status
-        """
-        with qdb.sql_connection.TRN:
-            # Get all available statuses
-            sql = "SELECT DISTINCT status FROM qiita.{0}_status".format(
-                self._table)
-            qdb.sql_connection.TRN.add(sql)
-            # We need to access to the results of the last SQL query,
-            # hence indexing using -1
-            avail_status = [
-                x[0] for x in qdb.sql_connection.TRN.execute_fetchindex()]
-
-            # Check that all the provided status are valid status
-            if set(status).difference(avail_status):
-                raise ValueError("%s are not valid status values"
-                                 % set(status).difference(avail_status))
-
-            # Get the DB status of the object
-            dbstatus = self.status
-            return dbstatus not in status if exclude else dbstatus in status

qiita_db/investigation.py

@@ -23,7 +23,7 @@
 REQUIRED_KEYS = {"name", "description", "contact_person"}
 
 
-class Investigation(qdb.base.QiitaStatusObject):
+class Investigation(qdb.base.QiitaObject):
     """
     Study object to access to the Qiita Study information
 

qiita_db/meta_util.py

@@ -109,17 +109,24 @@ def get_accessible_filepath_ids(user):
                 filepath_ids.update({fid for fid, _ in pt.get_filepaths()})
 
             # Then add the filepaths of the sample template
-            filepath_ids.update(
-                {fid
-                 for fid, _ in artifact.study.sample_template.get_filepaths()})
+            study = artifact.study
+            if study:
+                filepath_ids.update(
+                    {fid
+                     for fid, _ in study.sample_template.get_filepaths()})
 
         # Next, analyses
         # Same as before, there are public, private, and shared
         analyses = qdb.analysis.Analysis.get_by_status('public') | \
             user.private_analyses | user.shared_analyses
 
-        for analysis in analyses:
-            filepath_ids.update(analysis.all_associated_filepath_ids)
+        if analyses:
+            sql = """SELECT filepath_id
+                     FROM qiita.analysis_filepath
+                     WHERE analysis_id IN %s"""
+            sql_args = tuple([a.id for a in analyses])
+            qdb.sql_connection.TRN.add(sql, [sql_args])
+            filepath_ids.update(qdb.sql_connection.TRN.execute_fetchflatten())
 
         return filepath_ids
 

qiita_db/metadata_template/test/test_prep_template.py

@@ -876,8 +876,6 @@ def _common_creation_checks(self, pt, fp_count):
         # prep and qiime files have been created
         filepaths = pt.get_filepaths()
         self.assertEqual(len(filepaths), 2)
-        self.assertEqual(filepaths[0][0], fp_count + 2)
-        self.assertEqual(filepaths[1][0], fp_count + 1)
 
     def test_create(self):
         """Creates a new PrepTemplate"""
@@ -998,8 +996,6 @@ def test_create_warning(self):
         # prep and qiime files have been created
         filepaths = pt.get_filepaths()
         self.assertEqual(len(filepaths), 2)
-        self.assertEqual(filepaths[0][0], fp_count + 2)
-        self.assertEqual(filepaths[1][0], fp_count + 1)
 
     def test_create_investigation_type_error(self):
         """Create raises an error if the investigation_type does not exists"""

qiita_db/metadata_template/test/test_sample_template.py

@@ -1320,7 +1320,7 @@ def test_get_filepath(self):
         # change based on time and the same functionality is being tested
         # in data.py
         exp_id = self.conn_handler.execute_fetchone(
-            "SELECT count(1) FROM qiita.filepath")[0] + 1
+            "SELECT last_value FROM qiita.filepath_filepath_id_seq")[0] + 1
         st = qdb.metadata_template.sample_template.SampleTemplate.create(
             self.metadata, self.new_study)
         self.assertEqual(st.get_filepaths()[0][0], exp_id)

qiita_db/portal.py

@@ -91,14 +91,10 @@ def create(cls, portal, desc):
                         SELECT email FROM qiita.qiita_user
                     LOOP
                         INSERT INTO qiita.analysis
-                            (email, name, description, dflt,
-                             analysis_status_id)
-                        VALUES (eml, eml || '-dflt', 'dflt', true, 1)
+                            (email, name, description, dflt)
+                        VALUES (eml, eml || '-dflt', 'dflt', true)
                         RETURNING analysis_id INTO aid;
 
-                        INSERT INTO qiita.analysis_workflow (analysis_id, step)
-                        VALUES (aid, 2);
-
                         INSERT INTO qiita.analysis_portal
                             (analysis_id, portal_type_id)
                         VALUES (aid, pid);
@@ -162,9 +158,6 @@ def delete(portal):
                         DELETE FROM qiita.analysis_portal
                         WHERE analysis_id = aid;
 
-                        DELETE FROM qiita.analysis_workflow
-                        WHERE analysis_id = aid;
-
                         DELETE FROM qiita.analysis_sample
                         WHERE analysis_id = aid;
 

qiita_db/test/test_base.py

@@ -84,59 +84,5 @@ def test_not_equal_type(self):
         self.assertNotEqual(self.tester, new)
 
 
-@qiita_test_checker()
-class QiitaStatusObjectTest(TestCase):
-    """Tests that the QiitaStatusObject class functions act correctly"""
-
-    def setUp(self):
-        # We need an actual subclass in order to test the equality functions
-        self.tester = qdb.analysis.Analysis(1)
-
-    def test_status(self):
-        """Correctly returns the status of the object"""
-        self.assertEqual(self.tester.status, "in_construction")
-
-    def test_check_status_single(self):
-        """check_status works passing a single status"""
-        self.assertTrue(self.tester.check_status(["in_construction"]))
-        self.assertFalse(self.tester.check_status(["queued"]))
-
-    def test_check_status_exclude_single(self):
-        """check_status works passing a single status and the exclude flag"""
-        self.assertTrue(self.tester.check_status(["public"], exclude=True))
-        self.assertFalse(self.tester.check_status(["in_construction"],
-                         exclude=True))
-
-    def test_check_status_list(self):
-        """check_status work passing a list of status"""
-        self.assertTrue(self.tester.check_status(
-            ["in_construction", "queued"]))
-        self.assertFalse(self.tester.check_status(
-            ["public", "queued"]))
-
-    def test_check_status_exclude_list(self):
-        """check_status work passing a list of status and the exclude flag"""
-        self.assertTrue(self.tester.check_status(
-            ["public", "queued"], exclude=True))
-        self.assertFalse(self.tester.check_status(
-            ["in_construction", "queued"], exclude=True))
-
-    def test_check_status_unknown_status(self):
-        """check_status raises an error if an invalid status is provided"""
-        with self.assertRaises(ValueError):
-            self.tester.check_status(["foo"])
-
-        with self.assertRaises(ValueError):
-            self.tester.check_status(["foo"], exclude=True)
-
-    def test_check_status_unknown_status_list(self):
-        """check_status raises an error if an invalid status list is provided
-        """
-        with self.assertRaises(ValueError):
-            self.tester.check_status(["foo", "bar"])
-
-        with self.assertRaises(ValueError):
-            self.tester.check_status(["foo", "bar"], exclude=True)
-
 if __name__ == '__main__':
     main()

qiita_db/test/test_meta_util.py

@@ -46,13 +46,13 @@ def test_get_accessible_filepath_ids(self):
         obs = qdb.meta_util.get_accessible_filepath_ids(
             qdb.user.User('shared@foo.bar'))
         self.assertItemsEqual(obs, {
-            1, 2, 3, 4, 5, 9, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21})
+            1, 2, 3, 4, 5, 9, 12, 16, 17, 18, 19, 20, 21})
 
         # Now shared should not have access to the study files
         self._unshare_studies()
         obs = qdb.meta_util.get_accessible_filepath_ids(
             qdb.user.User('shared@foo.bar'))
-        self.assertItemsEqual(obs, {16, 14, 15, 13})
+        self.assertItemsEqual(obs, {16})
 
         # Now shared should not have access to any files
         self._unshare_analyses()
@@ -64,10 +64,11 @@ def test_get_accessible_filepath_ids(self):
         self._set_artifact_public()
         obs = qdb.meta_util.get_accessible_filepath_ids(
             qdb.user.User('shared@foo.bar'))
-        self.assertEqual(obs, {1, 2, 3, 4, 5, 9, 12, 17, 18, 19, 20, 21})
+        self.assertEqual(
+            obs, {1, 2, 3, 4, 5, 9, 12, 15, 16, 17, 18, 19, 20, 21, 22})
 
         # Test that it doesn't break: if the SampleTemplate hasn't been added
-        exp = {1, 2, 3, 4, 5, 9, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21}
+        exp = {1, 2, 3, 4, 5, 9, 12, 15, 16, 17, 18, 19, 20, 21, 22}
         obs = qdb.meta_util.get_accessible_filepath_ids(
             qdb.user.User('test@foo.bar'))
         self.assertEqual(obs, exp)
@@ -99,9 +100,11 @@ def test_get_accessible_filepath_ids(self):
         self.assertEqual(obs, exp)
 
         # admin should have access to everything
-        count = self.conn_handler.execute_fetchone("SELECT count(*) FROM "
-                                                   "qiita.filepath")[0]
+        count = self.conn_handler.execute_fetchone(
+            "SELECT last_value FROM qiita.filepath_filepath_id_seq")[0]
         exp = set(range(1, count + 1))
+        exp.discard(13)
+        exp.discard(14)
         obs = qdb.meta_util.get_accessible_filepath_ids(
             qdb.user.User('admin@foo.bar'))
         self.assertEqual(obs, exp)

qiita_db/test/test_portal.py

@@ -72,13 +72,6 @@ def test_remove_portal(self):
             qdb.portal.Portal.delete("QIITA")
 
         qdb.portal.Portal.create("NEWPORTAL2", "SOMEDESC")
-        # Add analysis to this new portal and make sure error raised
-        qiita_config.portal = "NEWPORTAL2"
-        qdb.analysis.Analysis.create(
-            qdb.user.User("test@foo.bar"), "newportal analysis", "desc")
-        qiita_config.portal = "QIITA"
-        with self.assertRaises(qdb.exceptions.QiitaDBError):
-            qdb.portal.Portal.delete("NEWPORTAL2")
 
         # Add study to this new portal and make sure error raised
         info = {

qiita_db/test/test_setup.py

@@ -62,18 +62,9 @@ def test_prep_1(self):
     def test_reference(self):
         self.assertEqual(get_count("qiita.reference"), 2)
 
-    def test_job(self):
-        self.assertEqual(get_count("qiita.job"), 3)
-
     def test_analysis(self):
         self.assertEqual(get_count("qiita.analysis"), 10)
 
-    def test_analysis_job(self):
-        self.assertEqual(get_count("qiita.analysis_job"), 3)
-
-    def test_analysis_workflow(self):
-        self.assertEqual(get_count("qiita.analysis_workflow"), 2)
-
     def test_analysis_filepath(self):
         self.assertEqual(get_count("qiita.analysis_filepath"), 2)
 
@@ -83,12 +74,6 @@ def test_analysis_sample(self):
     def test_analysis_users(self):
         self.assertEqual(get_count("qiita.analysis_users"), 1)
 
-    def test_job_results_filepath(self):
-        self.assertEqual(get_count("qiita.job_results_filepath"), 2)
-
-    def test_command_data_type(self):
-        self.assertEqual(get_count("qiita.command_data_type"), 14)
-
     def test_ontology(self):
         self.assertTrue(check_count('qiita.ontology', 1))
 

qiita_db/test/test_sql.py

@@ -20,26 +20,6 @@ def tearDown(self):
             if exists(fp):
                 remove(fp)
 
-    def test_collection_job_trigger_bad_insert(self):
-        # make sure an incorrect job raises an error
-        with self.assertRaises(ValueError):
-            self.conn_handler.execute(
-                'INSERT INTO qiita.collection_job (collection_id, job_id) '
-                'VALUES (1, 3)')
-        obs = self.conn_handler.execute_fetchall(
-            'SELECT * FROM qiita.collection_job')
-        self.assertNotIn([[1, 3]], obs)
-
-    def test_collection_job_trigger(self):
-        # make sure a correct job inserts successfully
-        self.conn_handler.execute(
-            'INSERT INTO qiita.collection_job (collection_id, job_id) '
-            'VALUES (1, 2)')
-        obs = self.conn_handler.execute_fetchall(
-            'SELECT * FROM qiita.collection_job')
-        exp = [[1, 1], [1, 2]]
-        self.assertEqual(obs, exp)
-
     def test_find_artifact_roots_is_root(self):
         """Correctly returns the root if the artifact is already the root"""
         sql = "SELECT * FROM qiita.find_artifact_roots(%s)"