Skip to content

Commit

Permalink
Return BadRequest instead of UnprocessableEntity for volumes API
Browse files Browse the repository at this point in the history
Changes exception from UnprocessableEntity to BadRequest when
input validation fails. Also raise BadRequest instead of a nova
exception InvalidParameterValue within the SnapshotController
create method. This is some preparation work to make sharing
unittests between V2.1 and V2 easier.

Partially implements blueprint v2-on-v3-api

Change-Id: I5c5b6f9680378d51f7fea054b52b99cd6cbe54b0
  • Loading branch information
Chris Yeoh committed Aug 25, 2014
1 parent 7fb7a2c commit eb59514
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 19 deletions.
14 changes: 9 additions & 5 deletions nova/api/openstack/compute/contrib/volumes.py
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,8 @@ def create(self, req, body):
authorize(context)

if not self.is_valid_body(body, 'volume'):
raise exc.HTTPUnprocessableEntity()
msg = _("volume not specified")
raise exc.HTTPBadRequest(explanation=msg)

vol = body['volume']

Expand Down Expand Up @@ -392,7 +393,8 @@ def create(self, req, server_id, body):
authorize_attach(context, action='create')

if not self.is_valid_body(body, 'volumeAttachment'):
raise exc.HTTPUnprocessableEntity()
msg = _("volumeAttachment not specified")
raise exc.HTTPBadRequest(explanation=msg)
try:
volume_id = body['volumeAttachment']['volumeId']
except KeyError:
Expand Down Expand Up @@ -449,7 +451,8 @@ def update(self, req, server_id, id, body):
authorize_attach(context, action='update')

if not self.is_valid_body(body, 'volumeAttachment'):
raise exc.HTTPUnprocessableEntity()
msg = _("volumeAttachment not specified")
raise exc.HTTPBadRequest(explanation=msg)

old_volume_id = id
old_volume = self.volume_api.get(context, old_volume_id)
Expand Down Expand Up @@ -681,7 +684,8 @@ def create(self, req, body):
authorize(context)

if not self.is_valid_body(body, 'snapshot'):
raise exc.HTTPUnprocessableEntity()
msg = _("snapshot not specified")
raise exc.HTTPBadRequest(explanation=msg)

snapshot = body['snapshot']
volume_id = snapshot['volume_id']
Expand All @@ -694,7 +698,7 @@ def create(self, req, body):
force = strutils.bool_from_string(force, strict=True)
except ValueError:
msg = _("Invalid value '%s' for force.") % force
raise exception.InvalidParameterValue(err=msg)
raise exc.HTTPBadRequest(explanation=msg)

if force:
create_func = self.volume_api.create_snapshot_force
Expand Down
28 changes: 14 additions & 14 deletions nova/tests/api/openstack/compute/contrib/test_volumes.py
Original file line number Diff line number Diff line change
Expand Up @@ -843,60 +843,60 @@ def test_full_volume(self):
self.assertEqual(request['body'], expected)


class CommonUnprocessableEntityTestCase(object):
class CommonBadRequestTestCase(object):

resource = None
entity_name = None
controller_cls = None
kwargs = {}

"""
Tests of places we throw 422 Unprocessable Entity from
Tests of places we throw 400 Bad Request from
"""

def setUp(self):
super(CommonUnprocessableEntityTestCase, self).setUp()
super(CommonBadRequestTestCase, self).setUp()
self.controller = self.controller_cls()

def _unprocessable_create(self, body):
def _bad_request_create(self, body):
req = fakes.HTTPRequest.blank('/v2/fake/' + self.resource)
req.method = 'POST'

kwargs = self.kwargs.copy()
kwargs['body'] = body
self.assertRaises(webob.exc.HTTPUnprocessableEntity,
self.assertRaises(webob.exc.HTTPBadRequest,
self.controller.create, req, **kwargs)

def test_create_no_body(self):
self._unprocessable_create(body=None)
self._bad_request_create(body=None)

def test_create_missing_volume(self):
body = {'foo': {'a': 'b'}}
self._unprocessable_create(body=body)
self._bad_request_create(body=body)

def test_create_malformed_entity(self):
body = {self.entity_name: 'string'}
self._unprocessable_create(body=body)
self._bad_request_create(body=body)


class UnprocessableVolumeTestCase(CommonUnprocessableEntityTestCase,
test.TestCase):
class BadRequestVolumeTestCase(CommonBadRequestTestCase,
test.TestCase):

resource = 'os-volumes'
entity_name = 'volume'
controller_cls = volumes.VolumeController


class UnprocessableAttachmentTestCase(CommonUnprocessableEntityTestCase,
test.TestCase):
class BadRequestAttachmentTestCase(CommonBadRequestTestCase,
test.TestCase):

resource = 'servers/' + FAKE_UUID + '/os-volume_attachments'
entity_name = 'volumeAttachment'
controller_cls = volumes.VolumeAttachmentController
kwargs = {'server_id': FAKE_UUID}


class UnprocessableSnapshotTestCase(CommonUnprocessableEntityTestCase,
class BadRequestSnapshotTestCase(CommonBadRequestTestCase,
test.TestCase):

resource = 'os-snapshots'
Expand Down Expand Up @@ -941,7 +941,7 @@ def test_force_false(self):

def test_force_invalid(self):
self.body['snapshot']['force'] = 'foo'
self.assertRaises(exception.InvalidParameterValue,
self.assertRaises(exc.HTTPBadRequest,
self.controller.create, self.req, body=self.body)


Expand Down

0 comments on commit eb59514

Please sign in to comment.