Merge lp:~alecu/ubuntu-sso-client/store-keyring-on-main-thread into lp:ubuntu-sso-client/stable-1-0

[Alejandro J. Cura]

Store credentials on the keyring *only* from the main thread (LP: #656545)

Store credentials on the keyring *only* from the main thread (LP: #656545)

[lp:~alecu/ubuntu-sso-client/store-keyring-on-main-thread]

641. By Alejandro J. Cura on 2010-11-03

NO_OP for consistency

[Natalia Bidart (nataliabidart)]

The solution is hacky but sadly we don't have other, cleaner option :-(

Thanks for the fix!

review: Approve

[John Lenton (chipaca)]

it's not at all clear to me that this fixes the issue. But I'm OK with us trying :)

review: Approve

=== modified file 'ubuntu_sso/main.py'
--- ubuntu_sso/main.py	2010-11-01 21:01:14 +0000
+++ ubuntu_sso/main.py	2010-11-03 19:34:01 +0000
@@ -55,6 +55,7 @@
 logger = setup_logging("ubuntu_sso.main")
 PING_URL = "https://one.ubuntu.com/oauth/sso-finished-so-get-tokens/"
 SERVICE_URL = "https://login.ubuntu.com/api/1.0"
+NO_OP = lambda *args, **kwargs: None
 
 
 class NoDefaultConfigError(Exception):
@@ -96,10 +97,16 @@
     """The new password could not be set."""
 
 
-def keyring_store_credentials(app_name, credentials):
+def keyring_store_credentials(app_name, credentials, callback, *cb_args):
     """Store the credentials in the keyring."""
-    logger.info('keyring_store_credentials: app_name "%s".', app_name)
-    Keyring(app_name).set_ubuntusso_attr(credentials)
+
+    def _inner():
+        """Store the credentials, and trigger the callback."""
+        logger.info('keyring_store_credentials: app_name "%s".', app_name)
+        Keyring(app_name).set_ubuntusso_attr(credentials)
+        callback(*cb_args)
+
+    gobject.idle_add(_inner)
 
 
 def keyring_get_credentials(app_name):
@@ -422,8 +429,8 @@
                                                        token_name)
             logger.debug('user is validated? %r.', is_validated)
             if is_validated:
-                keyring_store_credentials(app_name, credentials)
-                self.LoggedIn(app_name, email)
+                keyring_store_credentials(app_name, credentials,
+                                          self.LoggedIn, app_name, email)
             else:
                 self.UserNotValidated(app_name, email)
         blocking(f, app_name, success_cb, self.LoginError)
@@ -450,9 +457,14 @@
             token_name = get_token_name(app_name)
             credentials = self.processor.validate_email(email, password,
                                                       email_token, token_name)
-            keyring_store_credentials(app_name, credentials)
-            return email
-        blocking(f, app_name, self.EmailValidated, self.EmailValidationError)
+
+            def _email_stored():
+                """The email was stored, so call the signal."""
+                self.EmailValidated(app_name, email)
+
+            keyring_store_credentials(app_name, credentials, _email_stored)
+
+        blocking(f, app_name, NO_OP, self.EmailValidationError)
 
     # request_password_reset_token signals
     @dbus.service.signal(DBUS_IFACE_USER_NAME, signature="ss")
 
=== modified file 'ubuntu_sso/tests/test_main.py'
--- ubuntu_sso/tests/test_main.py	2010-11-01 21:01:14 +0000
+++ ubuntu_sso/tests/test_main.py	2010-11-03 19:34:01 +0000
@@ -417,12 +417,13 @@
         mockbus._register_object_path(ARGS)
         self.mockprocessorclass = None
 
-        def ksc(k, val):
+        def ksc(k, val, callback, *cb_args):
             """Assert over token and app_name."""
             self.assertEqual(k, APP_NAME)
             self.assertEqual(val, TOKEN)
             self.keyring_was_set = True
             self.keyring_values = k, val
+            callback(*cb_args)
 
         self.patch(ubuntu_sso.main, "keyring_store_credentials", ksc)
         self.keyring_was_set = False
@@ -870,7 +871,7 @@
         self.assertEqual(result["errtype"], e.__class__.__name__)
 
 
-class KeyringCredentialsTestCase(MockerTestCase):
+class KeyringCredentialsTestCase(TestCase, MockerTestCase):
     """Check the functions that access the keyring."""
 
     # Invalid name (should match ([a-z_][a-z0-9_]*|[A-Z_][A-Z0-9_]*)$)
@@ -878,15 +879,19 @@
 
     def test_keyring_store_cred(self):
         """Verify the method that stores credentials."""
+        idle_add = lambda f, *args, **kwargs: f(*args, **kwargs)
+        self.patch(gobject, "idle_add", idle_add)
         token_value = TOKEN
         mockKeyringClass = self.mocker.replace("ubuntu_sso.keyring.Keyring")
         mockKeyringClass(APP_NAME)
         mockKeyring = self.mocker.mock()
+        callback = self.mocker.mock()
         self.mocker.result(mockKeyring)
         mockKeyring.set_ubuntusso_attr(token_value)
+        callback(1, 2, 3)
         self.mocker.replay()
 
-        keyring_store_credentials(APP_NAME, token_value)
+        keyring_store_credentials(APP_NAME, token_value, callback, 1, 2, 3)
 
     def test_keyring_get_cred(self):
         """The method returns the right token."""