From b740a77efe4db3f6e9f81d17a256ce1ac12b5cd8 Mon Sep 17 00:00:00 2001 From: Dan Kolbman Date: Thu, 8 Mar 2018 15:23:40 -0500 Subject: [PATCH 1/2] :sparkles: Add study resource --- dataservice/api/__init__.py | 2 + dataservice/api/study/__init__.py | 2 + dataservice/api/study/resources.py | 134 +++++++++++++++++++++++++++++ dataservice/api/study/schemas.py | 16 ++++ 4 files changed, 154 insertions(+) create mode 100644 dataservice/api/study/__init__.py create mode 100644 dataservice/api/study/resources.py create mode 100644 dataservice/api/study/schemas.py diff --git a/dataservice/api/__init__.py b/dataservice/api/__init__.py index a67ee385c..0dad80e74 100644 --- a/dataservice/api/__init__.py +++ b/dataservice/api/__init__.py @@ -9,6 +9,8 @@ from dataservice.api.diagnosis import DiagnosisListAPI from dataservice.api.sample import SampleAPI from dataservice.api.sample import SampleListAPI +from dataservice.api.study import StudyAPI +from dataservice.api.study import StudyListAPI from dataservice.api.demographic import DemographicAPI from dataservice.api.demographic import DemographicListAPI diff --git a/dataservice/api/study/__init__.py b/dataservice/api/study/__init__.py new file mode 100644 index 000000000..2c0781aa6 --- /dev/null +++ b/dataservice/api/study/__init__.py @@ -0,0 +1,2 @@ +from dataservice.api.study.resources import StudyAPI +from dataservice.api.study.resources import StudyListAPI diff --git a/dataservice/api/study/resources.py b/dataservice/api/study/resources.py new file mode 100644 index 000000000..7e27d878d --- /dev/null +++ b/dataservice/api/study/resources.py @@ -0,0 +1,134 @@ +from flask import abort, request +from marshmallow import ValidationError + +from dataservice.extensions import db +from dataservice.api.common.pagination import paginated, Pagination +from dataservice.api.study.models import Study +from dataservice.api.study.schemas import StudySchema +from dataservice.api.common.views import CRUDView + + +class StudyListAPI(CRUDView): + """ + Study API + """ + endpoint = 'studies_list' + rule = '/studies' + schemas = {'Study': StudySchema} + + @paginated + def get(self, after, limit): + """ + Get a paginated studies + --- + template: + path: + get_list.yml + properties: + resource: + Study + """ + q = Study.query + + return (StudySchema(many=True) + .jsonify(Pagination(q, after, limit))) + + def post(self): + """ + Create a new study + --- + template: + path: + new_resource.yml + properties: + resource: + Study + """ + try: + st = StudySchema(strict=True).load(request.json).data + except ValidationError as err: + abort(400, 'could not create study: {}'.format(err.messages)) + + db.session.add(st) + db.session.commit() + return StudySchema( + 201, 'study {} created'.format(st.kf_id) + ).jsonify(st), 201 + + +class StudyAPI(CRUDView): + """ + Study API + """ + endpoint = 'studies' + rule = '/studies/' + schemas = {'Study': StudySchema} + + def get(self, kf_id): + """ + Get a study by id + --- + template: + path: + get_by_id.yml + properties: + resource: + Study + """ + st = Study.query.get(kf_id) + if st is None: + abort(404, 'could not find {} `{}`' + .format('study', kf_id)) + return StudySchema().jsonify(st) + + def patch(self, kf_id): + """ + Update an existing study. Allows partial update of resource + --- + template: + path: + update_by_id.yml + properties: + resource: + Study + """ + body = request.json + st = Study.query.get(kf_id) + if st is None: + abort(404, 'could not find {} `{}`' + .format('study', kf_id)) + + try: + st = (StudySchema(strict=True).load(body, instance=st, + partial=True).data) + except ValidationError as err: + abort(400, 'could not update study: {}'.format(err.messages)) + + db.session.add(st) + db.session.commit() + + return StudySchema( + 200, 'study {} updated'.format(st.kf_id) + ).jsonify(st), 200 + + def delete(self, kf_id): + """ + Delete study by id + --- + template: + path: + delete_by_id.yml + properties: + resource: + Study + """ + st = Study.query.get(kf_id) + if st is None: + abort(404, 'could not find {} `{}`'.format('study', kf_id)) + + db.session.delete(st) + db.session.commit() + + return StudySchema( + 200, 'study {} deleted'.format(st.kf_id) + ).jsonify(st), 200 diff --git a/dataservice/api/study/schemas.py b/dataservice/api/study/schemas.py new file mode 100644 index 000000000..f652cdd1b --- /dev/null +++ b/dataservice/api/study/schemas.py @@ -0,0 +1,16 @@ +from dataservice.api.study.models import Study +from dataservice.api.common.schemas import BaseSchema +from dataservice.extensions import ma + + +class StudySchema(BaseSchema): + + class Meta(BaseSchema.Meta): + model = Study + resource_url = 'api.studies' + collection_url = 'api.studies_list' + + _links = ma.Hyperlinks({ + 'self': ma.URLFor(Meta.resource_url, kf_id=''), + 'collection': ma.URLFor(Meta.collection_url) + }) From d458a98cda38ebb7b5e42476853c82f12d389037 Mon Sep 17 00:00:00 2001 From: Dan Kolbman Date: Thu, 8 Mar 2018 15:34:14 -0500 Subject: [PATCH 2/2] :white_check_mark: Add tests for study resource --- tests/conftest.py | 1 + tests/study/test_study_resources.py | 147 ++++++++++++++++++++++++++++ tests/test_api.py | 20 +++- tests/test_pagination.py | 12 +++ 4 files changed, 175 insertions(+), 5 deletions(-) create mode 100644 tests/study/test_study_resources.py diff --git a/tests/conftest.py b/tests/conftest.py index b729fc995..a5bc426eb 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -101,6 +101,7 @@ def entities(client): # Add kf_ids inputs['kf_ids'] = {'/participants': p.kf_id} + inputs['kf_ids'].update({'/studies': study.kf_id}) inputs['kf_ids'].update({'/demographics': p.demographic.kf_id}) inputs['kf_ids'].update({'/diagnoses': diagnosis.kf_id}) inputs['kf_ids'].update({'/samples': sample.kf_id}) diff --git a/tests/study/test_study_resources.py b/tests/study/test_study_resources.py new file mode 100644 index 000000000..cff02a06c --- /dev/null +++ b/tests/study/test_study_resources.py @@ -0,0 +1,147 @@ +import json + +from flask import url_for + +from dataservice.api.study.models import Study +from tests.utils import FlaskTestCase + +STUDY_URL = 'api.studies' +STUDY_LIST_URL = 'api.studies_list' + + +class StudyTest(FlaskTestCase): + ''' + Test study api endopoints + ''' + + def test_post_study(self): + ''' + Test creating a new study + ''' + response = self._make_study(external_id='TEST') + resp = json.loads(response.data.decode('utf-8')) + + self.assertEqual(response.status_code, 201) + + self.assertIn('study', resp['_status']['message']) + self.assertIn('created', resp['_status']['message']) + self.assertNotIn('_id', resp['results']) + + s = Study.query.first() + study = resp['results'] + self.assertEqual(s.kf_id, study['kf_id']) + self.assertEqual(s.external_id, study['external_id']) + + def test_get_study(self): + ''' + Test retrieving a study by id + ''' + resp = self._make_study('TEST') + resp = json.loads(resp.data.decode('utf-8')) + kf_id = resp['results']['kf_id'] + + response = self.client.get(url_for(STUDY_URL, + kf_id=kf_id), + headers=self._api_headers()) + resp = json.loads(response.data.decode('utf-8')) + self.assertEqual(response.status_code, 200) + + study = resp['results'] + self.assertEqual(kf_id, study['kf_id']) + + def test_patch_study(self): + ''' + Test updating an existing study + ''' + response = self._make_study(external_id='TEST') + resp = json.loads(response.data.decode('utf-8')) + study = resp['results'] + kf_id = study.get('kf_id') + external_id = study.get('external_id') + + # Update the study via http api + body = { + 'external_id': 'new_id' + } + response = self.client.patch(url_for(STUDY_URL, + kf_id=kf_id), + headers=self._api_headers(), + data=json.dumps(body)) + self.assertEqual(response.status_code, 200) + + self.assertEqual(Study.query.get(kf_id).external_id, + body['external_id']) + + resp = json.loads(response.data.decode('utf-8')) + self.assertIn('study', resp['_status']['message']) + self.assertIn('updated', resp['_status']['message']) + + study = resp['results'] + self.assertEqual(study['kf_id'], kf_id) + self.assertEqual(study['external_id'], body['external_id']) + + def test_patch_study_no_required_field(self): + ''' + Test that we may update the study without a required field + ''' + response = self._make_study(external_id='TEST') + resp = json.loads(response.data.decode('utf-8')) + study = resp['results'] + kf_id = study.get('kf_id') + external_id = study.get('external_id') + + # Update the study via http api + body = { + 'version': '2.0' + } + response = self.client.patch(url_for(STUDY_URL, + kf_id=kf_id), + headers=self._api_headers(), + data=json.dumps(body)) + self.assertEqual(response.status_code, 200) + + self.assertEqual(Study.query.get(kf_id).version, '2.0') + + resp = json.loads(response.data.decode('utf-8')) + self.assertIn('study', resp['_status']['message']) + self.assertIn('updated', resp['_status']['message']) + + study = resp['results'] + self.assertEqual(study['kf_id'], kf_id) + self.assertEqual(study['external_id'], external_id) + self.assertEqual(study['version'], body['version']) + + def test_delete_study(self): + ''' + Test deleting a study by id + ''' + resp = self._make_study('TEST') + resp = json.loads(resp.data.decode('utf-8')) + kf_id = resp['results']['kf_id'] + + response = self.client.delete(url_for(STUDY_URL, + kf_id=kf_id), + headers=self._api_headers()) + + resp = json.loads(response.data.decode('utf-8')) + self.assertEqual(response.status_code, 200) + + response = self.client.get(url_for(STUDY_URL, + kf_id=kf_id), + headers=self._api_headers()) + + resp = json.loads(response.data.decode('utf-8')) + self.assertEqual(response.status_code, 404) + + def _make_study(self, external_id='TEST-0001'): + ''' + Convenience method to create a study with a given source name + ''' + body = { + 'external_id': external_id, + 'version': '1.0' + } + response = self.client.post(url_for(STUDY_LIST_URL), + headers=self._api_headers(), + data=json.dumps(body)) + return response diff --git a/tests/test_api.py b/tests/test_api.py index 1a49158fe..adb92ed15 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -19,7 +19,9 @@ class TestAPI: ('/demographics/123', 'GET', 404), ('/participants', 'GET', 200), ('/persons', 'GET', 404), - ('/participants/123', 'GET', 404) + ('/participants/123', 'GET', 404), + ('/studies', 'GET', 200), + ('/studies/123', 'GET', 404) ]) def test_status_codes(self, client, endpoint, method, status_code): """ Test endpoint response codes """ @@ -47,7 +49,11 @@ def test_status_codes(self, client, endpoint, method, status_code): ('/participants', 'GET', 'success'), ('/participants/123', 'GET', 'could not find participant `123`'), ('/participants/123', 'PATCH', 'could not find participant `123`'), - ('/participants/123', 'DELETE', 'could not find participant `123`') + ('/participants/123', 'DELETE', 'could not find participant `123`'), + ('/studies', 'GET', 'success'), + ('/studies/123', 'GET', 'could not find study `123`'), + ('/studies/123', 'PATCH', 'could not find study `123`'), + ('/studies/123', 'DELETE', 'could not find study `123`'), ]) def test_status_messages(self, client, endpoint, method, status_message): """ @@ -63,7 +69,8 @@ def test_status_messages(self, client, endpoint, method, status_message): ('/participants', 'GET'), ('/samples', 'GET'), ('/diagnoses', 'GET'), - ('/demographics', 'GET') + ('/demographics', 'GET'), + ('/studies', 'GET') ]) def test_status_format(self, client, endpoint, method): """ Test that the _response field is consistent """ @@ -80,7 +87,9 @@ def test_status_format(self, client, endpoint, method): ('/participants', 'PATCH', ['created_at', 'modified_at']), ('/demographics', 'PATCH', ['created_at', 'modified_at']), ('/diagnoses', 'PATCH', ['created_at', 'modified_at']), - ('/samples', 'PATCH', ['created_at', 'modified_at']) + ('/samples', 'PATCH', ['created_at', 'modified_at']), + ('/studies', 'PATCH', ['created_at', 'modified_at']), + ('/studies', 'PATCH', ['created_at', 'modified_at']) ]) def test_read_only(self, client, entities, endpoint, method, fields): """ Test that given fields can not be written or modified """ @@ -103,7 +112,8 @@ def test_read_only(self, client, entities, endpoint, method, fields): @pytest.mark.parametrize('endpoint', ['/participants', '/demographics', '/diagnoses', - '/samples']) + '/samples', + '/studies']) def test_unknown_field(self, client, entities, endpoint, method): """ Test that unknown fields are rejected when trying to create """ inputs = entities[endpoint] diff --git a/tests/test_pagination.py b/tests/test_pagination.py index 57ca699b3..238b9e3d2 100644 --- a/tests/test_pagination.py +++ b/tests/test_pagination.py @@ -3,6 +3,7 @@ from dateutil import parser from dataservice.extensions import db +from dataservice.api.study.models import Study from dataservice.api.participant.models import Participant from dataservice.api.demographic.models import Demographic from dataservice.api.sample.models import Sample @@ -17,6 +18,13 @@ class TestPagination: @pytest.fixture(scope='module') def participants(client): + + # Add a bunch of studies for pagination + for _ in range(101): + s = Study(external_id='blah') + db.session.add(s) + db.session.commit() + s = Study(external_id='blah', name='test') db.session.add(s) db.session.flush() @@ -38,6 +46,7 @@ def participants(client): ('/demographics'), ('/samples'), ('/diagnoses'), + ('/studies'), ]) def test_pagination(self, client, participants, endpoint): """ Test pagination of resource """ @@ -69,6 +78,7 @@ def test_pagination(self, client, participants, endpoint): ('/demographics'), ('/samples'), ('/diagnoses'), + ('/studies'), ]) def test_limit(self, client, participants, endpoint): # Check that limit param operates correctly @@ -92,6 +102,7 @@ def test_limit(self, client, participants, endpoint): ('/demographics'), ('/samples'), ('/diagnoses'), + ('/studies'), ]) def test_after(self, client, participants, endpoint): """ Test `after` offeset paramater """ @@ -120,6 +131,7 @@ def test_after(self, client, participants, endpoint): ('/demographics'), ('/samples'), ('/diagnoses'), + ('/studies'), ]) def test_self(self, client, participants, endpoint): """ Test that the self link gives the same page """