Skip to content

Commit

Permalink
Set json template for DELETE requests
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
tobias-urdin committed Dec 4, 2020
1 parent 5eeb3f0 commit ba5d73d
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 27 deletions.
10 changes: 5 additions & 5 deletions gnocchi/rest/api.py
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -919,7 +919,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)
Expand Down Expand Up @@ -1048,7 +1048,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)
Expand Down
51 changes: 29 additions & 22 deletions gnocchi/tests/test_rest.py
Expand Up @@ -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",
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand Down

0 comments on commit ba5d73d

Please sign in to comment.