Merge lp:~rodrigo-moya/ubuntuone-client/use-sso-in-u1-prefs into lp:ubuntuone-client
- use-sso-in-u1-prefs
- Merge into trunk
Proposed by
Rodrigo Moya
| Status: | Merged |
|---|---|
| Approved by: | dobey |
| Approved revision: | 651 |
| Merged at revision: | 650 |
| Proposed branch: | lp:~rodrigo-moya/ubuntuone-client/use-sso-in-u1-prefs |
| Merge into: | lp:ubuntuone-client |
| Diff against target: |
517 lines (+71/-125) 2 files modified
bin/ubuntuone-preferences (+54/-82) tests/test_preferences.py (+17/-43) |
| To merge this branch: | bzr merge lp:~rodrigo-moya/ubuntuone-client/use-sso-in-u1-prefs |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| dobey (community) | Approve | ||
| Natalia Bidart (community) | Approve | ||
|
Review via email:
|
|||
Commit message
Make ubuntuone-
Description of the change
Make ubuntuone-
To post a comment you must log in.
- 648. By Rodrigo Moya
-
Some fixes
Revision history for this message
| dobey (dobey) wrote : | # |
Your change introduces a regression where multiple dialogs can be opened in the same process. There should only ever be one single dialog for this process.
review:
Needs Fixing
- 649. By Rodrigo Moya
-
Create the dialog only once
Revision history for this message
| dobey (dobey) wrote : | # |
It looks like test_login_check in the tests is skipped at the moment, but it should be updated to comply with the changes here too. I'd also avoid using globals for the oauth_token and oauth_consumer variables, and store them as attributes in the dialog (and pass them on to the other classes and methods that might need them).
I'm still confused about the changes for the dialog creation though. Why did you change it to not create the dialog when starting up? Shouldn't the dialog be created and shown, and its window id passed over to ubuntu_sso for setting as the parent?
The dialog creation was how it was, in order to show the dialog to the user as soon as possible, so that they know things are happening. Is there a good reason to not do that now?
review:
Needs Fixing
Revision history for this message
| Natalia Bidart (nataliabidart) wrote : | # |
The docstrings were not fixed (see lines 92-94 and 104-106)
review:
Needs Fixing
Revision history for this message
| Rodrigo Moya (rodrigo-moya) wrote : | # |
> The docstrings were not fixed (see lines 92-94 and 104-106)
I think they're fixed now, I guess you wanted them on just 1 line?
- 650. By Rodrigo Moya
-
Fixed docstrings
Revision history for this message
| Rodrigo Moya (rodrigo-moya) wrote : | # |
> It looks like test_login_check in the tests is skipped at the moment, but it
> should be updated to comply with the changes here too. I'd also avoid using
> globals for the oauth_token and oauth_consumer variables, and store them as
> attributes in the dialog (and pass them on to the other classes and methods
> that might need them).
>
test_login_check still depends on the real DBus service to be run, not sure how we can make it work without depending on the tests in ubuntu-sso-client. If there's a way, I'm ok with doing it, but not in this branch, which is needed for the new release to be done today or tomorrow at the latest.
> I'm still confused about the changes for the dialog creation though. Why did
> you change it to not create the dialog when starting up? Shouldn't the dialog
> be created and shown, and its window id passed over to ubuntu_sso for setting
> as the parent?
>
well, what was the point of having a dialog with all the widgets showing temporary data ('Unknown', etc)? So, now, we create the dialog associated to the LoginHandler, and show it when we get the credentials or when calling _present
> The dialog creation was how it was, in order to show the dialog to the user as
> soon as possible, so that they know things are happening. Is there a good
> reason to not do that now?
>
if the credentials are in the keyring, the dialog will show up inmediately, if not, they will see the SSO login/register screen
Revision history for this message
| Natalia Bidart (nataliabidart) wrote : | # |
Looks great now!
review:
Approve
- 651. By Rodrigo Moya
-
Move instantiation of dialog back to its original place
Revision history for this message
| dobey (dobey) : | # |
review:
Approve
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
| 1 | === modified file 'bin/ubuntuone-preferences' | |||
| 2 | --- bin/ubuntuone-preferences 2010-06-16 18:44:02 +0000 | |||
| 3 | +++ bin/ubuntuone-preferences 2010-08-24 21:19:41 +0000 | |||
| 4 | @@ -3,9 +3,10 @@ | |||
| 5 | 3 | 3 | ||
| 6 | 4 | # ubuntuone-client-applet - Tray icon applet for managing Ubuntu One | 4 | # ubuntuone-client-applet - Tray icon applet for managing Ubuntu One |
| 7 | 5 | # | 5 | # |
| 9 | 6 | # Author: Rodney Dawes <rodney.dawes@canonical.com> | 6 | # Authors: Rodney Dawes <rodney.dawes@canonical.com> |
| 10 | 7 | # Rodrigo Moya <rodrigo.moya@canonical.com> | ||
| 11 | 7 | # | 8 | # |
| 13 | 8 | # Copyright 2009 Canonical Ltd. | 9 | # Copyright 2009-2010 Canonical Ltd. |
| 14 | 9 | # | 10 | # |
| 15 | 10 | # This program is free software: you can redistribute it and/or modify it | 11 | # This program is free software: you can redistribute it and/or modify it |
| 16 | 11 | # under the terms of the GNU General Public License version 3, as published | 12 | # under the terms of the GNU General Public License version 3, as published |
| 17 | @@ -29,12 +30,11 @@ | |||
| 18 | 29 | import sys | 30 | import sys |
| 19 | 30 | import time | 31 | import time |
| 20 | 31 | import gettext | 32 | import gettext |
| 21 | 32 | import gnomekeyring | ||
| 22 | 33 | import subprocess | 33 | import subprocess |
| 23 | 34 | from threading import Thread | 34 | from threading import Thread |
| 24 | 35 | from oauth import oauth | 35 | from oauth import oauth |
| 25 | 36 | 36 | ||
| 27 | 37 | from ubuntu_sso import DBUS_IFACE_AUTH_NAME, DBUS_PATH_AUTH | 37 | from ubuntu_sso import DBUS_IFACE_AUTH_NAME, DBUS_IFACE_CRED_NAME, DBUS_CRED_PATH |
| 28 | 38 | from ubuntuone import clientdefs | 38 | from ubuntuone import clientdefs |
| 29 | 39 | from ubuntuone.syncdaemon.tools import SyncDaemonTool | 39 | from ubuntuone.syncdaemon.tools import SyncDaemonTool |
| 30 | 40 | from ubuntuone.api.restclient import RestClient | 40 | from ubuntuone.api.restclient import RestClient |
| 31 | @@ -62,7 +62,7 @@ | |||
| 32 | 62 | try: | 62 | try: |
| 33 | 63 | from desktopcouch.replication_services import ubuntuone as dcouch | 63 | from desktopcouch.replication_services import ubuntuone as dcouch |
| 34 | 64 | except ImportError: | 64 | except ImportError: |
| 36 | 65 | logger.error(_("DesktopCouch replication API not found")) | 65 | logger.error("DesktopCouch replication API not found") |
| 37 | 66 | 66 | ||
| 38 | 67 | DBusGMainLoop(set_as_default=True) | 67 | DBusGMainLoop(set_as_default=True) |
| 39 | 68 | 68 | ||
| 40 | @@ -92,7 +92,8 @@ | |||
| 41 | 92 | BW_CHCK_ARGS = ['dpkg', '-l', BW_PKG_NAME] | 92 | BW_CHCK_ARGS = ['dpkg', '-l', BW_PKG_NAME] |
| 42 | 93 | 93 | ||
| 43 | 94 | # This is a global so we can avoid creating multiple instances in some cases | 94 | # This is a global so we can avoid creating multiple instances in some cases |
| 45 | 95 | prefs_dialog = None | 95 | oauth_token = None |
| 46 | 96 | oauth_consumer = None | ||
| 47 | 96 | 97 | ||
| 48 | 97 | def dbus_async(*args, **kwargs): | 98 | def dbus_async(*args, **kwargs): |
| 49 | 98 | """Simple handler to make dbus do stuff async.""" | 99 | """Simple handler to make dbus do stuff async.""" |
| 50 | @@ -103,48 +104,31 @@ | |||
| 51 | 103 | """Make a login request to the login handling daemon.""" | 104 | """Make a login request to the login handling daemon.""" |
| 52 | 104 | try: | 105 | try: |
| 53 | 105 | client = bus.get_object(DBUS_IFACE_AUTH_NAME, | 106 | client = bus.get_object(DBUS_IFACE_AUTH_NAME, |
| 55 | 106 | DBUS_PATH_AUTH, | 107 | DBUS_CRED_PATH, |
| 56 | 107 | follow_name_owner_changes=True) | 108 | follow_name_owner_changes=True) |
| 61 | 108 | iface = dbus.Interface(client, DBUS_IFACE_AUTH_NAME) | 109 | iface = dbus.Interface(client, DBUS_IFACE_CRED_NAME) |
| 62 | 109 | iface.login('https://ubuntuone.com', 'ubuntuone', | 110 | iface.login_or_register_to_get_credentials( |
| 63 | 110 | reply_handler=dbus_async, | 111 | clientdefs.APP_NAME, |
| 64 | 111 | error_handler=error_handler) | 112 | clientdefs.TC_URL, |
| 65 | 113 | clientdefs.DESCRIPTION, | ||
| 66 | 114 | 0, | ||
| 67 | 115 | reply_handler=dbus_async, | ||
| 68 | 116 | error_handler=error_handler) | ||
| 69 | 112 | except DBusException, e: | 117 | except DBusException, e: |
| 70 | 113 | error_handler(e) | 118 | error_handler(e) |
| 71 | 114 | 119 | ||
| 91 | 115 | def get_access_token(keyring): | 120 | def do_rest_request(rest_client, url, method, callback): |
| 92 | 116 | """Get the access token from the keyring.""" | 121 | """Helper that handles the REST request.""" |
| 93 | 117 | items = [] | 122 | result = rest_client.call(url, method, oauth_consumer, oauth_token) |
| 75 | 118 | try: | ||
| 76 | 119 | items = keyring.find_items_sync( | ||
| 77 | 120 | keyring.ITEM_GENERIC_SECRET, | ||
| 78 | 121 | {'ubuntuone-realm': "https://ubuntuone.com", | ||
| 79 | 122 | 'oauth-consumer-key': 'ubuntuone'}) | ||
| 80 | 123 | secret = items[0].secret | ||
| 81 | 124 | return oauth.OAuthToken.from_string(secret) | ||
| 82 | 125 | except (gnomekeyring.NoMatchError, gnomekeyring.DeniedError): | ||
| 83 | 126 | return None | ||
| 84 | 127 | |||
| 85 | 128 | def do_rest_request(rest_client, url, method, token, callback): | ||
| 86 | 129 | """ | ||
| 87 | 130 | Helper that handles the REST response. | ||
| 88 | 131 | """ | ||
| 89 | 132 | consumer = oauth.OAuthConsumer('ubuntuone', 'hammertime') | ||
| 90 | 133 | result = rest_client.call(url, method, consumer, token) | ||
| 94 | 134 | 123 | ||
| 95 | 135 | gtk.gdk.threads_enter() | 124 | gtk.gdk.threads_enter() |
| 96 | 136 | callback(result) | 125 | callback(result) |
| 97 | 137 | gtk.gdk.threads_leave() | 126 | gtk.gdk.threads_leave() |
| 98 | 138 | 127 | ||
| 106 | 139 | def make_rest_request(url=None, method='GET', | 128 | def make_rest_request(url=None, method='GET', callback=None): |
| 107 | 140 | callback=None, keyring=None): | 129 | """Helper that makes an oauth-wrapped REST request.""" |
| 101 | 141 | """ | ||
| 102 | 142 | Helper that makes an oauth-wrapped REST request. | ||
| 103 | 143 | """ | ||
| 104 | 144 | token = get_access_token(keyring) | ||
| 105 | 145 | |||
| 108 | 146 | rest_client = RestClient(url) | 130 | rest_client = RestClient(url) |
| 110 | 147 | Thread(target=do_rest_request, args=(rest_client, url, method, token, callback)).start() | 131 | Thread(target=do_rest_request, args=(rest_client, url, method, callback)).start() |
| 111 | 148 | 132 | ||
| 112 | 149 | class DevicesWidget(gtk.Table): | 133 | class DevicesWidget(gtk.Table): |
| 113 | 150 | """ | 134 | """ |
| 114 | @@ -152,20 +136,14 @@ | |||
| 115 | 152 | """ | 136 | """ |
| 116 | 153 | def __init__(self, | 137 | def __init__(self, |
| 117 | 154 | bus, | 138 | bus, |
| 118 | 155 | keyring=gnomekeyring, | ||
| 119 | 156 | realm='https://ubuntuone.com', | ||
| 120 | 157 | consumer_key='ubuntuone', | ||
| 121 | 158 | url='https://one.ubuntu.com/api/1.0/devices/'): | 139 | url='https://one.ubuntu.com/api/1.0/devices/'): |
| 122 | 159 | super(DevicesWidget, self).__init__(rows=2, columns=3) | 140 | super(DevicesWidget, self).__init__(rows=2, columns=3) |
| 123 | 160 | self.bus = bus | 141 | self.bus = bus |
| 124 | 161 | self.keyring = keyring | ||
| 125 | 162 | self.sdtool = SyncDaemonTool(bus) | 142 | self.sdtool = SyncDaemonTool(bus) |
| 126 | 163 | self.set_border_width(6) | 143 | self.set_border_width(6) |
| 127 | 164 | self.set_row_spacings(6) | 144 | self.set_row_spacings(6) |
| 128 | 165 | self.set_col_spacings(6) | 145 | self.set_col_spacings(6) |
| 129 | 166 | self.devices = None | 146 | self.devices = None |
| 130 | 167 | self.realm = realm | ||
| 131 | 168 | self.consumer_key = consumer_key | ||
| 132 | 169 | self.base_url = url | 147 | self.base_url = url |
| 133 | 170 | self.conn = None | 148 | self.conn = None |
| 134 | 171 | self.consumer = None | 149 | self.consumer = None |
| 135 | @@ -301,14 +279,7 @@ | |||
| 136 | 301 | 279 | ||
| 137 | 302 | Hook up parse_devices to run on the result (when it gets here). | 280 | Hook up parse_devices to run on the result (when it gets here). |
| 138 | 303 | """ | 281 | """ |
| 147 | 304 | try: | 282 | make_rest_request(url=self.base_url, callback=self.parse_devices) |
| 140 | 305 | get_access_token(self.keyring) | ||
| 141 | 306 | except gnomekeyring.NoMatchError: | ||
| 142 | 307 | self.error("No token in the keyring") | ||
| 143 | 308 | self.devices = None | ||
| 144 | 309 | else: | ||
| 145 | 310 | make_rest_request(url=self.base_url, keyring=self.keyring, | ||
| 146 | 311 | callback=self.parse_devices) | ||
| 148 | 312 | 283 | ||
| 149 | 313 | def parse_devices(self, result): | 284 | def parse_devices(self, result): |
| 150 | 314 | """ | 285 | """ |
| 151 | @@ -356,8 +327,6 @@ | |||
| 152 | 356 | """ | 327 | """ |
| 153 | 357 | self.clear_devices_view() | 328 | self.clear_devices_view() |
| 154 | 358 | 329 | ||
| 155 | 359 | token = get_access_token(self.keyring) | ||
| 156 | 360 | |||
| 157 | 361 | fsync_enabled = self.sdtool.is_files_sync_enabled() | 330 | fsync_enabled = self.sdtool.is_files_sync_enabled() |
| 158 | 362 | 331 | ||
| 159 | 363 | if not self.devices: | 332 | if not self.devices: |
| 160 | @@ -365,7 +334,7 @@ | |||
| 161 | 365 | # a stopgap device so you can at least try to connect | 334 | # a stopgap device so you can at least try to connect |
| 162 | 366 | self.devices = [{'kind': 'Computer', | 335 | self.devices = [{'kind': 'Computer', |
| 163 | 367 | 'description': _("<LOCAL MACHINE>"), | 336 | 'description': _("<LOCAL MACHINE>"), |
| 165 | 368 | 'token': token.key if token else '', | 337 | 'token': oauth_token.key if oauth_token else '', |
| 166 | 369 | 'FAKE': 'YES'}] | 338 | 'FAKE': 'YES'}] |
| 167 | 370 | else: | 339 | else: |
| 168 | 371 | self.resize(len(self.devices)+1, 3) | 340 | self.resize(len(self.devices)+1, 3) |
| 169 | @@ -387,7 +356,7 @@ | |||
| 170 | 387 | butn.connect('clicked', self.remove, | 356 | butn.connect('clicked', self.remove, |
| 171 | 388 | row['kind'], row.get('token')) | 357 | row['kind'], row.get('token')) |
| 172 | 389 | self.attach(butn, 2, 3, i, i+1, xoptions=0, yoptions=0) | 358 | self.attach(butn, 2, 3, i, i+1, xoptions=0, yoptions=0) |
| 174 | 390 | if ((token and row.get('token') == token.key or 'FAKE' in row) | 359 | if ((oauth_token and row.get('token') == oauth_token.key or 'FAKE' in row) |
| 175 | 391 | and fsync_enabled): | 360 | and fsync_enabled): |
| 176 | 392 | self.bw_chk = ck_btn = gtk.CheckButton( | 361 | self.bw_chk = ck_btn = gtk.CheckButton( |
| 177 | 393 | _("_Limit Bandwidth Usage")) | 362 | _("_Limit Bandwidth Usage")) |
| 178 | @@ -463,20 +432,18 @@ | |||
| 179 | 463 | """ | 432 | """ |
| 180 | 464 | make_rest_request(url=('%sremove/%s/%s' % (self.base_url, | 433 | make_rest_request(url=('%sremove/%s/%s' % (self.base_url, |
| 181 | 465 | kind.lower(), token)), | 434 | kind.lower(), token)), |
| 182 | 466 | keyring=self.keyring, | ||
| 183 | 467 | callback=self.parse_devices) | 435 | callback=self.parse_devices) |
| 184 | 468 | local = get_access_token(self.keyring) | ||
| 185 | 469 | def local_removal_cb(*args, **kwargs): | 436 | def local_removal_cb(*args, **kwargs): |
| 186 | 470 | """Try to get a new token if we remove the local one.""" | 437 | """Try to get a new token if we remove the local one.""" |
| 187 | 471 | do_login_request(self.bus, self.error) | 438 | do_login_request(self.bus, self.error) |
| 188 | 472 | 439 | ||
| 190 | 473 | if token == local.key: | 440 | if token == oauth_token.key: |
| 191 | 474 | try: | 441 | try: |
| 194 | 475 | client = self.bus.get_object(DBUS_IFACE_AUTH_NAME, | 442 | client = self.bus.get_object(DBUS_IFACE_CRED_NAME, |
| 195 | 476 | DBUS_PATH_AUTH, | 443 | DBUS_CRED_PATH, |
| 196 | 477 | follow_name_owner_changes=True) | 444 | follow_name_owner_changes=True) |
| 199 | 478 | iface = dbus.Interface(client, DBUS_IFACE_AUTH_NAME) | 445 | iface = dbus.Interface(client, DBUS_IFACE_CRED_NAME) |
| 200 | 479 | iface.clear_token('https://ubuntuone.com', 'ubuntuone', | 446 | iface.clear_token('ubuntuone', |
| 201 | 480 | reply_handler=local_removal_cb, | 447 | reply_handler=local_removal_cb, |
| 202 | 481 | error_handler=self.error) | 448 | error_handler=self.error) |
| 203 | 482 | except DBusException, e: | 449 | except DBusException, e: |
| 204 | @@ -486,7 +453,7 @@ | |||
| 205 | 486 | class UbuntuOneDialog(gtk.Dialog): | 453 | class UbuntuOneDialog(gtk.Dialog): |
| 206 | 487 | """Preferences dialog.""" | 454 | """Preferences dialog.""" |
| 207 | 488 | 455 | ||
| 209 | 489 | def __init__(self, config=None, keyring=gnomekeyring, *args, **kw): | 456 | def __init__(self, config=None, *args, **kw): |
| 210 | 490 | """Initializes our config dialog.""" | 457 | """Initializes our config dialog.""" |
| 211 | 491 | super(UbuntuOneDialog, self).__init__(*args, **kw) | 458 | super(UbuntuOneDialog, self).__init__(*args, **kw) |
| 212 | 492 | self.set_title(_("Ubuntu One Preferences")) | 459 | self.set_title(_("Ubuntu One Preferences")) |
| 213 | @@ -506,7 +473,6 @@ | |||
| 214 | 506 | self.ums_id = None | 473 | self.ums_id = None |
| 215 | 507 | 474 | ||
| 216 | 508 | self.__bus = dbus.SessionBus() | 475 | self.__bus = dbus.SessionBus() |
| 217 | 509 | self.keyring = keyring | ||
| 218 | 510 | 476 | ||
| 219 | 511 | # Timeout ID to avoid spamming DBus from spinbutton changes | 477 | # Timeout ID to avoid spamming DBus from spinbutton changes |
| 220 | 512 | self.__update_id = 0 | 478 | self.__update_id = 0 |
| 221 | @@ -663,7 +629,6 @@ | |||
| 222 | 663 | def request_quota_info(self): | 629 | def request_quota_info(self): |
| 223 | 664 | """Request new quota info from server, and update display.""" | 630 | """Request new quota info from server, and update display.""" |
| 224 | 665 | make_rest_request(url='https://one.ubuntu.com/api/quota/', | 631 | make_rest_request(url='https://one.ubuntu.com/api/quota/', |
| 225 | 666 | keyring=self.keyring, | ||
| 226 | 667 | callback=self.got_quota_info) | 632 | callback=self.got_quota_info) |
| 227 | 668 | 633 | ||
| 228 | 669 | def got_account_info(self, user): | 634 | def got_account_info(self, user): |
| 229 | @@ -675,7 +640,6 @@ | |||
| 230 | 675 | def request_account_info(self): | 640 | def request_account_info(self): |
| 231 | 676 | """Request account info from server, and update display.""" | 641 | """Request account info from server, and update display.""" |
| 232 | 677 | make_rest_request(url='https://one.ubuntu.com/api/account/', | 642 | make_rest_request(url='https://one.ubuntu.com/api/account/', |
| 233 | 678 | keyring=self.keyring, | ||
| 234 | 679 | callback=self.got_account_info) | 643 | callback=self.got_account_info) |
| 235 | 680 | 644 | ||
| 236 | 681 | def __bw_inst_cb(self, button): | 645 | def __bw_inst_cb(self, button): |
| 237 | @@ -950,7 +914,7 @@ | |||
| 238 | 950 | self.notebook.append_page(sw) | 914 | self.notebook.append_page(sw) |
| 239 | 951 | self.notebook.set_tab_label_text(sw, _("Devices")) | 915 | self.notebook.set_tab_label_text(sw, _("Devices")) |
| 240 | 952 | sw.show() | 916 | sw.show() |
| 242 | 953 | self.devices = DevicesWidget(self.__bus, self.keyring) | 917 | self.devices = DevicesWidget(self.__bus) |
| 243 | 954 | sw.add_with_viewport(self.devices) | 918 | sw.add_with_viewport(self.devices) |
| 244 | 955 | self.devices.list_devices() | 919 | self.devices.list_devices() |
| 245 | 956 | self.devices.show_all() | 920 | self.devices.show_all() |
| 246 | @@ -1036,7 +1000,6 @@ | |||
| 247 | 1036 | def __init__(self, dialog, *args, **kw): | 1000 | def __init__(self, dialog, *args, **kw): |
| 248 | 1037 | self.bus = dbus.SessionBus() | 1001 | self.bus = dbus.SessionBus() |
| 249 | 1038 | 1002 | ||
| 250 | 1039 | # The actual UI | ||
| 251 | 1040 | self.dialog = dialog | 1003 | self.dialog = dialog |
| 252 | 1041 | 1004 | ||
| 253 | 1042 | # DBus object magic | 1005 | # DBus object magic |
| 254 | @@ -1059,20 +1022,28 @@ | |||
| 255 | 1059 | if self.dialog.get_property('visible'): | 1022 | if self.dialog.get_property('visible'): |
| 256 | 1060 | self.dialog.present_with_time(int(time.time())) | 1023 | self.dialog.present_with_time(int(time.time())) |
| 257 | 1061 | 1024 | ||
| 259 | 1062 | def got_newcredentials(self, realm, consumer_key): | 1025 | def got_newcredentials(self, app_name, credentials): |
| 260 | 1063 | """Show our dialog, since we can do stuff now.""" | 1026 | """Show our dialog, since we can do stuff now.""" |
| 265 | 1064 | self.present() | 1027 | global oauth_consumer |
| 266 | 1065 | 1028 | global oauth_token | |
| 267 | 1066 | def got_oautherror(self, message=None): | 1029 | |
| 268 | 1067 | """Got an error during oauth.""" | 1030 | if app_name == clientdefs.APP_NAME: |
| 269 | 1031 | oauth_consumer = oauth.OAuthConsumer(credentials['consumer_key'], | ||
| 270 | 1032 | credentials['consumer_secret']) | ||
| 271 | 1033 | oauth_token = oauth.OAuthToken(credentials['token'], | ||
| 272 | 1034 | credentials['token_secret']) | ||
| 273 | 1035 | self.present() | ||
| 274 | 1036 | |||
| 275 | 1037 | def got_credentialserror(self, message=None): | ||
| 276 | 1038 | """Got an error during authentication.""" | ||
| 277 | 1068 | if message: | 1039 | if message: |
| 278 | 1069 | logger.error(message) | 1040 | logger.error(message) |
| 279 | 1070 | else: | 1041 | else: |
| 281 | 1071 | logger.error(_("OAuthError with no message.")) | 1042 | logger.error("CredentialsError with no message.") |
| 282 | 1072 | 1043 | ||
| 283 | 1073 | def got_authdenied(self): | 1044 | def got_authdenied(self): |
| 284 | 1074 | """User denied access.""" | 1045 | """User denied access.""" |
| 286 | 1075 | logger.error(_("Authorization was denied.")) | 1046 | logger.error("Authorization was denied.") |
| 287 | 1076 | 1047 | ||
| 288 | 1077 | def got_dbus_error(self, error): | 1048 | def got_dbus_error(self, error): |
| 289 | 1078 | """Got a DBusError.""" | 1049 | """Got a DBusError.""" |
| 290 | @@ -1082,16 +1053,16 @@ | |||
| 291 | 1082 | """Register the dbus signal handlers.""" | 1053 | """Register the dbus signal handlers.""" |
| 292 | 1083 | self.bus.add_signal_receiver( | 1054 | self.bus.add_signal_receiver( |
| 293 | 1084 | handler_function=self.got_newcredentials, | 1055 | handler_function=self.got_newcredentials, |
| 296 | 1085 | signal_name='NewCredentials', | 1056 | signal_name='CredentialsFound', |
| 297 | 1086 | dbus_interface=DBUS_IFACE_AUTH_NAME) | 1057 | dbus_interface=DBUS_IFACE_CRED_NAME) |
| 298 | 1087 | self.bus.add_signal_receiver( | 1058 | self.bus.add_signal_receiver( |
| 302 | 1088 | handler_function=self.got_oautherror, | 1059 | handler_function=self.got_credentialserror, |
| 303 | 1089 | signal_name='OAuthError', | 1060 | signal_name='CredentialsError', |
| 304 | 1090 | dbus_interface=DBUS_IFACE_AUTH_NAME) | 1061 | dbus_interface=DBUS_IFACE_CRED_NAME) |
| 305 | 1091 | self.bus.add_signal_receiver( | 1062 | self.bus.add_signal_receiver( |
| 306 | 1092 | handler_function=self.got_authdenied, | 1063 | handler_function=self.got_authdenied, |
| 307 | 1093 | signal_name='AuthorizationDenied', | 1064 | signal_name='AuthorizationDenied', |
| 309 | 1094 | dbus_interface=DBUS_IFACE_AUTH_NAME) | 1065 | dbus_interface=DBUS_IFACE_CRED_NAME) |
| 310 | 1095 | 1066 | ||
| 311 | 1096 | 1067 | ||
| 312 | 1097 | if __name__ == "__main__": | 1068 | if __name__ == "__main__": |
| 313 | @@ -1120,6 +1091,7 @@ | |||
| 314 | 1120 | try: | 1091 | try: |
| 315 | 1121 | # The prefs dialog | 1092 | # The prefs dialog |
| 316 | 1122 | gtk.gdk.threads_enter() | 1093 | gtk.gdk.threads_enter() |
| 317 | 1094 | |||
| 318 | 1123 | prefs_dialog = UbuntuOneDialog() | 1095 | prefs_dialog = UbuntuOneDialog() |
| 319 | 1124 | prefs_dialog.show() | 1096 | prefs_dialog.show() |
| 320 | 1125 | 1097 | ||
| 321 | 1126 | 1098 | ||
| 322 | === modified file 'tests/test_preferences.py' | |||
| 323 | --- tests/test_preferences.py 2010-08-16 14:00:13 +0000 | |||
| 324 | +++ tests/test_preferences.py 2010-08-24 21:19:41 +0000 | |||
| 325 | @@ -20,7 +20,6 @@ | |||
| 326 | 20 | import new | 20 | import new |
| 327 | 21 | import os | 21 | import os |
| 328 | 22 | 22 | ||
| 329 | 23 | import gnomekeyring | ||
| 330 | 24 | import ubuntu_sso | 23 | import ubuntu_sso |
| 331 | 25 | 24 | ||
| 332 | 26 | from contrib.mocker import MockerTestCase | 25 | from contrib.mocker import MockerTestCase |
| 333 | @@ -71,31 +70,6 @@ | |||
| 334 | 71 | self._old_path = ubuntu_sso.DBUS_PATH_AUTH | 70 | self._old_path = ubuntu_sso.DBUS_PATH_AUTH |
| 335 | 72 | ubuntu_sso.DBUS_PATH_AUTH = '/oauthdesktop' | 71 | ubuntu_sso.DBUS_PATH_AUTH = '/oauthdesktop' |
| 336 | 73 | 72 | ||
| 337 | 74 | # For testing keyring queries | ||
| 338 | 75 | self.keyring = self.mocker.mock() | ||
| 339 | 76 | self.item = self.mocker.mock(gnomekeyring.Found) | ||
| 340 | 77 | |||
| 341 | 78 | self.item_id = 999 | ||
| 342 | 79 | |||
| 343 | 80 | self.item.item_id | ||
| 344 | 81 | self.mocker.result(self.item_id) | ||
| 345 | 82 | self.mocker.count(0, None) | ||
| 346 | 83 | |||
| 347 | 84 | self.item.secret | ||
| 348 | 85 | self.mocker.result('oauth_token=access_key' | ||
| 349 | 86 | '&oauth_token_secret=access_secret') | ||
| 350 | 87 | self.mocker.count(0, None) | ||
| 351 | 88 | |||
| 352 | 89 | self.keyring.find_items_sync( | ||
| 353 | 90 | None, | ||
| 354 | 91 | {'ubuntuone-realm': 'https://ubuntuone.com', | ||
| 355 | 92 | 'oauth-consumer-key': 'ubuntuone'}) | ||
| 356 | 93 | self.mocker.count(0, None) | ||
| 357 | 94 | self.mocker.result([self.item]) | ||
| 358 | 95 | self.keyring.ITEM_GENERIC_SECRET | ||
| 359 | 96 | self.mocker.count(0, None) | ||
| 360 | 97 | self.mocker.result(None) | ||
| 361 | 98 | |||
| 362 | 99 | self.u1prefs.make_rest_request = self.make_rest_request | 73 | self.u1prefs.make_rest_request = self.make_rest_request |
| 363 | 100 | self.u1prefs.SyncDaemonTool = SyncDaemonTool | 74 | self.u1prefs.SyncDaemonTool = SyncDaemonTool |
| 364 | 101 | 75 | ||
| 365 | @@ -114,7 +88,7 @@ | |||
| 366 | 114 | yield DBusTwistedTestCase.tearDown(self) | 88 | yield DBusTwistedTestCase.tearDown(self) |
| 367 | 115 | 89 | ||
| 368 | 116 | def make_rest_request(self, url=None, method='GET', | 90 | def make_rest_request(self, url=None, method='GET', |
| 370 | 117 | callback=None, keyring=None): | 91 | callback=None): |
| 371 | 118 | """Override the real request call to mock some stuff.""" | 92 | """Override the real request call to mock some stuff.""" |
| 372 | 119 | if callback: | 93 | if callback: |
| 373 | 120 | callback(self.content) | 94 | callback(self.content) |
| 374 | @@ -124,7 +98,7 @@ | |||
| 375 | 124 | def test_bw_throttling(self): | 98 | def test_bw_throttling(self): |
| 376 | 125 | """Test that toggling bw throttling works correctly.""" | 99 | """Test that toggling bw throttling works correctly.""" |
| 377 | 126 | self.mocker.replay() | 100 | self.mocker.replay() |
| 379 | 127 | widget = self.u1prefs.DevicesWidget(None, keyring=self.keyring) | 101 | widget = self.u1prefs.DevicesWidget(None) |
| 380 | 128 | self.assertFalse(widget.got_limits) | 102 | self.assertFalse(widget.got_limits) |
| 381 | 129 | widget.update_bw_settings = self.mocker.mock() | 103 | widget.update_bw_settings = self.mocker.mock() |
| 382 | 130 | try: | 104 | try: |
| 383 | @@ -160,7 +134,7 @@ | |||
| 384 | 160 | 134 | ||
| 385 | 161 | def test_list_devices_fills_devices_list_with_fake_result_when_empty(self): | 135 | def test_list_devices_fills_devices_list_with_fake_result_when_empty(self): |
| 386 | 162 | self.mocker.replay() | 136 | self.mocker.replay() |
| 388 | 163 | widget = self.u1prefs.DevicesWidget(None, keyring=self.keyring) | 137 | widget = self.u1prefs.DevicesWidget(None) |
| 389 | 164 | widget.update_bw_settings = self.mocker.mock() | 138 | widget.update_bw_settings = self.mocker.mock() |
| 390 | 165 | try: | 139 | try: |
| 391 | 166 | widget.devices = [] | 140 | widget.devices = [] |
| 392 | @@ -174,7 +148,7 @@ | |||
| 393 | 174 | 148 | ||
| 394 | 175 | def test_list_devices_shows_devices_list(self): | 149 | def test_list_devices_shows_devices_list(self): |
| 395 | 176 | self.mocker.replay() | 150 | self.mocker.replay() |
| 397 | 177 | widget = self.u1prefs.DevicesWidget(None, keyring=self.keyring) | 151 | widget = self.u1prefs.DevicesWidget(None) |
| 398 | 178 | widget.update_bw_settings = self.mocker.mock() | 152 | widget.update_bw_settings = self.mocker.mock() |
| 399 | 179 | try: | 153 | try: |
| 400 | 180 | widget.devices = [] | 154 | widget.devices = [] |
| 401 | @@ -205,7 +179,7 @@ | |||
| 402 | 205 | 179 | ||
| 403 | 206 | def test_list_devices_shows_real_devices_list(self): | 180 | def test_list_devices_shows_real_devices_list(self): |
| 404 | 207 | self.mocker.replay() | 181 | self.mocker.replay() |
| 406 | 208 | widget = self.u1prefs.DevicesWidget(None, keyring=self.keyring) | 182 | widget = self.u1prefs.DevicesWidget(None) |
| 407 | 209 | widget.update_bw_settings = self.mocker.mock() | 183 | widget.update_bw_settings = self.mocker.mock() |
| 408 | 210 | try: | 184 | try: |
| 409 | 211 | widget.devices = [{'kind': 'Computer', | 185 | widget.devices = [{'kind': 'Computer', |
| 410 | @@ -242,7 +216,7 @@ | |||
| 411 | 242 | def test_quota_display(self): | 216 | def test_quota_display(self): |
| 412 | 243 | """Test that quota display works correctly.""" | 217 | """Test that quota display works correctly.""" |
| 413 | 244 | self.mocker.replay() | 218 | self.mocker.replay() |
| 415 | 245 | dialog = self.u1prefs.UbuntuOneDialog(keyring=self.keyring) | 219 | dialog = self.u1prefs.UbuntuOneDialog() |
| 416 | 246 | self.assertTrue(dialog is not None) | 220 | self.assertTrue(dialog is not None) |
| 417 | 247 | self.assertEqual(dialog.usage_graph.get_fraction(), 0.0) | 221 | self.assertEqual(dialog.usage_graph.get_fraction(), 0.0) |
| 418 | 248 | dialog.update_quota_display(1024, 2048) | 222 | dialog.update_quota_display(1024, 2048) |
| 419 | @@ -254,7 +228,7 @@ | |||
| 420 | 254 | """Test that we can request the quota info properly.""" | 228 | """Test that we can request the quota info properly.""" |
| 421 | 255 | self.content = {"total":2048, "used":1024} | 229 | self.content = {"total":2048, "used":1024} |
| 422 | 256 | self.mocker.replay() | 230 | self.mocker.replay() |
| 424 | 257 | dialog = self.u1prefs.UbuntuOneDialog(keyring=self.keyring) | 231 | dialog = self.u1prefs.UbuntuOneDialog() |
| 425 | 258 | self.assertTrue(dialog is not None) | 232 | self.assertTrue(dialog is not None) |
| 426 | 259 | self.assertEqual(dialog.usage_graph.get_fraction(), 0.0) | 233 | self.assertEqual(dialog.usage_graph.get_fraction(), 0.0) |
| 427 | 260 | dialog.request_quota_info() | 234 | dialog.request_quota_info() |
| 428 | @@ -266,7 +240,7 @@ | |||
| 429 | 266 | """Test that the quota notice is not visible if usage is low.""" | 240 | """Test that the quota notice is not visible if usage is low.""" |
| 430 | 267 | self.content = {"total":2048, "used":1024} | 241 | self.content = {"total":2048, "used":1024} |
| 431 | 268 | self.mocker.replay() | 242 | self.mocker.replay() |
| 433 | 269 | dialog = self.u1prefs.UbuntuOneDialog(keyring=self.keyring) | 243 | dialog = self.u1prefs.UbuntuOneDialog() |
| 434 | 270 | self.assertTrue(dialog is not None) | 244 | self.assertTrue(dialog is not None) |
| 435 | 271 | dialog.request_quota_info() | 245 | dialog.request_quota_info() |
| 436 | 272 | # the label should just be the blank '\n' | 246 | # the label should just be the blank '\n' |
| 437 | @@ -283,7 +257,7 @@ | |||
| 438 | 283 | """ | 257 | """ |
| 439 | 284 | self.content = {"total": 100, "used": 95} | 258 | self.content = {"total": 100, "used": 95} |
| 440 | 285 | self.mocker.replay() | 259 | self.mocker.replay() |
| 442 | 286 | dialog = self.u1prefs.UbuntuOneDialog(keyring=self.keyring) | 260 | dialog = self.u1prefs.UbuntuOneDialog() |
| 443 | 287 | self.assertTrue(dialog is not None) | 261 | self.assertTrue(dialog is not None) |
| 444 | 288 | dialog.request_quota_info() | 262 | dialog.request_quota_info() |
| 445 | 289 | # don't check the exact text, as it will probably | 263 | # don't check the exact text, as it will probably |
| 446 | @@ -304,7 +278,7 @@ | |||
| 447 | 304 | """ | 278 | """ |
| 448 | 305 | self.content = {"total": 50<<30, "used": 49<<30} | 279 | self.content = {"total": 50<<30, "used": 49<<30} |
| 449 | 306 | self.mocker.replay() | 280 | self.mocker.replay() |
| 451 | 307 | dialog = self.u1prefs.UbuntuOneDialog(keyring=self.keyring) | 281 | dialog = self.u1prefs.UbuntuOneDialog() |
| 452 | 308 | self.assertTrue(dialog is not None) | 282 | self.assertTrue(dialog is not None) |
| 453 | 309 | dialog.request_quota_info() | 283 | dialog.request_quota_info() |
| 454 | 310 | # don't check the exact text, as it will probably | 284 | # don't check the exact text, as it will probably |
| 455 | @@ -326,7 +300,7 @@ | |||
| 456 | 326 | """ | 300 | """ |
| 457 | 327 | self.content = {"total": 100, "used": 100} | 301 | self.content = {"total": 100, "used": 100} |
| 458 | 328 | self.mocker.replay() | 302 | self.mocker.replay() |
| 460 | 329 | dialog = self.u1prefs.UbuntuOneDialog(keyring=self.keyring) | 303 | dialog = self.u1prefs.UbuntuOneDialog() |
| 461 | 330 | self.assertTrue(dialog is not None) | 304 | self.assertTrue(dialog is not None) |
| 462 | 331 | dialog.request_quota_info() | 305 | dialog.request_quota_info() |
| 463 | 332 | # don't check the exact text, as it will probably | 306 | # don't check the exact text, as it will probably |
| 464 | @@ -344,7 +318,7 @@ | |||
| 465 | 344 | """Test that the quota notice is visible if usage is 100%.""" | 318 | """Test that the quota notice is visible if usage is 100%.""" |
| 466 | 345 | self.content = {"total": 50<<30, "used": 50<<30} | 319 | self.content = {"total": 50<<30, "used": 50<<30} |
| 467 | 346 | self.mocker.replay() | 320 | self.mocker.replay() |
| 469 | 347 | dialog = self.u1prefs.UbuntuOneDialog(keyring=self.keyring) | 321 | dialog = self.u1prefs.UbuntuOneDialog() |
| 470 | 348 | self.assertTrue(dialog is not None) | 322 | self.assertTrue(dialog is not None) |
| 471 | 349 | dialog.request_quota_info() | 323 | dialog.request_quota_info() |
| 472 | 350 | # don't check the exact text, as it will probably | 324 | # don't check the exact text, as it will probably |
| 473 | @@ -361,7 +335,7 @@ | |||
| 474 | 361 | self.content = {"username": "ubuntuone", "nickname": "Ubuntu One", | 335 | self.content = {"username": "ubuntuone", "nickname": "Ubuntu One", |
| 475 | 362 | "email": "uone@example.com"} | 336 | "email": "uone@example.com"} |
| 476 | 363 | self.mocker.replay() | 337 | self.mocker.replay() |
| 478 | 364 | dialog = self.u1prefs.UbuntuOneDialog(keyring=self.keyring) | 338 | dialog = self.u1prefs.UbuntuOneDialog() |
| 479 | 365 | self.assertTrue(dialog is not None) | 339 | self.assertTrue(dialog is not None) |
| 480 | 366 | dialog.request_account_info() | 340 | dialog.request_account_info() |
| 481 | 367 | self.content = {"total":2048, "used":1024} | 341 | self.content = {"total":2048, "used":1024} |
| 482 | @@ -387,7 +361,7 @@ | |||
| 483 | 387 | self.expect(toggle_db_sync('bookmarks', True)) | 361 | self.expect(toggle_db_sync('bookmarks', True)) |
| 484 | 388 | self.expect(toggle_db_sync('bookmarks', False)) | 362 | self.expect(toggle_db_sync('bookmarks', False)) |
| 485 | 389 | self.mocker.replay() | 363 | self.mocker.replay() |
| 487 | 390 | dialog = self.u1prefs.UbuntuOneDialog(keyring=self.keyring) | 364 | dialog = self.u1prefs.UbuntuOneDialog() |
| 488 | 391 | self.assertTrue(dialog is not None) | 365 | self.assertTrue(dialog is not None) |
| 489 | 392 | dialog.toggle_db_sync = toggle_db_sync | 366 | dialog.toggle_db_sync = toggle_db_sync |
| 490 | 393 | dialog.bookmarks_check.set_active(True) | 367 | dialog.bookmarks_check.set_active(True) |
| 491 | @@ -406,7 +380,7 @@ | |||
| 492 | 406 | self.expect(toggle_db_sync('contacts', True)) | 380 | self.expect(toggle_db_sync('contacts', True)) |
| 493 | 407 | self.expect(toggle_db_sync('contacts', False)) | 381 | self.expect(toggle_db_sync('contacts', False)) |
| 494 | 408 | self.mocker.replay() | 382 | self.mocker.replay() |
| 496 | 409 | dialog = self.u1prefs.UbuntuOneDialog(keyring=self.keyring) | 383 | dialog = self.u1prefs.UbuntuOneDialog() |
| 497 | 410 | self.assertTrue(dialog is not None) | 384 | self.assertTrue(dialog is not None) |
| 498 | 411 | dialog.toggle_db_sync = toggle_db_sync | 385 | dialog.toggle_db_sync = toggle_db_sync |
| 499 | 412 | dialog.abook_check.set_active(True) | 386 | dialog.abook_check.set_active(True) |
| 500 | @@ -422,7 +396,7 @@ | |||
| 501 | 422 | def test_toggle_files(self): | 396 | def test_toggle_files(self): |
| 502 | 423 | """Test toggling the files service on/off.""" | 397 | """Test toggling the files service on/off.""" |
| 503 | 424 | self.mocker.replay() | 398 | self.mocker.replay() |
| 505 | 425 | dialog = self.u1prefs.UbuntuOneDialog(keyring=self.keyring) | 399 | dialog = self.u1prefs.UbuntuOneDialog() |
| 506 | 426 | self.assertTrue(dialog is not None) | 400 | self.assertTrue(dialog is not None) |
| 507 | 427 | dialog.files_check.set_active(True) | 401 | dialog.files_check.set_active(True) |
| 508 | 428 | self.assertTrue(dialog.files_check.get_active()) | 402 | self.assertTrue(dialog.files_check.get_active()) |
| 509 | @@ -436,7 +410,7 @@ | |||
| 510 | 436 | def test_toggle_files_and_music(self): | 410 | def test_toggle_files_and_music(self): |
| 511 | 437 | """Test toggling the files and music services on/off.""" | 411 | """Test toggling the files and music services on/off.""" |
| 512 | 438 | self.mocker.replay() | 412 | self.mocker.replay() |
| 514 | 439 | dialog = self.u1prefs.UbuntuOneDialog(keyring=self.keyring) | 413 | dialog = self.u1prefs.UbuntuOneDialog() |
| 515 | 440 | self.assertTrue(dialog is not None) | 414 | self.assertTrue(dialog is not None) |
| 516 | 441 | def files_toggled(checkbutton): | 415 | def files_toggled(checkbutton): |
| 517 | 442 | enabled = checkbutton.get_active() | 416 | enabled = checkbutton.get_active() |
For the constants use those in ubuntuone/
Please fix the docstrings "Helper that handles the REST response." (and the similar ones).
This needs to use APP_NAME instead of 'ubuntuone': iface.clear_
This also: + if app_name == "Ubuntu One":
Remove + print "Got credentials"
Remove _() for log messages.