Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
65 changes: 65 additions & 0 deletions qiita_db/handlers/sample_information.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
# -----------------------------------------------------------------------------
# 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 tornado.web import HTTPError

import qiita_db as qdb
from .oauth2 import OauthBaseHandler, authenticate_oauth


def _get_sample_info(sid):
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like _get_user_info and _get_sample_info are essentially the same function except for the class instantiating it. I would probably try to facto these two things into a single function and then make _get_sample_info a shortcut for a call to this new function. Something like:

def _get_instance(object_id, klass, reason):
    """ """
    try:
        object = klass(object_id)
    except qdb.exceptions.QiitaDBUnknownIDError:
        raise HTTPError(404)
    except Exception as e:
        raise HTTPError(500, reason=reason + ' %s: %s' % (object_id, str(e)))


def _get_study_info(sid):
    return _get_instance(email, qdb.user.User, 'Error instantiating user')

def _get_study_info(sid):
    return _get_instance(int(sid), qdb.metadata_template.sample_template.SampleTemplate, 'Error instantiating sample information')

Should ultimately reduce needed code and tests 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

k

"""Returns the sample information with the given `sid` if it exists

Parameters
----------
sid : str
The sample information id

Returns
-------
qiita_db.metadata_template.sample_template.SampleTemplate
The requested sample template

Raises
------
HTTPError
If the sample information does not exist, with error code 404
If there is a problem instantiating, with error code 500
"""
try:
sid = int(sid)
st = qdb.metadata_template.sample_template.SampleTemplate(sid)
except qdb.exceptions.QiitaDBUnknownIDError:
raise HTTPError(404)
except Exception as e:
raise HTTPError(500, reason='Error instantiating sample information '
'%s: %s' % (sid, str(e)))
Copy link
Contributor

Choose a reason for hiding this comment

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

I would probably change this to be 'id=%s: %s: % (sid, str(e)). So the message would be clear about what that number is, for example:

Error instantiating sample information id=11: Internal server error .....

If this makes sense, then it would be worth doing the same in the User's case.

Copy link
Member Author

Choose a reason for hiding this comment

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

k


return st


class SampleInfoDBHandler(OauthBaseHandler):
@authenticate_oauth
def get(self, study_id):
"""Retrieves the sample information content

Parameters
----------
study_id: str
The id of the study whose sample information is being retrieved

Returns
-------
dict
The contents of the sample information keyed by sample id
"""
with qdb.sql_connection.TRN:
st = _get_sample_info(study_id)
response = {'data': st.to_dataframe().to_dict(orient='index')}

self.write(response)
87 changes: 87 additions & 0 deletions qiita_db/handlers/tests/test_sample_information.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
# -----------------------------------------------------------------------------
# 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, TestCase
from json import loads

from tornado.web import HTTPError

from qiita_db.handlers.tests.oauthbase import OauthTestingBase
import qiita_db as qdb
from qiita_db.handlers.sample_information import _get_sample_info


class UtilTests(TestCase):
def test_get_sample_info(self):
obs = _get_sample_info(1)
exp = qdb.metadata_template.sample_template.SampleTemplate(1)
self.assertEqual(obs, exp)

# It does not exist
with self.assertRaises(HTTPError):
_get_sample_info(100)


class SampleInfoDBHandlerTests(OauthTestingBase):
def test_get_does_not_exist(self):
obs = self.get('/qiita_db/sample_information/100/data/',
headers=self.header)
self.assertEqual(obs.code, 404)

def test_get_no_header(self):
obs = self.get('/qiita_db/sample_information/100/data/')
self.assertEqual(obs.code, 400)

def test_get(self):
obs = self.get('/qiita_db/sample_information/1/data/',
headers=self.header)
self.assertEqual(obs.code, 200)

obs = loads(obs.body)
self.assertEqual(obs.keys(), ['data'])

# for simplicity we will only test that the keys are the same
# and that one of the key's info is correct
obs = obs['data']
exp = ['1.SKB2.640194', '1.SKM4.640180', '1.SKB3.640195',
'1.SKB6.640176', '1.SKD6.640190', '1.SKM6.640187',
'1.SKD9.640182', '1.SKM8.640201', '1.SKM2.640199',
'1.SKD2.640178', '1.SKB7.640196', '1.SKD4.640185',
'1.SKB8.640193', '1.SKM3.640197', '1.SKD5.640186',
'1.SKB1.640202', '1.SKM1.640183', '1.SKD1.640179',
'1.SKD3.640198', '1.SKB5.640181', '1.SKB4.640189',
'1.SKB9.640200', '1.SKM9.640192', '1.SKD8.640184',
'1.SKM5.640177', '1.SKM7.640188', '1.SKD7.640191']
self.assertItemsEqual(obs.keys(), exp)

obs = obs['1.SKB1.640202']
exp = {'qiita_study_id': '1', 'physical_specimen_location': 'ANL',
'tot_org_carb': '5', 'common_name': 'soil metagenome',
'water_content_soil': '0.164',
'env_feature': 'ENVO:plant-associated habitat',
'assigned_from_geo': 'n', 'altitude': '0',
'env_biome': ('ENVO:Temperate grasslands, savannas, and '
'shrubland biome'),
'texture': '64.6 sand, 17.6 silt, 17.8 clay',
'scientific_name': '1118232',
'description_duplicate': 'Burmese bulk',
'latitude': '4.59216095574', 'ph': '6.94', 'host_taxid': '3483',
'elevation': '114', 'description': 'Cannabis Soil Microbiome',
'collection_timestamp': '2011-11-11 13:00:00',
'physical_specimen_remaining': 'true', 'dna_extracted': 'true',
'taxon_id': '410658', 'samp_salinity': '7.15',
'host_subject_id': '1001:M2', 'sample_type': 'ENVO:soil',
'season_environment': 'winter', 'temp': '15',
'country': 'GAZ:United States of America',
'longitude': '63.5115213108', 'tot_nitro': '1.41',
'depth': '0.15', 'anonymized_name': 'SKB1'}
self.assertEqual(obs, exp)


if __name__ == '__main__':
main()
57 changes: 57 additions & 0 deletions qiita_db/handlers/tests/test_user.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
# -----------------------------------------------------------------------------
# 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, TestCase
from json import loads

from tornado.web import HTTPError

from qiita_db.handlers.tests.oauthbase import OauthTestingBase
import qiita_db as qdb
from qiita_db.handlers.user import _get_user_info


class UtilTests(TestCase):
def test_get_user_info(self):
obs = _get_user_info('shared@foo.bar')
exp = qdb.user.User('shared@foo.bar')
self.assertEqual(obs, exp)

# It does not exist
with self.assertRaises(HTTPError):
_get_user_info('no-exists@foo.bar')


class UserInfoDBHandlerTests(OauthTestingBase):
def test_get_does_not_exist(self):
obs = self.get('/qiita_db/user/no-exists@foo.bar/data/',
headers=self.header)
self.assertEqual(obs.code, 404)

def test_get_no_header(self):
obs = self.get('/qiita_db/user/no-exists@foo.bar/data/')
self.assertEqual(obs.code, 400)

def test_get(self):
obs = self.get('/qiita_db/user/shared@foo.bar/data/',
headers=self.header)
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe you should be able to add an argument to this method call asjson=True so that the returned data in body is already parsed as a JSON object.

Copy link
Contributor

Choose a reason for hiding this comment

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

If this work, can you fix where appropriate in this PR?

Copy link
Member Author

Choose a reason for hiding this comment

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

k

Copy link
Member Author

Choose a reason for hiding this comment

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

tried, and didn't work, perhaps we are using an older version

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for giving that a shot. I'm surprised that's the case, maybe there are slightly different test classes in Qiita.

self.assertEqual(obs.code, 200)

obs = loads(obs.body)
self.assertEqual(obs.keys(), ['data'])

# for simplicity we will only test that the keys are the same
# and that one of the key's info is correct
obs = obs['data']
exp = {"password": "$2a$12$gnUi8Qg.0tvW243v889BhOBhWLIHyIJjjgaG6dxuRJk"
"UM8nXG9Efe", "email": "shared@foo.bar", "level": "user"}
self.assertEqual(obs, exp)


if __name__ == '__main__':
main()
65 changes: 65 additions & 0 deletions qiita_db/handlers/user.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
# -----------------------------------------------------------------------------
# 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 tornado.web import HTTPError

import qiita_db as qdb
from .oauth2 import OauthBaseHandler, authenticate_oauth


def _get_user_info(email):
"""Returns the user information with the given `email` if it exists

Parameters
----------
email : str
The user email

Returns
-------
qiita_db.user.User
The requested user

Raises
------
HTTPError
If the user does not exist, with error code 404
If there is a problem instantiating, with error code 500
"""
try:
user = qdb.user.User(email)
except qdb.exceptions.QiitaDBUnknownIDError:
raise HTTPError(404)
except Exception as e:
raise HTTPError(500, reason='Error instantiating user %s: %s' %
(email, str(e)))

return user


class UserInfoDBHandler(OauthBaseHandler):
@authenticate_oauth
def get(self, email):
"""Retrieves the User information

Parameters
----------
email: str
The email of the user whose information is being retrieved

Returns
-------
dict
The user information as a dict
"""
with qdb.sql_connection.TRN:
user = _get_user_info(email)
response = {'data': {'email': email, 'level': user.level,
'password': user.password}}
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps I am missing a high-level design consideration but can you explain the motivation behind allowing consumers to query for a user's password?

Copy link
Member Author

Choose a reason for hiding this comment

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

So labman/other-tool can authenticate. Things to consider, the communication is started via keys that only live in qiita/other-tool (like plugins); qiita only stores hashed passwords, so the retrieve should be "safe". In the other tool, we will have a user add their log/pass, the other tool will hash the pass and then compare to this info.


self.write(response)
11 changes: 7 additions & 4 deletions qiita_db/test/test_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -327,10 +327,13 @@ def test_verify_code(self):
self.assertEqual(self.conn_handler.execute_fetchone(sql)[0], 2)

def _check_pass(self, user, passwd):
obspass = self.conn_handler.execute_fetchone(
"SELECT password FROM qiita.qiita_user WHERE email = %s",
(user.id, ))[0]
self.assertEqual(qdb.util.hash_password(passwd, obspass), obspass)
self.assertEqual(qdb.util.hash_password(passwd, user.password),
user.password)

def test_password(self):
user = qdb.user.User('shared@foo.bar')
self.assertEqual(user.password, '$2a$12$gnUi8Qg.0tvW243v889BhOBhWLIHy'
'IJjjgaG6dxuRJkUM8nXG9Efe')

def test_change_pass(self):
user = qdb.user.User.create('testchangepass@test.bar', 'password')
Expand Down
11 changes: 11 additions & 0 deletions qiita_db/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,17 @@ def email(self):
"""The email of the user"""
return self._id

@property
def password(self):
"""The password of the user"""
with qdb.sql_connection.TRN:
# pull password out of database
sql = "SELECT password FROM qiita.{0} WHERE email = %s".format(
self._table)
qdb.sql_connection.TRN.add(sql, [self.email])

return qdb.sql_connection.TRN.execute_fetchlast()

@property
def level(self):
"""The level of privileges of the user"""
Expand Down
4 changes: 3 additions & 1 deletion qiita_pet/support_files/doc/source/dev/rest.rst
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,11 @@ the important parts of the values are described in the Parameters column.
|PATCH | ``/qiita_db/artifacts/<artifact_id>/`` | ``op: operation``, ``path: path``, | Retrieves the artifact information | ArtifactHandler |
| | | ``value: value`` | | |
+--------+-----------------------------------------------------------------------------------+-----------------------------------------+-----------------------------------------------------+----------------------------+
|GET | ``/qiita_db/sample_information/<study_id>/data/` | | Retrieves the sample information contents | SampleInfoDBHandler |
+--------+-----------------------------------------------------------------------------------+-----------------------------------------+-----------------------------------------------------+----------------------------+
|GET | ``/qiita_db/prep_template/<prep_id>/data/`` | | Retrieves the preparation information contents | PrepTemplateDataHandler |
+--------+-----------------------------------------------------------------------------------+-----------------------------------------+-----------------------------------------------------+----------------------------+
|GET | ``/qiita_db/prep_template/<prep_id>/`` | | Retrieves the preparation template information | PrepTemplateDBHandler |
|GET | ``/qiita_db/prep_template/<prep_id>/`` | | Retrieves the preparation information | PrepTemplateDBHandler |
+--------+-----------------------------------------------------------------------------------+-----------------------------------------+-----------------------------------------------------+----------------------------+
|GET | ``/qiita_db/plugins/<plugin_name>/<version>/commands/<command_name>/activate/`` | | Activates the command | CommandActivateHandler |
+--------+-----------------------------------------------------------------------------------+-----------------------------------------+-----------------------------------------------------+----------------------------+
Expand Down
4 changes: 4 additions & 0 deletions qiita_pet/webserver.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@
ProcessingJobAPItestHandler)
from qiita_db.handlers.artifact import (
ArtifactHandler, ArtifactAPItestHandler, ArtifactTypeHandler)
from qiita_db.handlers.sample_information import SampleInfoDBHandler
from qiita_db.handlers.user import UserInfoDBHandler
from qiita_db.handlers.prep_template import (
PrepTemplateDataHandler, PrepTemplateAPItestHandler,
PrepTemplateDBHandler)
Expand Down Expand Up @@ -192,6 +194,8 @@ def __init__(self):
(r"/qiita_db/jobs/(.*)", JobHandler),
(r"/qiita_db/artifacts/types/", ArtifactTypeHandler),
(r"/qiita_db/artifacts/(.*)/", ArtifactHandler),
(r"/qiita_db/user/(.*)/data/", UserInfoDBHandler),
(r"/qiita_db/sample_information/(.*)/data/", SampleInfoDBHandler),
(r"/qiita_db/prep_template/(.*)/data/", PrepTemplateDataHandler),
(r"/qiita_db/prep_template/(.*)/", PrepTemplateDBHandler),
(r"/qiita_db/references/(.*)/", ReferenceHandler),
Expand Down