From b644a0de07ccbaa2561739831ebb0f9c0d929512 Mon Sep 17 00:00:00 2001 From: ghanshyam Date: Tue, 12 Jul 2016 09:50:40 +0900 Subject: [PATCH] Fix PUT server tag 201 to return empty content Update server tag API (PUT /servers/{server_id}/tags/{tag}) return the 201 and 204 success code if tag is created or already exist respectively. But in former case, API return response body with plain text- '{"code": "201 Created", "message": "

\\n\\n\\n", "title": "Created"}' This is because it use webob.exc to generate 201. This kind of response body may break parsing of the data for content type. Also this is consistent with other nova API. webob.Response is the right choice to generate 201 without content. Change-Id: Ib0958a7a99d866a38485715b5b613914e60ad425 Closes-Bug: #1602044 --- nova/api/openstack/compute/server_tags.py | 36 +++++++++---------- .../api_sample_tests/test_server_tags.py | 1 + .../api/openstack/compute/test_server_tags.py | 2 ++ 3 files changed, 21 insertions(+), 18 deletions(-) diff --git a/nova/api/openstack/compute/server_tags.py b/nova/api/openstack/compute/server_tags.py index 7461875f411f..a229e7b26c43 100644 --- a/nova/api/openstack/compute/server_tags.py +++ b/nova/api/openstack/compute/server_tags.py @@ -11,7 +11,7 @@ # under the License. import jsonschema -from webob import exc +import webob from nova.api.openstack import common from nova.api.openstack.compute.schemas import server_tags as schema @@ -62,12 +62,12 @@ class ServerTagsController(wsgi.Controller): try: exists = objects.Tag.exists(context, server_id, id) except exception.InstanceNotFound as e: - raise exc.HTTPNotFound(explanation=e.format_message()) + raise webob.exc.HTTPNotFound(explanation=e.format_message()) if not exists: msg = (_("Server %(server_id)s has no tag '%(tag)s'") % {'server_id': server_id, 'tag': id}) - raise exc.HTTPNotFound(explanation=msg) + raise webob.exc.HTTPNotFound(explanation=msg) @wsgi.Controller.api_version("2.26") @extensions.expected_errors(404) @@ -78,7 +78,7 @@ class ServerTagsController(wsgi.Controller): try: tags = objects.TagList.get_by_resource_id(context, server_id) except exception.InstanceNotFound as e: - raise exc.HTTPNotFound(explanation=e.format_message()) + raise webob.exc.HTTPNotFound(explanation=e.format_message()) return {'tags': _get_tags_names(tags)} @@ -96,36 +96,36 @@ class ServerTagsController(wsgi.Controller): msg = (_("Tag '%(tag)s' is invalid. It must be a string without " "characters '/' and ','. Validation error message: " "%(err)s") % {'tag': id, 'err': e.message}) - raise exc.HTTPBadRequest(explanation=msg) + raise webob.exc.HTTPBadRequest(explanation=msg) try: tags = objects.TagList.get_by_resource_id(context, server_id) except exception.InstanceNotFound as e: - raise exc.HTTPNotFound(explanation=e.format_message()) + raise webob.exc.HTTPNotFound(explanation=e.format_message()) if len(tags) >= objects.instance.MAX_TAG_COUNT: msg = (_("The number of tags exceeded the per-server limit %d") % objects.instance.MAX_TAG_COUNT) - raise exc.HTTPBadRequest(explanation=msg) + raise webob.exc.HTTPBadRequest(explanation=msg) if len(id) > objects.tag.MAX_TAG_LENGTH: msg = (_("Tag '%(tag)s' is too long. Maximum length of a tag " "is %(length)d") % {'tag': id, 'length': objects.tag.MAX_TAG_LENGTH}) - raise exc.HTTPBadRequest(explanation=msg) + raise webob.exc.HTTPBadRequest(explanation=msg) if id in _get_tags_names(tags): # NOTE(snikitin): server already has specified tag - return exc.HTTPNoContent() + return webob.Response(status_int=204) tag = objects.Tag(context=context, resource_id=server_id, tag=id) try: tag.create() except exception.InstanceNotFound as e: - raise exc.HTTPNotFound(explanation=e.format_message()) + raise webob.exc.HTTPNotFound(explanation=e.format_message()) - response = exc.HTTPCreated() + response = webob.Response(status_int=201) response.headers['Location'] = self._view_builder.get_location( req, server_id, id) return response @@ -147,7 +147,7 @@ class ServerTagsController(wsgi.Controller): if invalid_tags: msg = (_("Tags '%s' are invalid. Each tag must be a string " "without characters '/' and ','.") % invalid_tags) - raise exc.HTTPBadRequest(explanation=msg) + raise webob.exc.HTTPBadRequest(explanation=msg) tag_count = len(body['tags']) if tag_count > objects.instance.MAX_TAG_COUNT: @@ -155,7 +155,7 @@ class ServerTagsController(wsgi.Controller): "%(max)d. The number of tags in request is %(count)d.") % {'max': objects.instance.MAX_TAG_COUNT, 'count': tag_count}) - raise exc.HTTPBadRequest(explanation=msg) + raise webob.exc.HTTPBadRequest(explanation=msg) long_tags = [ t for t in body['tags'] if len(t) > objects.tag.MAX_TAG_LENGTH] @@ -163,12 +163,12 @@ class ServerTagsController(wsgi.Controller): msg = (_("Tags %(tags)s are too long. Maximum length of a tag " "is %(length)d") % {'tags': long_tags, 'length': objects.tag.MAX_TAG_LENGTH}) - raise exc.HTTPBadRequest(explanation=msg) + raise webob.exc.HTTPBadRequest(explanation=msg) try: tags = objects.TagList.create(context, server_id, body['tags']) except exception.InstanceNotFound as e: - raise exc.HTTPNotFound(explanation=e.format_message()) + raise webob.exc.HTTPNotFound(explanation=e.format_message()) return {'tags': _get_tags_names(tags)} @@ -183,9 +183,9 @@ class ServerTagsController(wsgi.Controller): try: objects.Tag.destroy(context, server_id, id) except exception.InstanceTagNotFound as e: - raise exc.HTTPNotFound(explanation=e.format_message()) + raise webob.exc.HTTPNotFound(explanation=e.format_message()) except exception.InstanceNotFound as e: - raise exc.HTTPNotFound(explanation=e.format_message()) + raise webob.exc.HTTPNotFound(explanation=e.format_message()) @wsgi.Controller.api_version("2.26") @wsgi.response(204) @@ -198,7 +198,7 @@ class ServerTagsController(wsgi.Controller): try: objects.TagList.destroy(context, server_id) except exception.InstanceNotFound as e: - raise exc.HTTPNotFound(explanation=e.format_message()) + raise webob.exc.HTTPNotFound(explanation=e.format_message()) class ServerTags(extensions.V21APIExtensionBase): diff --git a/nova/tests/functional/api_sample_tests/test_server_tags.py b/nova/tests/functional/api_sample_tests/test_server_tags.py index bf0884a4be7f..683098902ccb 100644 --- a/nova/tests/functional/api_sample_tests/test_server_tags.py +++ b/nova/tests/functional/api_sample_tests/test_server_tags.py @@ -90,6 +90,7 @@ class ServerTagsJsonTest(test_servers.ServersSampleBase): expected_location = "%s/servers/%s/tags/%s" % ( self._get_vers_compute_endpoint(), uuid, tag.tag) self.assertEqual(expected_location, response.headers['Location']) + self.assertEqual('', response.content) def test_server_tags_delete(self): uuid = self._put_server_tags() diff --git a/nova/tests/unit/api/openstack/compute/test_server_tags.py b/nova/tests/unit/api/openstack/compute/test_server_tags.py index 057acb5dddd8..a5269b7bad02 100644 --- a/nova/tests/unit/api/openstack/compute/test_server_tags.py +++ b/nova/tests/unit/api/openstack/compute/test_server_tags.py @@ -162,6 +162,7 @@ class ServerTagsTest(test.TestCase): res = self.controller.update(req, UUID, TAG2, body=None) self.assertEqual(201, res.status_int) + self.assertEqual(0, len(res.body)) self.assertEqual(location, res.headers['Location']) mock_db_add_inst_tags.assert_called_once_with(context, UUID, TAG2) mock_db_get_inst_tags.assert_called_once_with(context, UUID) @@ -177,6 +178,7 @@ class ServerTagsTest(test.TestCase): res = self.controller.update(req, UUID, TAG1, body=None) self.assertEqual(204, res.status_int) + self.assertEqual(0, len(res.body)) mock_db_get_inst_tags.assert_called_once_with(context, UUID) @mock.patch('nova.db.instance_tag_get_by_instance_uuid')