Unicode path fixes for drop-upload.

Signed-off-by: Daira Hopwood <daira@jacaranda.org>

src/allmydata/client.py

@@ -493,11 +493,12 @@ class Client(node.Node, pollmixin.PollMixin):
                                            "put the cap in a 'private/drop_upload_dircap' file, and delete this option.")
 
             upload_dircap = self.get_or_create_private_config("drop_upload_dircap")
-            local_dir_utf8 = self.get_config("drop_upload", "local.directory")
+            local_dir_config = self.get_config("drop_upload", "local.directory").decode("utf-8")
+            local_dir = abspath_expanduser_unicode(local_dir_config, base=self.basedir)
 
             try:
                 from allmydata.frontends import drop_upload
-                s = drop_upload.DropUploader(self, upload_dircap, local_dir_utf8)
+                s = drop_upload.DropUploader(self, upload_dircap, local_dir)
                 s.setServiceParent(self)
                 s.startService()
             except Exception, e:

src/allmydata/frontends/drop_upload.py

@@ -8,41 +8,37 @@ from foolscap.api import eventually
 
 from allmydata.interfaces import IDirectoryNode
 
-from allmydata.util.encodingutil import quote_output, get_filesystem_encoding
+from allmydata.util.fileutil import abspath_expanduser_unicode, precondition_abspath
-from allmydata.util.fileutil import abspath_expanduser_unicode
+from allmydata.util.encodingutil import listdir_unicode, to_filepath, \
+     unicode_from_filepath, quote_local_unicode_path, FilenameEncodingError
 from allmydata.immutable.upload import FileName
 
 
 class DropUploader(service.MultiService):
     name = 'drop-upload'
 
-    def __init__(self, client, upload_dircap, local_dir_utf8, inotify=None):
+    def __init__(self, client, upload_dircap, local_dir, inotify=None):
+        precondition_abspath(local_dir)
+
         service.MultiService.__init__(self)
-
+        self._local_dir = abspath_expanduser_unicode(local_dir)
-        try:
-            local_dir_u = abspath_expanduser_unicode(local_dir_utf8.decode('utf-8'))
-            if sys.platform == "win32":
-                local_dir = local_dir_u
-            else:
-                local_dir = local_dir_u.encode(get_filesystem_encoding())
-        except (UnicodeEncodeError, UnicodeDecodeError):
-            raise AssertionError("The '[drop_upload] local.directory' parameter %s was not valid UTF-8 or "
-                                 "could not be represented in the filesystem encoding."
-                                 % quote_output(local_dir_utf8))
-
         self._client = client
         self._stats_provider = client.stats_provider
         self._convergence = client.convergence
-        self._local_path = FilePath(local_dir)
+        self._local_path = to_filepath(self._local_dir)
 
         if inotify is None:
             from twisted.internet import inotify
         self._inotify = inotify
 
         if not self._local_path.exists():
-            raise AssertionError("The '[drop_upload] local.directory' parameter was %s but there is no directory at that location." % quote_output(local_dir_u))
+            raise AssertionError("The '[drop_upload] local.directory' parameter was %s "
+                                 "but there is no directory at that location."
+                                 % quote_local_unicode_path(local_dir))
         if not self._local_path.isdir():
-            raise AssertionError("The '[drop_upload] local.directory' parameter was %s but the thing at that location is not a directory." % quote_output(local_dir_u))
+            raise AssertionError("The '[drop_upload] local.directory' parameter was %s "
+                                 "but the thing at that location is not a directory."
+                                 % quote_local_unicode_path(local_dir))
 
         # TODO: allow a path rather than a cap URI.
         self._parent = self._client.create_node_from_uri(upload_dircap)

src/allmydata/test/test_client.py

@@ -27,7 +27,7 @@ BASECONFIG_I = ("[client]\n"
               "introducer.furl = %s\n"
               )
 
-class Basic(testutil.ReallyEqualMixin, unittest.TestCase):
+class Basic(testutil.ReallyEqualMixin, testutil.NonASCIIPathMixin, unittest.TestCase):
     def test_loadable(self):
         basedir = "test_client.Basic.test_loadable"
         os.mkdir(basedir)
@@ -316,7 +316,8 @@ class Basic(testutil.ReallyEqualMixin, unittest.TestCase):
         self.patch(allmydata.frontends.drop_upload, 'DropUploader', MockDropUploader)
 
         upload_dircap = "URI:DIR2:blah"
-        local_dir_utf8 = u"loc\u0101l_dir".encode('utf-8')
+        local_dir_u = self.unicode_or_fallback(u"loc\u0101l_dir", u"local_dir")
+        local_dir_utf8 = local_dir_u.encode('utf-8')
         config = (BASECONFIG +
                   "[storage]\n" +
                   "enabled = false\n" +
@@ -344,7 +345,7 @@ class Basic(testutil.ReallyEqualMixin, unittest.TestCase):
         self.failUnless(isinstance(uploader, MockDropUploader), uploader)
         self.failUnlessReallyEqual(uploader.client, c1)
         self.failUnlessReallyEqual(uploader.upload_dircap, upload_dircap)
-        self.failUnlessReallyEqual(uploader.local_dir_utf8, local_dir_utf8)
+        self.failUnlessReallyEqual(os.path.basename(uploader.local_dir), local_dir_u)
         self.failUnless(uploader.inotify is None, uploader.inotify)
         self.failUnless(uploader.running)
 

src/allmydata/test/test_drop_upload.py

@@ -2,19 +2,20 @@
 import os, sys
 
 from twisted.trial import unittest
-from twisted.python import filepath, runtime
+from twisted.python import runtime
 from twisted.internet import defer
 
 from allmydata.interfaces import IDirectoryNode, NoSuchChildError
 
-from allmydata.util import fake_inotify
+from allmydata.util import fake_inotify, fileutil
-from allmydata.util.encodingutil import get_filesystem_encoding
+from allmydata.util.encodingutil import get_filesystem_encoding, to_filepath
 from allmydata.util.consumer import download_to_data
 from allmydata.test.no_network import GridTestMixin
 from allmydata.test.common_util import ReallyEqualMixin, NonASCIIPathMixin
 from allmydata.test.common import ShouldFailMixin
 
 from allmydata.frontends.drop_upload import DropUploader
+from allmydata.util.fileutil import abspath_expanduser_unicode
 
 
 class DropUploadTestMixin(GridTestMixin, ShouldFailMixin, ReallyEqualMixin, NonASCIIPathMixin):
@@ -23,6 +24,10 @@ class DropUploadTestMixin(GridTestMixin, ShouldFailMixin, ReallyEqualMixin, NonA
     with the real INotify.
     """
 
+    def setUp(self):
+        GridTestMixin.setUp(self)
+        temp = self.mktemp()
+        self.basedir = abspath_expanduser_unicode(temp.decode(get_filesystem_encoding()))
     def _get_count(self, name):
         return self.stats_provider.get_stats()["counters"].get(name, 0)
 
@@ -84,23 +89,21 @@ class DropUploadTestMixin(GridTestMixin, ShouldFailMixin, ReallyEqualMixin, NonA
         # (otherwise we would get a defer.AlreadyCalledError). Should we be relying on that?
         self.uploader.set_uploaded_callback(d.callback)
 
-        path_u = os.path.join(self.local_dir, name_u)
+        path_u = abspath_expanduser_unicode(name_u, base=self.local_dir)
-        if sys.platform == "win32":
+        path = to_filepath(path_u)
-            path = filepath.FilePath(path_u)
-        else:
-            path = filepath.FilePath(path_u.encode(get_filesystem_encoding()))
 
         # We don't use FilePath.setContent() here because it creates a temporary file that
         # is renamed into place, which causes events that the test is not expecting.
-        f = open(path.path, "wb")
+        f = open(path_u, "wb")
         try:
             if temporary and sys.platform != "win32":
-                os.unlink(path.path)
+                os.unlink(path_u)
             f.write(data)
         finally:
             f.close()
         if temporary and sys.platform == "win32":
-            os.unlink(path.path)
+            os.unlink(path_u)
+        fileutil.flush_volume(path_u)
         self.notify_close_write(path)
 
         if temporary:
@@ -123,10 +126,14 @@ class MockTest(DropUploadTestMixin, unittest.TestCase):
     """This can run on any platform, and even if twisted.internet.inotify can't be imported."""
 
     def test_errors(self):
-        self.basedir = "drop_upload.MockTest.test_errors"
         self.set_up_grid()
-        errors_dir = os.path.join(self.basedir, "errors_dir")
+
+        errors_dir = abspath_expanduser_unicode(u"errors_dir", base=self.basedir)
         os.mkdir(errors_dir)
+        not_a_dir = abspath_expanduser_unicode(u"NOT_A_DIR", base=self.basedir)
+        fileutil.write(not_a_dir, "")
+        magicfolderdb = abspath_expanduser_unicode(u"magicfolderdb", base=self.basedir)
+        doesnotexist  = abspath_expanduser_unicode(u"doesnotexist", base=self.basedir)
 
         client = self.g.clients[0]
         d = client.create_dirnode()
@@ -135,16 +142,10 @@ class MockTest(DropUploadTestMixin, unittest.TestCase):
             upload_dircap = n.get_uri()
             readonly_dircap = n.get_readonly_uri()
 
-            self.shouldFail(AssertionError, 'invalid local.directory', 'could not be represented',
-                            DropUploader, client, upload_dircap, '\xFF', inotify=fake_inotify)
             self.shouldFail(AssertionError, 'nonexistent local.directory', 'there is no directory',
-                            DropUploader, client, upload_dircap, os.path.join(self.basedir, "Laputa"), inotify=fake_inotify)
+                            DropUploader, client, upload_dircap, doesnotexist, inotify=fake_inotify)
-
-            fp = filepath.FilePath(self.basedir).child('NOT_A_DIR')
-            fp.touch()
             self.shouldFail(AssertionError, 'non-directory local.directory', 'is not a directory',
-                            DropUploader, client, upload_dircap, fp.path, inotify=fake_inotify)
+                            DropUploader, client, upload_dircap, not_a_dir, inotify=fake_inotify)
-
             self.shouldFail(AssertionError, 'bad upload.dircap', 'does not refer to a directory',
                             DropUploader, client, 'bad', errors_dir, inotify=fake_inotify)
             self.shouldFail(AssertionError, 'non-directory upload.dircap', 'does not refer to a directory',

src/allmydata/util/encodingutil.py

@@ -8,6 +8,7 @@ from types import NoneType
 
 from allmydata.util.assertutil import precondition
 from twisted.python import usage
+from twisted.python.filepath import FilePath
 from allmydata.util import log
 from allmydata.util.fileutil import abspath_expanduser_unicode
 
@@ -35,9 +36,10 @@ def check_encoding(encoding):
 filesystem_encoding = None
 io_encoding = None
 is_unicode_platform = False
+use_unicode_filepath = False
 
 def _reload():
-    global filesystem_encoding, io_encoding, is_unicode_platform
+    global filesystem_encoding, io_encoding, is_unicode_platform, use_unicode_filepath
 
     filesystem_encoding = canonical_encoding(sys.getfilesystemencoding())
     check_encoding(filesystem_encoding)
@@ -61,6 +63,8 @@ def _reload():
 
     is_unicode_platform = sys.platform in ["win32", "darwin"]
 
+    use_unicode_filepath = sys.platform == "win32" or hasattr(FilePath, '_asTextPath')
+
 _reload()
 
 
@@ -245,6 +249,23 @@ def quote_local_unicode_path(path, quotemarks=True):
 
     return quote_output(path, quotemarks=quotemarks, quote_newlines=True)
 
+def to_filepath(path):
+    precondition(isinstance(path, basestring), path=path)
+
+    if isinstance(path, unicode) and not use_unicode_filepath:
+        path = path.encode(filesystem_encoding)
+
+    return FilePath(path)
+
+def unicode_from_filepath(fp):
+    precondition(isinstance(fp, FilePath), fp=fp)
+
+    path = fp.path
+    if isinstance(path, bytes):
+        path = path.decode(filesystem_encoding)
+
+    return path
+
 
 def unicode_platform():
     """