From d2c2060c0fe0892540c3e8348a328ffe1a33315c Mon Sep 17 00:00:00 2001 From: Tobias Urdin Date: Fri, 4 Dec 2020 02:14:09 +0100 Subject: [PATCH] Set json template for DELETE requests Setting expose to json will cause content-type internally for Pecan to be set to application/json. If this is not set Pecan will throw a NotAcceptable error if it cannot parse or understand what type of request it is. This does not affect the content-type in the response for DELETE calls. (cherry picked from commit ba5d73d5009c5fe5b71053546a240883a9719719) --- gnocchi/rest/api.py | 10 ++++---- gnocchi/tests/test_rest.py | 51 ++++++++++++++++++++++---------------- 2 files changed, 34 insertions(+), 27 deletions(-) diff --git a/gnocchi/rest/api.py b/gnocchi/rest/api.py index 836d7b2ef..2e66d4c2f 100644 --- a/gnocchi/rest/api.py +++ b/gnocchi/rest/api.py @@ -279,7 +279,7 @@ def patch(self): except indexer.UnsupportedArchivePolicyChange as e: abort(400, six.text_type(e)) - @pecan.expose() + @pecan.expose('json') def delete(self): # NOTE(jd) I don't think there's any point in fetching and passing the # archive policy here, as the rule is probably checking the actual role @@ -407,7 +407,7 @@ def patch(self): except indexer.UnsupportedArchivePolicyRuleChange as e: abort(400, six.text_type(e)) - @pecan.expose() + @pecan.expose('json') def delete(self): # NOTE(jd) I don't think there's any point in fetching and passing the # archive policy rule here, as the rule is probably checking the actual @@ -527,7 +527,7 @@ def get_measures(self, start=None, stop=None, aggregation='mean', except storage.MetricDoesNotExist: return [] - @pecan.expose() + @pecan.expose('json') def delete(self): self.enforce_metric("delete metric") try: @@ -922,7 +922,7 @@ def patch(self): except indexer.NoSuchResourceType as e: abort(400, six.text_type(e)) - @pecan.expose() + @pecan.expose('json') def delete(self): try: pecan.request.indexer.get_resource_type(self._name) @@ -1051,7 +1051,7 @@ def patch(self): etag_set_headers(resource) return resource - @pecan.expose() + @pecan.expose('json') def delete(self): resource = pecan.request.indexer.get_resource( self._resource_type, self.id) diff --git a/gnocchi/tests/test_rest.py b/gnocchi/tests/test_rest.py index 142343d71..392732980 100644 --- a/gnocchi/tests/test_rest.py +++ b/gnocchi/tests/test_rest.py @@ -389,7 +389,8 @@ def test_delete_metric_another_user(self): params={"archive_policy_name": "medium"}) metric = json.loads(result.text) with self.app.use_another_user(): - self.app.delete("/v1/metric/" + metric['id'], status=403) + r = self.app.delete("/v1/metric/" + metric['id'], status=403) + self.assertEqual("text/plain", r.content_type) def test_add_measure_with_another_user(self): result = self.app.post_json("/v1/metric", @@ -976,18 +977,20 @@ def test_delete_resource_named_metric(self): self.attributes['metrics'] = {'foo': {'archive_policy_name': "high"}} self.app.post_json("/v1/resource/" + self.resource_type, params=self.attributes) - self.app.delete("/v1/resource/" - + self.resource_type - + "/" - + self.attributes['id'] - + "/metric/foo", - status=204) - self.app.delete("/v1/resource/" - + self.resource_type - + "/" - + self.attributes['id'] - + "/metric/foo/measures", - status=404) + r = self.app.delete("/v1/resource/" + + self.resource_type + + "/" + + self.attributes['id'] + + "/metric/foo", + status=204) + self.assertEqual(None, r.content_type) + r = self.app.delete("/v1/resource/" + + self.resource_type + + "/" + + self.attributes['id'] + + "/metric/foo/measures", + status=404) + self.assertEqual("text/plain", r.content_type) def test_get_resource_unknown_named_metric(self): self.app.post_json("/v1/resource/" + self.resource_type, @@ -1246,9 +1249,10 @@ def test_delete_resource(self): self.app.get("/v1/resource/" + self.resource_type + "/" + self.attributes['id'], status=200) - self.app.delete("/v1/resource/" + self.resource_type + "/" - + self.attributes['id'], - status=204) + r = self.app.delete("/v1/resource/" + self.resource_type + "/" + + self.attributes['id'], + status=204) + self.assertEqual(None, r.content_type) self.app.get("/v1/resource/" + self.resource_type + "/" + self.attributes['id'], status=404) @@ -1267,9 +1271,10 @@ def test_delete_resource_with_metrics(self): self.app.get("/v1/resource/" + self.resource_type + "/" + self.attributes['id'], status=200) - self.app.delete("/v1/resource/" + self.resource_type + "/" - + self.attributes['id'], - status=204) + r = self.app.delete("/v1/resource/" + self.resource_type + "/" + + self.attributes['id'], + status=204) + self.assertEqual(None, r.content_type) self.app.get("/v1/resource/" + self.resource_type + "/" + self.attributes['id'], status=404) @@ -1280,14 +1285,16 @@ def test_delete_resource_unauthorized(self): self.app.post_json("/v1/resource/" + self.resource_type, params=self.attributes) with self.app.use_another_user(): - self.app.delete("/v1/resource/" + self.resource_type + "/" - + self.attributes['id'], - status=403) + r = self.app.delete("/v1/resource/" + self.resource_type + "/" + + self.attributes['id'], + status=403) + self.assertEqual("text/plain", r.content_type) def test_delete_resource_non_existent(self): result = self.app.delete("/v1/resource/" + self.resource_type + "/" + self.attributes['id'], status=404) + self.assertEqual("text/plain", result.content_type) self.assertIn( "Resource %s does not exist" % self.attributes['id'], result.text)