launchpad-reviewers team mailing list archive

["Jeroen T. Vermeulen" <jtv@xxxxxxxxxxxxx>]

Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/pre-986185-exception-responses into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jtv/maas/pre-986185-exception-responses/+merge/103510

This is preparation for another branch I'm working on.  Since MAASAPIExceptions translate so directly into HttpResponses, we might as well let the exception classes themselves handle that.  Some of the complexity leaves the middleware class, and can be used in other code that needs a very similar system of http-response exceptions.

This also adds another exception class I need: Redirect.  Thanks to the new method on the exception class, it's easy to specialize this out of MAASAPIException without any special-casing in the exception middleware.
-- 
https://code.launchpad.net/~jtv/maas/pre-986185-exception-responses/+merge/103510
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/pre-986185-exception-responses into lp:maas.

=== modified file 'src/maasserver/exceptions.py'
--- src/maasserver/exceptions.py	2012-04-17 09:20:48 +0000
+++ src/maasserver/exceptions.py	2012-04-25 16:23:40 +0000
@@ -22,6 +22,11 @@
 
 import httplib
 
+from django.http import (
+    HttpResponse,
+    HttpResponseRedirect,
+    )
+
 
 class MAASException(Exception):
     """Base class for MAAS' exceptions."""
@@ -44,6 +49,13 @@
     """
     api_error = httplib.INTERNAL_SERVER_ERROR
 
+    def make_http_response(self):
+        """Create an :class:`HttpResponse` representing this exception."""
+        encoding = b'utf-8'
+        return HttpResponse(
+            status=self.api_error, content=unicode(self).encode(encoding),
+            mimetype=b"text/plain; charset=%s" % encoding)
+
 
 class ExternalComponentException(MAASAPIException):
     """An external component failed."""
@@ -70,3 +82,11 @@
 class NodesNotAvailable(NodeStateViolation):
     """Requested node(s) are not available to be acquired."""
     api_error = httplib.CONFLICT
+
+
+class Redirect(MAASAPIException):
+    """Redirect.  The exception message is the target URL."""
+    api_error = httplib.FOUND
+
+    def make_http_response(self):
+        return HttpResponseRedirect(unicode(self))

=== modified file 'src/maasserver/middleware.py'
--- src/maasserver/middleware.py	2012-04-18 17:30:55 +0000
+++ src/maasserver/middleware.py	2012-04-25 16:23:40 +0000
@@ -175,12 +175,9 @@
 
         encoding = b'utf-8'
         if isinstance(exception, MAASAPIException):
-            # The exception is a MAASAPIException: exception.api_error
-            # will give us the proper error type.
-            return HttpResponse(
-                content=unicode(exception).encode(encoding),
-                status=exception.api_error,
-                mimetype=b"text/plain; charset=%s" % encoding)
+            # This type of exception knows how to translate itself into
+            # an http response.
+            return exception.make_http_response()
         elif isinstance(exception, ValidationError):
             if hasattr(exception, 'message_dict'):
                 # Complex validation error with multiple fields:

=== added file 'src/maasserver/tests/test_exceptions.py'
--- src/maasserver/tests/test_exceptions.py	1970-01-01 00:00:00 +0000
+++ src/maasserver/tests/test_exceptions.py	2012-04-25 16:23:40 +0000
@@ -0,0 +1,40 @@
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for the exceptions module."""
+
+from __future__ import (
+    absolute_import,
+    print_function,
+    unicode_literals,
+    )
+
+__metaclass__ = type
+__all__ = []
+
+import httplib
+
+from maasserver.exceptions import (
+    MAASAPIBadRequest,
+    Redirect,
+    )
+from maastesting.factory import factory
+from maastesting.testcase import TestCase
+
+
+class TestExceptions(TestCase):
+
+    def test_MAASAPIException_produces_http_response(self):
+        error = factory.getRandomString()
+        exception = MAASAPIBadRequest(error)
+        response = exception.make_http_response()
+        self.assertEqual(
+            (httplib.BAD_REQUEST, error),
+            (response.status_code, response.content))
+
+    def test_Redirect_produces_redirect_to_given_URL(self):
+        target = factory.getRandomString()
+        exception = Redirect(target)
+        response = exception.make_http_response()
+        self.assertEqual(httplib.FOUND, response.status_code)
+        self.assertEqual(target, response['Location'])