From 9d3fa5c43e83a0860855434d7493f128d3cd2e6e Mon Sep 17 00:00:00 2001 From: Antonio Gonzalez Date: Sat, 28 Jan 2017 18:15:34 -0700 Subject: [PATCH 01/11] fix #1805 --- qiita_db/util.py | 2 +- qiita_pet/handlers/download.py | 67 ++++++++++++++++++++++++++++++++- qiita_pet/test/test_download.py | 64 +++++++++++++++++++++++++++++++ qiita_pet/webserver.py | 4 +- 4 files changed, 134 insertions(+), 3 deletions(-) create mode 100644 qiita_pet/test/test_download.py diff --git a/qiita_db/util.py b/qiita_db/util.py index 938451450..9c9a09611 100644 --- a/qiita_db/util.py +++ b/qiita_db/util.py @@ -894,12 +894,12 @@ def filepath_id_to_rel_path(filepath_id): LEFT JOIN qiita.artifact_filepath USING (filepath_id) WHERE filepath_id = %s""" qdb.sql_connection.TRN.add(sql, [filepath_id]) + # It should be only one row mp, fp, sd, a_id = qdb.sql_connection.TRN.execute_fetchindex()[0] if sd: result = join(mp, str(a_id), fp) else: result = join(mp, fp) - # It should be only one row return result diff --git a/qiita_pet/handlers/download.py b/qiita_pet/handlers/download.py index bbf10699f..de4f4de36 100644 --- a/qiita_pet/handlers/download.py +++ b/qiita_pet/handlers/download.py @@ -1,10 +1,13 @@ from tornado.web import authenticated from os.path import basename +from datetime import datetime from .base_handlers import BaseHandler from qiita_pet.exceptions import QiitaPetAuthorizationError -from qiita_db.util import filepath_id_to_rel_path +from qiita_pet.handlers.api_proxy import study_get_req +from qiita_db.study import Study +from qiita_db.util import filepath_id_to_rel_path, get_db_files_base_dir from qiita_db.meta_util import validate_filepath_access_by_user from qiita_core.util import execute_as_transaction @@ -37,3 +40,65 @@ def get(self, filepath_id): 'attachment; filename=%s' % fname) self.finish() + + +class DownloadStudyBIOMSHandler(BaseHandler): + @authenticated + @execute_as_transaction + def get(self, study_id): + study_id = int(study_id) + # Check access to study + study_info = study_get_req(study_id, self.current_user.id) + + if study_info['status'] != 'success': + raise QiitaPetAuthorizationError( + self.current_user, 'study id %s' % str(study_id)) + + study = Study(study_id) + user = self.current_user + basedir = get_db_files_base_dir() + basedir_len = len(basedir) + 1 + # loop over artifacts and retrieve those that we have access to + to_download = [] + vfabu = validate_filepath_access_by_user + for a in study.artifacts(): + if a.artifact_type == 'BIOM': + to_add = True + for i, (fid, path, data_type) in enumerate(a.filepaths): + # validate access only of the first artifact filepath, + # the rest have the same permissions + if (i == 0 and not vfabu(user, fid)): + to_add = False + break + if path.startswith(basedir): + path = path[basedir_len:] + to_download.append((path, data_type)) + + if to_add: + for pt in a.prep_templates: + qmf = pt.qiime_map_fp + if qmf is not None: + if qmf.startswith(basedir): + qmf = qmf[basedir_len:] + to_download.append((qmf, 'QIIME map file')) + + # If we don't have nginx, write a file that indicates this + all_files = '\n'.join(['%s: %s' % (n, fp) for fp, n in to_download]) + self.write("This installation of Qiita was not equipped with nginx, " + "so it is incapable of serving files. The files you " + "attempted to download are located at:\n%s" % all_files) + + zip_fn = 'study_%d_%s.zip' % ( + study_id, datetime.now().strftime('%m%d%y-%H%M%S')) + + self.set_header('Content-Description', 'File Transfer') + self.set_header('Content-Type', 'application/octet-stream') + self.set_header('Content-Transfer-Encoding', 'binary') + self.set_header('Expires', '0') + self.set_header('Cache-Control', 'no-cache') + self.set_header('X-Accel-Files', 'zip') + for fp, n in to_download: + self.set_header('X-Accel-Redirect', '/protected/' + fp) + self.set_header('Content-Disposition', + 'attachment; filename=%s' % zip_fn) + self.finish() diff --git a/qiita_pet/test/test_download.py b/qiita_pet/test/test_download.py new file mode 100644 index 000000000..02b0aaf10 --- /dev/null +++ b/qiita_pet/test/test_download.py @@ -0,0 +1,64 @@ +# ----------------------------------------------------------------------------- +# Copyright (c) 2014--, The Qiita Development Team. +# +# Distributed under the terms of the BSD 3-clause License. +# +# The full license is in the file LICENSE, distributed with this software. +# ----------------------------------------------------------------------------- + +from unittest import main + +# from qiita_pet.exceptions import QiitaPetAuthorizationError +from qiita_pet.test.tornado_test_base import TestHandlerBase + + +class TestDownloadHandler(TestHandlerBase): + + def setUp(self): + super(TestDownloadHandler, self).setUp() + + def tearDown(self): + super(TestDownloadHandler, self).tearDown() + + def test_download(self): + # check success + response = self.get('/download/1') + self.assertEqual(response.code, 200) + self.assertEqual(response.body, ( + "This installation of Qiita was not equipped with nginx, so it " + "is incapable of serving files. The file you attempted to " + "download is located at raw_data/1_s_G1_L001_sequences.fastq.gz")) + + # failure + # response = self.get('/download/1000') + + +class TestDownloadStudyBIOMSHandler(TestHandlerBase): + + def setUp(self): + super(TestDownloadStudyBIOMSHandler, self).setUp() + + def tearDown(self): + super(TestDownloadStudyBIOMSHandler, self).tearDown() + + def test_download_study(self): + # check success + response = self.get('/download_study_bioms/1') + self.assertEqual(response.code, 200) + self.assertEqual(response.body, ( + "This installation of Qiita was not equipped with nginx, so it " + "is incapable of serving files. The files you attempted to " + "download are located at:\nbiom: processed_data/1_study_1001_" + "closed_reference_otu_table.biom\nQIIME map file: templates/" + "1_prep_1_qiime_19700101-000000.txt\nbiom: processed_data/" + "1_study_1001_closed_reference_otu_table.biom\nQIIME map file: " + "templates/1_prep_1_qiime_19700101-000000.txt\nbiom: " + "processed_data/1_study_1001_closed_reference_otu_table_Silva.biom" + "\nQIIME map file: templates/1_prep_1_qiime_19700101-000000.txt")) + + # failure + # response = self.get('/download_study_bioms/2') + + +if __name__ == '__main__': + main() diff --git a/qiita_pet/webserver.py b/qiita_pet/webserver.py index 42952d478..15185601d 100644 --- a/qiita_pet/webserver.py +++ b/qiita_pet/webserver.py @@ -38,7 +38,8 @@ from qiita_pet.handlers.logger_handlers import LogEntryViewerHandler from qiita_pet.handlers.upload import UploadFileHandler, StudyUploadFileHandler from qiita_pet.handlers.stats import StatsHandler -from qiita_pet.handlers.download import DownloadHandler +from qiita_pet.handlers.download import ( + DownloadHandler, DownloadStudyBIOMSHandler) from qiita_pet.handlers.prep_template import PrepTemplateHandler from qiita_pet.handlers.ontology import OntologyHandler from qiita_db.handlers.processing_job import ( @@ -144,6 +145,7 @@ def __init__(self): (r"/check_study/", CreateStudyAJAX), (r"/stats/", StatsHandler), (r"/download/(.*)", DownloadHandler), + (r"/download_study_bioms/(.*)", DownloadStudyBIOMSHandler), (r"/vamps/(.*)", VAMPSHandler), # Plugin handlers - the order matters here so do not change # qiita_db/jobs/(.*) should go after any of the From a70163b0076460bb6a2d508a29c527a50f3eb876 Mon Sep 17 00:00:00 2001 From: Antonio Gonzalez Date: Sat, 28 Jan 2017 18:34:27 -0700 Subject: [PATCH 02/11] adding button --- qiita_pet/templates/study_base.html | 1 + 1 file changed, 1 insertion(+) diff --git a/qiita_pet/templates/study_base.html b/qiita_pet/templates/study_base.html index 3e16e9fa9..34b604bf4 100644 --- a/qiita_pet/templates/study_base.html +++ b/qiita_pet/templates/study_base.html @@ -239,6 +239,7 @@ Upload Files {% end %} + All QIIME maps and BIOMs
From 1626859cca8c196a63ee2ee0b5849358524161db Mon Sep 17 00:00:00 2001 From: Antonio Gonzalez Date: Sun, 29 Jan 2017 06:53:13 -0700 Subject: [PATCH 03/11] fix errors --- qiita_db/test/test_meta_util.py | 41 ++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/qiita_db/test/test_meta_util.py b/qiita_db/test/test_meta_util.py index fe80059da..891ed3b66 100644 --- a/qiita_db/test/test_meta_util.py +++ b/qiita_db/test/test_meta_util.py @@ -84,7 +84,7 @@ def test_validate_filepath_access_by_user(self): "principal_investigator_id": 1, "lab_person_id": 1 } - qdb.study.Study.create( + study = qdb.study.Study.create( qdb.user.User('test@foo.bar'), "Test study", [1], info) for i in [1, 2, 3, 4, 5, 9, 12, 17, 18, 19, 20, 21]: self.assertTrue(qdb.meta_util.validate_filepath_access_by_user( @@ -105,6 +105,8 @@ def test_validate_filepath_access_by_user(self): self.assertTrue(qdb.meta_util.validate_filepath_access_by_user( admin, i[0])) + qdb.study.Study.delete(study.id) + def test_get_lat_longs(self): exp = [ [74.0894932572, 65.3283470202], @@ -179,18 +181,21 @@ def test_get_lat_longs_EMP_portal(self): self.assertItemsEqual(obs, exp) + qdb.metadata_template.sample_template.SampleTemplate.delete(study.id) + qdb.study.Study.delete(study.id) + def test_update_redis_stats(self): qdb.meta_util.update_redis_stats() portal = qiita_config.portal vals = [ - ('number_studies', {'sanbox': '2', 'public': '0', - 'private': '1'}, r_client.hgetall), - ('number_of_samples', {'sanbox': '1', 'public': '0', - 'private': '27'}, r_client.hgetall), + ('number_studies', {'sanbox': '0', 'public': '1', + 'private': '0'}, r_client.hgetall), + ('number_of_samples', {'sanbox': '0', 'public': '27', + 'private': '0'}, r_client.hgetall), ('num_users', '4', r_client.get), ('lat_longs', EXP_LAT_LONG, r_client.get), - ('num_studies_ebi', '3', r_client.get), + ('num_studies_ebi', '1', r_client.get), ('num_samples_ebi', '27', r_client.get), ('number_samples_ebi_prep', '54', r_client.get) # not testing img/time for simplicity @@ -203,19 +208,19 @@ def test_update_redis_stats(self): EXP_LAT_LONG = ( - '[[0.291867635913, 68.5945325743], [68.0991287718, 34.8360987059],' - ' [10.6655599093, 70.784770579], [40.8623799474, 6.66444220187],' + '[[60.1102854322, 74.7123248382], [23.1218032799, 42.838497795],' + ' [3.21190859967, 26.8138925876], [74.0894932572, 65.3283470202],' + ' [53.5050692395, 31.6056761814], [12.6245524972, 96.0693176066],' + ' [43.9614715197, 82.8516734159], [10.6655599093, 70.784770579],' + ' [78.3634273709, 74.423907894], [82.8302905615, 86.3615778099],' + ' [44.9725384282, 66.1920014699], [4.59216095574, 63.5115213108],' + ' [57.571893782, 32.5563076447], [40.8623799474, 6.66444220187],' + ' [95.2060749748, 27.3592668624], [38.2627021402, 3.48274264219],' ' [13.089194595, 92.5274472082], [84.0030227585, 66.8954849864],' - ' [12.7065957714, 84.9722975792], [78.3634273709, 74.423907894],' - ' [82.8302905615, 86.3615778099], [53.5050692395, 31.6056761814],' - ' [43.9614715197, 82.8516734159], [29.1499460692, 82.1270418227],' - ' [23.1218032799, 42.838497795], [12.6245524972, 96.0693176066],' - ' [38.2627021402, 3.48274264219], [74.0894932572, 65.3283470202],' - ' [35.2374368957, 68.5041623253], [4.59216095574, 63.5115213108],' - ' [95.2060749748, 27.3592668624], [68.51099627, 2.35063674718],' - ' [85.4121476399, 15.6526750776], [60.1102854322, 74.7123248382],' - ' [3.21190859967, 26.8138925876], [57.571893782, 32.5563076447],' - ' [44.9725384282, 66.1920014699], [42.42, 41.41]]') + ' [68.51099627, 2.35063674718], [29.1499460692, 82.1270418227],' + ' [35.2374368957, 68.5041623253], [12.7065957714, 84.9722975792],' + ' [0.291867635913, 68.5945325743], [85.4121476399, 15.6526750776],' + ' [68.0991287718, 34.8360987059]]') if __name__ == '__main__': main() From ade8cbe7844e47dcbee8f1123936b65f5d62f583 Mon Sep 17 00:00:00 2001 From: Antonio Gonzalez Date: Sun, 29 Jan 2017 20:08:57 -0700 Subject: [PATCH 04/11] fixing failures tests --- qiita_pet/handlers/download.py | 13 +++++++------ qiita_pet/test/test_download.py | 18 +++++++++++++----- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/qiita_pet/handlers/download.py b/qiita_pet/handlers/download.py index de4f4de36..b6d3e3fab 100644 --- a/qiita_pet/handlers/download.py +++ b/qiita_pet/handlers/download.py @@ -1,10 +1,9 @@ -from tornado.web import authenticated +from tornado.web import authenticated, HTTPError from os.path import basename from datetime import datetime from .base_handlers import BaseHandler -from qiita_pet.exceptions import QiitaPetAuthorizationError from qiita_pet.handlers.api_proxy import study_get_req from qiita_db.study import Study from qiita_db.util import filepath_id_to_rel_path, get_db_files_base_dir @@ -19,8 +18,9 @@ def get(self, filepath_id): fid = int(filepath_id) if not validate_filepath_access_by_user(self.current_user, fid): - raise QiitaPetAuthorizationError( - self.current_user, 'filepath id %s' % str(fid)) + raise HTTPError( + 404, "%s doesn't have access to " + "filepath_id: %s" % (self.current_user.email, str(fid))) relpath = filepath_id_to_rel_path(fid) fname = basename(relpath) @@ -51,8 +51,9 @@ def get(self, study_id): study_info = study_get_req(study_id, self.current_user.id) if study_info['status'] != 'success': - raise QiitaPetAuthorizationError( - self.current_user, 'study id %s' % str(study_id)) + raise HTTPError(405, "%s: %s, %s" % (study_info['message'], + self.current_user.email, + str(study_id))) study = Study(study_id) user = self.current_user diff --git a/qiita_pet/test/test_download.py b/qiita_pet/test/test_download.py index 02b0aaf10..aa3b94526 100644 --- a/qiita_pet/test/test_download.py +++ b/qiita_pet/test/test_download.py @@ -7,9 +7,11 @@ # ----------------------------------------------------------------------------- from unittest import main +from mock import Mock -# from qiita_pet.exceptions import QiitaPetAuthorizationError from qiita_pet.test.tornado_test_base import TestHandlerBase +from qiita_pet.handlers.base_handlers import BaseHandler +from qiita_db.user import User class TestDownloadHandler(TestHandlerBase): @@ -30,7 +32,8 @@ def test_download(self): "download is located at raw_data/1_s_G1_L001_sequences.fastq.gz")) # failure - # response = self.get('/download/1000') + response = self.get('/download/1000') + self.assertEqual(response.code, 404) class TestDownloadStudyBIOMSHandler(TestHandlerBase): @@ -42,7 +45,6 @@ def tearDown(self): super(TestDownloadStudyBIOMSHandler, self).tearDown() def test_download_study(self): - # check success response = self.get('/download_study_bioms/1') self.assertEqual(response.code, 200) self.assertEqual(response.body, ( @@ -56,8 +58,14 @@ def test_download_study(self): "processed_data/1_study_1001_closed_reference_otu_table_Silva.biom" "\nQIIME map file: templates/1_prep_1_qiime_19700101-000000.txt")) - # failure - # response = self.get('/download_study_bioms/2') + response = self.get('/download_study_bioms/200') + self.assertEqual(response.code, 405) + + # changing user so we can test the failures + BaseHandler.get_current_user = Mock( + return_value=User("demo@microbio.me")) + response = self.get('/download_study_bioms/1') + self.assertEqual(response.code, 405) if __name__ == '__main__': From 8e3ec27fc251ca048205eb97401a26e84eb65c48 Mon Sep 17 00:00:00 2001 From: Antonio Gonzalez Date: Tue, 21 Mar 2017 05:46:20 -0600 Subject: [PATCH 05/11] fixing errors --- qiita_db/test/test_meta_util.py | 2 +- qiita_pet/handlers/download.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/qiita_db/test/test_meta_util.py b/qiita_db/test/test_meta_util.py index 1b52c4468..4e4332764 100644 --- a/qiita_db/test/test_meta_util.py +++ b/qiita_db/test/test_meta_util.py @@ -219,7 +219,7 @@ def test_update_redis_stats(self): ' [35.2374368957, 68.5041623253], [12.7065957714, 84.9722975792],' ' [0.291867635913, 68.5945325743], [85.4121476399, 15.6526750776],' ' [68.0991287718, 34.8360987059]]') - + if __name__ == '__main__': main() diff --git a/qiita_pet/handlers/download.py b/qiita_pet/handlers/download.py index b6d3e3fab..425c068e7 100644 --- a/qiita_pet/handlers/download.py +++ b/qiita_pet/handlers/download.py @@ -97,7 +97,7 @@ def get(self, study_id): self.set_header('Content-Transfer-Encoding', 'binary') self.set_header('Expires', '0') self.set_header('Cache-Control', 'no-cache') - self.set_header('X-Accel-Files', 'zip') + self.set_header('X-Archive-Files', 'zip') for fp, n in to_download: self.set_header('X-Accel-Redirect', '/protected/' + fp) self.set_header('Content-Disposition', From 890d465779da653b9f332c3f15e993b4b7e6f07f Mon Sep 17 00:00:00 2001 From: Antonio Gonzalez Date: Tue, 21 Mar 2017 06:04:55 -0600 Subject: [PATCH 06/11] fixing errors due to update --- qiita_db/test/test_meta_util.py | 3 --- qiita_pet/test/test_download.py | 8 ++++---- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/qiita_db/test/test_meta_util.py b/qiita_db/test/test_meta_util.py index 4e4332764..7fff5cdf6 100644 --- a/qiita_db/test/test_meta_util.py +++ b/qiita_db/test/test_meta_util.py @@ -179,9 +179,6 @@ def test_get_lat_longs_EMP_portal(self): qdb.metadata_template.sample_template.SampleTemplate.delete(st.id) qdb.study.Study.delete(study.id) - qdb.metadata_template.sample_template.SampleTemplate.delete(study.id) - qdb.study.Study.delete(study.id) - def test_update_redis_stats(self): qdb.meta_util.update_redis_stats() diff --git a/qiita_pet/test/test_download.py b/qiita_pet/test/test_download.py index aa3b94526..77539e913 100644 --- a/qiita_pet/test/test_download.py +++ b/qiita_pet/test/test_download.py @@ -47,16 +47,16 @@ def tearDown(self): def test_download_study(self): response = self.get('/download_study_bioms/1') self.assertEqual(response.code, 200) - self.assertEqual(response.body, ( + self.assertRegexpMatches(response.body, ( "This installation of Qiita was not equipped with nginx, so it " "is incapable of serving files. The files you attempted to " "download are located at:\nbiom: processed_data/1_study_1001_" "closed_reference_otu_table.biom\nQIIME map file: templates/" - "1_prep_1_qiime_19700101-000000.txt\nbiom: processed_data/" + "1_prep_1_qiime_[0-9]*-[0-9]*.txt\nbiom: processed_data/" "1_study_1001_closed_reference_otu_table.biom\nQIIME map file: " - "templates/1_prep_1_qiime_19700101-000000.txt\nbiom: " + "templates/1_prep_1_qiime_[0-9]*-[0-9]*.txt\nbiom: " "processed_data/1_study_1001_closed_reference_otu_table_Silva.biom" - "\nQIIME map file: templates/1_prep_1_qiime_19700101-000000.txt")) + "\nQIIME map file: templates/1_prep_1_qiime_[0-9]*-[0-9]*.txt")) response = self.get('/download_study_bioms/200') self.assertEqual(response.code, 405) From 55fe223d1164480a86ebba6de2582fe331ef8a10 Mon Sep 17 00:00:00 2001 From: Jose Navas Date: Tue, 21 Mar 2017 15:41:24 -0700 Subject: [PATCH 07/11] Making the download work --- qiita_pet/handlers/download.py | 38 ++++++++++++++++++++++------------ 1 file changed, 25 insertions(+), 13 deletions(-) diff --git a/qiita_pet/handlers/download.py b/qiita_pet/handlers/download.py index 425c068e7..d122a5862 100644 --- a/qiita_pet/handlers/download.py +++ b/qiita_pet/handlers/download.py @@ -1,6 +1,7 @@ from tornado.web import authenticated, HTTPError -from os.path import basename +from os.path import basename, getsize, join +from os import walk from datetime import datetime from .base_handlers import BaseHandler @@ -71,35 +72,46 @@ def get(self, study_id): if (i == 0 and not vfabu(user, fid)): to_add = False break - if path.startswith(basedir): - path = path[basedir_len:] - to_download.append((path, data_type)) + if data_type == 'directory': + # If we have a directory, we actually need to list + # all the files from the directory so NGINX can + # actually download all of them + for dp, _, fps in walk(path): + for fname in fps: + fullpath = join(dp, fname) + spath = fullpath + if fullpath.startswith(basedir): + spath = fullpath[basedir_len:] + to_download.append((fullpath, spath, spath)) + elif path.startswith(basedir): + spath = path[basedir_len:] + to_download.append((fullpath, spath, spath)) + else: + to_download.append((path, path, path)) if to_add: for pt in a.prep_templates: qmf = pt.qiime_map_fp if qmf is not None: + sqmf = qmf if qmf.startswith(basedir): qmf = qmf[basedir_len:] - to_download.append((qmf, 'QIIME map file')) + to_download.append( + (qmf, sqmf, 'mapping_files/%s_mapping_file.txt' + % a.id)) # If we don't have nginx, write a file that indicates this - all_files = '\n'.join(['%s: %s' % (n, fp) for fp, n in to_download]) - self.write("This installation of Qiita was not equipped with nginx, " - "so it is incapable of serving files. The files you " - "attempted to download are located at:\n%s" % all_files) + all_files = '\n'.join(["- %s /protected/%s %s" % (getsize(fp), sfp, n) + for fp, sfp, n in to_download]) + self.write("%s\n" % all_files) zip_fn = 'study_%d_%s.zip' % ( study_id, datetime.now().strftime('%m%d%y-%H%M%S')) self.set_header('Content-Description', 'File Transfer') - self.set_header('Content-Type', 'application/octet-stream') - self.set_header('Content-Transfer-Encoding', 'binary') self.set_header('Expires', '0') self.set_header('Cache-Control', 'no-cache') self.set_header('X-Archive-Files', 'zip') - for fp, n in to_download: - self.set_header('X-Accel-Redirect', '/protected/' + fp) self.set_header('Content-Disposition', 'attachment; filename=%s' % zip_fn) self.finish() From d96da86689f76a3675a11518a65a2cb20d61d891 Mon Sep 17 00:00:00 2001 From: Jose Navas Date: Tue, 21 Mar 2017 17:07:38 -0700 Subject: [PATCH 08/11] Fixing tests --- qiita_pet/handlers/download.py | 4 ++-- qiita_pet/test/test_download.py | 29 +++++++++++++++++++---------- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/qiita_pet/handlers/download.py b/qiita_pet/handlers/download.py index d122a5862..0ad731854 100644 --- a/qiita_pet/handlers/download.py +++ b/qiita_pet/handlers/download.py @@ -85,7 +85,7 @@ def get(self, study_id): to_download.append((fullpath, spath, spath)) elif path.startswith(basedir): spath = path[basedir_len:] - to_download.append((fullpath, spath, spath)) + to_download.append((path, spath, spath)) else: to_download.append((path, path, path)) @@ -95,7 +95,7 @@ def get(self, study_id): if qmf is not None: sqmf = qmf if qmf.startswith(basedir): - qmf = qmf[basedir_len:] + sqmf = qmf[basedir_len:] to_download.append( (qmf, sqmf, 'mapping_files/%s_mapping_file.txt' % a.id)) diff --git a/qiita_pet/test/test_download.py b/qiita_pet/test/test_download.py index 77539e913..bc5b10485 100644 --- a/qiita_pet/test/test_download.py +++ b/qiita_pet/test/test_download.py @@ -47,16 +47,25 @@ def tearDown(self): def test_download_study(self): response = self.get('/download_study_bioms/1') self.assertEqual(response.code, 200) - self.assertRegexpMatches(response.body, ( - "This installation of Qiita was not equipped with nginx, so it " - "is incapable of serving files. The files you attempted to " - "download are located at:\nbiom: processed_data/1_study_1001_" - "closed_reference_otu_table.biom\nQIIME map file: templates/" - "1_prep_1_qiime_[0-9]*-[0-9]*.txt\nbiom: processed_data/" - "1_study_1001_closed_reference_otu_table.biom\nQIIME map file: " - "templates/1_prep_1_qiime_[0-9]*-[0-9]*.txt\nbiom: " - "processed_data/1_study_1001_closed_reference_otu_table_Silva.biom" - "\nQIIME map file: templates/1_prep_1_qiime_[0-9]*-[0-9]*.txt")) + exp = ( + '- 1256812 /protected/processed_data/1_study_1001_closed_' + 'reference_otu_table.biom processed_data/1_study_1001_closed_' + 'reference_otu_table.biom\n' + '- 36615 /protected/templates/1_prep_1_qiime_[0-9]*-[0-9]*.txt ' + 'mapping_files/4_mapping_file.txt\n' + '- 1256812 /protected/processed_data/1_study_1001_closed_reference' + '_otu_table.biom processed_data/1_study_1001_closed_' + 'reference_otu_table.biom\n' + '- 36615 /protected/templates/1_prep_1_qiime_[0-9]*-[0-9]*.txt ' + 'mapping_files/5_mapping_file.txt\n' + '- 1256812 /protected/processed_data/1_study_1001_closed_reference' + '_otu_table_Silva.biom processed_data/1_study_1001_closed_' + 'reference_otu_table_Silva.biom\n' + '- 36615 /protected/templates/1_prep_1_qiime_[0-9]*-[0-9]*.txt ' + 'mapping_files/6_mapping_file.txt\n' + '- 36615 /protected/templates/1_prep_2_qiime_[0-9]*-[0-9]*.txt ' + 'mapping_files/7_mapping_file.txt\n') + self.assertRegexpMatches(response.body, exp) response = self.get('/download_study_bioms/200') self.assertEqual(response.code, 405) From 1859c8864dd0165d933dcf5fe681778b7c3ad629 Mon Sep 17 00:00:00 2001 From: Jose Navas Date: Wed, 22 Mar 2017 14:18:40 -0700 Subject: [PATCH 09/11] Addressing @antgonza's comments --- qiita_pet/handlers/download.py | 10 ++++- qiita_pet/nginx_example.conf | 3 +- qiita_pet/test/test_download.py | 76 ++++++++++++++++++++++++++------- 3 files changed, 71 insertions(+), 18 deletions(-) diff --git a/qiita_pet/handlers/download.py b/qiita_pet/handlers/download.py index 0ad731854..b85b03dc4 100644 --- a/qiita_pet/handlers/download.py +++ b/qiita_pet/handlers/download.py @@ -7,7 +7,8 @@ from .base_handlers import BaseHandler from qiita_pet.handlers.api_proxy import study_get_req from qiita_db.study import Study -from qiita_db.util import filepath_id_to_rel_path, get_db_files_base_dir +from qiita_db.util import (filepath_id_to_rel_path, get_db_files_base_dir, + compute_checksum) from qiita_db.meta_util import validate_filepath_access_by_user from qiita_core.util import execute_as_transaction @@ -87,6 +88,10 @@ def get(self, study_id): spath = path[basedir_len:] to_download.append((path, spath, spath)) else: + # We are not aware of any case that can trigger this + # situation, but we wanted to be overly cautious + # There is no test for this line cause we don't know + # how to trigger it to_download.append((path, path, path)) if to_add: @@ -101,7 +106,8 @@ def get(self, study_id): % a.id)) # If we don't have nginx, write a file that indicates this - all_files = '\n'.join(["- %s /protected/%s %s" % (getsize(fp), sfp, n) + all_files = '\n'.join(["%s %s /protected/%s %s" + % (compute_checksum(fp), getsize(fp), sfp, n) for fp, sfp, n in to_download]) self.write("%s\n" % all_files) diff --git a/qiita_pet/nginx_example.conf b/qiita_pet/nginx_example.conf index f0cca6f81..27dd4d0f5 100644 --- a/qiita_pet/nginx_example.conf +++ b/qiita_pet/nginx_example.conf @@ -18,7 +18,7 @@ http { internal; # CHANGE ME: This should match the BASE_DATA_DIR in your qiita - # config. E.g., + # config. E.g., # alias /Users/username/qiita/qiita_db/support_files/test_data/; alias ; } @@ -30,6 +30,7 @@ http { proxy_set_header Host $host; proxy_set_header X-Real-IP $remote_addr; proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header Accept-Encoding identity; } } } diff --git a/qiita_pet/test/test_download.py b/qiita_pet/test/test_download.py index bc5b10485..4792e9326 100644 --- a/qiita_pet/test/test_download.py +++ b/qiita_pet/test/test_download.py @@ -8,10 +8,19 @@ from unittest import main from mock import Mock +from os.path import exists, isdir, join +from os import remove, makedirs +from shutil import rmtree +from tempfile import mkdtemp + +from biom.util import biom_open +from biom import example_table as et from qiita_pet.test.tornado_test_base import TestHandlerBase from qiita_pet.handlers.base_handlers import BaseHandler from qiita_db.user import User +from qiita_db.artifact import Artifact +from qiita_db.software import Parameters, Command class TestDownloadHandler(TestHandlerBase): @@ -40,31 +49,68 @@ class TestDownloadStudyBIOMSHandler(TestHandlerBase): def setUp(self): super(TestDownloadStudyBIOMSHandler, self).setUp() + self._clean_up_files = [] def tearDown(self): super(TestDownloadStudyBIOMSHandler, self).tearDown() + for fp in self._clean_up_files: + if exists(fp): + if isdir(fp): + rmtree(fp) + else: + remove(fp) def test_download_study(self): + tmp_dir = mkdtemp() + self._clean_up_files.append(tmp_dir) + + biom_fp = join(tmp_dir, 'otu_table.biom') + smr_dir = join(tmp_dir, 'sortmerna_picked_otus') + log_dir = join(smr_dir, 'seqs_otus.log') + + with biom_open(biom_fp, 'w') as f: + et.to_hdf5(f, "test") + makedirs(smr_dir) + with open(log_dir, 'w') as f: + f.write('\n') + + self._clean_up_files.append(tmp_dir) + + files_biom = [(biom_fp, 'biom'), (smr_dir, 'directory')] + + params = Parameters.from_default_params( + Command(3).default_parameter_sets.next(), {'input_data': 1}) + a = Artifact.create(files_biom, "BIOM", parents=[Artifact(2)], + processing_parameters=params) + for _, fp, _ in a.filepaths: + self._clean_up_files.append(fp) + response = self.get('/download_study_bioms/1') self.assertEqual(response.code, 200) exp = ( - '- 1256812 /protected/processed_data/1_study_1001_closed_' + '[0-9]* 1256812 /protected/processed_data/1_study_1001_closed_' 'reference_otu_table.biom processed_data/1_study_1001_closed_' 'reference_otu_table.biom\n' - '- 36615 /protected/templates/1_prep_1_qiime_[0-9]*-[0-9]*.txt ' - 'mapping_files/4_mapping_file.txt\n' - '- 1256812 /protected/processed_data/1_study_1001_closed_reference' - '_otu_table.biom processed_data/1_study_1001_closed_' - 'reference_otu_table.biom\n' - '- 36615 /protected/templates/1_prep_1_qiime_[0-9]*-[0-9]*.txt ' - 'mapping_files/5_mapping_file.txt\n' - '- 1256812 /protected/processed_data/1_study_1001_closed_reference' - '_otu_table_Silva.biom processed_data/1_study_1001_closed_' - 'reference_otu_table_Silva.biom\n' - '- 36615 /protected/templates/1_prep_1_qiime_[0-9]*-[0-9]*.txt ' - 'mapping_files/6_mapping_file.txt\n' - '- 36615 /protected/templates/1_prep_2_qiime_[0-9]*-[0-9]*.txt ' - 'mapping_files/7_mapping_file.txt\n') + '[0-9]* 36615 /protected/templates/1_prep_1_qiime_[0-9]*-' + '[0-9]*.txt mapping_files/4_mapping_file.txt\n' + '[0-9]* 1256812 /protected/processed_data/' + '1_study_1001_closed_reference_otu_table.biom processed_data/' + '1_study_1001_closed_reference_otu_table.biom\n' + '[0-9]* 36615 /protected/templates/1_prep_1_qiime_[0-9]*-' + '[0-9]*.txt mapping_files/5_mapping_file.txt\n' + '[0-9]* 1256812 /protected/processed_data/' + '1_study_1001_closed_reference_otu_table_Silva.biom processed_data' + '/1_study_1001_closed_reference_otu_table_Silva.biom\n' + '[0-9]* 36615 /protected/templates/1_prep_1_qiime_[0-9]*-' + '[0-9]*.txt mapping_files/6_mapping_file.txt\n' + '[0-9]* 36615 /protected/templates/1_prep_2_qiime_[0-9]*-' + '[0-9]*.txt mapping_files/7_mapping_file.txt\n' + '[0-9]* 39752 /protected/BIOM/{0}/otu_table.biom ' + 'BIOM/{0}/otu_table.biom\n' + '[0-9]* 1 /protected/BIOM/{0}/sortmerna_picked_otus/seqs_otus.log ' + 'BIOM/{0}/sortmerna_picked_otus/seqs_otus.log\n' + '[0-9]* 36615 /protected/templates/1_prep_1_qiime_[0-9]*-[0-9]*.' + 'txt mapping_files/{0}_mapping_file.txt\n'.format(a.id)) self.assertRegexpMatches(response.body, exp) response = self.get('/download_study_bioms/200') From e90ad508fca01d86117e4540a70e3ae08970e29a Mon Sep 17 00:00:00 2001 From: Jose Navas Date: Wed, 22 Mar 2017 14:24:53 -0700 Subject: [PATCH 10/11] Adding missing test --- qiita_pet/test/test_download.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/qiita_pet/test/test_download.py b/qiita_pet/test/test_download.py index 4792e9326..ddbf36997 100644 --- a/qiita_pet/test/test_download.py +++ b/qiita_pet/test/test_download.py @@ -122,6 +122,18 @@ def test_download_study(self): response = self.get('/download_study_bioms/1') self.assertEqual(response.code, 405) + a.visibility = 'public' + response = self.get('/download_study_bioms/1') + self.assertEqual(response.code, 200) + exp = ( + '[0-9]* 39752 /protected/BIOM/{0}/otu_table.biom ' + 'BIOM/{0}/otu_table.biom\n' + '[0-9]* 1 /protected/BIOM/{0}/sortmerna_picked_otus/seqs_otus.log ' + 'BIOM/{0}/sortmerna_picked_otus/seqs_otus.log\n' + '[0-9]* 36615 /protected/templates/1_prep_1_qiime_[0-9]*-[0-9]*.' + 'txt mapping_files/{0}_mapping_file.txt\n'.format(a.id)) + self.assertRegexpMatches(response.body, exp) + if __name__ == '__main__': main() From 587e5bf05ee89093d3720c2cc9a49c398f10085f Mon Sep 17 00:00:00 2001 From: Jose Navas Date: Fri, 24 Mar 2017 08:54:25 -0700 Subject: [PATCH 11/11] Ignoring tgz - thanks @antgonza --- qiita_pet/handlers/download.py | 10 ++++++---- qiita_pet/test/test_download.py | 31 +++++++++++++++++-------------- 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/qiita_pet/handlers/download.py b/qiita_pet/handlers/download.py index b85b03dc4..912471754 100644 --- a/qiita_pet/handlers/download.py +++ b/qiita_pet/handlers/download.py @@ -7,8 +7,7 @@ from .base_handlers import BaseHandler from qiita_pet.handlers.api_proxy import study_get_req from qiita_db.study import Study -from qiita_db.util import (filepath_id_to_rel_path, get_db_files_base_dir, - compute_checksum) +from qiita_db.util import filepath_id_to_rel_path, get_db_files_base_dir from qiita_db.meta_util import validate_filepath_access_by_user from qiita_core.util import execute_as_transaction @@ -73,6 +72,10 @@ def get(self, study_id): if (i == 0 and not vfabu(user, fid)): to_add = False break + # ignore if tgz as they could create problems and the + # raw data is in the folder + if data_type == 'tgz': + continue if data_type == 'directory': # If we have a directory, we actually need to list # all the files from the directory so NGINX can @@ -106,8 +109,7 @@ def get(self, study_id): % a.id)) # If we don't have nginx, write a file that indicates this - all_files = '\n'.join(["%s %s /protected/%s %s" - % (compute_checksum(fp), getsize(fp), sfp, n) + all_files = '\n'.join(["- %s /protected/%s %s" % (getsize(fp), sfp, n) for fp, sfp, n in to_download]) self.write("%s\n" % all_files) diff --git a/qiita_pet/test/test_download.py b/qiita_pet/test/test_download.py index ddbf36997..5fbc5a4d8 100644 --- a/qiita_pet/test/test_download.py +++ b/qiita_pet/test/test_download.py @@ -67,16 +67,19 @@ def test_download_study(self): biom_fp = join(tmp_dir, 'otu_table.biom') smr_dir = join(tmp_dir, 'sortmerna_picked_otus') log_dir = join(smr_dir, 'seqs_otus.log') + tgz = join(tmp_dir, 'sortmerna_picked_otus.tgz') with biom_open(biom_fp, 'w') as f: et.to_hdf5(f, "test") makedirs(smr_dir) with open(log_dir, 'w') as f: f.write('\n') + with open(tgz, 'w') as f: + f.write('\n') self._clean_up_files.append(tmp_dir) - files_biom = [(biom_fp, 'biom'), (smr_dir, 'directory')] + files_biom = [(biom_fp, 'biom'), (smr_dir, 'directory'), (tgz, 'tgz')] params = Parameters.from_default_params( Command(3).default_parameter_sets.next(), {'input_data': 1}) @@ -88,28 +91,28 @@ def test_download_study(self): response = self.get('/download_study_bioms/1') self.assertEqual(response.code, 200) exp = ( - '[0-9]* 1256812 /protected/processed_data/1_study_1001_closed_' + '- 1256812 /protected/processed_data/1_study_1001_closed_' 'reference_otu_table.biom processed_data/1_study_1001_closed_' 'reference_otu_table.biom\n' - '[0-9]* 36615 /protected/templates/1_prep_1_qiime_[0-9]*-' + '- 36615 /protected/templates/1_prep_1_qiime_[0-9]*-' '[0-9]*.txt mapping_files/4_mapping_file.txt\n' - '[0-9]* 1256812 /protected/processed_data/' + '- 1256812 /protected/processed_data/' '1_study_1001_closed_reference_otu_table.biom processed_data/' '1_study_1001_closed_reference_otu_table.biom\n' - '[0-9]* 36615 /protected/templates/1_prep_1_qiime_[0-9]*-' + '- 36615 /protected/templates/1_prep_1_qiime_[0-9]*-' '[0-9]*.txt mapping_files/5_mapping_file.txt\n' - '[0-9]* 1256812 /protected/processed_data/' + '- 1256812 /protected/processed_data/' '1_study_1001_closed_reference_otu_table_Silva.biom processed_data' '/1_study_1001_closed_reference_otu_table_Silva.biom\n' - '[0-9]* 36615 /protected/templates/1_prep_1_qiime_[0-9]*-' + '- 36615 /protected/templates/1_prep_1_qiime_[0-9]*-' '[0-9]*.txt mapping_files/6_mapping_file.txt\n' - '[0-9]* 36615 /protected/templates/1_prep_2_qiime_[0-9]*-' + '- 36615 /protected/templates/1_prep_2_qiime_[0-9]*-' '[0-9]*.txt mapping_files/7_mapping_file.txt\n' - '[0-9]* 39752 /protected/BIOM/{0}/otu_table.biom ' + '- 39752 /protected/BIOM/{0}/otu_table.biom ' 'BIOM/{0}/otu_table.biom\n' - '[0-9]* 1 /protected/BIOM/{0}/sortmerna_picked_otus/seqs_otus.log ' + '- 1 /protected/BIOM/{0}/sortmerna_picked_otus/seqs_otus.log ' 'BIOM/{0}/sortmerna_picked_otus/seqs_otus.log\n' - '[0-9]* 36615 /protected/templates/1_prep_1_qiime_[0-9]*-[0-9]*.' + '- 36615 /protected/templates/1_prep_1_qiime_[0-9]*-[0-9]*.' 'txt mapping_files/{0}_mapping_file.txt\n'.format(a.id)) self.assertRegexpMatches(response.body, exp) @@ -126,11 +129,11 @@ def test_download_study(self): response = self.get('/download_study_bioms/1') self.assertEqual(response.code, 200) exp = ( - '[0-9]* 39752 /protected/BIOM/{0}/otu_table.biom ' + '- 39752 /protected/BIOM/{0}/otu_table.biom ' 'BIOM/{0}/otu_table.biom\n' - '[0-9]* 1 /protected/BIOM/{0}/sortmerna_picked_otus/seqs_otus.log ' + '- 1 /protected/BIOM/{0}/sortmerna_picked_otus/seqs_otus.log ' 'BIOM/{0}/sortmerna_picked_otus/seqs_otus.log\n' - '[0-9]* 36615 /protected/templates/1_prep_1_qiime_[0-9]*-[0-9]*.' + '- 36615 /protected/templates/1_prep_1_qiime_[0-9]*-[0-9]*.' 'txt mapping_files/{0}_mapping_file.txt\n'.format(a.id)) self.assertRegexpMatches(response.body, exp)