openstack/deb-python-falcon
Code Issues Proposed changes

fix(HTTPError): Remove dependency on blank title to determine serialization

Browse Source 
This patch attempts to make the code easier to understand and maintain
going forward by removing the dependency between the title attribute and
the "serializable" property. The "serializable" property was renamed to
be more indicitive of the context in which it is actually used.

Closes #265
This commit is contained in:
Kurt Griffiths
2014-10-16 18:22:57 -05:00
parent d01af249b2
commit 4be86d1a60
5 changed files with 53 additions and 46 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
falcon/api.py
View File

@@ -371,7 +371,7 @@ class API(object):
if error.headers is not None:
resp.set_headers(error.headers)
if error.serializable:
if error.has_representation:
media_type, body = self._serialize_error(req, error)
if body is not None:

28
falcon/exceptions.py
View File

@@ -94,7 +94,11 @@ class HTTPNotFound(HTTPError):
"""
def __init__(self):
HTTPError.__init__(self, status.HTTP_404, None, None)
HTTPError.__init__(self, status.HTTP_404)
@property
def has_representation(self):
return False
class HTTPMethodNotAllowed(HTTPError):
@@ -116,21 +120,27 @@ class HTTPMethodNotAllowed(HTTPError):
HTTPError.__init__(self, status.HTTP_405, 'Method not allowed',
**kwargs)
# NOTE(kgriffs): Trigger an empty body in the response; 405
# responses don't usually have bodies, so we only send one
# if the caller indicates they want one by providing some
# details in the kwargs.
if kwargs:
title = 'Method not allowed'
self._has_representation = True
else:
# NOTE(kgriffs): Trigger an empty body in the response; 405
# responses don't usually have bodies, so we only send one
# if the caller indicates they want one, by way of specifying
# a description, href, and/or other details.
title = None
self._has_representation = False
# NOTE(kgriffs): Inject the "Allow" header so it will be included
# in the HTTP response.
headers = kwargs.setdefault('headers', {})
headers['Allow'] = ', '.join(allowed_methods)
HTTPError.__init__(self, status.HTTP_405, title, **kwargs)
HTTPError.__init__(self, status.HTTP_405, title=title, **kwargs)
@property
def has_representation(self):
return self._has_representation
class HTTPNotAcceptable(HTTPError):
@@ -262,7 +272,11 @@ class HTTPRangeNotSatisfiable(HTTPError):
def __init__(self, resource_length):
headers = {'Content-Range': 'bytes */' + str(resource_length)}
HTTPError.__init__(self, status.HTTP_416, None, headers=headers)
HTTPError.__init__(self, status.HTTP_416, headers=headers)
@property
def has_representation(self):
return False
class HTTPInternalServerError(HTTPError):

57
falcon/http_error.py
View File

@@ -35,14 +35,11 @@ class HTTPError(Exception):
Attributes:
status (str): HTTP status line, such as "748 Confounded by Ponies".
serializable (bool): Returns *True* IFF the error should be
serialized when composing the HTTP response.
Note:
If an app sets a custom error serializer, it will only
be called when the error's `serializable` property is
``True``.
has_representation (bool): Read-only property that determines
whether error details will be serialized when composing
the HTTP response. In ``HTTPError`` this property always
returns ``True``, but child classes may override this property
in order to return ``False`` when an empty HTTP body is desired.
title (str): Error title to send to the client. Will be ``None`` if
the error should result in an HTTP response with an empty body.
description (str): Description of the error to send to the client.
@@ -54,19 +51,9 @@ class HTTPError(Exception):
Args:
status (str): HTTP status code and text, such as "400 Bad Request"
title (str): Human-friendly error title. Set to ``None`` if you wish
Falcon to return an empty response body (all remaining args will
be ignored except for headers.) This will set the error's
`serializable` property to ``False``.
Note:
Set `title` to ``None`` when you don't wish to disclose
sensitive information about why a request was refused,
when the status and headers are self-descriptive, or when
the HTTP specification forbids returning a body for the
status code in question.
Keyword Args:
title (str): Human-friendly error title (default ``None``).
description (str): Human-friendly description of the error, along with
a helpful suggestion or two (default ``None``).
headers (dict or list): A dictionary of header names and values
@@ -95,7 +82,7 @@ class HTTPError(Exception):
for this error").
code (int): An internal code that customers can reference in their
support request or to help them when searching for knowledge
base articles related to this error.
base articles related to this error (default ``None``).
"""
__slots__ = (
@@ -104,7 +91,7 @@ class HTTPError(Exception):
'description',
'headers',
'link',
'code'
'code',
)
def __init__(self, status, title=None, description=None, headers=None,
@@ -124,8 +111,8 @@ class HTTPError(Exception):
self.link = None
@property
def serializable(self):
return self.title is not None
def has_representation(self):
return True
def raw(self, obj_type=dict):
"""Returns a raw dictionary representing the error.
@@ -142,18 +129,20 @@ class HTTPError(Exception):
"""
assert self.serializable
assert self.has_representation
obj = obj_type()
obj['title'] = self.title
if self.description:
if self.title is not None:
obj['title'] = self.title
if self.description is not None:
obj['description'] = self.description
if self.code:
if self.code is not None:
obj['code'] = self.code
if self.link:
if self.link is not None:
obj['link'] = self.link
return obj
@@ -178,18 +167,20 @@ class HTTPError(Exception):
"""
assert self.serializable
assert self.has_representation
error_element = et.Element('error')
et.SubElement(error_element, 'title').text = self.title
if self.description:
if self.title is not None:
et.SubElement(error_element, 'title').text = self.title
if self.description is not None:
et.SubElement(error_element, 'description').text = self.description
if self.code:
if self.code is not None:
et.SubElement(error_element, 'code').text = str(self.code)
if self.link:
if self.link is not None:
link_element = et.SubElement(error_element, 'link')
for key in ('text', 'href', 'rel'):

6
tests/test_after_hooks.py
View File

@@ -5,7 +5,7 @@ import falcon.testing as testing
def validate_output(req, resp):
raise falcon.HTTPError(falcon.HTTP_723, title=None)
raise falcon.HTTPError(falcon.HTTP_723, 'Tricky')
def serialize_body(req, resp):
@@ -257,7 +257,9 @@ class TestHooks(testing.TestBase):
def test_output_validator(self):
self.simulate_request(self.test_route)
self.assertEqual(falcon.HTTP_723, self.srmock.status)
self.assertEqual(None, self.resource.resp.body_encoded)
expected = b'{\n "title": "Tricky"\n}'
self.assertEqual(expected, self.resource.resp.body_encoded)
def test_serializer(self):
self.simulate_request(self.test_route, method='PUT')

6
tests/test_httperror.py
View File

@@ -37,7 +37,7 @@ class FaultyResource:
code=8733224)
def on_patch(self, req, resp):
raise falcon.HTTPError(falcon.HTTP_400, 'No-can-do')
raise falcon.HTTPError(falcon.HTTP_400)
class UnicodeFaultyResource(object):
@@ -166,7 +166,7 @@ class TestHTTPError(testing.TestBase):
def test_no_description_json(self):
body = self.simulate_request('/fail', method='PATCH')
self.assertEqual(self.srmock.status, falcon.HTTP_400)
self.assertEqual(body, [b'{\n "title": "No-can-do"\n}'])
self.assertEqual(body, [b'{}'])
def test_no_description_xml(self):
body = self.simulate_request('/fail', method='PATCH',
@@ -174,7 +174,7 @@ class TestHTTPError(testing.TestBase):
self.assertEqual(self.srmock.status, falcon.HTTP_400)
expected_xml = (b'<?xml version="1.0" encoding="UTF-8"?>'
b'<error><title>No-can-do</title></error>')
b'<error />')
self.assertEqual(body, [expected_xml])