Merge lp:~mandel/ubuntu-sso-client/fix-stale-broker into lp:ubuntu-sso-client

[Manuel de la Peña]

- Re-start sso if we get a stale broker amd re-request all the remote objects (LP: #1043367).

- Re-start sso if we get a stale broker amd re-request all the remote objects (LP: #1043367).

If you want to do IRL simply start sso in one terminal, launch control panel and kill sso in the middle of the work.

[Roberto Alsina (ralsina)]

Looks good to me

review: Approve

[Mike McCracken (mikemc)]

For posterity's sake: how to IRL test, so we know when this might happen in real life.

start with no creds
start sso-login
start u1-cp
cp will find no creds, show the initial screen asking you to log in or create a new account
** here, kill sso-login
then try to continue with CP, it should spawn a new sso-login and continue on its way.

if you already have creds, CP gets them once and caches them, so you shouldn't get a stale broker error if you've got creds successfully once, since it shouldn't call into sso again.

review: Approve

=== modified file 'ubuntu_sso/utils/ipc.py'
--- ubuntu_sso/utils/ipc.py	2012-08-09 21:17:19 +0000
+++ ubuntu_sso/utils/ipc.py	2012-08-30 01:47:20 +0000
@@ -237,10 +237,11 @@
     call_remote_functions = []  # methods that can be called on the remote obj
     signal_handlers = []  # signals that are of interest of this client
 
-    def __init__(self, remote_object):
+    def __init__(self, base_client, remote_object):
         """Create instance."""
         super(RemoteClient, self).__init__()
         self._mapping = defaultdict(list)
+        self.base_client = base_client
         self.remote = remote_object
 
         # for each function name in self.call_remote_functions,
@@ -284,6 +285,7 @@
 
         return callback_wrapper
 
+    @defer.inlineCallbacks
     def call_method(self, method_name, *args, **kwargs):
         """Call asynchronously 'method_name(*args)'.
 
@@ -293,13 +295,23 @@
         """
         logger.debug('Performing %r as a remote call (%r, %r).',
                      method_name, args, kwargs)
-        result = self.remote.callRemote(method_name, *args)
-        return result
+        try:
+            result = yield self.remote.callRemote(method_name, *args)
+        except DeadReferenceError:
+            yield self.base_client.reconnect()
+            result = yield self.call_method(method_name, *args, **kwargs)
+        defer.returnValue(result)
 
+    @defer.inlineCallbacks
     def register_to_signals(self):
         """Register to the signals."""
-        return self.remote.callRemote('register_to_signals', self,
-                                      self.signal_handlers)
+        try:
+            result = yield self.remote.callRemote('register_to_signals', self,
+                    self.signal_handlers)
+        except DeadReferenceError:
+            yield self.base_client.reconnect()
+            result = yield self.register_to_signals()
+        defer.returnValue(result)
 
     def unregister_to_signals(self):
         """Register to the signals."""
@@ -352,7 +364,7 @@
                      self.clients.keys(), self.__class__.__name__)
         for name, client_class in self.clients.items():
             remote = yield root.callRemote('get_%s' % name)
-            setattr(self, name, client_class(remote))
+            setattr(self, name, client_class(self, remote))
 
     @defer.inlineCallbacks
     def connect(self):
@@ -367,6 +379,22 @@
         yield self.register_to_signals()
 
     @defer.inlineCallbacks
+    def reconnect(self):
+        """Reconnect with the server."""
+        self.factory = PBClientFactory()
+        self.client = yield client_connect(self.factory,
+                                           self.service_name,
+                                           self.service_cmdline,
+                                           self.service_description)
+        root = yield self.factory.getRootObject()
+        # loop over the already present remote clients and reset their remotes
+        for name in self.clients:
+            remote = yield root.callRemote('get_%s' % name)
+            remote_client = getattr(self, name)
+            remote_client.remote = remote
+        yield self.register_to_signals()
+
+    @defer.inlineCallbacks
     def register_to_signals(self):
         """Register all the clients to their signals."""
         for name in self.clients:

=== modified file 'ubuntu_sso/utils/tests/test_ipc.py'
--- ubuntu_sso/utils/tests/test_ipc.py	2012-08-14 20:37:42 +0000
+++ ubuntu_sso/utils/tests/test_ipc.py	2012-08-30 01:47:20 +0000
@@ -619,3 +619,80 @@
             fake_remote_client.random_exception,
         )
         self.assertTrue(self.memento.check_warning(*expected))
+
+
+class FakeRootObject(object):
+    """A fake root object."""
+
+    def __init__(self, called, remote_obj):
+        """Create a new instance."""
+        self.called = called
+        self.remote_obj = remote_obj
+
+    # pylint: disable=C0103
+    def callRemote(self, method_name):
+        """A fake call remove method."""
+        self.called.append(method_name)
+        return defer.succeed(self.remote_obj)
+    # pylint: enable=C0103
+
+
+class FakeWorkingRemoteClient(object):
+    """A fake remote client."""
+
+    def __init__(self, called):
+        """Create a new instance."""
+        self.remote = None
+        self.called = called
+
+    def register_to_signals(self):
+        """Register to signals."""
+        self.called.append('register_to_signals')
+        return defer.succeed(True)
+
+
+class ReconnectTestCase(TestCase):
+    """Test the reconnection when sso is dead."""
+
+    @defer.inlineCallbacks
+    def setUp(self):
+        """Set the different tests."""
+        yield super(ReconnectTestCase, self).setUp()
+        self.called = []
+        self.remote_obj = 'remote'
+        self.root_obj = FakeRootObject(self.called, self.remote_obj)
+
+        def fake_get_root_object():
+            """Fake getting the root object."""
+            self.called.append('getRootObject')
+            return defer.succeed(self.root_obj)
+
+        def fake_client_connect(factory, service_name, cmd, description):
+            """Fake the client connect."""
+            self.called.append('client_connect')
+            self.patch(factory, 'getRootObject', fake_get_root_object)
+            return defer.succeed(True)
+
+        self.patch(ipc, 'client_connect', fake_client_connect)
+
+    @defer.inlineCallbacks
+    def test_reconnect_method(self):
+        """Test the execcution of the reconnect method."""
+        clients = dict(first=FakeWorkingRemoteClient(self.called),
+                       second=FakeWorkingRemoteClient(self.called))
+
+        base_client = ipc.BaseClient()
+        base_client.clients = clients
+        for name, client in clients.items():
+            setattr(base_client, name, client)
+
+        yield base_client.reconnect()
+        # assert that we did call the correct methods
+        self.assertIn('client_connect', self.called)
+        self.assertIn('getRootObject', self.called)
+
+        for name in clients:
+            self.assertIn('get_%s' % name, self.called)
+
+        self.assertEqual(len(clients),
+                self.called.count('register_to_signals'))