From adb8ba7bff2bc4e2657cf84e9f061433ff947a73 Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Wed, 13 Mar 2013 13:28:10 -0400 Subject: [PATCH] Switch all uses of 422 response code to 400. The 422 response is normally only for WebDAV, and these types of errors should normally return a 400 code, HTTPBadRequest. This commit goes through all the remaining 422 codes and changes them to 400 responses. Fixes bug 1153807 Change-Id: I9bf49643763c2cb0ac585641056c85a060152e36 --- cinder/api/contrib/backups.py | 4 +- cinder/api/contrib/services.py | 2 +- cinder/api/contrib/types_manage.py | 4 +- cinder/api/v2/snapshots.py | 6 +-- cinder/api/v2/volumes.py | 6 +-- cinder/tests/api/contrib/test_backups.py | 27 +++++----- cinder/tests/api/contrib/test_types_manage.py | 22 ++------ cinder/tests/api/v2/test_snapshots.py | 51 +++++++----------- cinder/tests/api/v2/test_volumes.py | 53 +++++++------------ 9 files changed, 71 insertions(+), 104 deletions(-) diff --git a/cinder/api/contrib/backups.py b/cinder/api/contrib/backups.py index 53b6b6cb1..02444ac55 100644 --- a/cinder/api/contrib/backups.py +++ b/cinder/api/contrib/backups.py @@ -184,7 +184,7 @@ class BackupsController(wsgi.Controller): """Create a new backup.""" LOG.debug(_('Creating new backup %s'), body) if not self.is_valid_body(body, 'backup'): - raise exc.HTTPUnprocessableEntity() + raise exc.HTTPBadRequest() context = req.environ['cinder.context'] @@ -220,7 +220,7 @@ class BackupsController(wsgi.Controller): backup_id = id LOG.debug(_('Restoring backup %(backup_id)s (%(body)s)') % locals()) if not self.is_valid_body(body, 'restore'): - raise exc.HTTPUnprocessableEntity() + raise exc.HTTPBadRequest() context = req.environ['cinder.context'] diff --git a/cinder/api/contrib/services.py b/cinder/api/contrib/services.py index ed4e4d0e6..db5d884ed 100644 --- a/cinder/api/contrib/services.py +++ b/cinder/api/contrib/services.py @@ -109,7 +109,7 @@ class ServiceController(object): host = body['host'] service = body['service'] except (TypeError, KeyError): - raise webob.exc.HTTPUnprocessableEntity() + raise webob.exc.HTTPBadRequest() try: svc = db.service_get_by_args(context, host, service) diff --git a/cinder/api/contrib/types_manage.py b/cinder/api/contrib/types_manage.py index e043db25b..ae1257e1d 100644 --- a/cinder/api/contrib/types_manage.py +++ b/cinder/api/contrib/types_manage.py @@ -43,14 +43,14 @@ class VolumeTypesManageController(wsgi.Controller): authorize(context) if not self.is_valid_body(body, 'volume_type'): - raise webob.exc.HTTPUnprocessableEntity() + raise webob.exc.HTTPBadRequest() vol_type = body['volume_type'] name = vol_type.get('name', None) specs = vol_type.get('extra_specs', {}) if name is None or name == "": - raise webob.exc.HTTPUnprocessableEntity() + raise webob.exc.HTTPBadRequest() try: volume_types.create(context, name, specs) diff --git a/cinder/api/v2/snapshots.py b/cinder/api/v2/snapshots.py index dc992b78b..28595da0e 100644 --- a/cinder/api/v2/snapshots.py +++ b/cinder/api/v2/snapshots.py @@ -167,7 +167,7 @@ class SnapshotsController(wsgi.Controller): context = req.environ['cinder.context'] if not self.is_valid_body(body, 'snapshot'): - raise exc.HTTPUnprocessableEntity() + raise exc.HTTPBadRequest() snapshot = body['snapshot'] kwargs['metadata'] = snapshot.get('metadata', None) @@ -212,10 +212,10 @@ class SnapshotsController(wsgi.Controller): context = req.environ['cinder.context'] if not body: - raise exc.HTTPUnprocessableEntity() + raise exc.HTTPBadRequest() if 'snapshot' not in body: - raise exc.HTTPUnprocessableEntity() + raise exc.HTTPBadRequest() snapshot = body['snapshot'] update_dict = {} diff --git a/cinder/api/v2/volumes.py b/cinder/api/v2/volumes.py index bd2e4e91b..3db80e7e2 100644 --- a/cinder/api/v2/volumes.py +++ b/cinder/api/v2/volumes.py @@ -220,7 +220,7 @@ class VolumeController(wsgi.Controller): def create(self, req, body): """Creates a new volume.""" if not self.is_valid_body(body, 'volume'): - raise exc.HTTPUnprocessableEntity() + raise exc.HTTPBadRequest() context = req.environ['cinder.context'] volume = body['volume'] @@ -306,10 +306,10 @@ class VolumeController(wsgi.Controller): context = req.environ['cinder.context'] if not body: - raise exc.HTTPUnprocessableEntity() + raise exc.HTTPBadRequest() if 'volume' not in body: - raise exc.HTTPUnprocessableEntity() + raise exc.HTTPBadRequest() volume = body['volume'] update_dict = {} diff --git a/cinder/tests/api/contrib/test_backups.py b/cinder/tests/api/contrib/test_backups.py index bc192cb26..1ac52fce6 100644 --- a/cinder/tests/api/contrib/test_backups.py +++ b/cinder/tests/api/contrib/test_backups.py @@ -395,10 +395,11 @@ class BackupsAPITestCase(test.TestCase): res = req.get_response(fakes.wsgi_app()) res_dict = json.loads(res.body) - self.assertEqual(res.status_int, 422) - self.assertEqual(res_dict['computeFault']['code'], 422) - self.assertEqual(res_dict['computeFault']['message'], - 'Unable to process the contained instructions') + self.assertEqual(res.status_int, 400) + self.assertEqual(res_dict['badRequest']['code'], 400) + self.assertEqual(res_dict['badRequest']['message'], + 'The server could not comply with the request since' + ' it is either malformed or otherwise incorrect.') def test_create_backup_with_body_KeyError(self): # omit volume_id from body @@ -576,10 +577,11 @@ class BackupsAPITestCase(test.TestCase): res = req.get_response(fakes.wsgi_app()) res_dict = json.loads(res.body) - self.assertEqual(res.status_int, 422) - self.assertEqual(res_dict['computeFault']['code'], 422) - self.assertEqual(res_dict['computeFault']['message'], - 'Unable to process the contained instructions') + self.assertEqual(res.status_int, 400) + self.assertEqual(res_dict['badRequest']['code'], 400) + self.assertEqual(res_dict['badRequest']['message'], + 'The server could not comply with the request since' + ' it is either malformed or otherwise incorrect.') db.backup_destroy(context.get_admin_context(), backup_id) @@ -597,10 +599,11 @@ class BackupsAPITestCase(test.TestCase): res_dict = json.loads(res.body) - self.assertEqual(res.status_int, 422) - self.assertEqual(res_dict['computeFault']['code'], 422) - self.assertEqual(res_dict['computeFault']['message'], - 'Unable to process the contained instructions') + self.assertEqual(res.status_int, 400) + self.assertEqual(res_dict['badRequest']['code'], 400) + self.assertEqual(res_dict['badRequest']['message'], + 'The server could not comply with the request since' + ' it is either malformed or otherwise incorrect.') def test_restore_backup_volume_id_unspecified(self): diff --git a/cinder/tests/api/contrib/test_types_manage.py b/cinder/tests/api/contrib/test_types_manage.py index 14fa778d0..a94916f3a 100644 --- a/cinder/tests/api/contrib/test_types_manage.py +++ b/cinder/tests/api/contrib/test_types_manage.py @@ -92,31 +92,19 @@ class VolumeTypesManageApiTest(test.TestCase): self.assertEqual(1, len(res_dict)) self.assertEqual('vol_type_1', res_dict['volume_type']['name']) - -class VolumeTypesUnprocessableEntityTestCase(test.TestCase): - - """ - Tests of places we throw 422 Unprocessable Entity from - """ - - def setUp(self): - super(VolumeTypesUnprocessableEntityTestCase, self).setUp() - self.controller = types_manage.VolumeTypesManageController() - - def _unprocessable_volume_type_create(self, body): + def _create_volume_type_bad_body(self, body): req = fakes.HTTPRequest.blank('/v2/fake/types') req.method = 'POST' - - self.assertRaises(webob.exc.HTTPUnprocessableEntity, + self.assertRaises(webob.exc.HTTPBadRequest, self.controller._create, req, body) def test_create_no_body(self): - self._unprocessable_volume_type_create(body=None) + self._create_volume_type_bad_body(body=None) def test_create_missing_volume(self): body = {'foo': {'a': 'b'}} - self._unprocessable_volume_type_create(body=body) + self._create_volume_type_bad_body(body=body) def test_create_malformed_entity(self): body = {'volume_type': 'string'} - self._unprocessable_volume_type_create(body=body) + self._create_volume_type_bad_body(body=body) diff --git a/cinder/tests/api/v2/test_snapshots.py b/cinder/tests/api/v2/test_snapshots.py index d32e01017..f003cb0b3 100644 --- a/cinder/tests/api/v2/test_snapshots.py +++ b/cinder/tests/api/v2/test_snapshots.py @@ -171,13 +171,13 @@ class SnapshotApiTest(test.TestCase): def test_snapshot_update_missing_body(self): body = {} req = fakes.HTTPRequest.blank('/v2/snapshots/%s' % UUID) - self.assertRaises(webob.exc.HTTPUnprocessableEntity, + self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update, req, UUID, body) def test_snapshot_update_invalid_body(self): body = {'name': 'missing top level snapshot key'} req = fakes.HTTPRequest.blank('/v2/snapshots/%s' % UUID) - self.assertRaises(webob.exc.HTTPUnprocessableEntity, + self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update, req, UUID, body) def test_snapshot_update_not_found(self): @@ -346,6 +346,24 @@ class SnapshotApiTest(test.TestCase): self.assertTrue('snapshots' in res) self.assertEqual(1, len(res['snapshots'])) + def _create_snapshot_bad_body(self, body): + req = fakes.HTTPRequest.blank('/v2/fake/snapshots') + req.method = 'POST' + + self.assertRaises(webob.exc.HTTPBadRequest, + self.controller.create, req, body) + + def test_create_no_body(self): + self._create_snapshot_bad_body(body=None) + + def test_create_missing_snapshot(self): + body = {'foo': {'a': 'b'}} + self._create_snapshot_bad_body(body=body) + + def test_create_malformed_entity(self): + body = {'snapshot': 'string'} + self._create_snapshot_bad_body(body=body) + class SnapshotSerializerTest(test.TestCase): def _verify_snapshot(self, snap, tree): @@ -405,32 +423,3 @@ class SnapshotSerializerTest(test.TestCase): self.assertEqual(len(raw_snapshots), len(tree)) for idx, child in enumerate(tree): self._verify_snapshot(raw_snapshots[idx], child) - - -class SnapshotsUnprocessableEntityTestCase(test.TestCase): - - """ - Tests of places we throw 422 Unprocessable Entity from - """ - - def setUp(self): - super(SnapshotsUnprocessableEntityTestCase, self).setUp() - self.controller = snapshots.SnapshotsController() - - def _unprocessable_snapshot_create(self, body): - req = fakes.HTTPRequest.blank('/v2/fake/snapshots') - req.method = 'POST' - - self.assertRaises(webob.exc.HTTPUnprocessableEntity, - self.controller.create, req, body) - - def test_create_no_body(self): - self._unprocessable_snapshot_create(body=None) - - def test_create_missing_snapshot(self): - body = {'foo': {'a': 'b'}} - self._unprocessable_snapshot_create(body=body) - - def test_create_malformed_entity(self): - body = {'snapshot': 'string'} - self._unprocessable_snapshot_create(body=body) diff --git a/cinder/tests/api/v2/test_volumes.py b/cinder/tests/api/v2/test_volumes.py index 78a9046c2..df2d92f3d 100644 --- a/cinder/tests/api/v2/test_volumes.py +++ b/cinder/tests/api/v2/test_volumes.py @@ -304,7 +304,7 @@ class VolumeApiTest(test.TestCase): def test_update_empty_body(self): body = {} req = fakes.HTTPRequest.blank('/v2/volumes/1') - self.assertRaises(webob.exc.HTTPUnprocessableEntity, + self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update, req, '1', body) @@ -313,7 +313,7 @@ class VolumeApiTest(test.TestCase): 'name': 'missing top level volume key' } req = fakes.HTTPRequest.blank('/v2/volumes/1') - self.assertRaises(webob.exc.HTTPUnprocessableEntity, + self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update, req, '1', body) @@ -661,6 +661,24 @@ class VolumeApiTest(test.TestCase): self.assertTrue('volumes' in res) self.assertEqual(1, len(res['volumes'])) + def _create_volume_bad_request(self, body): + req = fakes.HTTPRequest.blank('/v2/fake/volumes') + req.method = 'POST' + + self.assertRaises(webob.exc.HTTPBadRequest, + self.controller.create, req, body) + + def test_create_no_body(self): + self._create_volume_bad_request(body=None) + + def test_create_missing_volume(self): + body = {'foo': {'a': 'b'}} + self._create_volume_bad_request(body=body) + + def test_create_malformed_entity(self): + body = {'volume': 'string'} + self._create_volume_bad_request(body=body) + class VolumeSerializerTest(test.TestCase): def _verify_volume_attachment(self, attach, tree): @@ -904,34 +922,3 @@ class TestVolumeCreateRequestXMLDeserializer(test.TestCase): }, } self.assertEquals(request['body'], expected) - - -class VolumesUnprocessableEntityTestCase(test.TestCase): - - """ - Tests of places we throw 422 Unprocessable Entity from - """ - - def setUp(self): - super(VolumesUnprocessableEntityTestCase, self).setUp() - self.ext_mgr = extensions.ExtensionManager() - self.ext_mgr.extensions = {} - self.controller = volumes.VolumeController(self.ext_mgr) - - def _unprocessable_volume_create(self, body): - req = fakes.HTTPRequest.blank('/v2/fake/volumes') - req.method = 'POST' - - self.assertRaises(webob.exc.HTTPUnprocessableEntity, - self.controller.create, req, body) - - def test_create_no_body(self): - self._unprocessable_volume_create(body=None) - - def test_create_missing_volume(self): - body = {'foo': {'a': 'b'}} - self._unprocessable_volume_create(body=body) - - def test_create_malformed_entity(self): - body = {'volume': 'string'} - self._unprocessable_volume_create(body=body) -- 2.45.2