From: Daira Hopwood <daira@jacaranda.org>
Date: Mon, 27 Apr 2015 20:31:58 +0000 (+0100)
Subject: drop-upload: make paths consistently Unicode. Also fix import warnings.
X-Git-Url: https://git.rkrishnan.org/status?a=commitdiff_plain;h=77b88ecf75fa6ded4b8c4bae20329728253222b0;p=tahoe-lafs%2Ftahoe-lafs.git

drop-upload: make paths consistently Unicode. Also fix import warnings.

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

diff --git a/src/allmydata/client.py b/src/allmydata/client.py
index 131d2ed7..b584bfab 100644
--- a/src/allmydata/client.py
+++ b/src/allmydata/client.py
@@ -499,13 +499,14 @@ 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
                 dbfile = os.path.join(self.basedir, "private", "magicfolderdb.sqlite")
                 dbfile = abspath_expanduser_unicode(dbfile)
-                s = drop_upload.DropUploader(self, upload_dircap, local_dir_utf8, dbfile)
+                s = drop_upload.DropUploader(self, upload_dircap, local_dir, dbfile)
                 s.setServiceParent(self)
                 s.startService()
 
diff --git a/src/allmydata/frontends/drop_upload.py b/src/allmydata/frontends/drop_upload.py
index 1d52fd6b..eb60c1c6 100644
--- a/src/allmydata/frontends/drop_upload.py
+++ b/src/allmydata/frontends/drop_upload.py
@@ -4,47 +4,32 @@ from collections import deque
 
 from twisted.internet import defer, reactor, task
 from twisted.python.failure import Failure
-from twisted.python.filepath import FilePath
 from twisted.application import service
 
-from allmydata.interfaces import IDirectoryNode
+from allmydata.interfaces import IDirectoryNode, NoSuchChildError
 
-from allmydata.util.encodingutil import quote_output, get_filesystem_encoding
-from allmydata.util.fileutil import abspath_expanduser_unicode
+from allmydata.util.fileutil import abspath_expanduser_unicode, precondition_abspath
+from allmydata.util.encodingutil import listdir_unicode, to_filepath, \
+     unicode_from_filepath, quote_local_unicode_path, FilenameEncodingError
 from allmydata.immutable.upload import FileName
-from allmydata.scripts import backupdb, tahoe_backup
-
-from allmydata.util.encodingutil import listdir_unicode, quote_output, \
-     quote_local_unicode_path, to_str, FilenameEncodingError, unicode_to_url
-
-from allmydata.interfaces import NoSuchChildError
-from twisted.python import failure
+from allmydata.scripts import backupdb
 
 
 class DropUploader(service.MultiService):
     name = 'drop-upload'
 
-    def __init__(self, client, upload_dircap, local_dir_utf8, dbfile, inotify=None,
+    def __init__(self, client, upload_dircap, local_dir, dbfile, inotify=None,
                  pending_delay=1.0):
-        service.MultiService.__init__(self)
-        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))
+        precondition_abspath(local_dir)
 
+        service.MultiService.__init__(self)
+        self._local_dir = abspath_expanduser_unicode(local_dir)
         self._upload_lazy_tail = defer.succeed(None)
         self._pending = set()
         self._client = client
         self._stats_provider = client.stats_provider
         self._convergence = client.convergence
-        self._local_path = FilePath(local_dir)
-        self._local_dir = unicode(local_dir, 'UTF-8')
+        self._local_path = to_filepath(self._local_dir)
         self._dbfile = dbfile
 
         self._upload_deque = deque()
@@ -58,9 +43,13 @@ class DropUploader(service.MultiService):
         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)
@@ -174,49 +163,44 @@ class DropUploader(service.MultiService):
 
     def _notify(self, opaque, path, events_mask):
         self._log("inotify event %r, %r, %r\n" % (opaque, path, ', '.join(self._inotify.humanReadableMask(events_mask))))
-        if path.path not in self._pending:
-            self._append_to_deque(path.path)
+        path_u = unicode_from_filepath(path)
+        if path_u not in self._pending:
+            self._append_to_deque(path_u)
 
     def _process(self, path):
         d = defer.succeed(None)
 
         # FIXME (ticket #1712): if this already exists as a mutable file, we replace the
         # directory entry, but we should probably modify the file (as the SFTP frontend does).
-        def _add_file(ignore):
-            self._pending.remove(path)
-            name = os.path.basename(path)
-            # on Windows the name is already Unicode
-            if sys.platform != "win32":
-                name = name.decode(get_filesystem_encoding())
+        def _add_file(ignore, name):
             u = FileName(path, self._convergence)
             return self._parent.add_file(name, u)
 
-        def _add_dir(ignore):
+        def _add_dir(ignore, name):
+            d2 = self._parent.create_subdirectory(name)
+            d2.addCallback(lambda ign: self._log("created subdirectory %r" % (path,)))
+            d2.addCallback(lambda ign: self._scan(path))
+            return d2
+
+        def _maybe_upload(val):
+            print "_maybe_upload", path
             self._pending.remove(path)
             name = os.path.basename(path)
-            dirname = path
-            # on Windows the name is already Unicode
-            if sys.platform != "win32":
-                name = name.decode(get_filesystem_encoding())
-                # XXX
-                dirname = path
-            return self._parent.create_subdirectory(name)
 
-        def _maybe_upload(val):
             if not os.path.exists(path):
                 self._log("uploader: not uploading non-existent file.")
                 self._stats_provider.count('drop_upload.objects_disappeared', 1)
                 return NoSuchChildError("not uploading non-existent file")
             elif os.path.islink(path):
                 self._log("operator ERROR: symlink not being processed.")
-                return failure.Failure()
+                return Failure()
 
             if os.path.isdir(path):
-                d.addCallback(_add_dir)
+                d.addCallback(_add_dir, name)
                 self._stats_provider.count('drop_upload.directories_created', 1)
                 return None
             elif os.path.isfile(path):
-                d.addCallback(_add_file)
+                d.addCallback(_add_file, name)
                 def add_db_entry(filenode):
                     filecap = filenode.get_uri()
                     s = os.stat(path)
@@ -230,7 +214,7 @@ class DropUploader(service.MultiService):
                 return None
             else:
                 self._log("operator ERROR: non-directory/non-regular file not being processed.")
-                return failure.Failure()
+                return Failure()
 
         d.addCallback(_maybe_upload)
 
diff --git a/src/allmydata/test/test_client.py b/src/allmydata/test/test_client.py
index 01cfc40f..11b22d71 100644
--- a/src/allmydata/test/test_client.py
+++ b/src/allmydata/test/test_client.py
@@ -303,19 +303,20 @@ class Basic(testutil.ReallyEqualMixin, unittest.TestCase):
         class MockDropUploader(service.MultiService):
             name = 'drop-upload'
 
-            def __init__(self, client, upload_dircap, local_dir_utf8, dbfile, inotify=None,
+            def __init__(self, client, upload_dircap, local_dir, dbfile, inotify=None,
                          pending_delay=1.0):
                 service.MultiService.__init__(self)
                 self.client = client
                 self.upload_dircap = upload_dircap
-                self.local_dir_utf8 = local_dir_utf8
+                self.local_dir = local_dir
                 self.dbfile = dbfile
                 self.inotify = inotify
 
         mock_drop_uploader.side_effect = MockDropUploader
 
         upload_dircap = "URI:DIR2:blah"
-        local_dir_utf8 = u"loc\u0101l_dir".encode('utf-8')
+        local_dir_u = u"loc\u0101l_dir"
+        local_dir_utf8 = local_dir_u.encode('utf-8')
         config = (BASECONFIG +
                   "[storage]\n" +
                   "enabled = false\n" +
@@ -343,7 +344,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)
 
diff --git a/src/allmydata/test/test_drop_upload.py b/src/allmydata/test/test_drop_upload.py
index 348d53c1..9ff75a51 100644
--- a/src/allmydata/test/test_drop_upload.py
+++ b/src/allmydata/test/test_drop_upload.py
@@ -5,7 +5,6 @@ from twisted.trial import unittest
 from twisted.python import runtime
 from twisted.python.filepath import FilePath
 from twisted.internet import defer
-from twisted.application import service
 
 from allmydata.interfaces import IDirectoryNode, NoSuchChildError
 from allmydata.util import fileutil, fake_inotify
@@ -62,7 +61,7 @@ class DropUploadTestMixin(GridTestMixin, ShouldFailMixin, ReallyEqualMixin, NonA
     def _test_db_basic(self):
         fileutil.make_dirs(self.basedir)
         dbfile = os.path.join(self.basedir, "dbfile")
-        bdb = self._createdb(dbfile)
+        self._createdb(dbfile)
 
     def _test_db_persistence(self):
         """Test that a file upload creates an entry in the database.
diff --git a/src/allmydata/util/encodingutil.py b/src/allmydata/util/encodingutil.py
index d14b08f6..ae54afb9 100644
--- a/src/allmydata/util/encodingutil.py
+++ b/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():
     """