Skip to content

Fixes Qiita db #2046

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
931e566
New DB structure
josenavas Jan 9, 2017
a2b883e
Adding python patch
josenavas Jan 9, 2017
e737f64
Adding a biom so we can actually execute the patch
josenavas Jan 9, 2017
6bc0303
Fixing the patch to correctly transfer the information from the old s…
josenavas Jan 12, 2017
331012d
Fixing patch
josenavas Jan 12, 2017
eabe25d
Fixing patch and a few other bits to make the patch run successfully
josenavas Jan 12, 2017
0c40bc0
These files are no longer needed
josenavas Jan 12, 2017
602acc3
Removing unused code
josenavas Jan 12, 2017
ad4deea
Droping analysis status table
josenavas Jan 12, 2017
aa3e96f
Linking the analysis with all the artifacts
josenavas Jan 12, 2017
d222e08
Fixing typo
josenavas Jan 12, 2017
14be3b0
Merge branch 'analysis-refactor-db' into analysis-refactor-fix-analysis
josenavas Jan 12, 2017
39f6beb
Fixing HTML and dbschema files
josenavas Jan 12, 2017
8c2be56
Merge branch 'analysis-refactor-db' into analysis-refactor-fix-analysis
josenavas Jan 12, 2017
4dd357c
Adding analyisis jobs
josenavas Jan 13, 2017
209e838
Merge branch 'analysis-refactor-db' into analysis-refactor-fix-analysis
josenavas Jan 13, 2017
b24833d
Extending the artifact to work with the analysis
josenavas Jan 13, 2017
8b7f222
Allowing multiomics datatype
josenavas Jan 13, 2017
b381993
Adding private_job_submitter and modifying proc job handler to use it
josenavas Jan 13, 2017
ab65fa2
Adding logging column to the analysis
josenavas Jan 13, 2017
40656f2
Merge branch 'analysis-refactor-db' into analysis-refactor-fix-analysis
josenavas Jan 13, 2017
c4c8420
Adding datatype to the analysis-processing job table
josenavas Jan 15, 2017
0cd9f27
Adding REST endpoint to access the analysis metadata
josenavas Jan 15, 2017
8207513
Adding private jobs to plugin
josenavas Jan 15, 2017
f307cd4
Fixing typo
josenavas Jan 15, 2017
78c8095
Fixing analysis
josenavas Jan 16, 2017
b3936c8
Fixing the processing jobs complete
josenavas Jan 16, 2017
19c3f7d
Removing the old job code
josenavas Jan 16, 2017
055b53e
Oops removed the wrong file
josenavas Jan 16, 2017
46c322d
Removing QiitaStatusObject because it is not used
josenavas Jan 16, 2017
4d2e14d
fixing metautil
josenavas Jan 16, 2017
befb4e2
Fixing porntal, setup and sql tests
josenavas Jan 16, 2017
09f2722
Fixing user and util
josenavas Jan 16, 2017
d1cdc8e
Fixing qiita_db
josenavas Jan 16, 2017
43eb1ed
Addressing @antgonza's comments
josenavas Jan 16, 2017
d016833
Taking into account non-phylogenetic metrics in beta diversity
josenavas Jan 30, 2017
8d1f0c2
Merge branch 'analysis-refactor-fix-analysis' of github.com:josenavas…
josenavas Jan 30, 2017
894379c
Fixing merge conflicts
josenavas Jan 30, 2017
a80c0ac
Addressing @antgonza's comments
josenavas Feb 1, 2017
5d63afa
Merge branch 'analysis-refactor-fix-analysis' into analysis-refactor-…
josenavas Feb 1, 2017
c571bee
Merge branch 'analysis-refactor-remove-old' into analysis-refactor-qi…
josenavas Feb 1, 2017
5959420
Fixing merge conflicts
josenavas Feb 2, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
104 changes: 0 additions & 104 deletions qiita_db/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
:toctree: generated/

QiitaObject
QiitaStatusObject
"""

# -----------------------------------------------------------------------------
Expand Down Expand Up @@ -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
2 changes: 1 addition & 1 deletion qiita_db/investigation.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
17 changes: 12 additions & 5 deletions qiita_db/meta_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 0 additions & 4 deletions qiita_db/metadata_template/test/test_prep_template.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"""
Expand Down Expand Up @@ -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"""
Expand Down
2 changes: 1 addition & 1 deletion qiita_db/metadata_template/test/test_sample_template.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
11 changes: 2 additions & 9 deletions qiita_db/portal.py
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;

Expand Down
54 changes: 0 additions & 54 deletions qiita_db/test/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
15 changes: 9 additions & 6 deletions qiita_db/test/test_meta_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -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})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are these being deleted? Are the analysis permissions be different from now on?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those filepaths ids no longer exists. The jobs that where inserted in the DB didn't had any kind of command information, so the patch removes them.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to confirm, what you are saying is that those filepaths_ids where pushed but are not actually valid, right? If that's the case, are you creating new ones to test that the validate works or what's the plan?

My concern is that: we don't add some kind of permission checking for the analysis.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The permission checking on the analysis is done through the artifacts so that has been already tested. These files no longer exist, and 1 new file has been added (and it is seen in a test below)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for confirming! As the changes are mainly deletions I think 1 review is enough. Merging.


# 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()
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
7 changes: 0 additions & 7 deletions qiita_db/test/test_portal.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
15 changes: 0 additions & 15 deletions qiita_db/test/test_setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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))

Expand Down
20 changes: 0 additions & 20 deletions qiita_db/test/test_sql.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)"
Expand Down
Loading