From f3ad8bc784f829ae2f4561bef636b624c9d61099 Mon Sep 17 00:00:00 2001 From: yanyanhu Date: Tue, 12 Jul 2016 04:34:32 -0400 Subject: [PATCH] Fix delete API of profile/policy/receiver resources This patch revises implementation of profile/policy/receiver delete API interfaces: HTTPNoContent exception will be raised to generate 204 response code. Current implementation based on extra succeeded code will cause incorrect Content-Length which should be 0. Related tempest API tests are also improved to verify this change. This patch also fixes an error that exception could be incorrectly disguised twice. Change-Id: I8c3517f9bdc4499d90858b865e6c2d6d43fc810c Closes-Bug: #1602109 --- senlin/api/common/wsgi.py | 2 +- senlin/api/openstack/v1/policies.py | 1 + senlin/api/openstack/v1/profiles.py | 1 + senlin/api/openstack/v1/receivers.py | 1 + senlin/api/openstack/v1/router.py | 9 +++------ senlin/tests/tempest/api/policies/test_policy_delete.py | 1 + senlin/tests/tempest/api/profiles/test_profile_delete.py | 1 + .../tests/tempest/api/receivers/test_receiver_delete.py | 1 + senlin/tests/tempest/common/clustering_client.py | 9 +++++++-- senlin/tests/unit/api/openstack/v1/test_policies.py | 3 ++- senlin/tests/unit/api/openstack/v1/test_profiles.py | 3 ++- senlin/tests/unit/api/openstack/v1/test_receivers.py | 3 ++- senlin/tests/unit/api/openstack/v1/test_router.py | 9 +++------ 13 files changed, 26 insertions(+), 18 deletions(-) diff --git a/senlin/api/common/wsgi.py b/senlin/api/common/wsgi.py index 9d87ae4ea..f95698b68 100644 --- a/senlin/api/common/wsgi.py +++ b/senlin/api/common/wsgi.py @@ -661,7 +661,7 @@ class Resource(object): # openstacksdk's need. http_exc = translate_exception(err, request.best_match_language()) - raise exception.HTTPExceptionDisguise(http_exc) + raise http_exc if isinstance(err, exc.HTTPServerError): LOG.error( _LE("Returning %(code)s to user: %(explanation)s"), diff --git a/senlin/api/openstack/v1/policies.py b/senlin/api/openstack/v1/policies.py index f6503c422..109bd65d1 100644 --- a/senlin/api/openstack/v1/policies.py +++ b/senlin/api/openstack/v1/policies.py @@ -114,3 +114,4 @@ class PolicyController(wsgi.Controller): @util.policy_enforce def delete(self, req, policy_id): self.rpc_client.policy_delete(req.context, policy_id, cast=False) + raise exc.HTTPNoContent() diff --git a/senlin/api/openstack/v1/profiles.py b/senlin/api/openstack/v1/profiles.py index 8f69b0b84..77cde939e 100644 --- a/senlin/api/openstack/v1/profiles.py +++ b/senlin/api/openstack/v1/profiles.py @@ -137,3 +137,4 @@ class ProfileController(wsgi.Controller): @util.policy_enforce def delete(self, req, profile_id): self.rpc_client.profile_delete(req.context, profile_id, cast=False) + raise exc.HTTPNoContent() diff --git a/senlin/api/openstack/v1/receivers.py b/senlin/api/openstack/v1/receivers.py index 581f567d6..277395c9b 100644 --- a/senlin/api/openstack/v1/receivers.py +++ b/senlin/api/openstack/v1/receivers.py @@ -130,3 +130,4 @@ class ReceiverController(wsgi.Controller): @util.policy_enforce def delete(self, req, receiver_id): self.rpc_client.receiver_delete(req.context, receiver_id, cast=False) + raise exc.HTTPNoContent() diff --git a/senlin/api/openstack/v1/router.py b/senlin/api/openstack/v1/router.py index a18fc8d8b..178644ddc 100644 --- a/senlin/api/openstack/v1/router.py +++ b/senlin/api/openstack/v1/router.py @@ -82,8 +82,7 @@ class API(wsgi.Router): sub_mapper.connect("profile_delete", "/profiles/{profile_id}", action="delete", - conditions={'method': 'DELETE'}, - success=204) + conditions={'method': 'DELETE'}) # Policy Types res = wsgi.Resource(policy_types.PolicyTypeController(conf)) @@ -122,8 +121,7 @@ class API(wsgi.Router): sub_mapper.connect("policy_delete", "/policies/{policy_id}", action="delete", - conditions={'method': 'DELETE'}, - success=204) + conditions={'method': 'DELETE'}) # Clusters res = wsgi.Resource(clusters.ClusterController(conf)) @@ -247,8 +245,7 @@ class API(wsgi.Router): sub_mapper.connect("receiver_delete", "/receivers/{receiver_id}", action="delete", - conditions={'method': 'DELETE'}, - success=204) + conditions={'method': 'DELETE'}) # Webhooks res = wsgi.Resource(webhooks.WebhookController(conf)) diff --git a/senlin/tests/tempest/api/policies/test_policy_delete.py b/senlin/tests/tempest/api/policies/test_policy_delete.py index 286d87a43..381d926e1 100644 --- a/senlin/tests/tempest/api/policies/test_policy_delete.py +++ b/senlin/tests/tempest/api/policies/test_policy_delete.py @@ -28,3 +28,4 @@ class TestPolicyDelete(base.BaseSenlinAPITest): res = self.client.delete_obj('policies', self.policy_id) self.assertEqual(204, res['status']) self.assertIsNone(res['body']) + self.assertEqual('0', res['content-length']) diff --git a/senlin/tests/tempest/api/profiles/test_profile_delete.py b/senlin/tests/tempest/api/profiles/test_profile_delete.py index 980bf0e64..916e520a6 100644 --- a/senlin/tests/tempest/api/profiles/test_profile_delete.py +++ b/senlin/tests/tempest/api/profiles/test_profile_delete.py @@ -28,3 +28,4 @@ class TestProfileDelete(base.BaseSenlinAPITest): res = self.client.delete_obj('profiles', self.profile_id) self.assertEqual(204, res['status']) self.assertIsNone(res['body']) + self.assertEqual('0', res['content-length']) diff --git a/senlin/tests/tempest/api/receivers/test_receiver_delete.py b/senlin/tests/tempest/api/receivers/test_receiver_delete.py index 8691603c0..a4475eae0 100644 --- a/senlin/tests/tempest/api/receivers/test_receiver_delete.py +++ b/senlin/tests/tempest/api/receivers/test_receiver_delete.py @@ -35,3 +35,4 @@ class TestReceiverDelete(base.BaseSenlinAPITest): res = self.client.delete_obj('receivers', self.receiver_id) self.assertEqual(204, res['status']) self.assertIsNone(res['body']) + self.assertEqual('0', res['content-length']) diff --git a/senlin/tests/tempest/common/clustering_client.py b/senlin/tests/tempest/common/clustering_client.py index 4bb065bdb..c5adae2df 100644 --- a/senlin/tests/tempest/common/clustering_client.py +++ b/senlin/tests/tempest/common/clustering_client.py @@ -29,10 +29,15 @@ class ClusteringAPIClient(rest_client.RestClient): version = 'v1' def _parsed_resp(self, resp, body): + # Parse status code and location res = { - 'status': int(resp['status']), - 'location': resp.get('location', None), + 'status': int(resp.pop('status')), + 'location': resp.pop('location', None) } + # Parse other keys included in resp + res.update(resp) + + # Parse body if body and str(body) != 'null': res['body'] = self._parse_resp(body) else: diff --git a/senlin/tests/unit/api/openstack/v1/test_policies.py b/senlin/tests/unit/api/openstack/v1/test_policies.py index a39c6a4da..9d3af74b7 100644 --- a/senlin/tests/unit/api/openstack/v1/test_policies.py +++ b/senlin/tests/unit/api/openstack/v1/test_policies.py @@ -481,7 +481,8 @@ class PolicyControllerTest(shared.ControllerTest, base.SenlinTestCase): mock_call = self.patchobject(rpc_client.EngineClient, 'call', return_value=None) - self.controller.delete(req, policy_id=pid) + self.assertRaises(exc.HTTPNoContent, + self.controller.delete, req, policy_id=pid) mock_call.assert_called_with( req.context, ('policy_delete', {'identity': pid})) diff --git a/senlin/tests/unit/api/openstack/v1/test_profiles.py b/senlin/tests/unit/api/openstack/v1/test_profiles.py index 7e14368c6..3163690c8 100644 --- a/senlin/tests/unit/api/openstack/v1/test_profiles.py +++ b/senlin/tests/unit/api/openstack/v1/test_profiles.py @@ -537,7 +537,8 @@ class ProfileControllerTest(shared.ControllerTest, base.SenlinTestCase): mock_call = self.patchobject(rpc_client.EngineClient, 'call', return_value=None) - self.controller.delete(req, profile_id=pid) + self.assertRaises(exc.HTTPNoContent, + self.controller.delete, req, profile_id=pid) mock_call.assert_called_with( req.context, ('profile_delete', {'identity': pid})) diff --git a/senlin/tests/unit/api/openstack/v1/test_receivers.py b/senlin/tests/unit/api/openstack/v1/test_receivers.py index 9b081c059..f731ad907 100644 --- a/senlin/tests/unit/api/openstack/v1/test_receivers.py +++ b/senlin/tests/unit/api/openstack/v1/test_receivers.py @@ -495,7 +495,8 @@ class ReceiverControllerTest(shared.ControllerTest, base.SenlinTestCase): mock_call = self.patchobject(rpc_client.EngineClient, 'call', return_value=None) - self.controller.delete(req, receiver_id=wid) + self.assertRaises(exc.HTTPNoContent, + self.controller.delete, req, receiver_id=wid) mock_call.assert_called_with( req.context, diff --git a/senlin/tests/unit/api/openstack/v1/test_router.py b/senlin/tests/unit/api/openstack/v1/test_router.py index 0b98a3509..1a41eea92 100644 --- a/senlin/tests/unit/api/openstack/v1/test_router.py +++ b/senlin/tests/unit/api/openstack/v1/test_router.py @@ -106,8 +106,7 @@ class RoutesTest(base.SenlinTestCase): 'delete', 'ProfileController', { - 'profile_id': 'bbbb', - 'success': '204' + 'profile_id': 'bbbb' }) def test_policy_types_handling(self): @@ -173,8 +172,7 @@ class RoutesTest(base.SenlinTestCase): 'delete', 'PolicyController', { - 'policy_id': 'bbbb', - 'success': '204', + 'policy_id': 'bbbb' }) def test_cluster_collection(self): @@ -384,8 +382,7 @@ class RoutesTest(base.SenlinTestCase): 'delete', 'ReceiverController', { - 'receiver_id': 'bbbb', - 'success': '204', + 'receiver_id': 'bbbb' }) def test_webhook_collection(self):