From b92045a8a4a4afecf9c51fc057fa1af740afa5ab Mon Sep 17 00:00:00 2001 From: William Moore Date: Wed, 25 Jan 2017 21:46:23 +0000 Subject: [PATCH 01/19] Basic whitelist for allowing 'save' objects --- .../tools/OmeroWeb/omeroweb/api/views.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/components/tools/OmeroWeb/omeroweb/api/views.py b/components/tools/OmeroWeb/omeroweb/api/views.py index b1b8bcae317..09404fd0ae0 100644 --- a/components/tools/OmeroWeb/omeroweb/api/views.py +++ b/components/tools/OmeroWeb/omeroweb/api/views.py @@ -392,12 +392,27 @@ class SaveView(View): POST to create a new Object and PUT to replace existing one. """ + can_put = ['Project', 'Dataset', 'Screen'] + + can_post = [] + @method_decorator(login_required(useragent='OMERO.webapi')) @method_decorator(json_response()) def dispatch(self, *args, **kwargs): """Apply decorators for class methods below.""" return super(SaveView, self).dispatch(*args, **kwargs) + def get_type_name(self, marshalled): + """Get the '@type' name from marshalled data.""" + if '@type' not in marshalled: + return None + schema_type = marshalled['@type'] + # NB: Do we support saving from old SCHEMA to newer one? + if '#' not in schema_type: + return None + return schema_type.split('#')[1] + + def put(self, request, conn=None, **kwargs): """ PUT handles saving of existing objects. @@ -417,6 +432,9 @@ def post(self, request, conn=None, **kwargs): Therefore '@id' should not be set. """ object_json = json.loads(request.body) + obj_type = self.get_type_name(object_json) + if obj_type not in self.can_post: + raise BadRequestError("Creation of %s not supported" % obj_type) if '@id' in object_json: raise BadRequestError( "Object has '@id' attribute. Use PUT to update objects") From 39618ef92f53e2e21f65e7c5e877b9dfbdf1babc Mon Sep 17 00:00:00 2001 From: William Moore Date: Thu, 26 Jan 2017 09:55:37 +0000 Subject: [PATCH 02/19] SaveView restricts PUT to can_put whitelist --- components/tools/OmeroWeb/omeroweb/api/views.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/components/tools/OmeroWeb/omeroweb/api/views.py b/components/tools/OmeroWeb/omeroweb/api/views.py index 09404fd0ae0..781dca231f5 100644 --- a/components/tools/OmeroWeb/omeroweb/api/views.py +++ b/components/tools/OmeroWeb/omeroweb/api/views.py @@ -405,7 +405,7 @@ def dispatch(self, *args, **kwargs): def get_type_name(self, marshalled): """Get the '@type' name from marshalled data.""" if '@type' not in marshalled: - return None + raise BadRequestError('Need to specify @type attribute') schema_type = marshalled['@type'] # NB: Do we support saving from old SCHEMA to newer one? if '#' not in schema_type: @@ -420,6 +420,9 @@ def put(self, request, conn=None, **kwargs): Therefore '@id' should be set. """ object_json = json.loads(request.body) + obj_type = self.get_type_name(object_json) + if obj_type not in self.can_put: + raise BadRequestError("Update of %s not supported" % obj_type) if '@id' not in object_json: raise BadRequestError( "No '@id' attribute. Use POST to create new objects") @@ -447,8 +450,6 @@ def _save_object(self, request, conn, object_json, **kwargs): # Try to get group from request, OR from details below... group = getIntOrDefault(request, 'group', None) decoder = None - if '@type' not in object_json: - raise BadRequestError('Need to specify @type attribute') objType = object_json['@type'] decoder = get_decoder(objType) # If we are passed incomplete object, or decoder couldn't be found... From c1b626a2c5f2998ef986fd9b09a71e7b1233791a Mon Sep 17 00:00:00 2001 From: William Moore Date: Thu, 26 Jan 2017 11:11:00 +0000 Subject: [PATCH 03/19] Uppercase CAN_PUT, CAN_POST. Both allow P/D/S --- components/tools/OmeroWeb/omeroweb/api/views.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/components/tools/OmeroWeb/omeroweb/api/views.py b/components/tools/OmeroWeb/omeroweb/api/views.py index 781dca231f5..5fac8a04239 100644 --- a/components/tools/OmeroWeb/omeroweb/api/views.py +++ b/components/tools/OmeroWeb/omeroweb/api/views.py @@ -392,9 +392,9 @@ class SaveView(View): POST to create a new Object and PUT to replace existing one. """ - can_put = ['Project', 'Dataset', 'Screen'] + CAN_PUT = ['Project', 'Dataset', 'Screen'] - can_post = [] + CAN_POST = ['Project', 'Dataset', 'Screen'] @method_decorator(login_required(useragent='OMERO.webapi')) @method_decorator(json_response()) @@ -421,7 +421,7 @@ def put(self, request, conn=None, **kwargs): """ object_json = json.loads(request.body) obj_type = self.get_type_name(object_json) - if obj_type not in self.can_put: + if obj_type not in self.CAN_PUT: raise BadRequestError("Update of %s not supported" % obj_type) if '@id' not in object_json: raise BadRequestError( @@ -436,7 +436,7 @@ def post(self, request, conn=None, **kwargs): """ object_json = json.loads(request.body) obj_type = self.get_type_name(object_json) - if obj_type not in self.can_post: + if obj_type not in self.CAN_POST: raise BadRequestError("Creation of %s not supported" % obj_type) if '@id' in object_json: raise BadRequestError( From c52550b9e9d9ce88ed828fa58b65326d8b17a9ed Mon Sep 17 00:00:00 2001 From: William Moore Date: Fri, 27 Jan 2017 13:16:40 +0000 Subject: [PATCH 04/19] Add CAN_DELETE check into ObjectView.delete() --- components/tools/OmeroWeb/omeroweb/api/views.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/components/tools/OmeroWeb/omeroweb/api/views.py b/components/tools/OmeroWeb/omeroweb/api/views.py index 5fac8a04239..bde38b03c73 100644 --- a/components/tools/OmeroWeb/omeroweb/api/views.py +++ b/components/tools/OmeroWeb/omeroweb/api/views.py @@ -148,6 +148,8 @@ def add_data(self, marshalled, request, **kwargs): class ObjectView(ApiView): """Handle access to an individual Object to GET or DELETE it.""" + CAN_DELETE = True + def get_opts(self, request): """Return a dict for use in conn.getObjects() based on request.""" return {} @@ -170,6 +172,9 @@ def delete(self, request, object_id, conn=None, **kwargs): Return 404 if not found. """ + if not self.CAN_DELETE: + raise BadRequestError( + "Delete of %s not supported" % self.OMERO_TYPE) try: obj = conn.getQueryService().get(self.OMERO_TYPE, long(object_id), conn.SERVICE_OPTS) @@ -211,6 +216,8 @@ class ImageView(ObjectView): OMERO_TYPE = 'Image' + CAN_DELETE = False + def get_opts(self, request): """Add support for load_pixels and load_channels.""" opts = super(ImageView, self).get_opts(request) From 756ae7da1d5d6835c78a98c03d5c5d3e51359b6e Mon Sep 17 00:00:00 2001 From: William Moore Date: Fri, 27 Jan 2017 13:59:21 +0000 Subject: [PATCH 05/19] Return status=405 'method not supported' for disallowed methods --- .../OmeroWeb/omeroweb/api/api_exceptions.py | 16 ++++++++++++++++ .../tools/OmeroWeb/omeroweb/api/decorators.py | 5 +++-- components/tools/OmeroWeb/omeroweb/api/views.py | 10 ++++++---- 3 files changed, 25 insertions(+), 6 deletions(-) diff --git a/components/tools/OmeroWeb/omeroweb/api/api_exceptions.py b/components/tools/OmeroWeb/omeroweb/api/api_exceptions.py index bfcfda7380c..f50b406d0f2 100644 --- a/components/tools/OmeroWeb/omeroweb/api/api_exceptions.py +++ b/components/tools/OmeroWeb/omeroweb/api/api_exceptions.py @@ -50,6 +50,22 @@ def __init__(self, message, stacktrace=None): self.stacktrace = stacktrace +class MethodNotSupportedError(Exception): + """ + An exception that will result in a response status of 405. + + Raised if user tries to DELETE, POST or PUT for an Object + where we don't support those methods. + """ + + status = 405 + + def __init__(self, message, stacktrace=None): + """Override init to handle message and stacktrace.""" + super(MethodNotSupportedError, self).__init__(message) + self.stacktrace = stacktrace + + class CreatedObject(Exception): """ An exception that is thrown when new object created. diff --git a/components/tools/OmeroWeb/omeroweb/api/decorators.py b/components/tools/OmeroWeb/omeroweb/api/decorators.py index 59718240112..445dc18b94d 100644 --- a/components/tools/OmeroWeb/omeroweb/api/decorators.py +++ b/components/tools/OmeroWeb/omeroweb/api/decorators.py @@ -27,7 +27,8 @@ import traceback from django.http import JsonResponse from functools import update_wrapper -from api_exceptions import NotFoundError, BadRequestError, CreatedObject +from api_exceptions import NotFoundError, BadRequestError, CreatedObject, \ + MethodNotSupportedError logger = logging.getLogger(__name__) @@ -75,7 +76,7 @@ def handle_error(self, ex, trace): # But we try to handle all 'expected' errors appropriately # TODO: handle omero.ConcurrencyException status = 500 - if isinstance(ex, NotFoundError): + if isinstance(ex, (NotFoundError, MethodNotSupportedError)): status = ex.status if isinstance(ex, BadRequestError): status = ex.status diff --git a/components/tools/OmeroWeb/omeroweb/api/views.py b/components/tools/OmeroWeb/omeroweb/api/views.py index bde38b03c73..b0047fac9f3 100644 --- a/components/tools/OmeroWeb/omeroweb/api/views.py +++ b/components/tools/OmeroWeb/omeroweb/api/views.py @@ -33,7 +33,7 @@ from omero import ValidationException from omeroweb.connector import Server from omeroweb.api.api_exceptions import BadRequestError, NotFoundError, \ - CreatedObject + CreatedObject, MethodNotSupportedError from omeroweb.api.decorators import login_required, json_response from omeroweb.webgateway.util import getIntOrDefault @@ -173,7 +173,7 @@ def delete(self, request, object_id, conn=None, **kwargs): Return 404 if not found. """ if not self.CAN_DELETE: - raise BadRequestError( + raise MethodNotSupportedError( "Delete of %s not supported" % self.OMERO_TYPE) try: obj = conn.getQueryService().get(self.OMERO_TYPE, long(object_id), @@ -429,7 +429,8 @@ def put(self, request, conn=None, **kwargs): object_json = json.loads(request.body) obj_type = self.get_type_name(object_json) if obj_type not in self.CAN_PUT: - raise BadRequestError("Update of %s not supported" % obj_type) + raise MethodNotSupportedError( + "Update of %s not supported" % obj_type) if '@id' not in object_json: raise BadRequestError( "No '@id' attribute. Use POST to create new objects") @@ -444,7 +445,8 @@ def post(self, request, conn=None, **kwargs): object_json = json.loads(request.body) obj_type = self.get_type_name(object_json) if obj_type not in self.CAN_POST: - raise BadRequestError("Creation of %s not supported" % obj_type) + raise MethodNotSupportedError( + "Creation of %s not supported" % obj_type) if '@id' in object_json: raise BadRequestError( "Object has '@id' attribute. Use PUT to update objects") From afcf5817dca44d6be73b3520d49a6385f0011e76 Mon Sep 17 00:00:00 2001 From: William Moore Date: Fri, 27 Jan 2017 14:00:40 +0000 Subject: [PATCH 06/19] Add test to check POST, PUT & DELETE not supported for Images --- .../test/integration/test_api_images.py | 30 +++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/components/tools/OmeroWeb/test/integration/test_api_images.py b/components/tools/OmeroWeb/test/integration/test_api_images.py index e0fadf2a9d6..01dffde7377 100644 --- a/components/tools/OmeroWeb/test/integration/test_api_images.py +++ b/components/tools/OmeroWeb/test/integration/test_api_images.py @@ -19,12 +19,13 @@ """Tests querying Images with web json api.""" -from omeroweb.testlib import IWebTest, _get_response_json +from omeroweb.testlib import IWebTest, _get_response_json, \ + _csrf_post_json, _csrf_put_json, _csrf_delete_response_json from django.core.urlresolvers import reverse from django.conf import settings import pytest from omero.gateway import BlitzGateway -from omero_marshal import get_encoder +from omero_marshal import get_encoder, OME_SCHEMA_URL from omero.model import DatasetI, ImageI from omero.rtypes import rstring, unwrap import json @@ -182,3 +183,28 @@ def test_dataset_images(self, user1, dataset_images): assert len(rsp['Pixels']['Channels']) == 1 assert_objects(conn, [rsp], [orphaned], dtype='Image', opts={'load_channels': True}) + + + def test_image_create_update_delete(self, user1): + """Test that create, update & delete are NOT supported for Images.""" + conn = get_connection(user1) + user_name = conn.getUser().getName() + django_client = self.new_django_client(user_name, user_name) + version = settings.API_VERSIONS[-1] + save_url = reverse('api_save', kwargs={'api_version': version}) + payload = {'Name': 'Image test', + '@type': OME_SCHEMA_URL + '#Image'} + # Test POST creation + rsp = _csrf_post_json(django_client, save_url, payload, + status_code=405) + assert rsp['message'] == 'Creation of Image not supported' + # Test PUT update + rsp = _csrf_put_json(django_client, save_url, payload, + status_code=405) + assert rsp['message'] == 'Update of Image not supported' + # Delete (fake url - image doesn't need to exist for test) + delete_url = reverse('api_image', kwargs={'api_version': version, + 'object_id': 1}) + rsp = _csrf_delete_response_json(django_client, delete_url, {}, + status_code=405) + assert rsp['message'] == 'Delete of Image not supported' From a1a769c0c0e3c8bb34901ed67878120332c00957 Mon Sep 17 00:00:00 2001 From: William Moore Date: Fri, 27 Jan 2017 15:35:00 +0000 Subject: [PATCH 07/19] flake8 fixes --- components/tools/OmeroWeb/omeroweb/api/views.py | 1 - components/tools/OmeroWeb/test/integration/test_api_images.py | 1 - 2 files changed, 2 deletions(-) diff --git a/components/tools/OmeroWeb/omeroweb/api/views.py b/components/tools/OmeroWeb/omeroweb/api/views.py index b0047fac9f3..8d434f4e8b6 100644 --- a/components/tools/OmeroWeb/omeroweb/api/views.py +++ b/components/tools/OmeroWeb/omeroweb/api/views.py @@ -419,7 +419,6 @@ def get_type_name(self, marshalled): return None return schema_type.split('#')[1] - def put(self, request, conn=None, **kwargs): """ PUT handles saving of existing objects. diff --git a/components/tools/OmeroWeb/test/integration/test_api_images.py b/components/tools/OmeroWeb/test/integration/test_api_images.py index 01dffde7377..7c1884eb814 100644 --- a/components/tools/OmeroWeb/test/integration/test_api_images.py +++ b/components/tools/OmeroWeb/test/integration/test_api_images.py @@ -184,7 +184,6 @@ def test_dataset_images(self, user1, dataset_images): assert_objects(conn, [rsp], [orphaned], dtype='Image', opts={'load_channels': True}) - def test_image_create_update_delete(self, user1): """Test that create, update & delete are NOT supported for Images.""" conn = get_connection(user1) From 030316133a07310202fe0e657a632bda060ef4f1 Mon Sep 17 00:00:00 2001 From: William Moore Date: Mon, 30 Jan 2017 15:46:20 +0000 Subject: [PATCH 08/19] Remove 'Plate' from test_container_crud() --- .../tools/OmeroWeb/test/integration/test_api_containers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/tools/OmeroWeb/test/integration/test_api_containers.py b/components/tools/OmeroWeb/test/integration/test_api_containers.py index eb4a19d73a9..1363b1e2a68 100644 --- a/components/tools/OmeroWeb/test/integration/test_api_containers.py +++ b/components/tools/OmeroWeb/test/integration/test_api_containers.py @@ -171,7 +171,7 @@ def user_screens(self, user1): return screens @pytest.mark.parametrize("dtype", ['Project', 'Dataset', - 'Screen', 'Plate']) + 'Screen']) def test_container_crud(self, dtype): """ Test create, read, update and delete of Containers. From d1baadc58544b5ec24864ef40fd1096b6eb4da11 Mon Sep 17 00:00:00 2001 From: William Moore Date: Tue, 14 Feb 2017 15:56:15 +0000 Subject: [PATCH 09/19] Tiny fix to test_api_images to avoid merge conflict --- components/tools/OmeroWeb/test/integration/test_api_images.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/components/tools/OmeroWeb/test/integration/test_api_images.py b/components/tools/OmeroWeb/test/integration/test_api_images.py index 7c1884eb814..40d81c471f0 100644 --- a/components/tools/OmeroWeb/test/integration/test_api_images.py +++ b/components/tools/OmeroWeb/test/integration/test_api_images.py @@ -25,10 +25,11 @@ from django.conf import settings import pytest from omero.gateway import BlitzGateway -from omero_marshal import get_encoder, OME_SCHEMA_URL +from omero_marshal import get_encoder from omero.model import DatasetI, ImageI from omero.rtypes import rstring, unwrap import json +from omero_marshal import OME_SCHEMA_URL def get_update_service(user): From fd204cc53caa27dc597830db0549a48b7704bdd7 Mon Sep 17 00:00:00 2001 From: William Moore Date: Tue, 14 Feb 2017 16:23:46 +0000 Subject: [PATCH 10/19] Test handling of create, update & delete for unsupported types --- .../tools/OmeroWeb/omeroweb/api/views.py | 4 +++ .../test/integration/test_api_images.py | 25 +++++++++++-------- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/components/tools/OmeroWeb/omeroweb/api/views.py b/components/tools/OmeroWeb/omeroweb/api/views.py index ffc7509ec6b..3a2329c2fdf 100644 --- a/components/tools/OmeroWeb/omeroweb/api/views.py +++ b/components/tools/OmeroWeb/omeroweb/api/views.py @@ -244,6 +244,8 @@ class PlateView(ObjectView): OMERO_TYPE = 'Plate' + CAN_DELETE = False + # Urls to add to marshalled object. See ProjectsView for more details urls = { 'url:wells': {'name': 'api_plate_wells', @@ -256,6 +258,8 @@ class WellView(ObjectView): OMERO_TYPE = 'Well' + CAN_DELETE = False + def get_opts(self, request): """Add support for load_images.""" opts = super(WellView, self).get_opts(request) diff --git a/components/tools/OmeroWeb/test/integration/test_api_images.py b/components/tools/OmeroWeb/test/integration/test_api_images.py index 957b9325971..220c4930cd8 100644 --- a/components/tools/OmeroWeb/test/integration/test_api_images.py +++ b/components/tools/OmeroWeb/test/integration/test_api_images.py @@ -156,26 +156,29 @@ def test_dataset_images(self, user1, dataset_images): assert_objects(conn, [rsp], [orphaned], dtype='Image', opts={'load_channels': True}) - def test_image_create_update_delete(self, user1): - """Test that create, update & delete are NOT supported for Images.""" + @pytest.mark.parametrize("dtype", ['Plate', 'Image', 'Well', 'Channel', 'foo']) + def test_create_update_delete_405(self, user1, dtype): + """Test create, update & delete are rejected for unsupported types.""" conn = get_connection(user1) user_name = conn.getUser().getName() django_client = self.new_django_client(user_name, user_name) version = settings.API_VERSIONS[-1] save_url = reverse('api_save', kwargs={'api_version': version}) - payload = {'Name': 'Image test', - '@type': OME_SCHEMA_URL + '#Image'} + payload = {'Name': 'test', + '@type': OME_SCHEMA_URL + '#%s' % dtype} # Test POST creation rsp = _csrf_post_json(django_client, save_url, payload, status_code=405) - assert rsp['message'] == 'Creation of Image not supported' + assert rsp['message'] == 'Creation of %s not supported' % dtype # Test PUT update rsp = _csrf_put_json(django_client, save_url, payload, status_code=405) - assert rsp['message'] == 'Update of Image not supported' + assert rsp['message'] == 'Update of %s not supported' % dtype # Delete (fake url - image doesn't need to exist for test) - delete_url = reverse('api_image', kwargs={'api_version': version, - 'object_id': 1}) - rsp = _csrf_delete_response_json(django_client, delete_url, {}, - status_code=405) - assert rsp['message'] == 'Delete of Image not supported' + if dtype in ('Plate', 'Image', 'Well'): + url_name = 'api_%s' % dtype.lower() + delete_url = reverse(url_name, kwargs={'api_version': version, + 'object_id': 1}) + rsp = _csrf_delete_response_json(django_client, delete_url, {}, + status_code=405) + assert rsp['message'] == 'Delete of %s not supported' % dtype From da755139e5068d6245b8428336cd0386f3c0abc9 Mon Sep 17 00:00:00 2001 From: William Moore Date: Tue, 14 Feb 2017 16:27:51 +0000 Subject: [PATCH 11/19] Move test_crud_unsupported to test_api_containers.py Since this test is really the counterpart to test_container_crud() above --- .../test/integration/test_api_containers.py | 29 +++++++++++++++++++ .../test/integration/test_api_images.py | 28 ------------------ 2 files changed, 29 insertions(+), 28 deletions(-) diff --git a/components/tools/OmeroWeb/test/integration/test_api_containers.py b/components/tools/OmeroWeb/test/integration/test_api_containers.py index ea554d8a2a5..72d01f81561 100644 --- a/components/tools/OmeroWeb/test/integration/test_api_containers.py +++ b/components/tools/OmeroWeb/test/integration/test_api_containers.py @@ -35,6 +35,7 @@ WellI, \ WellSampleI from omero.rtypes import rstring, rint +from omero_marshal import OME_SCHEMA_URL def build_url(client, url_name, url_kwargs): @@ -178,6 +179,34 @@ def user_screens(self, user1): screens.sort(cmp_name_insensitive) return screens + @pytest.mark.parametrize("dtype", ['Plate', 'Image', 'Well', + 'Channel', 'foo']) + def test_crud_unsupported(self, user1, dtype): + """Test create, update & delete are rejected for unsupported types.""" + conn = get_connection(user1) + user_name = conn.getUser().getName() + django_client = self.new_django_client(user_name, user_name) + version = settings.API_VERSIONS[-1] + save_url = reverse('api_save', kwargs={'api_version': version}) + payload = {'Name': 'test', + '@type': OME_SCHEMA_URL + '#%s' % dtype} + # Test POST creation + rsp = _csrf_post_json(django_client, save_url, payload, + status_code=405) + assert rsp['message'] == 'Creation of %s not supported' % dtype + # Test PUT update + rsp = _csrf_put_json(django_client, save_url, payload, + status_code=405) + assert rsp['message'] == 'Update of %s not supported' % dtype + # Delete (fake url - image doesn't need to exist for test) + if dtype in ('Plate', 'Image', 'Well'): + url_name = 'api_%s' % dtype.lower() + delete_url = reverse(url_name, kwargs={'api_version': version, + 'object_id': 1}) + rsp = _csrf_delete_response_json(django_client, delete_url, {}, + status_code=405) + assert rsp['message'] == 'Delete of %s not supported' % dtype + @pytest.mark.parametrize("dtype", ['Project', 'Dataset', 'Screen']) def test_container_crud(self, dtype): diff --git a/components/tools/OmeroWeb/test/integration/test_api_images.py b/components/tools/OmeroWeb/test/integration/test_api_images.py index 220c4930cd8..1419b08bbb0 100644 --- a/components/tools/OmeroWeb/test/integration/test_api_images.py +++ b/components/tools/OmeroWeb/test/integration/test_api_images.py @@ -29,7 +29,6 @@ from omero.model import DatasetI, ImageI from omero.rtypes import rstring import json -from omero_marshal import OME_SCHEMA_URL def get_query_service(user): @@ -155,30 +154,3 @@ def test_dataset_images(self, user1, dataset_images): assert len(rsp['Pixels']['Channels']) == 1 assert_objects(conn, [rsp], [orphaned], dtype='Image', opts={'load_channels': True}) - - @pytest.mark.parametrize("dtype", ['Plate', 'Image', 'Well', 'Channel', 'foo']) - def test_create_update_delete_405(self, user1, dtype): - """Test create, update & delete are rejected for unsupported types.""" - conn = get_connection(user1) - user_name = conn.getUser().getName() - django_client = self.new_django_client(user_name, user_name) - version = settings.API_VERSIONS[-1] - save_url = reverse('api_save', kwargs={'api_version': version}) - payload = {'Name': 'test', - '@type': OME_SCHEMA_URL + '#%s' % dtype} - # Test POST creation - rsp = _csrf_post_json(django_client, save_url, payload, - status_code=405) - assert rsp['message'] == 'Creation of %s not supported' % dtype - # Test PUT update - rsp = _csrf_put_json(django_client, save_url, payload, - status_code=405) - assert rsp['message'] == 'Update of %s not supported' % dtype - # Delete (fake url - image doesn't need to exist for test) - if dtype in ('Plate', 'Image', 'Well'): - url_name = 'api_%s' % dtype.lower() - delete_url = reverse(url_name, kwargs={'api_version': version, - 'object_id': 1}) - rsp = _csrf_delete_response_json(django_client, delete_url, {}, - status_code=405) - assert rsp['message'] == 'Delete of %s not supported' % dtype From 17682f32da63a705cc564f83c0d4eb0ccea75983 Mon Sep 17 00:00:00 2001 From: William Moore Date: Tue, 14 Feb 2017 21:42:11 +0000 Subject: [PATCH 12/19] Remove comment --- components/tools/OmeroWeb/omeroweb/api/views.py | 1 - 1 file changed, 1 deletion(-) diff --git a/components/tools/OmeroWeb/omeroweb/api/views.py b/components/tools/OmeroWeb/omeroweb/api/views.py index 3a2329c2fdf..eef75b11a32 100644 --- a/components/tools/OmeroWeb/omeroweb/api/views.py +++ b/components/tools/OmeroWeb/omeroweb/api/views.py @@ -511,7 +511,6 @@ def get_type_name(self, marshalled): if '@type' not in marshalled: raise BadRequestError('Need to specify @type attribute') schema_type = marshalled['@type'] - # NB: Do we support saving from old SCHEMA to newer one? if '#' not in schema_type: return None return schema_type.split('#')[1] From b7e936e2e41d9ff490ca6b6a99127f3c0d697272 Mon Sep 17 00:00:00 2001 From: William Moore Date: Tue, 14 Feb 2017 21:44:59 +0000 Subject: [PATCH 13/19] flake8 fix --- components/tools/OmeroWeb/test/integration/test_api_images.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/components/tools/OmeroWeb/test/integration/test_api_images.py b/components/tools/OmeroWeb/test/integration/test_api_images.py index 1419b08bbb0..067ccc62528 100644 --- a/components/tools/OmeroWeb/test/integration/test_api_images.py +++ b/components/tools/OmeroWeb/test/integration/test_api_images.py @@ -19,8 +19,7 @@ """Tests querying Images with web json api.""" -from omeroweb.testlib import IWebTest, _get_response_json, \ - _csrf_post_json, _csrf_put_json, _csrf_delete_response_json +from omeroweb.testlib import IWebTest, _get_response_json from django.core.urlresolvers import reverse from django.conf import settings import pytest From ddd187a3da49f4da492ce70d69465f35b4107849 Mon Sep 17 00:00:00 2001 From: William Moore Date: Thu, 16 Feb 2017 16:44:40 +0000 Subject: [PATCH 14/19] Fix failing test_api_errors.py tests --- .../test/integration/test_api_errors.py | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/components/tools/OmeroWeb/test/integration/test_api_errors.py b/components/tools/OmeroWeb/test/integration/test_api_errors.py index 7c9a5cfdaa0..0c94a51fbdb 100644 --- a/components/tools/OmeroWeb/test/integration/test_api_errors.py +++ b/components/tools/OmeroWeb/test/integration/test_api_errors.py @@ -24,7 +24,7 @@ from django.conf import settings import pytest from test_api_projects import get_connection -from omero.model import ProjectI +from omero.model import ProjectI, TagAnnotationI from omero.rtypes import rstring from omero_marshal import get_encoder, get_decoder, OME_SCHEMA_URL from omero import ValidationException @@ -59,6 +59,7 @@ def test_save_post_no_id(self): version = settings.API_VERSIONS[-1] save_url = reverse('api_save', kwargs={'api_version': version}) payload = {'Name': 'test_save_post_no_id', + '@type': '%s#Project' % OME_SCHEMA_URL, '@id': 1} rsp = _csrf_post_json(django_client, save_url, payload, status_code=400) @@ -70,7 +71,8 @@ def test_save_put_id(self): django_client = self.django_root_client version = settings.API_VERSIONS[-1] save_url = reverse('api_save', kwargs={'api_version': version}) - payload = {'Name': 'test_save_put_id'} + payload = {'Name': 'test_save_put_id', + '@type': '%s#Project' % OME_SCHEMA_URL,} rsp = _csrf_put_json(django_client, save_url, payload, status_code=400) assert (rsp['message'] == @@ -81,7 +83,7 @@ def test_marshal_type(self): django_client = self.django_root_client version = settings.API_VERSIONS[-1] save_url = reverse('api_save', kwargs={'api_version': version}) - objType = 'SomeInvalid#Type' + objType = 'SomeInvalidSchema#Project' payload = {'Name': 'test_marshal_type', '@type': objType} rsp = _csrf_post_json(django_client, save_url, payload, @@ -141,16 +143,17 @@ def test_validation_exception(self, user_A): save_url += '?group=' + str(group) # Create Tag - tag = {'Value': 'test_tag', - '@type': OME_SCHEMA_URL + '#TagAnnotation'} - tag_rsp = _csrf_post_json(django_client, save_url, tag, - status_code=201) + tag = TagAnnotationI() + tag.textValue = rstring('test_tag') + tag = conn.getUpdateService().saveAndReturnObject(tag) + tag_json = {'Value': 'test_tag', + '@id': tag.id.val, + '@type': OME_SCHEMA_URL + '#TagAnnotation'} # Add Tag twice to Project to get Validation Exception - del tag_rsp['omero:details'] payload = {'Name': 'test_validation', '@type': OME_SCHEMA_URL + '#Project', - 'Annotations': [tag_rsp, tag_rsp]} + 'Annotations': [tag_json, tag_json]} rsp = _csrf_post_json(django_client, save_url, payload, status_code=400) # NB: message contains whole stack trace From 4b72827597c531406b3a8f0b4bdda8795de055d6 Mon Sep 17 00:00:00 2001 From: William Moore Date: Thu, 16 Feb 2017 16:50:46 +0000 Subject: [PATCH 15/19] Tidy api_exceptions imports --- components/tools/OmeroWeb/omeroweb/api/decorators.py | 6 ++++-- components/tools/OmeroWeb/omeroweb/api/views.py | 6 ++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/components/tools/OmeroWeb/omeroweb/api/decorators.py b/components/tools/OmeroWeb/omeroweb/api/decorators.py index 445dc18b94d..dab11da2d8a 100644 --- a/components/tools/OmeroWeb/omeroweb/api/decorators.py +++ b/components/tools/OmeroWeb/omeroweb/api/decorators.py @@ -27,8 +27,10 @@ import traceback from django.http import JsonResponse from functools import update_wrapper -from api_exceptions import NotFoundError, BadRequestError, CreatedObject, \ - MethodNotSupportedError +from api_exceptions import BadRequestError, \ + CreatedObject, \ + MethodNotSupportedError, \ + NotFoundError logger = logging.getLogger(__name__) diff --git a/components/tools/OmeroWeb/omeroweb/api/views.py b/components/tools/OmeroWeb/omeroweb/api/views.py index eef75b11a32..b7ebba0cad6 100644 --- a/components/tools/OmeroWeb/omeroweb/api/views.py +++ b/components/tools/OmeroWeb/omeroweb/api/views.py @@ -32,8 +32,10 @@ from omero_marshal import get_encoder, get_decoder, OME_SCHEMA_URL from omero import ValidationException from omeroweb.connector import Server -from omeroweb.api.api_exceptions import BadRequestError, NotFoundError, \ - CreatedObject, MethodNotSupportedError +from api_exceptions import BadRequestError, \ + CreatedObject, \ + MethodNotSupportedError, \ + NotFoundError from omeroweb.api.decorators import login_required, json_response from omeroweb.webgateway.util import getIntOrDefault From 102c6c9c8fb2a43a6262908b484196b88a206d19 Mon Sep 17 00:00:00 2001 From: William Moore Date: Thu, 16 Feb 2017 22:40:47 +0000 Subject: [PATCH 16/19] Split test_crud_unsupported into separate tests Single test for PUT, POST & DELETE is spilt into separate tests for each method. To try and reduce code duplication, PUT and POST are covered by parametrizing method in a single test. --- .../test/integration/test_api_containers.py | 37 +++++++++++-------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/components/tools/OmeroWeb/test/integration/test_api_containers.py b/components/tools/OmeroWeb/test/integration/test_api_containers.py index 72d01f81561..7bc4fb6ca15 100644 --- a/components/tools/OmeroWeb/test/integration/test_api_containers.py +++ b/components/tools/OmeroWeb/test/integration/test_api_containers.py @@ -181,8 +181,10 @@ def user_screens(self, user1): @pytest.mark.parametrize("dtype", ['Plate', 'Image', 'Well', 'Channel', 'foo']) - def test_crud_unsupported(self, user1, dtype): - """Test create, update & delete are rejected for unsupported types.""" + @pytest.mark.parametrize("method", [(_csrf_post_json, 'Creation'), + (_csrf_put_json, 'Update')]) + def test_create_update_unsupported(self, user1, dtype, method): + """Test create and update are rejected for unsupported types.""" conn = get_connection(user1) user_name = conn.getUser().getName() django_client = self.new_django_client(user_name, user_name) @@ -190,22 +192,25 @@ def test_crud_unsupported(self, user1, dtype): save_url = reverse('api_save', kwargs={'api_version': version}) payload = {'Name': 'test', '@type': OME_SCHEMA_URL + '#%s' % dtype} - # Test POST creation - rsp = _csrf_post_json(django_client, save_url, payload, + # Test PUT/POST + rsp = method[0](django_client, save_url, payload, status_code=405) - assert rsp['message'] == 'Creation of %s not supported' % dtype - # Test PUT update - rsp = _csrf_put_json(django_client, save_url, payload, - status_code=405) - assert rsp['message'] == 'Update of %s not supported' % dtype + assert rsp['message'] == '%s of %s not supported' % (method[1], dtype) + + @pytest.mark.parametrize("dtype", ['Plate', 'Image', 'Well']) + def test_delete_unsupported(self, user1, dtype): + """Test delete is rejected for unsupported types.""" + conn = get_connection(user1) + user_name = conn.getUser().getName() + django_client = self.new_django_client(user_name, user_name) + version = settings.API_VERSIONS[-1] # Delete (fake url - image doesn't need to exist for test) - if dtype in ('Plate', 'Image', 'Well'): - url_name = 'api_%s' % dtype.lower() - delete_url = reverse(url_name, kwargs={'api_version': version, - 'object_id': 1}) - rsp = _csrf_delete_response_json(django_client, delete_url, {}, - status_code=405) - assert rsp['message'] == 'Delete of %s not supported' % dtype + url_name = 'api_%s' % dtype.lower() + delete_url = reverse(url_name, kwargs={'api_version': version, + 'object_id': 1}) + rsp = _csrf_delete_response_json(django_client, delete_url, {}, + status_code=405) + assert rsp['message'] == 'Delete of %s not supported' % dtype @pytest.mark.parametrize("dtype", ['Project', 'Dataset', 'Screen']) From a377f3d9156aed17ead016cc12b2959f2b8e409a Mon Sep 17 00:00:00 2001 From: William Moore Date: Thu, 16 Feb 2017 22:45:43 +0000 Subject: [PATCH 17/19] flake8 fix --- components/tools/OmeroWeb/test/integration/test_api_errors.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/tools/OmeroWeb/test/integration/test_api_errors.py b/components/tools/OmeroWeb/test/integration/test_api_errors.py index 0c94a51fbdb..9621ab2fdd7 100644 --- a/components/tools/OmeroWeb/test/integration/test_api_errors.py +++ b/components/tools/OmeroWeb/test/integration/test_api_errors.py @@ -72,7 +72,7 @@ def test_save_put_id(self): version = settings.API_VERSIONS[-1] save_url = reverse('api_save', kwargs={'api_version': version}) payload = {'Name': 'test_save_put_id', - '@type': '%s#Project' % OME_SCHEMA_URL,} + '@type': '%s#Project' % OME_SCHEMA_URL} rsp = _csrf_put_json(django_client, save_url, payload, status_code=400) assert (rsp['message'] == From 242c551d4fe6be56cac458f618441b8cfcc8f80d Mon Sep 17 00:00:00 2001 From: William Moore Date: Thu, 16 Feb 2017 22:46:58 +0000 Subject: [PATCH 18/19] flake8 fix --- .../tools/OmeroWeb/test/integration/test_api_containers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/tools/OmeroWeb/test/integration/test_api_containers.py b/components/tools/OmeroWeb/test/integration/test_api_containers.py index 7bc4fb6ca15..ec151341ca8 100644 --- a/components/tools/OmeroWeb/test/integration/test_api_containers.py +++ b/components/tools/OmeroWeb/test/integration/test_api_containers.py @@ -194,7 +194,7 @@ def test_create_update_unsupported(self, user1, dtype, method): '@type': OME_SCHEMA_URL + '#%s' % dtype} # Test PUT/POST rsp = method[0](django_client, save_url, payload, - status_code=405) + status_code=405) assert rsp['message'] == '%s of %s not supported' % (method[1], dtype) @pytest.mark.parametrize("dtype", ['Plate', 'Image', 'Well']) From eb18edcb38a5c89c68b7fa0d1f5221ed4af836d7 Mon Sep 17 00:00:00 2001 From: William Moore Date: Fri, 17 Feb 2017 11:10:12 +0000 Subject: [PATCH 19/19] ApiException base class to avoid code duplication --- .../OmeroWeb/omeroweb/api/api_exceptions.py | 30 ++++++++----------- 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/components/tools/OmeroWeb/omeroweb/api/api_exceptions.py b/components/tools/OmeroWeb/omeroweb/api/api_exceptions.py index f50b406d0f2..607793140e8 100644 --- a/components/tools/OmeroWeb/omeroweb/api/api_exceptions.py +++ b/components/tools/OmeroWeb/omeroweb/api/api_exceptions.py @@ -20,7 +20,16 @@ """Exceptions used by the api/views methods.""" -class BadRequestError(Exception): +class ApiException(Exception): + """A base exception class that handles message and stactrace.""" + + def __init__(self, message, stacktrace=None): + """Override init to handle message and stacktrace.""" + super(ApiException, self).__init__(message) + self.stacktrace = stacktrace + + +class BadRequestError(ApiException): """ An exception that will result in a response status of 400. @@ -29,13 +38,8 @@ class BadRequestError(Exception): status = 400 - def __init__(self, message, stacktrace=None): - """Override init to handle message and stacktrace.""" - super(BadRequestError, self).__init__(message) - self.stacktrace = stacktrace - -class NotFoundError(Exception): +class NotFoundError(ApiException): """ An exception that will result in a response status of 404. @@ -44,13 +48,8 @@ class NotFoundError(Exception): status = 404 - def __init__(self, message, stacktrace=None): - """Override init to handle message and stacktrace.""" - super(NotFoundError, self).__init__(message) - self.stacktrace = stacktrace - -class MethodNotSupportedError(Exception): +class MethodNotSupportedError(ApiException): """ An exception that will result in a response status of 405. @@ -60,11 +59,6 @@ class MethodNotSupportedError(Exception): status = 405 - def __init__(self, message, stacktrace=None): - """Override init to handle message and stacktrace.""" - super(MethodNotSupportedError, self).__init__(message) - self.stacktrace = stacktrace - class CreatedObject(Exception): """