Merge lp:~mandel/ubuntuone-client/guard-ipc-connect into lp:ubuntuone-client

[Manuel de la Peña]

- Add a guard so that async calls are perform after each other and sd is not started more than once (LP: #1043287).

- Add a guard so that async calls are perform after each other and sd is not started more than once (LP: #1043287).

[Roberto Alsina (ralsina)]

I am not confident I am following the tests correctly, but it seems to work, ad the lock part is fairly obvious.

review: Approve

[Alejandro J. Cura (alecu)]

Looks good.

review: Approve

=== modified file 'tests/platform/ipc/test_perspective_broker.py'
--- tests/platform/ipc/test_perspective_broker.py	2012-05-22 15:27:36 +0000
+++ tests/platform/ipc/test_perspective_broker.py	2012-08-29 12:47:18 +0000
@@ -27,6 +27,7 @@
 # version.  If you delete this exception statement from all source
 # files in the program, then also delete it here.
 """IPC tests on perspective broker."""
+import itertools
 import os
 
 from mocker import MockerTestCase, ANY
@@ -62,6 +63,7 @@
     Status,
     SyncDaemon,
 )
+from ubuntuone.platform.ipc import ipc_client
 from ubuntuone.platform.ipc.ipc_client import (
     signal,
     ConfigClient,
@@ -764,3 +766,110 @@
                    lambda _: defer.fail(ipc.AlreadyStartedError()))
         is_running = yield ipc.is_already_running()
         self.assertTrue(is_running, "Should be running by now.")
+
+
+class MultipleConnectionsTestCase(TestCase):
+    """Test the execution of the client with multiple connections."""
+
+    @defer.inlineCallbacks
+    def setUp(self):
+        """Set the different tests."""
+        yield super(MultipleConnectionsTestCase, self).setUp()
+        self.client_connect_d = defer.Deferred()  # called when we connected
+        self.client_root_obj_d = defer.Deferred()  # called when we got root
+        self.remote_obj_d = defer.Deferred()  # called when we got the remotes
+        self.register_to_signals_d = defer.Deferred()  # called with signals
+        self.called = []
+        self.num_clients = 4
+
+        @defer.inlineCallbacks
+        def fake_get_root_object():
+            """Fake getting a root objects."""
+            yield self.client_root_obj_d
+            self.called.append('getRootObject')
+            defer.returnValue(True)
+
+        @defer.inlineCallbacks
+        def fake_ipc_client_connect(factory):
+            """Fake ipc_client_connect."""
+            yield self.client_connect_d
+            self.called.append('ipc_client_connect')
+
+            # lets patch the factory getRootObjects function
+            self.patch(factory, 'getRootObject', fake_get_root_object)
+            defer.returnValue(True)
+
+        self.patch(ipc_client, 'ipc_client_connect', fake_ipc_client_connect)
+
+        @defer.inlineCallbacks
+        def fake_request_remote_objects(my_self, root):
+            """Fake request_remote_objects."""
+            yield self.remote_obj_d
+            self.called.append('_request_remote_objects')
+            defer.returnValue(True)
+
+        self.patch(ipc_client.UbuntuOneClient, '_request_remote_objects',
+                fake_request_remote_objects)
+
+        @defer.inlineCallbacks
+        def fake_register_to_signals(my_self):
+            """Fake registering to signals."""
+            yield self.register_to_signals_d
+            self.called.append('register_to_signals')
+            defer.returnValue(True)
+
+        self.patch(ipc_client.UbuntuOneClient, 'register_to_signals',
+                fake_register_to_signals)
+
+    def grouper(self, n, iterable, fillvalue=None):
+        "grouper(3, 'ABCDEFG', 'x') --> ABC DEF Gxx"
+        args = [iter(iterable)] * n
+        return itertools.izip_longest(*args, fillvalue=fillvalue)
+
+    @defer.inlineCallbacks
+    def test_multiple_connections(self):
+        """Test that we only perform connect once but all other are correct."""
+        deferreds = [self.client_connect_d, self.client_root_obj_d,
+                     self.remote_obj_d, self.register_to_signals_d]
+
+        # the order in which the calls are expected
+        expected_calls = ('ipc_client_connect','getRootObject',
+                '_request_remote_objects', 'register_to_signals')
+
+        clients = []
+        while len(clients) < self.num_clients:
+            clients.append(ipc_client.UbuntuOneClient())
+
+        connected_d = []
+
+        for num_steps in range(len(deferreds)):
+            # tell the first client to connect
+            connected_d.append(clients[0].connect())
+
+            # perform the number of connection steps so far
+            for index, step_d in enumerate(deferreds):
+                if index > num_steps:
+                    break
+                step_d.callback(True)
+
+            # call connect to all the other clients
+            for client in clients[1:]:
+                connected_d.append(client.connect())
+
+            # perform the rest of steps
+            for step_d in deferreds[num_steps + 1:]:
+                step_d.callback(True)
+
+            yield defer.gatherResults(connected_d)
+            # reset all the deferreds for the next round of testing
+            for index, d_name in enumerate(('client_connect_d',
+                    'client_root_obj_d', 'remote_obj_d',
+                    'register_to_signals_d')):
+                new_d = defer.Deferred()
+                setattr(self, d_name, new_d)
+                deferreds[index] = new_d
+
+            # assert that all connect calls have been done in the correct
+            # order
+            for calls in self.grouper(4, self.called, None):
+                self.assertEqual(expected_calls, calls)

=== modified file 'ubuntuone/platform/ipc/ipc_client.py'
--- ubuntuone/platform/ipc/ipc_client.py	2012-08-27 15:22:13 +0000
+++ ubuntuone/platform/ipc/ipc_client.py	2012-08-29 12:47:18 +0000
@@ -719,6 +719,8 @@
 class UbuntuOneClient(object):
     """Root object that provides access to all the remote objects."""
 
+    connection_lock = defer.DeferredLock()
+
     def __init__(self):
         """Create a new instance."""
         self.status = None
@@ -765,17 +767,21 @@
     def connect(self):
         """Connect to the syncdaemon service."""
         # pylint: disable=W0702
+        yield self.connection_lock.acquire()
         try:
-            # connect to the remote objects
-            self.factory = PBClientFactory()
-            self.client = yield ipc_client_connect(self.factory)
-            root = yield self.factory.getRootObject()
-            yield self._request_remote_objects(root)
-            yield self.register_to_signals()
+            if self.client is None:
+                # connect to the remote objects
+                self.factory = PBClientFactory()
+                self.client = yield ipc_client_connect(self.factory)
+                root = yield self.factory.getRootObject()
+                yield self._request_remote_objects(root)
+                yield self.register_to_signals()
             defer.returnValue(self)
         except Exception, e:
             raise SyncDaemonClientConnectionError(
                             'Could not connect to the syncdaemon ipc.', e)
+        finally:
+            self.connection_lock.release()
         # pylint: disable=W0702
 
     @defer.inlineCallbacks