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

Proposed by Alejandro J. Cura on 2010-11-03
Status: Merged
Approved by: John Lenton on 2010-11-04
Approved revision: 641
Merged at revision: 640
Proposed branch: lp:~alecu/ubuntu-sso-client/store-keyring-on-main-thread
Merge into: lp:ubuntu-sso-client/stable-1-0
Diff against target: 109 lines (+28/-11)
2 files modified
ubuntu_sso/main.py (+20/-8)
ubuntu_sso/tests/test_main.py (+8/-3)
To merge this branch: bzr merge lp:~alecu/ubuntu-sso-client/store-keyring-on-main-thread
Reviewer Review Type Date Requested Status
John Lenton Approve on 2010-11-04
Natalia Bidart 2010-11-03 Approve on 2010-11-03
Review via email: mp+40005@code.launchpad.net

Commit message

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

Description of the change

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

To post a comment you must log in.
641. By Alejandro J. Cura on 2010-11-03

NO_OP for consistency

Natalia Bidart (nataliabidart) wrote :

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

Thanks for the fix!

review: Approve
John Lenton (chipaca) wrote :

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

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'ubuntu_sso/main.py'
2--- ubuntu_sso/main.py 2010-11-01 21:01:14 +0000
3+++ ubuntu_sso/main.py 2010-11-03 19:34:01 +0000
4@@ -55,6 +55,7 @@
5 logger = setup_logging("ubuntu_sso.main")
6 PING_URL = "https://one.ubuntu.com/oauth/sso-finished-so-get-tokens/"
7 SERVICE_URL = "https://login.ubuntu.com/api/1.0"
8+NO_OP = lambda *args, **kwargs: None
9
10
11 class NoDefaultConfigError(Exception):
12@@ -96,10 +97,16 @@
13 """The new password could not be set."""
14
15
16-def keyring_store_credentials(app_name, credentials):
17+def keyring_store_credentials(app_name, credentials, callback, *cb_args):
18 """Store the credentials in the keyring."""
19- logger.info('keyring_store_credentials: app_name "%s".', app_name)
20- Keyring(app_name).set_ubuntusso_attr(credentials)
21+
22+ def _inner():
23+ """Store the credentials, and trigger the callback."""
24+ logger.info('keyring_store_credentials: app_name "%s".', app_name)
25+ Keyring(app_name).set_ubuntusso_attr(credentials)
26+ callback(*cb_args)
27+
28+ gobject.idle_add(_inner)
29
30
31 def keyring_get_credentials(app_name):
32@@ -422,8 +429,8 @@
33 token_name)
34 logger.debug('user is validated? %r.', is_validated)
35 if is_validated:
36- keyring_store_credentials(app_name, credentials)
37- self.LoggedIn(app_name, email)
38+ keyring_store_credentials(app_name, credentials,
39+ self.LoggedIn, app_name, email)
40 else:
41 self.UserNotValidated(app_name, email)
42 blocking(f, app_name, success_cb, self.LoginError)
43@@ -450,9 +457,14 @@
44 token_name = get_token_name(app_name)
45 credentials = self.processor.validate_email(email, password,
46 email_token, token_name)
47- keyring_store_credentials(app_name, credentials)
48- return email
49- blocking(f, app_name, self.EmailValidated, self.EmailValidationError)
50+
51+ def _email_stored():
52+ """The email was stored, so call the signal."""
53+ self.EmailValidated(app_name, email)
54+
55+ keyring_store_credentials(app_name, credentials, _email_stored)
56+
57+ blocking(f, app_name, NO_OP, self.EmailValidationError)
58
59 # request_password_reset_token signals
60 @dbus.service.signal(DBUS_IFACE_USER_NAME, signature="ss")
61
62=== modified file 'ubuntu_sso/tests/test_main.py'
63--- ubuntu_sso/tests/test_main.py 2010-11-01 21:01:14 +0000
64+++ ubuntu_sso/tests/test_main.py 2010-11-03 19:34:01 +0000
65@@ -417,12 +417,13 @@
66 mockbus._register_object_path(ARGS)
67 self.mockprocessorclass = None
68
69- def ksc(k, val):
70+ def ksc(k, val, callback, *cb_args):
71 """Assert over token and app_name."""
72 self.assertEqual(k, APP_NAME)
73 self.assertEqual(val, TOKEN)
74 self.keyring_was_set = True
75 self.keyring_values = k, val
76+ callback(*cb_args)
77
78 self.patch(ubuntu_sso.main, "keyring_store_credentials", ksc)
79 self.keyring_was_set = False
80@@ -870,7 +871,7 @@
81 self.assertEqual(result["errtype"], e.__class__.__name__)
82
83
84-class KeyringCredentialsTestCase(MockerTestCase):
85+class KeyringCredentialsTestCase(TestCase, MockerTestCase):
86 """Check the functions that access the keyring."""
87
88 # Invalid name (should match ([a-z_][a-z0-9_]*|[A-Z_][A-Z0-9_]*)$)
89@@ -878,15 +879,19 @@
90
91 def test_keyring_store_cred(self):
92 """Verify the method that stores credentials."""
93+ idle_add = lambda f, *args, **kwargs: f(*args, **kwargs)
94+ self.patch(gobject, "idle_add", idle_add)
95 token_value = TOKEN
96 mockKeyringClass = self.mocker.replace("ubuntu_sso.keyring.Keyring")
97 mockKeyringClass(APP_NAME)
98 mockKeyring = self.mocker.mock()
99+ callback = self.mocker.mock()
100 self.mocker.result(mockKeyring)
101 mockKeyring.set_ubuntusso_attr(token_value)
102+ callback(1, 2, 3)
103 self.mocker.replay()
104
105- keyring_store_credentials(APP_NAME, token_value)
106+ keyring_store_credentials(APP_NAME, token_value, callback, 1, 2, 3)
107
108 def test_keyring_get_cred(self):
109 """The method returns the right token."""

Subscribers

People subscribed via source and target branches