Merge lp:~nataliabidart/ubuntuone-client/platform-details into lp:ubuntuone-client

[Natalia Bidart]

- Platform details are now sent in the PING_URL (LP: #827343).

[Roberto Alsina (ralsina)]

looks nice!

review: Approve

[lp:~nataliabidart/ubuntuone-client/platform-details]

1110. By Natalia Bidart on 2011-08-17

* Using urlencode to encode ping url platform params.
* Added the logging of the result when using the log_call decorator.

1111. By Natalia Bidart on 2011-08-17

Merged trunk in.

[John Lenton (chipaca)]

even without the fix to the test, this is +1.

review: Approve

[lp:~nataliabidart/ubuntuone-client/platform-details]

1112. By Natalia Bidart on 2011-08-17

Removed randomness to test_ping_url to ensure platform data is correct.

1113. By Natalia Bidart on 2011-08-17

Removing unused import, and debug print.

=== modified file 'bin/ubuntuone-login'
--- bin/ubuntuone-login	2011-07-28 21:28:35 +0000
+++ bin/ubuntuone-login	2011-08-17 13:31:24 +0000
@@ -32,9 +32,10 @@
 
 from ubuntuone.logger import basic_formatter
 from ubuntuone.platform.credentials import logger
-from ubuntuone.platform.linux.credentials import (
+from ubuntuone.platform.credentials.linux import (
     DBUS_BUS_NAME, DBUS_CREDENTIALS_PATH,
-    CredentialsManagement)
+    CredentialsManagement,
+)
 
 dbus.mainloop.glib.DBusGMainLoop(set_as_default=True)
 
@@ -52,6 +53,7 @@
         debug_handler = logging.StreamHandler(sys.stderr)
         debug_handler.setFormatter(basic_formatter)
         debug_handler.setLevel(logging.DEBUG)
+        logger.setLevel(logging.DEBUG)
         logger.addHandler(debug_handler)
 
     logger.info("Starting Ubuntu One login manager for bus %r.", DBUS_BUS_NAME)

=== modified file 'po/POTFILES.in'
--- po/POTFILES.in	2011-07-21 15:24:02 +0000
+++ po/POTFILES.in	2011-08-17 13:31:24 +0000
@@ -1,6 +1,6 @@
 ubuntuone/clientdefs.py.in
 ubuntuone/status/aggregator.py
-ubuntuone/credentials.py
+ubuntuone/platform/credentials/__init__.py
 data/emblem-ubuntuone-downloading.icon.in
 data/emblem-ubuntuone-unsynchronized.icon.in
 data/emblem-ubuntuone-uploading.icon.in

=== modified file 'tests/platform/linux/test_credentials.py'
--- tests/platform/linux/test_credentials.py	2011-07-27 17:31:25 +0000
+++ tests/platform/linux/test_credentials.py	2011-08-17 13:31:24 +0000
@@ -18,18 +18,22 @@
 
 """Tests for the Ubuntu One credentials management dbus service."""
 
+import logging
+
 from functools import wraps
 
 from twisted.internet.defer import Deferred, inlineCallbacks
 from ubuntuone.devtools.testcase import DBusTestCase
 from ubuntuone.devtools.handlers import MementoHandler
 
-from ubuntuone.credentials import logging, logger
-from ubuntuone.platform.linux.credentials import (
+from ubuntuone.platform.credentials import (
+    CredentialsError,
+    CredentialsManagementTool,
+    logger,
+)
+from ubuntuone.platform.credentials.linux import (
     APP_NAME,
-    CredentialsError,
     CredentialsManagement,
-    CredentialsManagementTool,
     dbus,
     DBUS_BUS_NAME,
     DBUS_CREDENTIALS_IFACE,

=== renamed file 'tests/test_credentials.py' => 'tests/platform/test_credentials.py'
--- tests/test_credentials.py	2011-07-29 16:03:27 +0000
+++ tests/platform/test_credentials.py	2011-08-17 13:31:24 +0000
@@ -18,10 +18,20 @@
 # with this program.  If not, see <http://www.gnu.org/licenses/>.
 """Platform independent tests for the credentials management."""
 
+import platform
+import urlparse
+
 from mocker import MockerTestCase, MATCH
 from twisted.internet import defer
+from twisted.trial.unittest import TestCase
 
-from ubuntuone.platform.credentials import CredentialsManagementTool
+from ubuntuone import clientdefs
+from ubuntuone.platform.credentials import (
+    BASE_PING_URL,
+    CredentialsManagementTool,
+    PING_URL,
+    PLATFORM_DATA,
+)
 
 
 class TestCredentialsManagementTool(MockerTestCase):
@@ -132,3 +142,25 @@
         self.get_cred_proxy = lambda: defer.succeed(real_get_cred_proxy())
         self.management_tool = CredentialsManagementTool()
         self.management_tool.get_creds_proxy = self.get_cred_proxy
+
+
+class PingURLPlatformDetails(TestCase):
+
+    def test_ping_url(self):
+        """The PING_URL is BASE_PING_URL followed by encoded PLATFORM_DATA."""
+        result = urlparse.urlparse(PING_URL)
+
+        expected_base = urlparse.urljoin(result.scheme + '://' + result.netloc,
+                                         result.path)
+        self.assertEqual(expected_base, BASE_PING_URL)
+
+        expected_query = dict(urlparse.parse_qsl(result.query))
+        self.assertEqual(expected_query, PLATFORM_DATA)
+
+    def test_platform_data(self):
+        """The platform data is correct."""
+        expected = {"platform": platform.system(),
+                    "platform_version": platform.release(),
+                    "platform_arch": platform.machine(),
+                    "client_version": clientdefs.VERSION}
+        self.assertEqual(expected, PLATFORM_DATA)

=== modified file 'tests/platform/windows/test_credentials.py'
--- tests/platform/windows/test_credentials.py	2011-07-19 00:06:04 +0000
+++ tests/platform/windows/test_credentials.py	2011-08-17 13:31:24 +0000
@@ -20,7 +20,10 @@
 from twisted.internet import defer
 from twisted.trial.unittest import TestCase
 
-from ubuntuone.platform.windows import credentials
+from ubuntuone.platform.credentials.windows import (
+    CredentialsManagement,
+    RemovableSignal,
+)
 
 TEST_APP_NAME = "test application"
 TEST_ERROR_DICT = {}
@@ -93,8 +96,7 @@
         """__call__ works as expected."""
         sample_store = []
         test_cb = lambda app_name, creds: sample_store.append(creds)
-        rs = credentials.RemovableSignal(self.proxy, "on_credentials_found_cb",
-                                         test_cb)
+        rs = RemovableSignal(self.proxy, "on_credentials_found_cb", test_cb)
         rs(TEST_APP_NAME, TEST_CREDENTIALS)
         self.assertEqual(sample_store[0], TEST_CREDENTIALS)
 
@@ -102,8 +104,7 @@
         """The signal has a .remove that removes the callback."""
         sample_store = []
         test_cb = lambda app_name, creds: sample_store.append(creds)
-        rs = credentials.RemovableSignal(self.proxy, "on_credentials_found_cb",
-                                         test_cb)
+        rs = RemovableSignal(self.proxy, "on_credentials_found_cb", test_cb)
         rs.remove()
         rs(TEST_APP_NAME, TEST_CREDENTIALS)
         self.assertEqual(len(sample_store), 0)
@@ -117,7 +118,7 @@
     def setUp(self):
         """Initialize these tests."""
         self.proxy = FakeSSOProxy()
-        self.cm = credentials.CredentialsManagement(self.proxy)
+        self.cm = CredentialsManagement(self.proxy)
 
     def test_find_credentials(self):
         """Test the find_credentials method."""
@@ -177,7 +178,7 @@
         d = defer.Deferred()
 
         def signal_handler(*args):
-            """Fake the behaviour of CredentialsManagementToolRoot."""
+            """Fake the behaviour of CredentialsManagementTool."""
             d.callback(args[0] if len(args) > 0 else None)
 
         register = getattr(self.cm, "register_to_" + signal_name)

=== modified file 'ubuntuone/logger.py'
--- ubuntuone/logger.py	2011-07-26 16:07:08 +0000
+++ ubuntuone/logger.py	2011-08-17 13:31:24 +0000
@@ -240,6 +240,33 @@
             return True
 
 
+def log_call(log_func):
+    """Decorator to add a log entry using 'log_func'.
+
+    An example of use would be:
+
+    @log_call(logger.debug)
+    def f(a, b, c):
+        ....
+
+    """
+
+    def middle(f):
+        """Add logging when calling 'f'."""
+
+        @functools.wraps(f)
+        def inner(*args, **kwargs):
+            """Call f(*args, **kwargs)."""
+            log_func('%s: args %r, kwargs %r.', f.__name__, args, kwargs)
+            res = f(*args, **kwargs)
+            log_func('%s: result %r.', f.__name__, res)
+            return res
+
+        return inner
+
+    return middle
+
+
 ### configure the thing ###
 LOGBACKUP = 5 # the number of log files to keep around
 

=== modified file 'ubuntuone/platform/__init__.py'
--- ubuntuone/platform/__init__.py	2011-08-02 17:26:22 +0000
+++ ubuntuone/platform/__init__.py	2011-08-17 13:31:24 +0000
@@ -10,6 +10,8 @@
     from ubuntuone.platform import linux
     source = linux
 
+from ubuntuone.platform import credentials
+
 target = sys.modules[__name__]
 for k in dir(source):
     setattr(target, k, getattr(source, k))

=== added directory 'ubuntuone/platform/credentials'
=== renamed file 'ubuntuone/credentials.py' => 'ubuntuone/platform/credentials/__init__.py'
--- ubuntuone/credentials.py	2011-07-29 16:15:53 +0000
+++ ubuntuone/platform/credentials/__init__.py	2011-08-17 13:31:24 +0000
@@ -18,16 +18,20 @@
 # with this program.  If not, see <http://www.gnu.org/licenses/>.
 """Common code for the credentials management."""
 
+import gettext
 import logging
 import os
-import gettext
+import platform
+import urllib
+import sys
 
 from twisted.internet import defer
 
-from ubuntuone.clientdefs import GETTEXT_PACKAGE
+from ubuntuone import clientdefs
 from ubuntuone.logger import (
     basic_formatter,
     CustomRotatingFileHandler,
+    log_call,
 )
 from ubuntuone.platform.xdg_base_directory import ubuntuone_log_dir
 
@@ -42,29 +46,35 @@
 logger.addHandler(MAIN_HANDLER)
 
 NO_OP = lambda *args, **kwargs: None
-Q_ = lambda string: gettext.dgettext(GETTEXT_PACKAGE, string)
+Q_ = lambda string: gettext.dgettext(clientdefs.GETTEXT_PACKAGE, string)
 APP_NAME = "Ubuntu One"
 TC_URL = "https://one.ubuntu.com/terms/"
-PING_URL = "https://one.ubuntu.com/oauth/sso-finished-so-get-tokens/"
+BASE_PING_URL = \
+    "https://one.ubuntu.com/oauth/sso-finished-so-get-tokens/{email}"
+PLATFORM_DATA = {'platform': platform.system(),
+                 'platform_version': platform.release(),
+                 'platform_arch': platform.machine(),
+                 'client_version': clientdefs.VERSION}
+PING_URL = BASE_PING_URL + "?" + urllib.urlencode(PLATFORM_DATA)
 DESCRIPTION = Q_('Ubuntu One requires an Ubuntu Single Sign On (SSO) account. '
                  'This process will allow you to create a new account, '
                  'if you do not yet have one.')
 
+
 class CredentialsError(Exception):
     """A general exception when hadling credentilas."""
 
 
-class CredentialsManagementToolRoot(object):
+class CredentialsManagementTool(object):
     """Wrapper to CredentialsManagement.
 
-    The goal of this class is to abstract the caller from calling the DBus
+    The goal of this class is to abstract the caller from calling the IPC
     service implemented in the class CredentialsManagement.
 
     """
 
-    def __init__(self, get_proxy_fn):
+    def __init__(self):
         self._cleanup_signals = []
-        self.get_proxy_fn = get_proxy_fn
 
     def cleanup(self, _):
         """Disconnect all the DBus signals."""
@@ -77,8 +87,16 @@
 
     def get_creds_proxy(self):
         """Call the get_proxy_fn, and perhaps wrap the result in a defer."""
-        return defer.maybeDeferred(self.get_proxy_fn)
+        if sys.platform == 'win32':
+            from ubuntuone.platform.credentials import windows
+            source = windows
+        else:
+            from ubuntuone.platform.credentials import linux
+            source = linux
+        result = defer.maybeDeferred(source.get_creds_proxy)
+        return result
 
+    @log_call(logger.debug)
     @defer.inlineCallbacks
     def find_credentials(self):
         """Find credentials for Ubuntu One.
@@ -121,6 +139,7 @@
         result = yield d
         defer.returnValue(result)
 
+    @log_call(logger.debug)
     @defer.inlineCallbacks
     def clear_credentials(self):
         """Clear credentials for Ubuntu One.
@@ -149,6 +168,7 @@
 
         yield d
 
+    @log_call(logger.debug)
     @defer.inlineCallbacks
     def store_credentials(self, token):
         """Store credentials for Ubuntu One.
@@ -181,6 +201,7 @@
 
         yield d
 
+    @log_call(logger.debug)
     @defer.inlineCallbacks
     def register(self, window_id=0):
         """Register to Ubuntu One.
@@ -226,6 +247,7 @@
         result = yield d
         defer.returnValue(result)
 
+    @log_call(logger.debug)
     @defer.inlineCallbacks
     def login(self, window_id=0):
         """Login to Ubuntu One.
@@ -272,6 +294,7 @@
         result = yield d
         defer.returnValue(result)
 
+    @log_call(logger.debug)
     @defer.inlineCallbacks
     def login_email_password(self, email, password):
         """Login to Ubuntu One.

=== renamed file 'ubuntuone/platform/linux/credentials.py' => 'ubuntuone/platform/credentials/linux.py'
--- ubuntuone/platform/linux/credentials.py	2011-07-29 15:36:12 +0000
+++ ubuntuone/platform/credentials/linux.py	2011-08-17 13:31:24 +0000
@@ -17,7 +17,6 @@
 # with this program.  If not, see <http://www.gnu.org/licenses/>.
 """Ubuntu One credentials management dbus service."""
 
-
 import dbus.service
 import ubuntu_sso
 
@@ -26,16 +25,14 @@
     PING_URL_KEY,
     TC_URL_KEY,
 )
-from ubuntuone.credentials import (
-    CredentialsManagementToolRoot,
-    logger,
+from ubuntuone.platform.credentials import (
     APP_NAME,
     DESCRIPTION,
+    logger,
     NO_OP,
     PING_URL,
     TC_URL,
 )
-from ubuntuone.credentials import CredentialsError as CredentialsErrorRoot
 
 
 TIMEOUT_INTERVAL = 10000  # 10 seconds
@@ -45,8 +42,6 @@
 DBUS_CREDENTIALS_PATH = "/credentials"
 DBUS_CREDENTIALS_IFACE = "com.ubuntuone.CredentialsManagement"
 
-CredentialsError = CredentialsErrorRoot
-
 
 class CredentialsManagement(dbus.service.Object):
     """DBus object that manages Ubuntu One credentials."""
@@ -242,7 +237,7 @@
     """Proxy that allows to work with the dbus api.
 
     This proxy has been added to hide the exact details of
-    how the signals are connected. This allows the reuse of 
+    how the signals are connected. This allows the reuse of
     the CredentialsManagementTool in diff platforms.
     """
 
@@ -307,10 +302,6 @@
         return self.proxy.connect_to_signal('CredentialsError', callback)
 
 
-class CredentialsManagementTool(CredentialsManagementToolRoot):
-    """Wrapper used to manage the creds of ubuntu one."""
-
-    def __init__(self):
-        """Create a new instance."""
-        super(CredentialsManagementTool, self).__init__(
-                                                CredentialsManagementProxy)
+def get_creds_proxy():
+    """Get the CredentialsManagement proxy."""
+    return CredentialsManagementProxy()

=== renamed file 'ubuntuone/platform/windows/credentials.py' => 'ubuntuone/platform/credentials/windows.py'
--- ubuntuone/platform/windows/credentials.py	2011-07-29 18:12:27 +0000
+++ ubuntuone/platform/credentials/windows.py	2011-08-17 13:31:24 +0000
@@ -27,17 +27,13 @@
     PING_URL_KEY,
     TC_URL_KEY,
 )
-from ubuntuone.credentials import (
-    CredentialsManagementToolRoot,
+from ubuntuone.platform.credentials import (
     APP_NAME,
     DESCRIPTION,
     NO_OP,
     PING_URL,
     TC_URL,
 )
-from ubuntuone.credentials import CredentialsError as CredentialsErrorRoot
-
-CredentialsError = CredentialsErrorRoot
 
 
 class RemovableSignal(object):
@@ -144,16 +140,8 @@
 
 @defer.inlineCallbacks
 def get_creds_proxy():
-    """Get the CredentialsManagement dbus proxy."""
+    """Get the CredentialsManagement proxy."""
     client = UbuntuSSOClient()
     client = yield client.connect()
     yield client.cred_management.register_to_signals()
     defer.returnValue(CredentialsManagement(client.cred_management))
-
-
-class CredentialsManagementTool(CredentialsManagementToolRoot):
-    """Wrapper used to manage the creds of ubuntu one."""
-
-    def __init__(self):
-        """Create a new instance."""
-        super(CredentialsManagementTool, self).__init__(get_creds_proxy)

=== modified file 'ubuntuone/platform/linux/__init__.py'
--- ubuntuone/platform/linux/__init__.py	2011-08-11 16:07:23 +0000
+++ ubuntuone/platform/linux/__init__.py	2011-08-17 13:31:24 +0000
@@ -56,7 +56,6 @@
     stat_path,
     walk,
 )
-from ubuntuone.platform.linux.credentials import CredentialsManagementTool
 from ubuntuone.platform.linux.logger import setup_filesystem_logging, get_filesystem_logger
 from ubuntuone.platform.linux.filesystem_notifications import FilesystemMonitor
 from ubuntuone.platform.linux.notification import Notification

=== modified file 'ubuntuone/platform/linux/dbus_interface.py'
--- ubuntuone/platform/linux/dbus_interface.py	2011-07-21 20:39:54 +0000
+++ ubuntuone/platform/linux/dbus_interface.py	2011-08-17 13:31:24 +0000
@@ -28,7 +28,7 @@
 from twisted.internet import defer
 from twisted.python.failure import Failure
 from ubuntuone.syncdaemon.interfaces import IMarker
-from ubuntuone.platform.linux.credentials import (
+from ubuntuone.platform.credentials.linux import (
     DBUS_BUS_NAME, DBUS_CREDENTIALS_PATH,
     DBUS_CREDENTIALS_IFACE)
 from ubuntuone.platform.linux.launcher import UbuntuOneLauncher

=== modified file 'ubuntuone/platform/windows/__init__.py'
--- ubuntuone/platform/windows/__init__.py	2011-08-11 16:07:23 +0000
+++ ubuntuone/platform/windows/__init__.py	2011-08-17 13:31:24 +0000
@@ -55,7 +55,6 @@
     stat_path,
     walk,
 )
-from ubuntuone.platform.windows.credentials import CredentialsManagementTool
 from ubuntuone.platform.windows import event_logging
 from ubuntuone.platform.windows.logger import (
     setup_filesystem_logging,

=== modified file 'ubuntuone/platform/windows/ipc.py'
--- ubuntuone/platform/windows/ipc.py	2011-08-08 13:20:23 +0000
+++ ubuntuone/platform/windows/ipc.py	2011-08-17 13:31:24 +0000
@@ -38,7 +38,7 @@
     AlreadyStartedError,
 )
 from ubuntuone.syncdaemon.interfaces import IMarker
-from ubuntuone.platform.windows import CredentialsManagementTool
+from ubuntuone.platform.credentials import CredentialsManagementTool
 from ubuntuone.platform.windows.network_manager import NetworkManager
 from ubuntuone.syncdaemon.interaction_interfaces import (
     bool_str,