From afd292631632505c5ce713a359343fe7a932f3bc Mon Sep 17 00:00:00 2001 From: Harsha Kethineni Date: Tue, 18 Jul 2017 16:39:44 -0500 Subject: [PATCH 1/4] analysis put implemented --- api/api.py | 2 +- api/config.py | 1 + api/handlers/refererhandler.py | 26 +++++++++++++++++++ raml/schemas/input/analysis-update.json | 6 +++++ .../python/test_containers.py | 22 ++++++++++++++++ 5 files changed, 56 insertions(+), 1 deletion(-) create mode 100644 raml/schemas/input/analysis-update.json diff --git a/api/api.py b/api/api.py index 85412dbe1..bddf373c4 100644 --- a/api/api.py +++ b/api/api.py @@ -247,7 +247,7 @@ def prefix(path, routes): route( '/analyses', AnalysesHandler, m=['POST']), prefix('/analyses', [ - route('/<_id:{cid}>', AnalysesHandler, m=['GET', 'DELETE']), + route('/<_id:{cid}>', AnalysesHandler, m=['GET', 'PUT', 'DELETE']), route('/<_id:{cid}>/files', AnalysesHandler, h='download', m=['GET']), route('/<_id:{cid}>/files/', AnalysesHandler, h='download', m=['GET']), ]), diff --git a/api/config.py b/api/config.py index 4ce82ab5a..b991f58e0 100644 --- a/api/config.py +++ b/api/config.py @@ -152,6 +152,7 @@ def apply_env_variables(config): 'acquisition-update.json', 'analysis.json', 'analysis-job.json', + 'analysis-update.json', 'avatars.json', 'collection.json', 'collection-update.json', diff --git a/api/handlers/refererhandler.py b/api/handlers/refererhandler.py index 8cdac06e1..127ae4850 100644 --- a/api/handlers/refererhandler.py +++ b/api/handlers/refererhandler.py @@ -8,6 +8,7 @@ import os import zipfile +import datetime from abc import ABCMeta, abstractproperty from .. import config @@ -88,6 +89,31 @@ def post(self, cont_name, cid): else: self.abort(500, 'Analysis not added for container {} {}'.format(cont_name, cid)) + def put(self, cont_name, **kwargs): + cid = kwargs.pop('cid') + _id = kwargs.pop('_id') + + parent = self.storage.get_parent(cont_name, cid) + permchecker = self.get_permchecker(parent) + permchecker(noop)('PUT') + + + payload = self.request.json_body + if not payload: + self.abort(400, 'PUT request body cannot be empty') + self.input_validator(payload, 'PUT') + + payload['modified'] = datetime.datetime.utcnow() + + try: + result = self.storage.update_el(_id, payload) + except APIStorageException as e: + self.abort(400, e.message) + + if result.modified_count == 1: + return {'modified': result.modified_count} + else: + self.abort(404, 'Element not updated in container {} {}'.format(self.storage.cont_name, _id)) def get(self, cont_name, cid, _id): parent = self.storage.get_parent(cont_name, cid) diff --git a/raml/schemas/input/analysis-update.json b/raml/schemas/input/analysis-update.json new file mode 100644 index 000000000..73fbc4cb3 --- /dev/null +++ b/raml/schemas/input/analysis-update.json @@ -0,0 +1,6 @@ +{ + "$schema": "http://json-schema.org/draft-04/schema#", + "title": "Analysis", + "type": "object", + "allOf":[{"$ref":"../definitions/analysis.json#/definitions/analysis-input"}] +} \ No newline at end of file diff --git a/test/integration_tests/python/test_containers.py b/test/integration_tests/python/test_containers.py index b6fca9c4e..6c21a59c7 100644 --- a/test/integration_tests/python/test_containers.py +++ b/test/integration_tests/python/test_containers.py @@ -495,3 +495,25 @@ def test_subject_no_name_hashes(data_builder, as_admin): } }) assert r.status_code == 400 + +def test_analysis_put(data_builder, as_admin): + project = data_builder.create_project() + session = data_builder.create_session() + gear = data_builder.create_gear() + r = as_admin.post('/sessions/' + session + '/analyses', params={'job': 'true'}, json={ + 'analysis': {'label': 'with-job'}, + 'job': { + 'gear_id': gear, + 'inputs': { + 'csv': {'type': 'project', 'id': project, 'name': 'job_1.csv'} + } + } + }) + + assert r.ok + analysis = r.json()['_id'] + r = as_admin.put('/sessions/'+session + '/analyses/' + analysis, json={'label': 'ayo'}) + assert r.ok + r = as_admin.get('/sessions/'+session + '/analyses/' + analysis) + assert r.ok + assert r.json()['label'] == 'ayo' \ No newline at end of file From e8d4ad3d94afa6b6b0a4d2d09856c735868bd10a Mon Sep 17 00:00:00 2001 From: Harsha Kethineni Date: Wed, 19 Jul 2017 09:34:31 -0500 Subject: [PATCH 2/4] Can only change labels of analyses --- api/handlers/refererhandler.py | 9 ++++++++- raml/schemas/definitions/analysis.json | 7 +++++++ raml/schemas/input/analysis-update.json | 2 +- test/integration_tests/python/test_containers.py | 5 ++++- 4 files changed, 20 insertions(+), 3 deletions(-) diff --git a/api/handlers/refererhandler.py b/api/handlers/refererhandler.py index 127ae4850..b1bd32df7 100644 --- a/api/handlers/refererhandler.py +++ b/api/handlers/refererhandler.py @@ -38,6 +38,12 @@ def input_validator(self): input_validator = validators.from_schema_path(input_schema_uri) return input_validator + @property + def update_validator(self): + update_schema_uri = validators.schema_uri('input', self.update_payload_schema_file) + update_validator = validators.from_schema_path(update_schema_uri) + return update_validator + def get_permchecker(self, parent_container): if self.superuser_request: return always_ok @@ -51,6 +57,7 @@ def get_permchecker(self, parent_container): class AnalysesHandler(RefererHandler): storage = containerstorage.AnalysisStorage() payload_schema_file = 'analysis.json' + update_payload_schema_file = 'analysis-update.json' def post(self, cont_name, cid): @@ -101,7 +108,7 @@ def put(self, cont_name, **kwargs): payload = self.request.json_body if not payload: self.abort(400, 'PUT request body cannot be empty') - self.input_validator(payload, 'PUT') + self.update_validator(payload, 'PUT') payload['modified'] = datetime.datetime.utcnow() diff --git a/raml/schemas/definitions/analysis.json b/raml/schemas/definitions/analysis.json index 729e815fb..1a8da4353 100644 --- a/raml/schemas/definitions/analysis.json +++ b/raml/schemas/definitions/analysis.json @@ -31,6 +31,13 @@ }, "additionalProperties": false }, + "analysis-update":{ + "type":"object", + "properties":{ + "label":{"$ref":"#/definitions/label"} + }, + "additionalProperties":false + }, "analysis-output":{ "type":"object", "properties":{ diff --git a/raml/schemas/input/analysis-update.json b/raml/schemas/input/analysis-update.json index 73fbc4cb3..b0a5eec24 100644 --- a/raml/schemas/input/analysis-update.json +++ b/raml/schemas/input/analysis-update.json @@ -2,5 +2,5 @@ "$schema": "http://json-schema.org/draft-04/schema#", "title": "Analysis", "type": "object", - "allOf":[{"$ref":"../definitions/analysis.json#/definitions/analysis-input"}] + "allOf":[{"$ref":"../definitions/analysis.json#/definitions/analysis-update"}] } \ No newline at end of file diff --git a/test/integration_tests/python/test_containers.py b/test/integration_tests/python/test_containers.py index 6c21a59c7..3f14de218 100644 --- a/test/integration_tests/python/test_containers.py +++ b/test/integration_tests/python/test_containers.py @@ -516,4 +516,7 @@ def test_analysis_put(data_builder, as_admin): assert r.ok r = as_admin.get('/sessions/'+session + '/analyses/' + analysis) assert r.ok - assert r.json()['label'] == 'ayo' \ No newline at end of file + assert r.json()['label'] == 'ayo' + + r = as_admin.put('/sessions/'+session + '/analyses/' + analysis, json={'input': 'ayo'}) + assert r.status_code == 400 From 6acb7380d36cff2603bd65a2a121f51b8723ca6f Mon Sep 17 00:00:00 2001 From: Harsha Kethineni Date: Thu, 20 Jul 2017 16:59:02 -0500 Subject: [PATCH 3/4] payload exists decorator in validators.py --- api/handlers/collectionshandler.py | 2 ++ api/handlers/containerhandler.py | 3 +- api/handlers/grouphandler.py | 3 +- api/handlers/refererhandler.py | 8 ++---- api/handlers/userhandler.py | 1 + api/validators.py | 28 +++++++++++++++++++ .../python/test_containers.py | 7 +++++ 7 files changed, 42 insertions(+), 10 deletions(-) diff --git a/api/handlers/collectionshandler.py b/api/handlers/collectionshandler.py index 000fc2efc..06e5fd069 100644 --- a/api/handlers/collectionshandler.py +++ b/api/handlers/collectionshandler.py @@ -5,6 +5,7 @@ from ..auth import containerauth, always_ok from ..dao import containerstorage, containerutil from ..dao import APIStorageException +from ..validators import verify_payload_exists from .containerhandler import ContainerHandler @@ -51,6 +52,7 @@ def post(self): else: self.abort(404, 'Element not added in collection {}'.format(self.uid)) + @verify_payload_exists('collections') def put(self, **kwargs): _id = kwargs.pop('cid') container = self._get_container(_id) diff --git a/api/handlers/containerhandler.py b/api/handlers/containerhandler.py index ae6ab4874..5cd00f08d 100644 --- a/api/handlers/containerhandler.py +++ b/api/handlers/containerhandler.py @@ -430,6 +430,7 @@ def post(self, cont_name): else: self.abort(404, 'Element not added in container {}'.format(self.storage.cont_name)) + @validators.verify_payload_exists() def put(self, cont_name, **kwargs): _id = kwargs.pop('cid') self.config = self.container_handler_configurations[cont_name] @@ -438,8 +439,6 @@ def put(self, cont_name, **kwargs): mongo_validator, payload_validator = self._get_validators() payload = self.request.json_body - if not payload: - self.abort(400, 'PUT request body cannot be empty') payload_validator(payload, 'PUT') # Check if any payload keys are any propogated property, add to r_payload diff --git a/api/handlers/grouphandler.py b/api/handlers/grouphandler.py index ba695b872..b6221072c 100644 --- a/api/handlers/grouphandler.py +++ b/api/handlers/grouphandler.py @@ -46,12 +46,11 @@ def get_all(self, uid=None): results = ContainerHandler.join_user_info(results) return results + @validators.verify_payload_exists('groups') def put(self, _id): group = self._get_group(_id) permchecker = groupauth.default(self, group) payload = self.request.json_body - if not payload: - self.abort(400, 'PUT request body cannot be empty') mongo_schema_uri = validators.schema_uri('mongo', 'group.json') mongo_validator = validators.decorator_from_schema_path(mongo_schema_uri) payload_schema_uri = validators.schema_uri('input', 'group-update.json') diff --git a/api/handlers/refererhandler.py b/api/handlers/refererhandler.py index b1bd32df7..5fa0d0922 100644 --- a/api/handlers/refererhandler.py +++ b/api/handlers/refererhandler.py @@ -96,6 +96,7 @@ def post(self, cont_name, cid): else: self.abort(500, 'Analysis not added for container {} {}'.format(cont_name, cid)) + @validators.verify_payload_exists() def put(self, cont_name, **kwargs): cid = kwargs.pop('cid') _id = kwargs.pop('_id') @@ -106,16 +107,11 @@ def put(self, cont_name, **kwargs): payload = self.request.json_body - if not payload: - self.abort(400, 'PUT request body cannot be empty') self.update_validator(payload, 'PUT') payload['modified'] = datetime.datetime.utcnow() - try: - result = self.storage.update_el(_id, payload) - except APIStorageException as e: - self.abort(400, e.message) + result = self.storage.update_el(_id, payload) if result.modified_count == 1: return {'modified': result.modified_count} diff --git a/api/handlers/userhandler.py b/api/handlers/userhandler.py index bf55b8849..a4a426847 100644 --- a/api/handlers/userhandler.py +++ b/api/handlers/userhandler.py @@ -64,6 +64,7 @@ def delete(self, _id): self.abort(404, 'User {} not removed'.format(_id)) return result + @validators.verify_payload_exists('users') def put(self, _id): """Update a user""" user = self._get_user(_id) diff --git a/api/validators.py b/api/validators.py index a0ddbb6a9..7df012f0a 100644 --- a/api/validators.py +++ b/api/validators.py @@ -146,3 +146,31 @@ def _check_query_params(keys, query_params): is different from expected: {} """.format(query_params.keys(), keys) +def verify_payload_exists(args=False): + def verify_payload_dec(handler_method): + if args in ['users', 'groups']: + def payload_checker(self, _id): + try: + if not self.request.json_body: + raise InputValidationException("Empty Payload") + except ValueError: + raise InputValidationException("Empty Payload") + return handler_method(self, _id) + elif args in ['collections']: + def payload_checker(self, **kwargs): + try: + if not self.request.json_body: + raise InputValidationException("Empty Payload") + except ValueError: + raise InputValidationException("Empty Payload") + return handler_method(self, **kwargs) + else: + def payload_checker(self, cont_name, **kwargs): + try: + if not self.request.json_body: + raise InputValidationException("Empty Payload") + except ValueError: + raise InputValidationException("Empty Payload") + return handler_method(self, cont_name, **kwargs) + return payload_checker + return verify_payload_dec diff --git a/test/integration_tests/python/test_containers.py b/test/integration_tests/python/test_containers.py index 3f14de218..22645f475 100644 --- a/test/integration_tests/python/test_containers.py +++ b/test/integration_tests/python/test_containers.py @@ -520,3 +520,10 @@ def test_analysis_put(data_builder, as_admin): r = as_admin.put('/sessions/'+session + '/analyses/' + analysis, json={'input': 'ayo'}) assert r.status_code == 400 + + r = as_admin.put('/sessions/'+session + '/analyses/' + analysis, json={}) + assert r.status_code == 400 + + r = as_admin.put('/sessions/'+session + '/analyses/' + analysis) + assert r.status_code == 400 + From c54e3fc8e2c7e48eb5f6dfa70390c14d12812ce2 Mon Sep 17 00:00:00 2001 From: Megan Henning Date: Wed, 26 Jul 2017 12:10:43 -0500 Subject: [PATCH 4/4] Simplify payload validator decorator --- api/handlers/collectionshandler.py | 2 +- api/handlers/containerhandler.py | 2 +- api/handlers/grouphandler.py | 2 +- api/handlers/refererhandler.py | 4 +-- api/handlers/userhandler.py | 2 +- api/validators.py | 41 ++++++++++-------------------- 6 files changed, 20 insertions(+), 33 deletions(-) diff --git a/api/handlers/collectionshandler.py b/api/handlers/collectionshandler.py index 06e5fd069..68577a186 100644 --- a/api/handlers/collectionshandler.py +++ b/api/handlers/collectionshandler.py @@ -52,7 +52,7 @@ def post(self): else: self.abort(404, 'Element not added in collection {}'.format(self.uid)) - @verify_payload_exists('collections') + @verify_payload_exists def put(self, **kwargs): _id = kwargs.pop('cid') container = self._get_container(_id) diff --git a/api/handlers/containerhandler.py b/api/handlers/containerhandler.py index 5cd00f08d..ff57e24d3 100644 --- a/api/handlers/containerhandler.py +++ b/api/handlers/containerhandler.py @@ -430,7 +430,7 @@ def post(self, cont_name): else: self.abort(404, 'Element not added in container {}'.format(self.storage.cont_name)) - @validators.verify_payload_exists() + @validators.verify_payload_exists def put(self, cont_name, **kwargs): _id = kwargs.pop('cid') self.config = self.container_handler_configurations[cont_name] diff --git a/api/handlers/grouphandler.py b/api/handlers/grouphandler.py index b6221072c..b908b7204 100644 --- a/api/handlers/grouphandler.py +++ b/api/handlers/grouphandler.py @@ -46,7 +46,7 @@ def get_all(self, uid=None): results = ContainerHandler.join_user_info(results) return results - @validators.verify_payload_exists('groups') + @validators.verify_payload_exists def put(self, _id): group = self._get_group(_id) permchecker = groupauth.default(self, group) diff --git a/api/handlers/refererhandler.py b/api/handlers/refererhandler.py index 5fa0d0922..41056c169 100644 --- a/api/handlers/refererhandler.py +++ b/api/handlers/refererhandler.py @@ -96,7 +96,7 @@ def post(self, cont_name, cid): else: self.abort(500, 'Analysis not added for container {} {}'.format(cont_name, cid)) - @validators.verify_payload_exists() + @validators.verify_payload_exists def put(self, cont_name, **kwargs): cid = kwargs.pop('cid') _id = kwargs.pop('_id') @@ -110,7 +110,7 @@ def put(self, cont_name, **kwargs): self.update_validator(payload, 'PUT') payload['modified'] = datetime.datetime.utcnow() - + result = self.storage.update_el(_id, payload) if result.modified_count == 1: diff --git a/api/handlers/userhandler.py b/api/handlers/userhandler.py index a4a426847..7269b8217 100644 --- a/api/handlers/userhandler.py +++ b/api/handlers/userhandler.py @@ -64,7 +64,7 @@ def delete(self, _id): self.abort(404, 'User {} not removed'.format(_id)) return result - @validators.verify_payload_exists('users') + @validators.verify_payload_exists def put(self, _id): """Update a user""" user = self._get_user(_id) diff --git a/api/validators.py b/api/validators.py index 7df012f0a..b2d804900 100644 --- a/api/validators.py +++ b/api/validators.py @@ -146,31 +146,18 @@ def _check_query_params(keys, query_params): is different from expected: {} """.format(query_params.keys(), keys) -def verify_payload_exists(args=False): - def verify_payload_dec(handler_method): - if args in ['users', 'groups']: - def payload_checker(self, _id): - try: - if not self.request.json_body: - raise InputValidationException("Empty Payload") - except ValueError: - raise InputValidationException("Empty Payload") - return handler_method(self, _id) - elif args in ['collections']: - def payload_checker(self, **kwargs): - try: - if not self.request.json_body: - raise InputValidationException("Empty Payload") - except ValueError: - raise InputValidationException("Empty Payload") - return handler_method(self, **kwargs) - else: - def payload_checker(self, cont_name, **kwargs): - try: - if not self.request.json_body: - raise InputValidationException("Empty Payload") - except ValueError: - raise InputValidationException("Empty Payload") - return handler_method(self, cont_name, **kwargs) - return payload_checker + +def verify_payload_exists(handler_method): + """ + A decorator to ensure the json payload exists and is not empty + + Useful for POST and PUT handler operations + """ + def verify_payload_dec(self, *args, **kwargs): + try: + if not self.request.json_body: + raise InputValidationException("Empty Payload") + except ValueError: + raise InputValidationException("Empty Payload") + return handler_method(self, *args, **kwargs) return verify_payload_dec