Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion api/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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/<filename:{fname}>', AnalysesHandler, h='download', m=['GET']),
]),
Expand Down
1 change: 1 addition & 0 deletions api/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
2 changes: 2 additions & 0 deletions api/handlers/collectionshandler.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -51,6 +52,7 @@ def post(self):
else:
self.abort(404, 'Element not added in collection {}'.format(self.uid))

@verify_payload_exists
def put(self, **kwargs):
_id = kwargs.pop('cid')
container = self._get_container(_id)
Expand Down
3 changes: 1 addition & 2 deletions api/handlers/containerhandler.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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
Expand Down
3 changes: 1 addition & 2 deletions api/handlers/grouphandler.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,11 @@ def get_all(self, uid=None):
results = ContainerHandler.join_user_info(results)
return results

@validators.verify_payload_exists
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')
Expand Down
29 changes: 29 additions & 0 deletions api/handlers/refererhandler.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

import os
import zipfile
import datetime
from abc import ABCMeta, abstractproperty

from .. import config
Expand Down Expand Up @@ -37,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
Expand All @@ -50,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):
Expand Down Expand Up @@ -88,6 +96,27 @@ 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')

parent = self.storage.get_parent(cont_name, cid)
permchecker = self.get_permchecker(parent)
permchecker(noop)('PUT')


payload = self.request.json_body
Copy link
Contributor

Choose a reason for hiding this comment

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

This would technically 500 if the PUT contained no request body, not just {}.

Copy link
Contributor

@nagem nagem Jul 20, 2017

Choose a reason for hiding this comment

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

We check that the payload exists everywhere, it would be nice to have a decorator such as @verify_payload_exists that just does this for us. Feel free to add one if you'd like :)

self.update_validator(payload, 'PUT')

payload['modified'] = datetime.datetime.utcnow()

result = self.storage.update_el(_id, payload)

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)
Expand Down
1 change: 1 addition & 0 deletions api/handlers/userhandler.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ def delete(self, _id):
self.abort(404, 'User {} not removed'.format(_id))
return result

@validators.verify_payload_exists
def put(self, _id):
"""Update a user"""
user = self._get_user(_id)
Expand Down
15 changes: 15 additions & 0 deletions api/validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,3 +146,18 @@ def _check_query_params(keys, query_params):
is different from expected:
{}
""".format(query_params.keys(), keys)

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
7 changes: 7 additions & 0 deletions raml/schemas/definitions/analysis.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,13 @@
},
"additionalProperties": false
},
"analysis-update":{
"type":"object",
"properties":{
"label":{"$ref":"#/definitions/label"}
},
"additionalProperties":false
},
"analysis-output":{
"type":"object",
"properties":{
Expand Down
6 changes: 6 additions & 0 deletions raml/schemas/input/analysis-update.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"$schema": "http://json-schema.org/draft-04/schema#",
"title": "Analysis",
"type": "object",
"allOf":[{"$ref":"../definitions/analysis.json#/definitions/analysis-update"}]
}
32 changes: 32 additions & 0 deletions test/integration_tests/python/test_containers.py
Original file line number Diff line number Diff line change
Expand Up @@ -495,3 +495,35 @@ 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'

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