Merge lp:~diegosarmentero/ubuntuone-control-panel/folder-show-garbage into lp:ubuntuone-control-panel

[Diego Sarmentero]

Fixed: QT UI: Folder list may show garbage for user homes non-ascii (LP: #851356).

To test it IRL you will need to have a windows user with unicode chars in the username and be able to open the control panel and see the Ubuntu One folder.

[lp:~diegosarmentero/ubuntuone-control-panel/folder-show-garbage]

247. By Diego Sarmentero on 2012-01-18

get_home_dir modified

[Roberto Alsina (ralsina)]

I just don't understand how get_home_dir is supposed to work. It will set self.home_dir in some eventual future, so after calling get_home_dir, thereisno guarantee self.home_dir will have anything useful.

I think that should be blocking code, and not return until self.home_dir is set.

Unless of course, I missed something...

review: Needs Information

[lp:~diegosarmentero/ubuntuone-control-panel/folder-show-garbage]

248. By Diego Sarmentero on 2012-01-19

removing unnecessary self.home_dir

[Natalia Bidart (nataliabidart)]

The branch looks good. There are a couple of this that confuse me:

* you're mixing unicode and non unicode in 200 to 202. Now it works because there is automatic promotion to unicode, but we should avoid that and be explicit.

* when trying to create a UDF using u1client trunk and this branch, the creation fails and the syncdaemon log shows:

2012-01-19 11:00:45,206 - ubuntuone.SyncDaemon.VM - DEBUG - create udf: 'C:\\Users\\\xe5\x8d\x97\xe6\xb8\xa1\xe6\xb1\x9f \xd7\xa0\xd7\x95\xd7\xa0\xd7\x95\\Links'
C:\Python27\lib\genericpath.py:71: UnicodeWarning: Unicode equal comparison failed to convert both arguments to Unicode - interpreting them as being unequal
  s1 = min(m)

2012-01-19 11:00:45,207 - ubuntuone.SyncDaemon.EQ - DEBUG - push_event: VM_UDF_CREATE_ERROR, kwargs: {'path': 'C:\\Users\\\xe5\x8d\x97\xe6\xb8\xa1\xe6\xb1\x9f \xd7\xa0\xd7\x95\xd7\xa0\xd7\x95\\Links', 'error': "INVALID_PATH: 'ascii' codec can't decode byte 0xe5 in position 0: ordinal not in range(128)"}

2012-01-19 11:00:45,207 - ubuntuone.SyncDaemon.InteractionInterfaces - DEBUG - handle_VM_UDF_CREATE_ERROR: args (<ubuntuone.syncdaemon.interaction_interfaces.SyncdaemonEventListener object at 0x033AFED0>,), kwargs {'path': 'C:\\Users\\\xe5\x8d\x97\xe6\xb8\xa1\xe6\xb1\x9f \xd7\xa0\xd7\x95\xd7\xa0\xd7\x95\\Links', 'error
': "INVALID_PATH: 'ascii' codec can't decode byte 0xe5 in position 0: ordinal not in range(128)"}.

Would you please confirm if the problem is in syncdaemon's end or controlpanel's end?

review: Needs Fixing

[Natalia Bidart (nataliabidart)]

Let me know if this branch is meant to only solve the showing of the folders, and I'll make a new bug report and change the voting.

[lp:~diegosarmentero/ubuntuone-control-panel/folder-show-garbage]

249. By Diego Sarmentero on 2012-01-23

merge

250. By Diego Sarmentero on 2012-01-25

_process_path improved

[Diego Sarmentero (diegosarmentero)]

> Let me know if this branch is meant to only solve the showing of the folders,
> and I'll make a new bug report and change the voting.

This is a Ubuntu One Client issue, I already create a bug for it and i'm working on that:
https://bugs.launchpad.net/ubuntuone-client/+bug/921703

[Roberto Alsina (ralsina)]

+1

review: Approve

[Natalia Bidart (nataliabidart)]

- Seems like the import of USER_HOME was moved in ubuntuone/controlpanel/tests/test_backend.py and now is the imports are not alphabetically ordered.

- While this test_volumes_info_process_path is a great adding, the assertion self.assertTrue(USER_HOME in display_name) is a bit weak. Can you please change that for:

prefix = 'My Ubuntu' + USER_HOME
self.assertTrue(display_name.startswith(prefix))

- Tests in windows are not padding for me, but neither are the suite from trunk, so seems unrelated.

I'm approving but please fix the first two items before landing.

review: Approve

[lp:~diegosarmentero/ubuntuone-control-panel/folder-show-garbage]

251. By Diego Sarmentero on 2012-01-26

Tests improved.

=== modified file 'ubuntuone/controlpanel/backend.py'
--- ubuntuone/controlpanel/backend.py	2011-09-16 16:38:43 +0000
+++ ubuntuone/controlpanel/backend.py	2012-01-26 12:48:23 +0000
@@ -139,6 +139,10 @@
 
         logger.info('ControlBackend: instance started.')
 
+    def get_home_dir(self):
+        """Return a defer with the home_dir from ubuntu one client."""
+        return self.sd_client.get_home_dir()
+
     def _process_file_sync_status(self, status):
         """Process raw file sync status into custom format.
 
@@ -289,11 +293,15 @@
 
         returnValue(local_device)
 
+    @inlineCallbacks
     def _process_path(self, path):
         """Trim 'path' so the '~' is removed."""
-        home = os.path.expanduser('~')
-        result = path.replace(os.path.join(home, ''), '')
-        return result
+        home_dir = yield self.get_home_dir()
+        if not home_dir.endswith(os.path.sep):
+            home_dir += os.path.sep
+        if path.startswith(home_dir):
+            path = path[len(home_dir):]
+        returnValue(path)
 
     @inlineCallbacks
     def get_credentials(self):
@@ -578,9 +586,10 @@
         folders = yield self.sd_client.get_folders()
         shares = yield self.sd_client.get_shares()
 
+        display_name = yield self._process_path(root_dir)
         root_volume = {u'volume_id': u'', u'path': root_dir,
                        u'subscribed': True, u'type': self.ROOT_TYPE,
-                       u'display_name': self._process_path(root_dir)}
+                       u'display_name': display_name}
         self._volumes[u''] = root_volume
 
         # group shares by the offering user
@@ -618,7 +627,8 @@
                 logger.warning('volumes_info: udf %r already in the volumes '
                                'list (%r).', vid, self._volumes[vid])
             folder[u'subscribed'] = bool(folder[u'subscribed'])
-            folder[u'display_name'] = self._process_path(folder[u'path'])
+            display_name = yield self._process_path(folder[u'path'])
+            folder[u'display_name'] = display_name
             self._volumes[vid] = folder
 
         folders.sort(key=operator.itemgetter('path'))
@@ -678,7 +688,7 @@
     @inlineCallbacks
     def validate_path_for_folder(self, folder_path):
         """Validate 'folder_path' for folder creation."""
-        user_home = os.path.expanduser('~')
+        user_home = yield self.get_home_dir()
         folder_path = append_path_sep(folder_path)
 
         # handle folder_path not within '~' or links

=== modified file 'ubuntuone/controlpanel/gui/qt/addfolder.py'
--- ubuntuone/controlpanel/gui/qt/addfolder.py	2011-12-26 19:45:13 +0000
+++ ubuntuone/controlpanel/gui/qt/addfolder.py	2012-01-26 12:48:23 +0000
@@ -20,8 +20,6 @@
 
 from __future__ import division
 
-import os
-
 from PyQt4 import QtGui, QtCore
 from twisted.internet import defer
 
@@ -56,8 +54,9 @@
     def on_clicked(self):
         """The 'Sync another folder' button was clicked."""
         # The options argument is because of LP: #835013
+        home_dir = yield self.backend.get_home_dir()
         folder = QtGui.QFileDialog.getExistingDirectory(
-            parent=self, directory=os.path.expanduser('~'),
+            parent=self, directory=home_dir,
             options=QtGui.QFileDialog.DontUseNativeDialog)
         folder = unicode(QtCore.QDir.toNativeSeparators(folder))
         logger.debug('on_add_folder_button_clicked: user requested folder '
@@ -68,9 +67,8 @@
 
         is_valid = yield self.backend.validate_path_for_folder(folder)
         if not is_valid:
-            user_home = os.path.expanduser('~')
             text = FOLDER_INVALID_PATH % {'folder_path': folder,
-                                          'home_folder': user_home}
+                                          'home_folder': home_dir}
             QtGui.QMessageBox.warning(self, '', text, CLOSE)
             return
 

=== modified file 'ubuntuone/controlpanel/gui/qt/tests/__init__.py'
--- ubuntuone/controlpanel/gui/qt/tests/__init__.py	2011-10-24 21:48:27 +0000
+++ ubuntuone/controlpanel/gui/qt/tests/__init__.py	2012-01-26 12:48:23 +0000
@@ -127,6 +127,10 @@
         self._called['get_credentials'] = ((), {})
         return TOKEN
 
+    def get_home_dir(self):
+        """Fake home return."""
+        return USER_HOME
+
 
 class CrashyBackendException(Exception):
     """A faked backend crash."""

=== modified file 'ubuntuone/controlpanel/gui/qt/tests/test_addfolder.py'
--- ubuntuone/controlpanel/gui/qt/tests/test_addfolder.py	2011-12-26 19:45:13 +0000
+++ ubuntuone/controlpanel/gui/qt/tests/test_addfolder.py	2012-01-26 12:48:23 +0000
@@ -80,10 +80,11 @@
         """When adding a new folder, the proper file chooser is raised."""
         yield self.ui.click()
 
+        home_dir = yield self.ui.backend.get_home_dir()
         self.assertEqual(FakedFileDialog.args, ())
         self.assertEqual(FakedFileDialog.kwargs, {
             'options': gui.QtGui.QFileDialog.DontUseNativeDialog,
-            'directory': os.path.expanduser('~'),
+            'directory': home_dir,
             'parent': self.ui,
         })
 

=== modified file 'ubuntuone/controlpanel/sd_client/__init__.py'
--- ubuntuone/controlpanel/sd_client/__init__.py	2011-08-24 19:56:19 +0000
+++ ubuntuone/controlpanel/sd_client/__init__.py	2012-01-26 12:48:23 +0000
@@ -108,6 +108,10 @@
         """Disable udf_autosubscribe in the syncdaemon."""
         return self.proxy.enable_udf_autosubscribe(False)
 
+    def get_home_dir(self):
+        """Retrieve the root information from syncdaemon."""
+        return self.proxy.get_home_dir()
+
     def get_root_dir(self):
         """Retrieve the root information from syncdaemon."""
         return self.proxy.get_root_dir()

=== modified file 'ubuntuone/controlpanel/tests/test_backend.py'
--- ubuntuone/controlpanel/tests/test_backend.py	2011-10-24 21:48:27 +0000
+++ ubuntuone/controlpanel/tests/test_backend.py	2012-01-26 12:48:23 +0000
@@ -248,6 +248,10 @@
         """Stop the file_sync service."""
         self.actions.append('stop')
 
+    def get_home_dir(self):
+        """Grab the home dir."""
+        return USER_HOME
+
     def get_root_dir(self):
         """Grab the root dir."""
         return ROOT_PATH
@@ -784,10 +788,11 @@
         self.be.wc.results[QUOTA_API] = SAMPLE_QUOTA_JSON
 
         # root dir info
+        display_name = yield self.be._process_path(ROOT_PATH)
         self.root_volume = {
             u'volume_id': u'', u'path': ROOT_PATH, u'subscribed': True,
             u'type': self.be.ROOT_TYPE,
-            u'display_name': self.be._process_path(ROOT_PATH),
+            u'display_name': display_name,
         }
 
     @inlineCallbacks
@@ -831,7 +836,8 @@
             folder = folder.copy()
             folder[u'type'] = self.be.FOLDER_TYPE
             folder[u'subscribed'] = bool(folder[u'subscribed'])
-            folder[u'display_name'] = self.be._process_path(folder[u'path'])
+            display_name = yield self.be._process_path(folder[u'path'])
+            folder[u'display_name'] = display_name
             folders.append(folder)
 
         # sort folders by path
@@ -863,6 +869,26 @@
         self.assertEqual(result, expected)
 
     @inlineCallbacks
+    def test_volumes_info_process_path(self):
+        """The volumes_info method exercises its callback."""
+        root_path = USER_HOME + os.path.sep + 'My Ubuntu' + USER_HOME
+        self.patch(self.be.sd_client, 'get_root_dir', lambda: root_path)
+        for item in SAMPLE_FOLDERS:
+            path = item[u'path']
+            path = path[len(USER_HOME) + 1:]
+            item[u'path'] = os.path.join(root_path, path)
+        self.patch(self.be.sd_client, 'shares', SAMPLE_SHARES)
+        self.patch(self.be.sd_client, 'folders', SAMPLE_FOLDERS)
+
+        yield self.be.volumes_info()
+        for item in SAMPLE_FOLDERS:
+            key = item[u'volume_id']
+            folder = self.be._volumes[key]
+            display_name = folder['display_name']
+            prefix = 'My Ubuntu' + USER_HOME
+            self.assertTrue(display_name.startswith(prefix))
+
+    @inlineCallbacks
     def test_volumes_info_without_storage_info(self):
         """The volumes_info method exercises its callback."""
         self.patch(self.be.sd_client, 'shares', SAMPLE_SHARES)

=== modified file 'ubuntuone/controlpanel/tests/test_sd_client.py'
--- ubuntuone/controlpanel/tests/test_sd_client.py	2012-01-23 20:15:09 +0000
+++ ubuntuone/controlpanel/tests/test_sd_client.py	2012-01-26 12:48:23 +0000
@@ -69,9 +69,10 @@
     called = {}
     shares = {}
     folders = {}
-    root_dir = os.path.expanduser('~/Ubuntu One')
-    shares_dir = os.path.expanduser('~/.cache/ubuntuone/shares')
-    shares_dir_link = os.path.join(root_dir, 'Shared With Me')
+    home_dir = u'/home/ñandu úser'
+    root_dir = os.path.join(home_dir, u'Ubuntu One')
+    shares_dir = os.path.join(home_dir, u'.cache/ubuntuone/shares')
+    shares_dir_link = os.path.join(root_dir, u'Shared With Me')
 
     @classmethod
     def create_share(cls, name, accepted=False, subscribed=False):
@@ -302,6 +303,10 @@
         """Request a rescan from scratch for volume_id."""
         self.called['rescan_from_scratch'] = volume_id
 
+    def get_home_dir(self):
+        """Return the root directory."""
+        return self.home_dir
+
     def get_root_dir(self):
         """Return the root directory."""
         return self.root_dir