Skip to content

Commit

Permalink
api: return 404 if a resource is not found
Browse files Browse the repository at this point in the history
This also checks for the error message that is returned, and fixes a
problem with the error encoding middleware that was doing double JSON
encoding.

Change-Id: Ieb39a991ddc9ecba0a7e71450a1e57ede18ccbe6
Fixes-Bug: #1218760
Fixes-Bug: #1208552
  • Loading branch information
jd committed Sep 27, 2013
1 parent 12eab1c commit acb89bd
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 22 deletions.
24 changes: 14 additions & 10 deletions ceilometer/api/controllers/v2.py
Expand Up @@ -2,10 +2,12 @@
#
# Copyright © 2012 New Dream Network, LLC (DreamHost)
# Copyright 2013 IBM Corp.
# Copyright © 2013 eNovance <licensing@enovance.com>
#
# Author: Doug Hellmann <doug.hellmann@dreamhost.com>
# Angus Salkeld <asalkeld@redhat.com>
# Eoghan Glynn <eglynn@redhat.com>
# Authors: Doug Hellmann <doug.hellmann@dreamhost.com>
# Angus Salkeld <asalkeld@redhat.com>
# Eoghan Glynn <eglynn@redhat.com>
# Julien Danjou <julien@danjou.info>
#
# Licensed under the Apache License, Version 2.0 (the "License"); you may
# not use this file except in compliance with the License. You may obtain
Expand Down Expand Up @@ -864,6 +866,14 @@ def sample(cls):
)


class ResourceNotFound(Exception):
code = 404

def __init__(self, id):
super(ResourceNotFound, self).__init__(
_("Resource %s Not Found") % id)


class ResourcesController(rest.RestController):
"""Works on resources."""

Expand All @@ -886,14 +896,8 @@ def get_one(self, resource_id):
authorized_project = acl.get_limited_to_project(pecan.request.headers)
resources = list(pecan.request.storage_conn.get_resources(
resource=resource_id, project=authorized_project))
# FIXME (flwang): Need to change this to return a 404 error code when
# we get a release of WSME that supports it.
if not resources:
error = _("Unknown resource")
pecan.response.translatable_error = error
raise wsme.exc.InvalidInput("resource_id",
resource_id,
unicode(error))
raise ResourceNotFound(resource_id)
return Resource.from_db_and_links(resources[0],
self._resource_links(resource_id))

Expand Down
2 changes: 1 addition & 1 deletion ceilometer/api/middleware.py
Expand Up @@ -125,7 +125,7 @@ def replacement_start_response(status, headers, exc_info=None):
fault['faultstring'] = (
gettextutils.get_localized_message(
error, user_locale))
body = [json.dumps({'error_message': json.dumps(fault)})]
body = [json.dumps({'error_message': fault})]
except ValueError as err:
body = [json.dumps({'error_message': '\n'.join(app_iter)})]
state['headers'].append(('Content-Type', 'application/json'))
Expand Down
12 changes: 5 additions & 7 deletions tests/api/v2/test_alarm_scenarios.py
Expand Up @@ -243,9 +243,8 @@ def test_post_alarm_wsme_workaround(self):
resp = self.post_json('/alarms', params=json, expect_errors=True,
status=400, headers=self.auth_headers)
self.assertEqual(
resp.json['error_message'],
'{"debuginfo": null, "faultcode": "Client", "faultstring": '
'"%s is mandatory"}' % field)
resp.json['error_message']['faultstring'],
'%s is mandatory' % field)
alarms = list(self.conn.get_alarms())
self.assertEqual(4, len(alarms))

Expand Down Expand Up @@ -323,10 +322,9 @@ def test_post_invalid_alarm_have_multiple_rules(self):
alarms = list(self.conn.get_alarms())
self.assertEqual(4, len(alarms))
self.assertEqual(
resp.json['error_message'],
'{"debuginfo": null, "faultcode": "Client", "faultstring": '
'"threshold_rule and combination_rule cannot '
'be set at the same time"}')
resp.json['error_message']['faultstring'],
'threshold_rule and combination_rule cannot '
'be set at the same time')

def test_post_alarm_defaults(self):
to_check = {
Expand Down
5 changes: 2 additions & 3 deletions tests/api/v2/test_app.py
Expand Up @@ -18,7 +18,6 @@
# under the License.
"""Test basic ceilometer-api app
"""
import json
import os

from oslo.config import cfg
Expand Down Expand Up @@ -141,8 +140,8 @@ def test_json_parsable_error_middleware_translation_400(self):
self.assertEqual(response.status_int, 400)
self.assertEqual(response.content_type, "application/json")
self.assertTrue(response.json['error_message'])
fault = json.loads(response.json['error_message'])
self.assertEqual(fault['faultstring'], self.no_lang_translated_error)
self.assertEqual(response.json['error_message']['faultstring'],
self.no_lang_translated_error)

def test_xml_parsable_error_middleware_404(self):
response = self.get_json('/invalid_path',
Expand Down
6 changes: 5 additions & 1 deletion tests/api/v2/test_list_resources_scenarios.py
Expand Up @@ -19,6 +19,7 @@
"""

import datetime
import json
import logging
import testscenarios

Expand Down Expand Up @@ -249,7 +250,10 @@ def test_with_invalid_resource_id(self):
self.assertEqual(resp2["resource_id"], "resource-id-2")

resp3 = self.get_json('/resources/resource-id-3', expect_errors=True)
self.assertEqual(resp3.status_code, 400)
self.assertEqual(resp3.status_code, 404)
self.assertEqual(json.loads(resp3.body)['error_message']
['faultstring'],
"Resource resource-id-3 Not Found")

def test_with_user(self):
sample1 = sample.Sample(
Expand Down

0 comments on commit acb89bd

Please sign in to comment.