Issue #431: extractes the generic handling for error callbacks in init and handles login, logout, mail and tags resources

7 files changed, 74 insertions, 17 deletions

diff --git a/service/pixelated/resources/__init__.py b/service/pixelated/resources/__init__.py
index 469c8bc8..77425cc5 100644
--- a/service/pixelated/resources/__init__.py
+++ b/service/pixelated/resources/__init__.py
@@ -15,6 +15,7 @@
 # along with Pixelated. If not, see <http://www.gnu.org/licenses/>.
 
 import json
+import logging
 
 from twisted.web.http import UNAUTHORIZED
 from twisted.web.resource import Resource
@@ -23,6 +24,9 @@ from twisted.web.resource import Resource
 from pixelated.resources.session import IPixelatedSession
 from pixelated.support import log_time
 
+from twisted.web.http import INTERNAL_SERVER_ERROR
+log = logging.getLogger(__name__)
+
 
 class SetEncoder(json.JSONEncoder):
     def default(self, obj):
@@ -47,7 +51,17 @@ def respond_json_deferred(entity, request, status_code=200):
     request.finish()
 
 
-class BaseResource(Resource):
+class GenericDeferredErrorHandler(object):
+
+    @classmethod
+    def generic_error_handling(cls, e, request):
+        log.error(e)
+        request.setResponseCode(INTERNAL_SERVER_ERROR)
+        request.write('Something went wrong!')
+        request.finish()
+
+
+class BaseResource(Resource, GenericDeferredErrorHandler):
 
     def __init__(self, services_factory):
         Resource.__init__(self)
diff --git a/service/pixelated/resources/login_resource.py b/service/pixelated/resources/login_resource.py
index aca266cf..13a78fbc 100644
--- a/service/pixelated/resources/login_resource.py
+++ b/service/pixelated/resources/login_resource.py
@@ -141,6 +141,7 @@ class LoginResource(BaseResource):
 
         d = self._handle_login(request)
         d.addCallbacks(render_response, render_error)
+        d.addErrback(self.generic_error_handling, request)
 
         return NOT_DONE_YET
 
diff --git a/service/pixelated/resources/logout_resource.py b/service/pixelated/resources/logout_resource.py
index e26ec4b2..c22815ce 100644
--- a/service/pixelated/resources/logout_resource.py
+++ b/service/pixelated/resources/logout_resource.py
@@ -1,15 +1,26 @@
-import logging
+#
+# Copyright (c) 2014 ThoughtWorks, Inc.
+#
+# Pixelated is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# Pixelated is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with Pixelated. If not, see <http://www.gnu.org/licenses/>.
 
 from twisted.internet import defer
 from twisted.web import util
-from twisted.web.http import INTERNAL_SERVER_ERROR
 from twisted.web.server import NOT_DONE_YET
 
 from pixelated.resources import BaseResource
 from pixelated.resources.login_resource import LoginResource
 
-log = logging.getLogger(__name__)
-
 
 class LogoutResource(BaseResource):
     BASE_URL = "logout"
@@ -27,14 +38,8 @@ class LogoutResource(BaseResource):
             request.write(content)
             request.finish()
 
-        def handle_error(e):
-            log.error(e)
-            request.setResponseCode(INTERNAL_SERVER_ERROR)
-            request.write('Something went wrong!')
-            request.finish()
-
         d = self._execute_logout(request)
         d.addCallback(_redirect_to_login)
-        d.addErrback(handle_error)
+        d.addErrback(self.generic_error_handling, request)
 
         return NOT_DONE_YET
diff --git a/service/pixelated/resources/mail_resource.py b/service/pixelated/resources/mail_resource.py
index 37fceb9b..6f9ec828 100644
--- a/service/pixelated/resources/mail_resource.py
+++ b/service/pixelated/resources/mail_resource.py
@@ -4,7 +4,7 @@ from twisted.python.log import err
 from twisted.web.resource import Resource
 from twisted.web.server import NOT_DONE_YET
 
-from pixelated.resources import respond_json_deferred, BaseResource
+from pixelated.resources import respond_json_deferred, BaseResource, GenericDeferredErrorHandler
 from pixelated.support import replier
 
 
@@ -30,7 +30,7 @@ class MailTags(Resource):
         return NOT_DONE_YET
 
 
-class Mail(Resource):
+class Mail(Resource, GenericDeferredErrorHandler):
 
     def __init__(self, mail_id, mail_service):
         Resource.__init__(self)
@@ -51,6 +51,8 @@ class Mail(Resource):
         d = self._mail_service.mail(self._mail_id)
         d.addCallback(lambda mail: populate_reply(mail))
         d.addCallback(lambda mail_dict: respond_json_deferred(mail_dict, request))
+        d.addErrback(self.generic_error_handling, request)
+
         return NOT_DONE_YET
 
     def render_DELETE(self, request):
diff --git a/service/pixelated/resources/tags_resource.py b/service/pixelated/resources/tags_resource.py
index 6d4b7335..26da9dae 100644
--- a/service/pixelated/resources/tags_resource.py
+++ b/service/pixelated/resources/tags_resource.py
@@ -16,7 +16,6 @@
 
 from pixelated.resources import respond_json_deferred, BaseResource
 from twisted.internet.threads import deferToThread
-from twisted.web.resource import Resource
 from twisted.web.server import NOT_DONE_YET
 
 
@@ -34,5 +33,6 @@ class TagsResource(BaseResource):
 
         d = deferToThread(lambda: _search_engine.tags(query=query, skip_default_tags=skip_default_tags))
         d.addCallback(lambda tags: respond_json_deferred(tags, request))
+        d.addErrback(self.generic_error_handling, request)
 
         return NOT_DONE_YET
diff --git a/service/test/unit/resources/test_logout_resources.py b/service/test/unit/resources/test_logout_resources.py
index e8b88171..49866057 100644
--- a/service/test/unit/resources/test_logout_resources.py
+++ b/service/test/unit/resources/test_logout_resources.py
@@ -50,7 +50,7 @@ class TestLogoutResource(unittest.TestCase):
         request.method = 'POST'
 
         session = self.resource.get_session(request)
-        exception = Exception('haha')
+        exception = Exception('')
         session.expire = MagicMock(side_effect=exception)
 
         d = self.web.get(request)
@@ -61,4 +61,3 @@ class TestLogoutResource(unittest.TestCase):
 
         d.addCallback(assert_500_when_exception_is_thrown)
         return d
-
diff --git a/service/test/unit/resources/test_tags_resource.py b/service/test/unit/resources/test_tags_resource.py
new file mode 100644
index 00000000..684e95f2
--- /dev/null
+++ b/service/test/unit/resources/test_tags_resource.py
@@ -0,0 +1,36 @@
+import logging
+
+from mock import MagicMock
+from twisted.trial import unittest
+from twisted.web.test.requesthelper import DummyRequest
+from pixelated.resources.tags_resource import TagsResource
+from test.unit.resources import DummySite
+
+logging.getLogger('pixelated.resources').addHandler(logging.NullHandler())
+
+
+class TestTagsResource(unittest.TestCase):
+    def setUp(self):
+        self.services_factory = MagicMock()
+        self.resource = TagsResource(self.services_factory)
+
+    def test_errback_is_called(self):
+        exception = Exception('')
+        mock_search_engine = MagicMock()
+        mock_search_engine.tags = MagicMock(side_effect=exception)
+        mock_service = MagicMock()
+        mock_service.search_engine = mock_search_engine
+        self.services_factory.services.return_value = mock_service
+        self.web = DummySite(self.resource)
+
+        request = DummyRequest(['/tags'])
+        request.method = 'GET'
+
+        d = self.web.get(request)
+
+        def assert_500_when_exception_is_thrown(_):
+            self.assertEqual(500, request.responseCode)
+            self.assertEqual('Something went wrong!', request.written[0])
+
+        d.addCallback(assert_500_when_exception_is_thrown)
+        return d