Refactoring to allow logging from _write_downloaded_file and _rename_conflicted_file.

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

src/allmydata/frontends/magic_folder.py

@@ -400,7 +400,54 @@ class Uploader(QueueMixin):
         return d
 
 
-class Downloader(QueueMixin):
+class WriteFileMixin(object):
+    FUDGE_SECONDS = 10.0
+
+    def _write_downloaded_file(self, abspath_u, file_contents, is_conflict=False, now=None):
+        self._log("_write_downloaded_file(%r, <%d bytes>, is_conflict=%r, now=%r)"
+                  % (abspath_u, len(file_contents), is_conflict, now))
+
+        # 1. Write a temporary file, say .foo.tmp.
+        # 2. is_conflict determines whether this is an overwrite or a conflict.
+        # 3. Set the mtime of the replacement file to be T seconds before the
+        #    current local time.
+        # 4. Perform a file replacement with backup filename foo.backup,
+        #    replaced file foo, and replacement file .foo.tmp. If any step of
+        #    this operation fails, reclassify as a conflict and stop.
+        #
+        # Returns the path of the destination file.
+
+        precondition_abspath(abspath_u)
+        replacement_path_u = abspath_u + u".tmp"  # FIXME more unique
+        backup_path_u = abspath_u + u".backup"
+        if now is None:
+            now = time.time()
+
+        # ensure parent directory exists
+        head, tail = os.path.split(abspath_u)
+        mode = 0777 # XXX
+        fileutil.make_dirs(head, mode)
+
+        fileutil.write(replacement_path_u, file_contents)
+        os.utime(replacement_path_u, (now, now - self.FUDGE_SECONDS))
+        if is_conflict:
+            return self._rename_conflicted_file(abspath_u, replacement_path_u)
+        else:
+            try:
+                fileutil.replace_file(abspath_u, replacement_path_u, backup_path_u)
+                return abspath_u
+            except fileutil.ConflictError:
+                return self._rename_conflicted_file(abspath_u, replacement_path_u)
+
+    def _rename_conflicted_file(self, abspath_u, replacement_path_u):
+        self._log("_rename_conflicted_file(%r, %r)" % (abspath_u, replacement_path_u))
+
+        conflict_path_u = abspath_u + u".conflict"
+        fileutil.rename_no_overwrite(replacement_path_u, conflict_path_u)
+        return conflict_path_u
+
+
+class Downloader(QueueMixin, WriteFileMixin):
     REMOTE_SCAN_INTERVAL = 3  # facilitates tests
 
     def __init__(self, client, local_path_u, db, collective_dircap, clock):
@@ -589,45 +636,3 @@ class Downloader(QueueMixin):
             return res
         d.addBoth(remove_from_pending)
         return d
-
-    FUDGE_SECONDS = 10.0
-
-    @classmethod
-    def _write_downloaded_file(cls, abspath_u, file_contents, is_conflict=False, now=None):
-        # 1. Write a temporary file, say .foo.tmp.
-        # 2. is_conflict determines whether this is an overwrite or a conflict.
-        # 3. Set the mtime of the replacement file to be T seconds before the
-        #    current local time.
-        # 4. Perform a file replacement with backup filename foo.backup,
-        #    replaced file foo, and replacement file .foo.tmp. If any step of
-        #    this operation fails, reclassify as a conflict and stop.
-        #
-        # Returns the path of the destination file.
-
-        precondition_abspath(abspath_u)
-        replacement_path_u = abspath_u + u".tmp"  # FIXME more unique
-        backup_path_u = abspath_u + u".backup"
-        if now is None:
-            now = time.time()
-
-        # ensure parent directory exists
-        head, tail = os.path.split(abspath_u)
-        mode = 0777 # XXX
-        fileutil.make_dirs(head, mode)
-
-        fileutil.write(replacement_path_u, file_contents)
-        os.utime(replacement_path_u, (now, now - cls.FUDGE_SECONDS))
-        if is_conflict:
-            return cls._rename_conflicted_file(abspath_u, replacement_path_u)
-        else:
-            try:
-                fileutil.replace_file(abspath_u, replacement_path_u, backup_path_u)
-                return abspath_u
-            except fileutil.ConflictError:
-                return cls._rename_conflicted_file(abspath_u, replacement_path_u)
-
-    @classmethod
-    def _rename_conflicted_file(self, abspath_u, replacement_path_u):
-        conflict_path_u = abspath_u + u".conflict"
-        fileutil.rename_no_overwrite(replacement_path_u, conflict_path_u)
-        return conflict_path_u

src/allmydata/test/test_magic_folder.py

@@ -16,7 +16,7 @@ from allmydata.test.common import ShouldFailMixin
 from .test_cli_magic_folder import MagicFolderCLITestMixin
 
 from allmydata.frontends import magic_folder
-from allmydata.frontends.magic_folder import MagicFolder, Downloader
+from allmydata.frontends.magic_folder import MagicFolder, Downloader, WriteFileMixin
 from allmydata import magicfolderdb, magicpath
 from allmydata.util.fileutil import abspath_expanduser_unicode
 
@@ -482,13 +482,19 @@ class MockTest(MagicFolderTestMixin, unittest.TestCase):
         workdir = u"cli/MagicFolder/write-downloaded-file"
         local_file = fileutil.abspath_expanduser_unicode(os.path.join(workdir, "foobar"))
 
+        class TestWriteFileMixin(WriteFileMixin):
+            def _log(self, msg):
+                pass
+
+        writefile = TestWriteFileMixin()
+
         # create a file with name "foobar" with content "foo"
         # write downloaded file content "bar" into "foobar" with is_conflict = False
         fileutil.make_dirs(workdir)
         fileutil.write(local_file, "foo")
 
         # if is_conflict is False, then the .conflict file shouldn't exist.
-        Downloader._write_downloaded_file(local_file, "bar", False, None)
+        writefile._write_downloaded_file(local_file, "bar", False, None)
         conflicted_path = local_file + u".conflict"
         self.failIf(os.path.exists(conflicted_path))
 
@@ -504,7 +510,7 @@ class MockTest(MagicFolderTestMixin, unittest.TestCase):
         self.failUnlessEqual(fileutil.read(local_file), "bar")
 
         # now a test for conflicted case
-        Downloader._write_downloaded_file(local_file, "bar", True, None)
+        writefile._write_downloaded_file(local_file, "bar", True, None)
         self.failUnless(os.path.exists(conflicted_path))
 
         # .tmp file shouldn't exist