Merge lp:~dobey/ubuntuone-client/no-httplib2 into lp:ubuntuone-client

[dobey]

Unify the 2 rest request methods to do async requests
Drops the httplib2 dependency
Also updated the tests to fix #537443

[Natalia Bidart (nataliabidart)]

There is a pyflakes issue:

== Python Lint Notices ==

./tests/test_preferences.py:
    24: 'KWARGS' imported but unused

make: *** [lint] Error 1

review: Needs Fixing

[Natalia Bidart (nataliabidart)]

> There is a pyflakes issue:
>
> == Python Lint Notices ==
>
> ./tests/test_preferences.py:
> 24: 'KWARGS' imported but unused
>
> make: *** [lint] Error 1

I'm also getting:

nessita@dali:~/canonical/ubuntuone-client/review_no-httplib2$ ./contrib/test tests/test_preferences.py
contrib.testing.testcase
  DBusTwistedTestCase
    runTest ... [OK]
tests.test_preferences
  PreferencesTests
    test_bw_throttling ... Traceback (most recent call last):
  File "/usr/lib/pymodules/python2.6/dbus/connection.py", line 576, in msg_reply_handler
    reply_handler(*message.get_args_list(**get_args_opts))
  File "/usr/lib/pymodules/python2.6/dbus/proxies.py", line 391, in _introspect_reply_handler
    self._introspect_execute_queue()
  File "/usr/lib/pymodules/python2.6/dbus/proxies.py", line 378, in _introspect_execute_queue
    proxy_method(*args, **keywords)
  File "/usr/lib/pymodules/python2.6/dbus/proxies.py", line 132, in __call__
    **keywords)
  File "/usr/lib/pymodules/python2.6/dbus/connection.py", line 585, in call_async
    require_main_loop=require_main_loop)
dbus.exceptions.DBusException: Connection is disconnected - unable to make method call
                                                [OK]
    test_list_devices_fills_devices_list_with_fake_result_when_empty ... Traceback (most recent call last):
  File "/usr/lib/pymodules/python2.6/dbus/connection.py", line 576, in msg_reply_handler
    reply_handler(*message.get_args_list(**get_args_opts))
  File "/usr/lib/pymodules/python2.6/dbus/proxies.py", line 391, in _introspect_reply_handler
    self._introspect_execute_queue()
  File "/usr/lib/pymodules/python2.6/dbus/proxies.py", line 378, in _introspect_execute_queue
    proxy_method(*args, **keywords)
  File "/usr/lib/pymodules/python2.6/dbus/proxies.py", line 132, in __call__
    **keywords)
  File "/usr/lib/pymodules/python2.6/dbus/connection.py", line 585, in call_async
    require_main_loop=require_main_loop)
dbus.exceptions.DBusException: Connection is disconnected - unable to make method call
  [OK]
    test_list_devices_shows_devices_list ... Traceback (most recent call last):
  File "/usr/lib/pymodules/python2.6/dbus/connection.py", line 576, in msg_reply_handler
    reply_handler(*message.get_args_list(**get_args_opts))
  File "/usr/lib/pymodules/python2.6/dbus/proxies.py", line 391, in _introspect_reply_handler
    self._introspect_execute_queue()
  File "/usr/lib/pymodules/python2.6/dbus/proxies.py", line 378, in _introspect_execute_queue
    proxy_method(*args, **keywords)
  File "/usr/lib/pymodules/python2.6/dbus/proxies.py", line 132, in __call__
    **keywords)
  File "/usr/lib/pymodules/python2.6/dbus/connection.py", line 585, in call_async
    require_main_loop=require_main_loop)
dbus.exceptions.DBusException: Connection is disconnected - unable to make method call
                              [OK]
    test_list_devices_shows_real_devices_list ... Traceback (most recent call last):
  File "/usr/lib/pymodules/python2.6/dbus/connection.py", line 576, in msg_reply_handler
    reply_handler(*message.get_args_list(**get_args_opts))
  File "/usr/lib/pymodules/pyt...

[lp:~dobey/ubuntuone-client/no-httplib2]

423. By dobey on 2010-03-12

Fix lint issue

[dobey (dobey)]

The lint is fixed.

The DBusExceptions seem to be a result of how contrib/test is called. Perhaps a bug in that script, but I am not sure...

./contrib/test tests/test_preferences.py gives those exceptions

./contrib/test -t tests.test_preferences however, does not

[John Lenton (chipaca)]

I'm getting a lot of very strange, apparently unrelated test failures with this branch (and not with trunk). It's got something to do with some kind of resource not going away, because I can run each of the test modules individually, or all the tests except test_preferences, they pass; if I run test_preferences and one other one, I get these failures.

They look like this:
    test_udf_deleted ... Traceback (most recent call last):
  File "/usr/lib/python2.6/dist-packages/twisted/trial/unittest.py", line 951, in _cleanUp
    clean = util._Janitor(self, result).postCaseCleanup()
  File "/usr/lib/python2.6/dist-packages/twisted/trial/util.py", line 131, in postCaseCleanup
    calls = self._cleanPending()
  File "/usr/lib/python2.6/dist-packages/twisted/internet/utils.py", line 211, in warningSuppressingWrapper
    return runWithWarningsSuppressed(suppressedWarnings, f, *a, **kw)
  File "/usr/lib/python2.6/dist-packages/twisted/internet/utils.py", line 191, in runWithWarningsSuppressed
    result = f(*a, **kw)
  File "/usr/lib/python2.6/dist-packages/twisted/trial/util.py", line 172, in _cleanPending
    reactor.iterate(0)
  File "/usr/lib/python2.6/dist-packages/twisted/trial/unittest.py", line 981, in _
    stacklevel=2, category=DeprecationWarning)
exceptions.TypeError: __nonzero__ should return bool or int, returned Mock
[ERROR]

review: Needs Fixing

[lp:~dobey/ubuntuone-client/no-httplib2]

424. By dobey on 2010-03-15

Upstream patch to mocker.py to fix the __nonzero__ test failures

[dobey (dobey)]

The weird __nonzero__ tracebacks are fixed now.

[John Lenton (chipaca)]

On Mon, Mar 15, 2010 at 08:48:31PM -0000, Rodney Dawes wrote:
> The weird __nonzero__ tracebacks are fixed now.

yes, they are. Good to go!

 review approve

review: Approve

[Natalia Bidart (nataliabidart)]

> The weird __nonzero__ tracebacks are fixed now.

Approving now.

review: Approve

=== modified file 'bin/ubuntuone-preferences'
--- bin/ubuntuone-preferences	2010-03-11 19:10:01 +0000
+++ bin/ubuntuone-preferences	2010-03-15 20:46:20 +0000
@@ -28,7 +28,6 @@
 import os
 import gettext
 import gnomekeyring
-import httplib2
 import simplejson
 from oauth import oauth
 from ubuntuone import clientdefs
@@ -82,6 +81,50 @@
       secret = items[0].secret
       return oauth.OAuthToken.from_string(secret)
 
+def rest_response_handler(source, condition, conn, callback):
+    """
+    Helper that handles the REST response.
+    """
+    result = None
+    response = conn.getresponse() # shouldn't block
+    if response.status == 200:
+        data = response.read() # neither should this
+        result = simplejson.loads(data)
+    else:
+        result = response.reason
+    callback(result)
+    return False
+
+def make_rest_request(url=None, method='GET',
+                      callback=None, keyring=None):
+    """
+    Helper that makes an oauth-wrapped REST request.
+    """
+    token = get_access_token(keyring)
+    consumer = oauth.OAuthConsumer('ubuntuone', 'hammertime')
+
+    oauth_request = oauth.OAuthRequest.from_consumer_and_token(
+        http_url=url,
+        http_method=method,
+        oauth_consumer=consumer,
+        token=token,
+        parameters='')
+    oauth_request.sign_request(oauth.OAuthSignatureMethod_HMAC_SHA1(),
+                               consumer, token)
+
+    scheme, netloc, path, query, fragment = urlparse.urlsplit(url)
+
+    conn = httplib.HTTPSConnection(netloc)
+    try:
+        conn.request(method, path, headers=oauth_request.to_header())
+    except socket.error:
+        callback(None)
+    else:
+        glib.io_add_watch(
+            conn.sock,
+            glib.IO_IN | glib.IO_PRI | glib.IO_ERR | glib.IO_HUP,
+            rest_response_handler, conn, callback)
+
 
 class DevicesWidget(gtk.Table):
     """
@@ -224,34 +267,6 @@
         self.status_label.set_markup("<b>Error:</b> %s" % msg)
         logger.error(msg)
 
-    def request(self, path='', method='GET'):
-        """
-        Helper that makes an oauth-wrapped rest request.
-
-        XXX duplication with request_REST_info (but this one should be async).
-        """
-        url = self.base_url + path
-
-        token = get_access_token(self.keyring)
-
-        oauth_request = oauth.OAuthRequest.from_consumer_and_token(
-            http_url=url,
-            http_method=method,
-            oauth_consumer=self.consumer,
-            token=token,
-            parameters='')
-        oauth_request.sign_request(oauth.OAuthSignatureMethod_HMAC_SHA1(),
-                                   self.consumer, token)
-
-        scheme, netloc, path, query, fragment = urlparse.urlsplit(url)
-
-        conn = httplib.HTTPSConnection(netloc)
-        try:
-            conn.request(method, path, headers=oauth_request.to_header())
-        except socket.error:
-            return None
-        return conn
-
     def get_devices(self):
         """
         Ask the server for a list of devices
@@ -264,31 +279,19 @@
             self.error("No token in the keyring")
             self.devices = []
         else:
-            self.consumer = oauth.OAuthConsumer("ubuntuone", "hammertime")
-
-            self.conn = self.request()
-            if self.conn is None:
-                self.clear_devices_view()
-                self.error('Unable to connect')
-            else:
-                glib.io_add_watch(
-                    self.conn.sock,
-                    glib.IO_IN | glib.IO_PRI | glib.IO_ERR | glib.IO_HUP,
-                    self.parse_devices)
-
-    def parse_devices(self, *a):
+            make_rest_request(url=self.base_url, keyring=self.keyring,
+                              callback=self.parse_devices)
+
+    def parse_devices(self, result):
         """
         Parse the list of devices, and hook up list_devices if it worked.
         """
-        response = self.conn.getresponse() # shouldn't block
-        if response.status == 200:
-            response = response.read() # neither should this
-            self.devices = simplejson.loads(response)
+        if result:
+            self.devices = result
             gobject.idle_add(self.list_devices)
         else:
             self.clear_devices_view()
-            self.error(response.reason)
-        return False
+            self.error(result)
 
     def clear_devices_view(self):
         """
@@ -414,16 +417,10 @@
 
         Starts an async request to remove a device.
         """
-        self.conn = self.request('remove/%s/%s' % (kind.lower(), token))
-        if self.conn is None:
-            self.clear_devices_view()
-            self.error('Unable to connect')
-        else:
-            glib.io_add_watch(
-                self.conn.sock,
-                glib.IO_IN | glib.IO_PRI | glib.IO_ERR | glib.IO_HUP,
-                self.parse_devices)
-
+        make_rest_request(url=('%sremove/%s/%s' % (self.base_url,
+                                                   kind.lower(), token)),
+                          keyring=self.keyring,
+                          callback=self.parse_devices)
 
 
 class UbuntuOneDialog(gtk.Dialog):
@@ -479,11 +476,11 @@
 
       def __dbus_error(self, error):
             """Error getting throttling config."""
-            print repr(error)
+            logger.error(error)
 
       def __sd_error(self, error):
             """Error talking to syncdaemon."""
-            print repr(error)
+            logger.error(error)
 
       def __got_state(self, state):
             """Got the state of syncdaemon."""
@@ -525,42 +522,30 @@
                         'percent' : percent })
             self.usage_graph.set_fraction(percent / 100)
 
-      def request_REST_info(self, url, method):
-            """Make a REST request and return the resulting dict, or None."""
-            consumer = oauth.OAuthConsumer("ubuntuone", "hammertime")
-            token = get_access_token(self.keyring)
-            request = oauth.OAuthRequest.from_consumer_and_token(
-                  http_url=url, http_method=method, oauth_consumer=consumer,
-                  token=token)
-            request.sign_request(oauth.OAuthSignatureMethod_HMAC_SHA1(),
-                                 consumer, token)
-            client = httplib2.Http()
-            headers = {}
-            headers.update(request.to_header())
-            resp, content = client.request(url, method, headers=headers)
-            if resp['status'] == '200':
-                  return simplejson.loads(content)
-                  # FIXME: Log json parsing failures
-            else:
-                  # FIXME: Log errors
-                  return None
+      def got_quota_info(self, quota):
+            """Handle the result from the quota REST call."""
+            if quota:
+                  self.update_quota_display(quota['used'], quota['total'])
 
       def request_quota_info(self):
             """Request new quota info from server, and update display."""
-            quota = self.request_REST_info('https://one.ubuntu.com/api/quota/',
-                                           'GET')
-            if quota:
-                  self.update_quota_display(quota['used'], quota['total'])
+            make_rest_request(url='https://one.ubuntu.com/api/quota/',
+                              keyring=self.keyring,
+                              callback=self.got_quota_info)
 
-      def request_account_info(self):
-            """Request account info from server, and update display."""
-            user = self.request_REST_info('https://one.ubuntu.com/api/account/',
-                                          'GET')
+      def got_account_info(self, user):
+            """Handle the result from the account REST call."""
             if user:
                   self.name_label.set_text(user['nickname'])
                   self.user_label.set_text(user['username'])
                   self.mail_label.set_text(user['email'])
 
+      def request_account_info(self):
+            """Request account info from server, and update display."""
+            make_rest_request(url='https://one.ubuntu.com/api/account/',
+                              keyring=self.keyring,
+                              callback=self.got_account_info)
+
       def toggle_db_sync(self, dbname, disable=False):
             """
             Toggle whether a db in desktopcouch is synchronized to the
@@ -683,7 +668,6 @@
             self.notebook.append_page(self.devices)
             self.notebook.set_tab_label_text(self.devices, _("Devices"))
             self.devices.show_all()
-            self.devices.get_devices()
 
             # Services tab
             services = gtk.VBox(spacing=12)
@@ -742,12 +726,17 @@
             """Show our dialog, since we can do stuff now."""
             self.disconnect_signal_handlers()
             dialog = UbuntuOneDialog()
+            dialog.show()
             dialog.request_quota_info()
             dialog.request_account_info()
-            dialog.show()
+            dialog.devices.get_devices()
 
       def got_oautherror(self, message=None):
             """Got an error during oauth."""
+            if message:
+                  logger.error(message)
+            else:
+                  logger.error(_("OAuthError with no message."))
             gtk.main_quit()
 
       def got_authdenied(self):
@@ -756,6 +745,7 @@
 
       def got_dbus_error(self, error):
             """Got a DBusError."""
+            logger.error(error)
             gtk.main_quit()
 
       def register_signal_handlers(self):
@@ -804,7 +794,7 @@
       gettext.textdomain(clientdefs.GETTEXT_PACKAGE)
 
       gtk.rc_parse_string(RCSTYLE)
-      gobject.set_application_name("Ubuntu One")
+      gobject.set_application_name("ubuntuone-preferences")
 
       try:
             login = UbuntuoneLoginHandler()

=== modified file 'contrib/mocker.py'
--- contrib/mocker.py	2009-05-12 13:36:05 +0000
+++ contrib/mocker.py	2010-03-15 20:46:20 +0000
@@ -1092,9 +1092,12 @@
 
     def __nonzero__(self):
         try:
-            return self.__mocker_act__("nonzero")
+            result = self.__mocker_act__("nonzero")
         except MatchError, e:
             return True
+        if type(result) is Mock:
+            return True
+        return result
 
     def __iter__(self):
         # XXX On py3k, when next() becomes __next__(), we'll be able

=== modified file 'tests/test_preferences.py'
--- tests/test_preferences.py	2010-03-08 05:24:51 +0000
+++ tests/test_preferences.py	2010-03-15 20:46:20 +0000
@@ -21,7 +21,7 @@
 import os
 import gnomekeyring
 
-from contrib.mocker import MockerTestCase, KWARGS
+from contrib.mocker import MockerTestCase
 from contrib.testing.testcase import DBusTwistedTestCase, FakeLogin
 from twisted.internet import defer
 from twisted.python.failure import Failure
@@ -48,7 +48,6 @@
 
         # For testing keyring queries
         self.keyring = self.mocker.mock()
-        self.u1prefs.httplib2 = self.mocker.mock()
         self.item = self.mocker.mock(gnomekeyring.Found)
 
         self.item_id = 999
@@ -72,6 +71,8 @@
         self.mocker.count(0, None)
         self.mocker.result(None)
 
+        self.u1prefs.make_rest_request = self.make_rest_request
+
     def tearDown(self):
         # collect all signal receivers registered during the test
         signal_receivers = set()
@@ -88,10 +89,19 @@
         d.addBoth(lambda _: DBusTwistedTestCase.tearDown(self))
         return d
 
+    def make_rest_request(self, url=None, method='GET',
+                          callback=None, keyring=None):
+        """Override the real request call to mock some stuff."""
+        if callback:
+            callback(self.content)
+        else:
+            raise Exception('No callback provided.')
+
     def test_bw_throttling(self):
         """Test that toggling bw throttling works correctly."""
         self.mocker.replay()
         widget = self.u1prefs.DevicesWidget(None, keyring=self.keyring)
+        widget.update_bw_settings = self.mocker.mock()
         try:
             widget.devices = []
             widget.list_devices()
@@ -116,6 +126,7 @@
     def test_list_devices_fills_devices_list_with_fake_result_when_empty(self):
         self.mocker.replay()
         widget = self.u1prefs.DevicesWidget(None, keyring=self.keyring)
+        widget.update_bw_settings = self.mocker.mock()
         try:
             widget.devices = []
             widget.list_devices()
@@ -129,6 +140,7 @@
     def test_list_devices_shows_devices_list(self):
         self.mocker.replay()
         widget = self.u1prefs.DevicesWidget(None, keyring=self.keyring)
+        widget.update_bw_settings = self.mocker.mock()
         try:
             widget.devices = []
             widget.list_devices()
@@ -159,6 +171,7 @@
     def test_list_devices_shows_real_devices_list(self):
         self.mocker.replay()
         widget = self.u1prefs.DevicesWidget(None, keyring=self.keyring)
+        widget.update_bw_settings = self.mocker.mock()
         try:
             widget.devices = [{'kind': 'Computer',
                                'description': 'xyzzy',
@@ -203,12 +216,7 @@
 
     def test_request_quota_info(self):
         """Test that we can request the quota info properly."""
-        response = { 'status' : '200' }
-        content = '{"total":2048, "used":1024}'
-        client = self.mocker.mock()
-        self.expect(self.u1prefs.httplib2.Http()).result(client)
-        self.expect(client.request('https://one.ubuntu.com/api/quota/',
-                                   'GET', KWARGS)).result((response, content))
+        self.content = {"total":2048, "used":1024}
         self.mocker.replay()
         dialog = self.u1prefs.UbuntuOneDialog(keyring=self.keyring)
         self.assertTrue(dialog is not None)
@@ -219,14 +227,8 @@
 
     def test_request_account_info(self):
         """Test that we can request the account info properly."""
-        response = { 'status' : '200' }
-        content = '''{"username": "ubuntuone", "nickname": "Ubuntu One",
-                     "email": "[e-mail]"}
-                  '''
-        client = self.mocker.mock()
-        self.expect(self.u1prefs.httplib2.Http()).result(client)
-        self.expect(client.request('https://one.ubuntu.com/api/account/',
-                                   'GET', KWARGS)).result((response, content))
+        self.content = {"username": "ubuntuone", "nickname": "Ubuntu One",
+                        "email": "[e-mail]"}
         self.mocker.replay()
         dialog = self.u1prefs.UbuntuOneDialog(keyring=self.keyring)
         self.assertTrue(dialog is not None)