Merge lp:~nataliabidart/ubuntuone-control-panel/simpler-devices-backend into lp:ubuntuone-control-panel

[Natalia Bidart]

- Backend now provides a simpler device query method: device_names_info (LP: #798413).

[John Lenton (chipaca)]

review: Approve

[lp:~nataliabidart/ubuntuone-control-panel/simpler-devices-backend]

168. By Natalia Bidart on 2011-06-21

Merged trunk in.

[Roberto Alsina (ralsina)]

+1

review: Approve

=== modified file 'ubuntuone/controlpanel/backend.py'
--- ubuntuone/controlpanel/backend.py	2011-06-17 21:11:45 +0000
+++ ubuntuone/controlpanel/backend.py	2011-06-21 16:37:25 +0000
@@ -46,6 +46,7 @@
 SHOW_ALL_NOTIFICATIONS_KEY = 'show_all_notifications'
 SHARE_AUTOSUBSCRIBE_KEY = 'share_autosubscribe'
 UDF_AUTOSUBSCRIBE_KEY = 'udf_autosubscribe'
+LIMIT_BW_KEY = 'limit_bandwidth'
 UPLOAD_KEY = "max_upload_speed"
 DOWNLOAD_KEY = "max_download_speed"
 
@@ -191,17 +192,20 @@
                                       _set_status_changed_handler)
 
     @inlineCallbacks
-    def _process_device_web_info(self, devices, enabled, limit_bw, limits,
-                                 autoconnect, show_notifs,
-                                 share_autosubscribe, udf_autosubscribe):
-        """Return a lis of processed devices."""
+    def _process_device_web_info(self, devices,
+        enabled=None, limit_bw=None, limits=None, autoconnect=None,
+        show_notifs=None, share_autosubscribe=None, udf_autosubscribe=None):
+        """Return a lis of processed devices.
+
+        If all the file sync settings are None, do not attach that info.
+
+        """
         result = []
         for d in devices:
             di = {}
             di["type"] = d["kind"]
             di["name"] = d["description"] if d["description"] \
                                           else self.NAME_NOT_SET
-            di["configurable"] = False
             if di["type"] == DEVICE_TYPE_COMPUTER:
                 di["device_id"] = di["type"] + d["token"]
             if di["type"] == DEVICE_TYPE_PHONE:
@@ -209,12 +213,22 @@
 
             is_local = yield self.device_is_local(di["device_id"])
             di["is_local"] = is_local
+
+            if is_local:  # prepend the local device!
+                result.insert(0, di)
+            else:
+                result.append(di)
+
+            if enabled is None:
+                # without knowing if file sync is enabled or not,
+                # we can't add any extra info about the device.
+                continue
+
             # currently, only local devices are configurable.
-            # eventually, more devices will be configurable.
             di["configurable"] = is_local and enabled
 
             if di["configurable"]:
-                di["limit_bandwidth"] = limit_bw
+                di[LIMIT_BW_KEY] = limit_bw
                 di[AUTOCONNECT_KEY] = autoconnect
                 di[SHOW_ALL_NOTIFICATIONS_KEY] = show_notifs
                 di[SHARE_AUTOSUBSCRIBE_KEY] = share_autosubscribe
@@ -222,20 +236,41 @@
                 di[UPLOAD_KEY] = limits["upload"]
                 di[DOWNLOAD_KEY] = limits["download"]
 
-            # date_added is not in the webservice yet (LP: #673668)
-            # di["date_added"] = ""
-
-            # missing values (LP: #673668)
-            # di["available_services"] = ""
-            # di["enabled_services"] = ""
-
-            if is_local:  # prepend the local device!
-                result.insert(0, di)
-            else:
-                result.append(di)
-
         returnValue(result)
 
+    @inlineCallbacks
+    def _process_device_local_info(self,
+        enabled=None, limit_bw=None, limits=None, autoconnect=None,
+        show_notifs=None, share_autosubscribe=None, udf_autosubscribe=None):
+        """Return the information for the local device.
+
+        If all the file sync settings are None, do not attach that info.
+
+        """
+        credentials = yield login_client.get_credentials()
+
+        local_device = {}
+        local_device["type"] = DEVICE_TYPE_COMPUTER
+        local_device["name"] = credentials['name']
+        device_id = local_device["type"] + credentials["token"]
+        local_device["device_id"] = device_id
+        local_device["is_local"] = True
+
+        if enabled is not None:
+            local_device["configurable"] = enabled
+            if local_device["configurable"]:
+                local_device[LIMIT_BW_KEY] = limit_bw
+                local_device[AUTOCONNECT_KEY] = autoconnect
+                local_device[SHOW_ALL_NOTIFICATIONS_KEY] = show_notifs
+                local_device[SHARE_AUTOSUBSCRIBE_KEY] = share_autosubscribe
+                local_device[UDF_AUTOSUBSCRIBE_KEY] = udf_autosubscribe
+                upload = limits["upload"]
+                download = limits["download"]
+                local_device[UPLOAD_KEY] = upload
+                local_device[DOWNLOAD_KEY] = download
+
+        returnValue(local_device)
+
     def _process_path(self, path):
         """Trim 'path' so the '~' is removed."""
         home = os.path.expanduser('~')
@@ -329,33 +364,39 @@
             result = yield self._process_device_web_info(devices, enabled,
                         limit_bw, limits, autoconnect, show_notifs,
                         share_autosubscribe, udf_autosubscribe)
+
         if result is None:
-            logger.info('devices_info: result is None after calling '
-                        'devices/ API, building the local device.')
-            credentials = yield login_client.get_credentials()
-            local_device = {}
-            local_device["type"] = DEVICE_TYPE_COMPUTER
-            local_device["name"] = credentials['name']
-            device_id = local_device["type"] + credentials["token"]
-            local_device["device_id"] = device_id
-            local_device["is_local"] = True
-            local_device["configurable"] = enabled
-            if local_device["configurable"]:
-                local_device["limit_bandwidth"] = limit_bw
-                local_device[AUTOCONNECT_KEY] = autoconnect
-                local_device[SHOW_ALL_NOTIFICATIONS_KEY] = show_notifs
-                local_device[SHARE_AUTOSUBSCRIBE_KEY] = share_autosubscribe
-                local_device[UDF_AUTOSUBSCRIBE_KEY] = udf_autosubscribe
-                upload = limits["upload"]
-                download = limits["download"]
-                local_device[UPLOAD_KEY] = upload
-                local_device[DOWNLOAD_KEY] = download
-            logger.debug('devices_info: local device built: %r.', local_device)
+            local_device = yield self._process_device_local_info(enabled,
+                            limit_bw, limits, autoconnect, show_notifs,
+                            share_autosubscribe, udf_autosubscribe)
             result = [local_device]
+
+        logger.info('devices_info: result is %r',
+                    filter_field(result, field='device_id'))
+
+        returnValue(result)
+
+    @log_call(logger.debug)
+    @process_unauthorized
+    @inlineCallbacks
+    def device_names_info(self):
+        """Get the user devices info, only list names and kind."""
+        result = None
+        try:
+            devices = yield self.wc.call_api(DEVICES_API)
+        except UnauthorizedError:
+            raise
+        except WebClientError:
+            logger.exception('device_names_info: web client failure:')
         else:
-            logger.info('devices_info: result is not None after calling '
-                        'devices/ API: %r',
-                        filter_field(result, field='device_id'))
+            result = yield self._process_device_web_info(devices)
+
+        if result is None:
+            local_device = yield self._process_device_local_info()
+            result = [local_device]
+
+        logger.info('device_names_info: result is %r',
+                    filter_field(result, field='device_id'))
 
         returnValue(result)
 
@@ -379,8 +420,8 @@
             else:
                 yield sd_client.enable_show_all_notifications()
 
-        if is_local and "limit_bandwidth" in settings:
-            if not settings["limit_bandwidth"]:
+        if is_local and LIMIT_BW_KEY in settings:
+            if not settings[LIMIT_BW_KEY]:
                 yield sd_client.disable_bandwidth_throttling()
             else:
                 yield sd_client.enable_bandwidth_throttling()

=== modified file 'ubuntuone/controlpanel/tests/__init__.py'
--- ubuntuone/controlpanel/tests/__init__.py	2011-06-14 20:18:30 +0000
+++ ubuntuone/controlpanel/tests/__init__.py	2011-06-21 16:37:25 +0000
@@ -20,6 +20,10 @@
 
 from ubuntuone.devtools.testcase import TestCase as BaseTestCase
 
+from ubuntuone.controlpanel.backend import (
+    DEVICE_TYPE_COMPUTER, DEVICE_TYPE_PHONE,
+)
+
 
 TOKEN = {u'consumer_key': u'xQ7xDAz',
          u'consumer_secret': u'KzCJWCTNbbntwfyCKKjomJDzlgqxLy',
@@ -141,19 +145,41 @@
     {
         "device_id": "ComputerABCDEF01234token",
         "name": "Ubuntu One @ darkstar",
-        "type": "Computer",
+        "type": DEVICE_TYPE_COMPUTER,
         "is_local": False,
         "configurable": False,
     },
     {
         "device_id": "Phone1000",
         "name": "Nokia E65",
-        "type": "Phone",
+        "type": DEVICE_TYPE_PHONE,
         "configurable": False,
         "is_local": False,
     },
 ]
 
+# note that local computer should be first, do not change!
+EXPECTED_DEVICE_NAMES_INFO = [
+    {
+        'device_id': 'ComputerABCDEF01234-localtoken',
+        'name': 'Ubuntu One @ localhost',
+        'type': DEVICE_TYPE_COMPUTER,
+        'is_local': True,
+    },
+    {
+        'device_id': 'ComputerABCDEF01234token',
+        'name': 'Ubuntu One @ darkstar',
+        'type': DEVICE_TYPE_COMPUTER,
+        'is_local': False,
+    },
+    {
+        'device_id': 'Phone1000',
+        'name': 'Nokia E65',
+        'type': DEVICE_TYPE_PHONE,
+        'is_local': False,
+    },
+]
+
 SAMPLE_FOLDERS = [
     {u'generation': u'2',
      u'node_id': u'341da068-81d8-437a-8f75-5bb9d86455ba',

=== modified file 'ubuntuone/controlpanel/tests/test_backend.py'
--- ubuntuone/controlpanel/tests/test_backend.py	2011-06-17 21:11:45 +0000
+++ ubuntuone/controlpanel/tests/test_backend.py	2011-06-21 16:37:25 +0000
@@ -48,6 +48,7 @@
     EMPTY_DESCRIPTION_JSON,
     EXPECTED_ACCOUNT_INFO,
     EXPECTED_ACCOUNT_INFO_WITH_CURRENT_PLAN,
+    EXPECTED_DEVICE_NAMES_INFO,
     EXPECTED_DEVICES_INFO,
     LOCAL_DEVICE,
     ROOT_PATH,
@@ -440,7 +441,7 @@
         yield d
 
     @inlineCallbacks
-    def test_devices_info_if_files_disable(self):
+    def test_devices_info_if_files_disabled(self):
         """The devices_info returns device only info if files is disabled."""
         yield self.be.disable_files()
         status = yield self.be.file_sync_status()
@@ -485,6 +486,75 @@
         self.assertTrue(device_id_logged)
 
     @inlineCallbacks
+    def test_device_names_info(self):
+        """The device_names_info returns the correct info."""
+        self.be.wc.results[DEVICES_API] = SAMPLE_DEVICES_JSON
+        result = yield self.be.device_names_info()
+        self.assertEqual(result, EXPECTED_DEVICE_NAMES_INFO)
+
+    @inlineCallbacks
+    def test_device_names_info_fails(self):
+        """The device_names_info method exercises its errback."""
+        def fail(*args, **kwargs):
+            """Raise any error other than WebClientError."""
+            raise ValueError(args)
+
+        self.patch(self.be.wc, 'call_api', fail)
+        yield self.assertFailure(self.be.device_names_info(), ValueError)
+
+    @inlineCallbacks
+    def test_device_names_info_with_webclient_error(self):
+        """The device_names_info returns local info if webclient error."""
+        MockWebClient.failure = 404
+        result = yield self.be.device_names_info()
+
+        device = LOCAL_DEVICE.copy()
+        device.pop('configurable', None)
+        device.pop('limit_bandwidth', None)
+        device.pop(backend.DOWNLOAD_KEY, None)
+        device.pop(backend.UPLOAD_KEY, None)
+        device.pop(backend.AUTOCONNECT_KEY, None)
+        device.pop(backend.SHOW_ALL_NOTIFICATIONS_KEY, None)
+        device.pop(backend.SHARE_AUTOSUBSCRIBE_KEY, None)
+        device.pop(backend.UDF_AUTOSUBSCRIBE_KEY, None)
+        self.assertEqual(result, [device])
+        self.assertTrue(self.memento.check_error('device_names_info',
+                                                 'web client failure'))
+
+    @inlineCallbacks
+    def test_device_names_info_fails_with_unauthorized(self):
+        """The device_names_info clears the credentials on unauthorized."""
+        MockWebClient.failure = 401
+        d = defer.Deferred()
+        self.patch(backend.login_client, 'clear_credentials',
+                   lambda: d.callback('called'))
+        yield self.assertFailure(self.be.device_names_info(),
+                                 backend.UnauthorizedError)
+        yield d
+
+    @inlineCallbacks
+    def test_device_names_info_when_token_name_is_empty(self):
+        """The device_names_info can handle empty token names."""
+        self.be.wc.results[DEVICES_API] = EMPTY_DESCRIPTION_JSON
+        result = yield self.be.device_names_info()
+        expected = {'device_id': 'ComputerABCDEF01234token',
+                    'is_local': False, 'name': self.be.NAME_NOT_SET,
+                    'type': DEVICE_TYPE_COMPUTER}
+        self.assertEqual(result, [expected])
+
+    @inlineCallbacks
+    def test_device_names_info_does_not_log_device_id(self):
+        """The device_names_info does not log the device_id."""
+        self.be.wc.results[DEVICES_API] = SAMPLE_DEVICES_JSON
+        yield self.be.device_names_info()
+
+        dids = (d['device_id'] for d in EXPECTED_DEVICES_INFO)
+        device_id_logged = all(all(did not in r.getMessage()
+                                   for r in self.memento.records)
+                               for did in dids)
+        self.assertTrue(device_id_logged)
+
+    @inlineCallbacks
     def test_remove_device(self):
         """The remove_device method calls the right api."""
         dtype, did = DEVICE_TYPE_COMPUTER, "SAMPLE-TOKEN"
@@ -549,10 +619,10 @@
         """The device settings are updated."""
         backend.sd_client.throttling = False
         yield self.be.change_device_settings(self.local_token,
-                                        {"limit_bandwidth": True})
+                                        {backend.LIMIT_BW_KEY: True})
         self.assertEqual(backend.sd_client.throttling, True)
         yield self.be.change_device_settings(self.local_token,
-                                        {"limit_bandwidth": False})
+                                        {backend.LIMIT_BW_KEY: False})
         self.assertEqual(backend.sd_client.throttling, False)
 
     @inlineCallbacks
@@ -560,7 +630,7 @@
         """The device settings are updated."""
         backend.sd_client.limits = {"download": -1, "upload": -1}
         yield self.be.change_device_settings(self.local_token,
-                                        {"max_upload_speed": 1111})
+                                        {backend.UPLOAD_KEY: 1111})
         self.assertEqual(backend.sd_client.limits["upload"], 1111)
         self.assertEqual(backend.sd_client.limits["download"], -1)
 
@@ -569,7 +639,7 @@
         """The device settings are updated."""
         backend.sd_client.limits = {"download": -1, "upload": -1}
         yield self.be.change_device_settings(self.local_token,
-                                        {"max_download_speed": 99})
+                                        {backend.DOWNLOAD_KEY: 99})
         self.assertEqual(backend.sd_client.limits["upload"], -1)
         self.assertEqual(backend.sd_client.limits["download"], 99)
 
@@ -579,9 +649,9 @@
         backend.sd_client.throttling = False
         backend.sd_client.limits = {"download": -1, "upload": -1}
         new_settings = {
-            "max_download_speed": 99,
-            "max_upload_speed": 99,
-            "limit_bandwidth": True,
+            backend.DOWNLOAD_KEY: 99,
+            backend.UPLOAD_KEY: 99,
+            backend.LIMIT_BW_KEY: True,
         }
         yield self.be.change_device_settings("wrong token!", new_settings)
         self.assertEqual(backend.sd_client.throttling, False)