Merge lp:~alecu/ubuntuone-control-panel/webclient-shutdowns into lp:ubuntuone-control-panel

[Alejandro J. Cura]

- Do not throw a webclient error when closing (LP: #845105).

Do not throw a webclient error when closing (LP: #845105)

[Natalia Bidart (nataliabidart)]

The branch looks good, but I'm getting this in windows:

===============================================================================
[ERROR]
Traceback (most recent call last):
Failure: twisted.trial.util.DirtyReactorAggregateError: Reactor was unclean.
DelayedCalls: (set twisted.internet.base.DelayedCall.debug = True to debug)
<DelayedCall 0x5dad710 [29.004999876s] called=0 cancelled=0 Client.failIfNotConn
ected(TimeoutError('',))>

ubuntuone.controlpanel.web_client.tests.test_txwebclient.WebClientShutdownTestCa
se.test_shutdown
===============================================================================
[ERROR]
Traceback (most recent call last):
Failure: twisted.trial.util.DirtyReactorAggregateError: Reactor was unclean.
Selectables:
<<class 'twisted.internet.tcp.Client'> to ('localhost', 50832) at 5dac6d0>

ubuntuone.controlpanel.web_client.tests.test_txwebclient.WebClientShutdownTestCa
se.test_shutdown
-------------------------------------------------------------------------------

As far as I can see, if the test finished because d2 was fired, the deferred d3 leaves the reactor unclean with the callLater thing added.

review: Needs Fixing

[Natalia Bidart (nataliabidart)]

It works great IRL!!!

[Natalia Bidart (nataliabidart)]

This was superseded by https://code.launchpad.net/~nataliabidart/ubuntuone-control-panel/webclient-shutdowns/+merge/75609.

Unmerged revisions

=== added directory 'ubuntuone/controlpanel/web_client/tests'
=== added file 'ubuntuone/controlpanel/web_client/tests/__init__.py'
--- ubuntuone/controlpanel/web_client/tests/__init__.py	1970-01-01 00:00:00 +0000
+++ ubuntuone/controlpanel/web_client/tests/__init__.py	2011-09-09 00:01:23 +0000
@@ -0,0 +1,19 @@
+# -*- coding: utf-8 -*-
+
+# Authors: Alejandro J. Cura <[e-mail]>
+#
+# Copyright 2011 Canonical Ltd.
+#
+# This program is free software: you can redistribute it and/or modify it
+# under the terms of the GNU General Public License version 3, as published
+# by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranties of
+# MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
+# PURPOSE.  See the GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License along
+# with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Unit tests for the control panel backend webservice clients."""

=== added file 'ubuntuone/controlpanel/web_client/tests/test_txwebclient.py'
--- ubuntuone/controlpanel/web_client/tests/test_txwebclient.py	1970-01-01 00:00:00 +0000
+++ ubuntuone/controlpanel/web_client/tests/test_txwebclient.py	2011-09-09 00:01:23 +0000
@@ -0,0 +1,153 @@
+# -*- coding: utf-8 -*-
+
+# Authors: Alejandro J. Cura <[e-mail]>
+#
+# Copyright 2011 Canonical Ltd.
+#
+# This program is free software: you can redistribute it and/or modify it
+# under the terms of the GNU General Public License version 3, as published
+# by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranties of
+# MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
+# PURPOSE.  See the GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License along
+# with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Unit tests for the control panel backend twisted webservice client."""
+
+from twisted.application import internet, service
+from twisted.internet import defer, reactor
+from twisted.internet.defer import inlineCallbacks
+from twisted.web import server, resource
+
+from ubuntuone.controlpanel.web_client import txwebclient
+from ubuntuone.controlpanel.tests import TestCase
+
+
+SAMPLE_KEY = "result"
+SAMPLE_VALUE = "sample result"
+SAMPLE_RESOURCE = '{"%s": "%s"}' % (SAMPLE_KEY, SAMPLE_VALUE)
+SAMPLE_CREDENTIALS = dict(
+    consumer_key="consumer key",
+    consumer_secret="consumer secret",
+    token="the real token",
+    token_secret="the token secret",
+)
+
+
+def sample_get_credentials():
+    """Will return the sample credentials right now."""
+    return defer.succeed(SAMPLE_CREDENTIALS)
+
+
+class MockResource(resource.Resource):
+    """A simple web resource."""
+    isLeaf = True
+    contents = ""
+
+    # pylint: disable=C0103
+    # t.w.resource methods have freeform cased names
+
+    def getChild(self, name, request):
+        """Get a given child resource."""
+        if name == '':
+            return self
+        return resource.Resource.getChild(self, name, request)
+
+    def render_GET(self, request):
+        """Make a bit of html out of these resource's content."""
+        return self.contents
+
+
+class MockWebService(object):
+    """A mock webservice for testing"""
+
+    def __init__(self):
+        """Start up this instance."""
+        root = resource.Resource()
+        devices_resource = MockResource()
+        devices_resource.contents = SAMPLE_RESOURCE
+        root.putChild("devices", devices_resource)
+        root.putChild("throwerror", resource.NoResource())
+        unauthorized = resource.ErrorPage(resource.http.UNAUTHORIZED,
+                                          "Unauthrorized", "Unauthrorized")
+        root.putChild("unauthorized", unauthorized)
+
+        site = server.Site(root)
+        application = service.Application('web')
+        self.service_collection = service.IServiceCollection(application)
+        #pylint: disable=E1101
+        self.tcpserver = internet.TCPServer(0, site)
+        self.tcpserver.setServiceParent(self.service_collection)
+        self.service_collection.startService()
+
+    def get_url(self):
+        """Build the url for this mock server."""
+        #pylint: disable=W0212
+        port_num = self.tcpserver._port.getHost().port
+        return "http://localhost:%d/" % port_num
+
+    def stop(self):
+        """Shut it down."""
+        #pylint: disable=E1101
+        self.service_collection.stopService()
+
+
+class WebClientTestCase(TestCase):
+    """Test for the webservice client."""
+
+    timeout = 8
+
+    def setUp(self):
+        super(WebClientTestCase, self).setUp()
+        self.ws = MockWebService()
+        test_base_url = self.ws.get_url()
+        self.wc = txwebclient.WebClient(sample_get_credentials, test_base_url)
+
+    @inlineCallbacks
+    def tearDown(self):
+        super(WebClientTestCase, self).tearDown()
+        yield self.ws.stop()
+        self.wc.shutdown()
+
+    @inlineCallbacks
+    def test_get_url(self):
+        """A method is successfully called in the mock webservice."""
+        result = yield self.wc.call_api("devices")
+        self.assertIn(SAMPLE_KEY, result)
+        self.assertEqual(SAMPLE_VALUE, result[SAMPLE_KEY])
+
+    @inlineCallbacks
+    def test_get_url_error(self):
+        """The errback is called when there's some error."""
+        yield self.assertFailure(self.wc.call_api("throwerror"),
+                                 txwebclient.WebClientError)
+
+    @inlineCallbacks
+    def test_unauthorized(self):
+        """Detect when a request failed with UNAUTHORIZED."""
+        yield self.assertFailure(self.wc.call_api("unauthorized"),
+                                 txwebclient.UnauthorizedError)
+
+
+class WebClientShutdownTestCase(TestCase):
+    """The webclient behaviour during shutdown."""
+
+    @inlineCallbacks
+    def test_shutdown(self):
+        """The webclient behaves well during shutdown."""
+        d3 = defer.Deferred()
+        # pylint: disable=E1101
+        reactor.callLater(1, d3.callback, None)
+        ws = MockWebService()
+        test_base_url = ws.get_url()
+        wc = txwebclient.WebClient(sample_get_credentials, test_base_url)
+        d1 = wc.call_api("throwerror")
+        d2 = ws.stop()
+        wc.shutdown()
+        yield d2
+        yield defer.DeferredList([d1, d3], fireOnOneCallback=True,
+                                 fireOnOneErrback=True)

=== modified file 'ubuntuone/controlpanel/web_client/txwebclient.py'
--- ubuntuone/controlpanel/web_client/txwebclient.py	2011-06-02 13:14:51 +0000
+++ ubuntuone/controlpanel/web_client/txwebclient.py	2011-09-09 00:01:23 +0000
@@ -20,6 +20,7 @@
 
 import simplejson
 
+from twisted.internet import defer, reactor
 from twisted.web import client, error, http
 
 from ubuntuone.controlpanel import WEBSERVICE_BASE_URL
@@ -39,6 +40,10 @@
         """Initialize the webclient."""
         self.base_url = base_url
         self.get_credentials = get_credentials
+        self.running = True
+        # pylint: disable=E1101
+        self.trigger_id = reactor.addSystemEventTrigger("before", "shutdown",
+                                                        self.shutdown)
 
     def _handle_response(self, result):
         """Handle the response of the webservice call."""
@@ -74,7 +79,19 @@
         d = self.get_credentials()
         d.addErrback(self._handle_error)
         d.addCallback(self._call_api_with_creds, api_name)
-        return d
+        d2 = defer.Deferred()
+        d.addCallback(d2.callback)
+
+        def mask_errors_on_shutdown(failure):
+            """Do not fire the errbacks if we are shutting down."""
+            if self.running:
+                d2.errback(failure)
+
+        d.addErrback(mask_errors_on_shutdown)
+        return d2
 
     def shutdown(self):
         """End the pending webclient calls."""
+        self.running = False
+        # pylint: disable=E1101
+        reactor.removeSystemEventTrigger(self.trigger_id)