Merge lp:~alecu/ubuntuone-client/removable-signals into lp:ubuntuone-client

[Alejandro J. Cura]

Make signals removable, so the pb proxy looks a bit more like dbus. (LP: #805471)

Make signals removable, so the pb proxy looks a bit more like dbus. (LP: #805471)

[Roberto Alsina (ralsina)]

+1

review: Approve

[Natalia Bidart (nataliabidart)]

review: Approve

=== modified file 'tests/platform/windows/test_credentials.py'
--- tests/platform/windows/test_credentials.py	2011-06-24 05:30:34 +0000
+++ tests/platform/windows/test_credentials.py	2011-07-04 15:23:09 +0000
@@ -23,7 +23,7 @@
 from ubuntuone.platform.windows import credentials
 
 TEST_APP_NAME = "test application"
-
+TEST_ERROR_DICT = {}
 TEST_CREDENTIALS = {
     "token": "1234token",
 }
@@ -32,11 +32,67 @@
 class FakeSSOProxy(object):
     """A fake SSOProxy."""
 
+    def __init__(self):
+        """Initialize this fake."""
+        signals = [
+            "credentials_stored",
+            "credentials_cleared",
+            "credentials_found",
+            "credentials_not_found",
+            "authorization_denied",
+            "credentials_error",
+        ]
+        for s in signals:
+            handler_name = "on_%s" % s
+            callback_name = "on_%s_cb" % s
+
+            def make_handler(callback_name):
+                """Create a handler for a given callback_name."""
+
+                def handler(*args):
+                    """A signal was called."""
+                    callback = getattr(self, callback_name, None)
+                    if callback is not None:
+                        callback(*args)
+
+                return handler
+
+            setattr(self, handler_name, make_handler(callback_name))
+            setattr(self, callback_name, None)
+
+
     def find_credentials(self, app_name, options, **kwargs):
         """Ask the U1 credentials."""
         return defer.succeed(TEST_CREDENTIALS)
 
 
+class RemovableSignalTestCase(TestCase):
+    """Tests for RemovableSignal."""
+
+    def setUp(self):
+        """Initialize this fake instance."""
+        self.proxy = FakeSSOProxy()
+
+    def test_dunder_callable(self):
+        """__call__ works as expected."""
+        sample_store = []
+        test_cb = lambda app_name, creds: sample_store.append(creds)
+        rs = credentials.RemovableSignal(self.proxy, "on_credentials_found_cb",
+                                         test_cb)
+        rs(TEST_APP_NAME, TEST_CREDENTIALS)
+        self.assertEqual(sample_store[0], TEST_CREDENTIALS)
+
+    def test_remove(self):
+        """The signal has a .remove that removes the callback."""
+        sample_store = []
+        test_cb = lambda app_name, creds: sample_store.append(creds)
+        rs = credentials.RemovableSignal(self.proxy, "on_credentials_found_cb",
+                                         test_cb)
+        rs.remove()
+        rs(TEST_APP_NAME, TEST_CREDENTIALS)
+        self.assertEqual(len(sample_store), 0)
+
+
 class CredentialsManagementTestCase(TestCase):
     """Tests for CredentialsManagement."""
 
@@ -66,3 +122,44 @@
         cb = lambda: None
         other_cb = self.cm.register_to_credentials_not_found(cb)
         other_cb(TEST_APP_NAME)
+
+    def _verify_not_called_twice(self, signal_name, *args):
+        """Test that the callback is not called twice."""
+        d = defer.Deferred()
+
+        def signal_handler(*args):
+            """Fake the behaviour of CredentialsManagementToolRoot."""
+            d.callback(args[0] if len(args) > 0 else None)
+
+        register = getattr(self.cm, "register_to_" + signal_name)
+        signal = register(signal_handler)
+        proxy_cb = getattr(self.proxy, "on_" + signal_name)
+        proxy_cb(*args)
+        if getattr(signal, "remove", False):
+            signal.remove()
+        proxy_cb(*args)
+
+    def test_not_called_twice_credentials_stored(self):
+        """Test that on_credentials_stored is not called twice."""
+        self._verify_not_called_twice("credentials_stored")
+
+    def test_not_called_twice_credentials_cleared(self):
+        """Test that on_credentials_cleared is not called twice."""
+        self._verify_not_called_twice("credentials_cleared")
+
+    def test_not_called_twice_credentials_found(self):
+        """Test that on_credentials_found is not called twice."""
+        self._verify_not_called_twice("credentials_found", TEST_APP_NAME,
+                                      TEST_CREDENTIALS)
+
+    def test_not_called_twice_credentials_not_found(self):
+        """Test that on_credentials_not_found is not called twice."""
+        self._verify_not_called_twice("credentials_not_found")
+
+    def test_not_called_twice_authorization_denied(self):
+        """Test that on_authorization_denied is not called twice."""
+        self._verify_not_called_twice("authorization_denied")
+
+    def test_not_called_twice_credentials_error(self):
+        """Test that on_credentials_error is not called twice."""
+        self._verify_not_called_twice("credentials_error", TEST_ERROR_DICT)

=== modified file 'ubuntuone/platform/windows/credentials.py'
--- ubuntuone/platform/windows/credentials.py	2011-06-24 05:30:34 +0000
+++ ubuntuone/platform/windows/credentials.py	2011-07-04 15:23:09 +0000
@@ -40,6 +40,28 @@
 CredentialsError = CredentialsErrorRoot
 
 
+class RemovableSignal(object):
+    """A signal that can be removed."""
+
+    def __init__(self, proxy, signal_name, callback):
+        """Initialize this instance."""
+        self.proxy = proxy
+        self.signal_name = signal_name
+        self.callback = callback
+        setattr(self.proxy, signal_name, callback)
+
+    def __call__(self, *args, **kwargs):
+        """Call this instance."""
+        func = getattr(self.proxy, self.signal_name, None)
+        if func is not None:
+            return func(*args, **kwargs)
+
+    def remove(self):
+        """Remove this signal."""
+        if getattr(self.proxy, self.signal_name, False):
+            setattr(self.proxy, self.signal_name, None)
+
+
 class CredentialsManagement(object):
     """Object that manages Ubuntu One credentials."""
 
@@ -81,35 +103,35 @@
 
     def register_to_credentials_stored(self, callback):
         """Register to the CredentialsStored dbus signal."""
-        self.sso_proxy.on_credentials_stored_cb = callback
-        return self.sso_proxy.on_credentials_stored_cb
+        return RemovableSignal(self.sso_proxy, "on_credentials_stored_cb",
+                               callback)
 
     def register_to_credentials_cleared(self, callback):
         """Register to the CredentialsCleared dbus signal."""
-        self.sso_proxy.on_credentials_cleared_cb = callback
-        return self.sso_proxy.on_credentials_cleared_cb
+        return RemovableSignal(self.sso_proxy, "on_credentials_cleared_cb",
+                               callback)
 
     def register_to_credentials_found(self, callback):
         """Register to the CredentialsFound dbus signal."""
         pass_credentials = lambda app_name, creds: callback(creds)
-        self.sso_proxy.on_credentials_found_cb = pass_credentials
-        return self.sso_proxy.on_credentials_found_cb
+        return RemovableSignal(self.sso_proxy, "on_credentials_found_cb",
+                               pass_credentials)
 
     def register_to_credentials_not_found(self, callback):
         """Register to the CredentialsFound dbus signal."""
         pass_empty = lambda *args: callback()
-        self.sso_proxy.on_credentials_not_found_cb = pass_empty
-        return self.sso_proxy.on_credentials_not_found_cb
+        return RemovableSignal(self.sso_proxy, "on_credentials_not_found_cb",
+                               pass_empty)
 
     def register_to_authorization_denied(self, callback):
         """Register to the AuthorizationDenied dbus signal."""
-        self.sso_proxy.on_authorization_denied_cb = callback
-        return self.sso_proxy.on_authorization_denied_cb
+        return RemovableSignal(self.sso_proxy, "on_authorization_denied_cb",
+                               callback)
 
     def register_to_credentials_error(self, callback):
         """Register to the CredentialsError dbus signal."""
-        self.sso_proxy.on_credentials_error_cb = callback
-        return self.sso_proxy.on_credentials_error_cb
+        return RemovableSignal(self.sso_proxy, "on_credentials_error_cb",
+                               callback)
 
 
 @defer.inlineCallbacks