Ubuntu One Client

Merge lp:~dobey/ubuntuone-client/devices-remove-local into lp:ubuntuone-client

  1. devices-remove-local
  2. Merge into trunk
Proposed by dobey
Status: Merged
Approved by: Natalia Bidart
Approved revision: 456
Merged at revision: not available
Proposed branch: lp:~dobey/ubuntuone-client/devices-remove-local
Merge into: lp:ubuntuone-client
Diff against target: 424 lines (+119/-59)
3 files modified
bin/ubuntuone-preferences (+115/-55)
tests/test_preferences.py (+3/-2)
ubuntuone/oauthdesktop/main.py (+1/-2)
To merge this branch: bzr merge lp:~dobey/ubuntuone-client/devices-remove-local
Reviewer Review Type Date Requested Status
Natalia Bidart (community) Approve
Joshua Hoover (community) ran branch and verified bug fixes Approve
Review via email: mp+22266@code.launchpad.net

Commit message

Fix the code to create and show the dialog sooner
Delete the token from the keyring if we're deleting the local computer
Start a new login request when the local computer is deleted
Always make sure prefs are available in Devices tab for editing
Don't exit preferences when auth is denied or there is an oauth error

To post a comment you must log in.
lp:~dobey/ubuntuone-client/devices-remove-local updated
453. By dobey

Don't set content_area sensitivity, as it apparently includes action_area
Push the login check and subsequent network activity to a timeout

Revision history for this message
Natalia Bidart (nataliabidart) wrote :

Startup time of the blank window takes ~13 seconds for me. After that, I get a blank window during approx 1 minute, and then I have buttons and tabs.

As per what I talked with dobey on IRC, base window should appear immediately, so I'm setting this as Needs fixing.

review: Needs Fixing
Revision history for this message
Natalia Bidart (nataliabidart) wrote :

> Startup time of the blank window takes ~13 seconds for me. After that, I get a
> blank window during approx 1 minute, and then I have buttons and tabs.
>
> As per what I talked with dobey on IRC, base window should appear immediately,
> so I'm setting this as Needs fixing.

As per Chipaca's request I measured this against current trunk (revno 452).
I created a screencast showing how log does this branch's u1-preferences takes to start and to be functional, and the same for the binary under trunk.

Using my cellphone as a chronometer, I got:

 * for this branch: time until a non-functional window is shown, ~14 seconds. A functional window appeared at 1 minute and 13 seconds.

 * for trunk: the first window that is displayed is clickable and usable, but this window is shown after 27 seconds.

So, deciding if this branch is slower or not is relative. I personally prefer to have a faster functional window, so I'll keep my vote here, but I'll listen to the User Experience / QA team :-)

Revision history for this message
Natalia Bidart (nataliabidart) wrote :

The screencast is at https://chinstrap.canonical.com/~nessita/u1-preferences-startup.ogv

lp:~dobey/ubuntuone-client/devices-remove-local updated
454. By dobey

Merged with trunk

455. By dobey

Fix some resize issues
Change the unknowns to default to "Unknown" instead of ""
Remove the lockup caused by calling our own dbus interface from within

456. By dobey

Also fix the clear_token method to not defer the keyring call to a thread

Revision history for this message
Joshua Hoover (joshuahoover) wrote :

Functioning as expected now. Window opens almost immediately and all other tabs are responsive so the performance issues seem to be addressed.

Verified the following are fixed by this branch:

 * Preferences stays open after adding computer to account (#538278)
 * Preferences still work even when network connection is not up (#540304)
 * Removing the local computer deletes the local ubuntuone token and prompts to re-add the computer as designed (#545506)

review: Approve (ran branch and verified bug fixes)
Revision history for this message
Natalia Bidart (nataliabidart) wrote :

I'm getting a test failure:

===============================================================================
[FAIL]: tests.test_preferences.PreferencesTests.test_request_account_info

Traceback (most recent call last):
  File "/home/nessita/canonical/ubuntuone-client/review_devices-remove-local/contrib/mocker.py", line 102, in test_method_wrapper
    result = test_method()
  File "/home/nessita/canonical/ubuntuone-client/review_devices-remove-local/tests/test_preferences.py", line 337, in test_request_account_info
    self.assertFalse(dialog.upgrade_link.get_visible())
twisted.trial.unittest.FailTest:

lp:~dobey/ubuntuone-client/devices-remove-local updated
457. By dobey

Fix the test... if not foo: is not the opposite of if foo: in python?

Revision history for this message
Natalia Bidart (nataliabidart) wrote :

Approving. Startup time is now 12 seconds for me, which is a great improvement.

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-03-30 18:19:51 +0000
3+++ bin/ubuntuone-preferences 2010-03-31 14:48:29 +0000
4@@ -26,6 +26,8 @@
5 import gobject
6 import gtk
7 import os
8+import sys
9+import time
10 import gettext
11 import gnomekeyring
12 import simplejson
13@@ -69,6 +71,9 @@
14 DBUS_IFACE_AUTH_NAME = "com.ubuntuone.Authentication"
15 DBUS_IFACE_AUTH_PATH = "/"
16
17+# Our own DBus interface
18+PREFS_BUS_NAME = "com.ubuntuone.Preferences"
19+
20 # This is where thy music lies
21 U1MSPATH = os.path.expanduser("~/.ubuntuone/Purchased from Ubuntu One")
22
23@@ -82,20 +87,39 @@
24 widget_class '*Dialog*' style 'dialogs'
25 """
26
27-def dbus_async(*args):
28+# This is a global so we can avoid creating multiple instances in some cases
29+prefs_dialog = None
30+
31+def dbus_async(*args, **kwargs):
32 """Simple handler to make dbus do stuff async."""
33 pass
34
35
36+def do_login_request(bus, error_handler):
37+ """Make a login request to the login handling daemon."""
38+ try:
39+ client = bus.get_object(DBUS_IFACE_AUTH_NAME,
40+ DBUS_IFACE_AUTH_PATH,
41+ follow_name_owner_changes=True)
42+ iface = dbus.Interface(client, DBUS_IFACE_AUTH_NAME)
43+ iface.login('https://ubuntuone.com', 'ubuntuone',
44+ reply_handler=dbus_async,
45+ error_handler=error_handler)
46+ except DBusException, e:
47+ error_handler(e)
48+
49 def get_access_token(keyring):
50 """Get the access token from the keyring."""
51 items = []
52- items = keyring.find_items_sync(
53- keyring.ITEM_GENERIC_SECRET,
54- {'ubuntuone-realm': "https://ubuntuone.com",
55- 'oauth-consumer-key': 'ubuntuone'})
56- secret = items[0].secret
57- return oauth.OAuthToken.from_string(secret)
58+ try:
59+ items = keyring.find_items_sync(
60+ keyring.ITEM_GENERIC_SECRET,
61+ {'ubuntuone-realm': "https://ubuntuone.com",
62+ 'oauth-consumer-key': 'ubuntuone'})
63+ secret = items[0].secret
64+ return oauth.OAuthToken.from_string(secret)
65+ except (gnomekeyring.NoMatchError, gnomekeyring.DeniedError):
66+ return None
67
68 def do_rest_request(url, method, token, callback):
69 """
70@@ -291,7 +315,7 @@
71 get_access_token(self.keyring)
72 except gnomekeyring.NoMatchError:
73 self.error("No token in the keyring")
74- self.devices = []
75+ self.devices = None
76 else:
77 make_rest_request(url=self.base_url, keyring=self.keyring,
78 callback=self.parse_devices)
79@@ -321,7 +345,6 @@
80
81 gobject.idle_add(self.list_devices)
82
83-
84 def clear_devices_view(self):
85 """
86 Clear out almost all the widgets.
87@@ -352,7 +375,7 @@
88 # a stopgap device so you can at least try to connect
89 self.devices = [{'kind': 'Computer',
90 'description': _("<LOCAL MACHINE>"),
91- 'token': token.key,
92+ 'token': token.key if token else '',
93 'FAKE': 'YES'}]
94 else:
95 self.resize(len(self.devices)+1, 3)
96@@ -374,12 +397,13 @@
97 butn.connect('clicked', self.remove,
98 row['kind'], row.get('token'))
99 self.attach(butn, 2, 3, i, i+1, xoptions=0, yoptions=0)
100- if row.get('token') == token.key:
101+ if token and row.get('token') == token.key or 'FAKE' in row:
102 self.bw_chk = ck_btn = gtk.CheckButton(
103 _("_Limit Bandwidth Usage"))
104 ck_btn.set_active(self.bw_limited)
105 up_lbl = gtk.Label(_("Maximum _upload speed (KB/s):"))
106 up_lbl.set_alignment(0., .5)
107+ up_lbl.set_use_underline(True)
108 adj = gtk.Adjustment(value=self.up_limit/1024.,
109 lower=0.0, upper=4096.0,
110 step_incr=1.0, page_incr=16.0)
111@@ -388,6 +412,7 @@
112 up_lbl.set_mnemonic_widget(up_btn)
113 dn_lbl = gtk.Label(_("Maximum _download speed (KB/s):"))
114 dn_lbl.set_alignment(0., .5)
115+ dn_lbl.set_use_underline(True)
116 adj = gtk.Adjustment(value=self.dn_limit/1024.,
117 lower=0.0, upper=4096.0,
118 step_incr=1.0, page_incr=16.0)
119@@ -450,6 +475,22 @@
120 kind.lower(), token)),
121 keyring=self.keyring,
122 callback=self.parse_devices)
123+ local = get_access_token(self.keyring)
124+ def local_removal_cb(*args, **kwargs):
125+ """Try to get a new token if we remove the local one."""
126+ do_login_request(self.bus, self.error)
127+
128+ if token == local.key:
129+ try:
130+ client = self.bus.get_object(DBUS_IFACE_AUTH_NAME,
131+ DBUS_IFACE_AUTH_PATH,
132+ follow_name_owner_changes=True)
133+ iface = dbus.Interface(client, DBUS_IFACE_AUTH_NAME)
134+ iface.clear_token('https://ubuntuone.com', 'ubuntuone',
135+ reply_handler=local_removal_cb,
136+ error_handler=self.error)
137+ except DBusException, e:
138+ self.error(e)
139
140
141 class UbuntuOneDialog(gtk.Dialog):
142@@ -463,6 +504,7 @@
143 self.add_button(gtk.STOCK_CLOSE, gtk.RESPONSE_CLOSE)
144 self.set_default_response(gtk.RESPONSE_CLOSE)
145 self.set_icon_name("ubuntuone")
146+ self.set_default_size(400, 300)
147
148 self.connect("close", self.__handle_response, gtk.RESPONSE_CLOSE)
149 self.connect("response", self.__handle_response)
150@@ -479,12 +521,13 @@
151 # Timeout ID to avoid spamming DBus from spinbutton changes
152 self.__update_id = 0
153
154+ # Build the dialog
155+ self.__construct()
156+
157 # SD Tool object
158 self.sdtool = SyncDaemonTool(self.__bus)
159 self.sdtool.get_status().addCallbacks(self.__got_state,
160 self.__sd_error)
161- # Build the dialog
162- self.__construct()
163
164 # hook dbus up
165 self.__bus.add_signal_receiver(
166@@ -579,6 +622,8 @@
167 self.plan_label.set_label(_("Paid") if is_paid_account else _("Free"))
168 if is_paid_account:
169 self.upgrade_link.hide()
170+ else:
171+ self.upgrade_link.show()
172
173 if percent >= 100.:
174 self.overquota_img.set_from_icon_name('dialog-warning',
175@@ -765,12 +810,12 @@
176 hbox.pack_start(self.usage_graph, False, False)
177 self.usage_graph.show()
178
179- self.usage_label = gtk.Label("")
180+ self.usage_label = gtk.Label(_("Unknown"))
181 self.usage_label.set_alignment(0.0, 0.5)
182- hbox.pack_start(self.usage_label, True, True)
183+ hbox.pack_start(self.usage_label, False, False)
184 self.usage_label.show()
185
186- self.status_label = gtk.Label("")
187+ self.status_label = gtk.Label(_("Unknown"))
188 self.status_label.set_alignment(0.0, 0.5)
189 rbox.pack_start(self.status_label, False, False)
190 self.status_label.show()
191@@ -791,7 +836,6 @@
192 self.overquota = gtk.HBox(spacing=6)
193 self.overquota.set_border_width(6)
194 account.pack_start(self.overquota, False, False)
195- self.overquota.show()
196
197 self.overquota_img = gtk.Image()
198 self.overquota.pack_start(self.overquota_img, False, False)
199@@ -801,7 +845,7 @@
200 label.set_line_wrap(True)
201 label.set_alignment(0.0, 0.5)
202 self.overquota.pack_start(label, False, False)
203- label.show()
204+ self.overquota_label.show()
205
206
207 # User info in account tab
208@@ -817,7 +861,7 @@
209 table.attach(label, 0, 1, 0, 1)
210 label.show()
211
212- self.name_label = gtk.Label("")
213+ self.name_label = gtk.Label(_("Unknown"))
214 self.name_label.set_use_underline(True)
215 self.name_label.set_alignment(0.0, 0.5)
216 table.attach(self.name_label, 1, 2, 0, 1)
217@@ -829,7 +873,7 @@
218 table.attach(label, 0, 1, 1, 2)
219 label.show()
220
221- self.mail_label = gtk.Label("")
222+ self.mail_label = gtk.Label(_("Unknown"))
223 self.mail_label.set_use_underline(True)
224 self.mail_label.set_alignment(0.0, 0.5)
225 table.attach(self.mail_label, 1, 2, 1, 2)
226@@ -840,7 +884,7 @@
227 table.attach(label, 0, 1, 2, 3)
228 label.show()
229
230- self.plan_label = gtk.Label("")
231+ self.plan_label = gtk.Label(_("Unknown"))
232 self.plan_label.set_alignment(0.0, 0.5)
233 table.attach(self.plan_label, 1, 2, 2, 3)
234 self.plan_label.show()
235@@ -857,6 +901,7 @@
236 table.attach(link, 0, 2, 5-n, 6-n)
237 link.show()
238 self.upgrade_link = link
239+ self.upgrade_link.hide()
240
241 # Devices tab
242 sw = gtk.ScrolledWindow()
243@@ -923,30 +968,42 @@
244 alignment.add(self.music_check)
245 self.music_check.show()
246
247- # get the quota
248- self.update_quota_display(0, 2)
249
250-class UbuntuoneLoginHandler(object):
251+class UbuntuoneLoginHandler(dbus.service.Object):
252 """Class to handle registration/login, in case we aren't already."""
253
254- def __init__(self, *args, **kw):
255+ def __init__(self, dialog, *args, **kw):
256 self.bus = dbus.SessionBus()
257
258+ # Signal handler match IDs
259 self.newcreds = None
260 self.oautherr = None
261 self.authdeny = None
262
263+ # The actual UI
264+ self.dialog = dialog
265+
266+ # DBus object magic
267+ self.path = '/'
268+ bus_name = dbus.service.BusName(PREFS_BUS_NAME, bus=self.bus)
269+ dbus.service.Object.__init__(self, bus_name=bus_name,
270+ object_path=self.path)
271+
272+
273+ @dbus.service.method(PREFS_BUS_NAME, in_signature='', out_signature='')
274+ def present(self):
275+ """Raise the dialog window."""
276+ self.dialog.connect_desktopcouch_exclusion()
277+ self.dialog.get_syncdaemon_sync_config()
278+ self.dialog.connect_file_sync_callbacks()
279+ self.dialog.request_quota_info()
280+ self.dialog.request_account_info()
281+ self.dialog.devices.get_devices()
282+ self.dialog.present_with_time(int(time.time()))
283+
284 def got_newcredentials(self, realm, consumer_key):
285 """Show our dialog, since we can do stuff now."""
286- self.disconnect_signal_handlers()
287- dialog = UbuntuOneDialog()
288- dialog.connect_desktopcouch_exclusion()
289- dialog.get_syncdaemon_sync_config()
290- dialog.connect_file_sync_callbacks()
291- dialog.show()
292- dialog.request_quota_info()
293- dialog.request_account_info()
294- dialog.devices.get_devices()
295+ self.present()
296
297 def got_oautherror(self, message=None):
298 """Got an error during oauth."""
299@@ -954,28 +1011,26 @@
300 logger.error(message)
301 else:
302 logger.error(_("OAuthError with no message."))
303- gtk.main_quit()
304
305 def got_authdenied(self):
306 """User denied access."""
307- gtk.main_quit()
308+ logger.error(_("Authorization was denied."))
309
310 def got_dbus_error(self, error):
311 """Got a DBusError."""
312 logger.error(error)
313- gtk.main_quit()
314
315 def register_signal_handlers(self):
316 """Register the dbus signal handlers."""
317- self.newcreds = self.bus.add_signal_receiver(
318+ self.bus.add_signal_receiver(
319 handler_function=self.got_newcredentials,
320 signal_name='NewCredentials',
321 dbus_interface=DBUS_IFACE_AUTH_NAME)
322- self.oautherr = self.bus.add_signal_receiver(
323+ self.bus.add_signal_receiver(
324 handler_function=self.got_oautherror,
325 signal_name='OAuthError',
326 dbus_interface=DBUS_IFACE_AUTH_NAME)
327- self.authdeny = self.bus.add_signal_receiver(
328+ self.bus.add_signal_receiver(
329 handler_function=self.got_authdenied,
330 signal_name='AuthorizationDenied',
331 dbus_interface=DBUS_IFACE_AUTH_NAME)
332@@ -992,19 +1047,6 @@
333 self.authdeny,
334 dbus_interface=DBUS_IFACE_AUTH_NAME)
335
336- def do_login_check(self):
337- """Log in to U1 or validate that we already are."""
338- try:
339- client = self.bus.get_object(DBUS_IFACE_AUTH_NAME,
340- DBUS_IFACE_AUTH_PATH,
341- follow_name_owner_changes=True)
342- iface = dbus.Interface(client, DBUS_IFACE_AUTH_NAME)
343- iface.login('https://ubuntuone.com', 'ubuntuone',
344- reply_handler=dbus_async,
345- error_handler=self.got_dbus_error)
346- except DBusException, e:
347- self.got_dbus_error(e)
348-
349
350 if __name__ == "__main__":
351 gettext.bindtextdomain(clientdefs.GETTEXT_PACKAGE, clientdefs.LOCALEDIR)
352@@ -1016,11 +1058,29 @@
353 gtk.rc_parse_string(RCSTYLE)
354 gobject.set_application_name("ubuntuone-preferences")
355
356+ bus = dbus.SessionBus()
357+ result = bus.request_name(PREFS_BUS_NAME,
358+ dbus.bus.NAME_FLAG_DO_NOT_QUEUE)
359+ if result == dbus.bus.REQUEST_NAME_REPLY_EXISTS:
360+ try:
361+ client = bus.get_object(PREFS_BUS_NAME, '/',
362+ follow_name_owner_changes=True)
363+ iface= dbus.Interface(client, PREFS_BUS_NAME)
364+ iface.present()
365+ except DBusException, e:
366+ logger.error(e)
367+ sys.exit(0)
368+
369 try:
370+ # The prefs dialog
371 gtk.gdk.threads_enter()
372- login = UbuntuoneLoginHandler()
373+ prefs_dialog = UbuntuOneDialog()
374+ prefs_dialog.show()
375+
376+ login = UbuntuoneLoginHandler(dialog=prefs_dialog)
377 login.register_signal_handlers()
378- login.do_login_check()
379+ gobject.timeout_add_seconds(1, do_login_request,
380+ bus, login.got_dbus_error)
381 gtk.main()
382 gtk.gdk.threads_leave()
383 except KeyboardInterrupt:
384
385=== modified file 'tests/test_preferences.py'
386--- tests/test_preferences.py 2010-03-29 21:57:10 +0000
387+++ tests/test_preferences.py 2010-03-31 14:48:29 +0000
388@@ -439,12 +439,13 @@
389
390 d = defer.Deferred()
391
392- login = self.u1prefs.UbuntuoneLoginHandler()
393+ login = self.u1prefs.UbuntuoneLoginHandler(dialog=None)
394 login.got_newcredentials = got_new_creds
395 login.got_authdenied = got_auth_denied
396 login.got_oautherror = got_oauth_error
397 login.got_dbus_error = got_dbus_error
398 login.register_signal_handlers()
399- login.do_login_check()
400+ self.u1prefs.do_login_request(bus=self.bus,
401+ error_handler=got_dbus_error)
402
403 return d
404
405=== modified file 'ubuntuone/oauthdesktop/main.py'
406--- ubuntuone/oauthdesktop/main.py 2010-03-30 15:36:29 +0000
407+++ ubuntuone/oauthdesktop/main.py 2010-03-31 14:48:29 +0000
408@@ -31,7 +31,6 @@
409 from dbus.mainloop.glib import DBusGMainLoop
410
411 from ubuntuone.oauthdesktop.config import get_config
412-from twisted.internet.threads import deferToThread
413
414 from ubuntuone.oauthdesktop.logger import setupLogging
415 logger = setupLogging("UbuntuOne.OAuthDesktop.main")
416@@ -109,7 +108,7 @@
417 callback_denied=self.got_denial,
418 callback_notoken=self.got_no_token,
419 callback_error=self.got_error)
420- deferToThread(client.clear_token).addErrback(self.error_handler)
421+ gobject.timeout_add_seconds(1, client.clear_token)
422
423 def error_handler(self, failure):
424 """Deal with errors returned from auth process"""

Subscribers

People subscribed via source and target branches