implement 'delete' functionality, with tests

src/allmydata/frontends/magic_folder.py

@@ -1,6 +1,7 @@
 
 import sys, os
 import os.path
+import shutil
 from collections import deque
 import time
 
@@ -321,8 +322,11 @@ class Uploader(QueueMixin):
                 current_version = self._db.get_local_file_version(relpath_u)
                 if current_version is None:
                     new_version = 0
-                else:
+                elif self._db.is_new_file(pathinfo, relpath_u):
                     new_version = current_version + 1
+                else:
+                    self._log("ignoring {}".format(relpath_u))
+                    return
 
                 metadata = { 'version': new_version,
                              'deleted': True,
@@ -646,7 +650,10 @@ class Downloader(QueueMixin, WriteFileMixin):
             d.addCallback(lambda ign: abspath_u)
         else:
             d.addCallback(lambda ign: file_node.download_best_version())
-            d.addCallback(lambda contents: self._write_downloaded_file(abspath_u, contents, is_conflict=False))
+            if metadata.get('deleted', False):
+                d.addCallback(lambda result: self._unlink_deleted_file(abspath_u, result))
+            else:
+                d.addCallback(lambda contents: self._write_downloaded_file(abspath_u, contents, is_conflict=False))
 
         def do_update_db(written_abspath_u):
             filecap = file_node.get_uri()
@@ -654,7 +661,7 @@ class Downloader(QueueMixin, WriteFileMixin):
             last_downloaded_uri = filecap
             last_downloaded_timestamp = now
             written_pathinfo = get_pathinfo(written_abspath_u)
-            if not written_pathinfo.exists:
+            if not written_pathinfo.exists and not metadata.get('deleted', False):
                 raise Exception("downloaded object %s disappeared" % quote_local_unicode_path(written_abspath_u))
 
             self._db.did_upload_version(relpath_u, metadata['version'], last_uploaded_uri,
@@ -670,3 +677,11 @@ class Downloader(QueueMixin, WriteFileMixin):
             return res
         d.addBoth(remove_from_pending)
         return d
+
+    def _unlink_deleted_file(self, abspath_u, result):
+        try:
+            self._log('unlinking: %s' % (abspath_u,))
+            shutil.move(abspath_u, abspath_u + '.backup')
+        except IOError:
+            self._log("Already gone: '%s'" % (abspath_u,))
+        return abspath_u

src/allmydata/magicfolderdb.py

@@ -135,4 +135,6 @@ class MagicFolderDB(object):
         row = self.cursor.fetchone()
         if not row:
             return True
+        if not pathinfo.exists and row[0] is None:
+            return False
         return (pathinfo.size, pathinfo.mtime, pathinfo.ctime) != row

src/allmydata/test/check_magicfolder_smoke.py

@@ -261,8 +261,7 @@ if True:
             break
         time.sleep(1)
 
-    # XXX this doesn't work; shouldn't a .tmp file appear on bob's side?
-    bob_tmp = bob_foo + '.tmp'
+    bob_tmp = bob_foo + '.backup'
     print("Waiting for '%s' to appear" % (bob_tmp,))
     while True:
         if exists(bob_tmp):

src/allmydata/test/test_magic_folder.py

@@ -241,6 +241,331 @@ class MagicFolderTestMixin(MagicFolderCLITestMixin, ShouldFailMixin, ReallyEqual
         d.addBoth(self.cleanup)
         return d
 
+    @defer.inlineCallbacks
+    def test_delete(self):
+        self.set_up_grid()
+        self.local_dir = os.path.join(self.basedir, u"local_dir")
+        self.mkdir_nonascii(self.local_dir)
+
+        yield self.create_invite_join_magic_folder(u"Alice\u0101", self.local_dir)
+        yield self._restart_client(None)
+
+        try:
+            # create a file
+            up_proc = self.magicfolder.uploader.set_hook('processed')
+            # down_proc = self.magicfolder.downloader.set_hook('processed')
+            path = os.path.join(self.local_dir, u'foo')
+            with open(path, 'w') as f:
+                f.write('foo\n')
+            self.notify(to_filepath(path), self.inotify.IN_CLOSE_WRITE)
+            yield up_proc
+            self.assertTrue(os.path.exists(path))
+
+            # the real test part: delete the file
+            up_proc = self.magicfolder.uploader.set_hook('processed')
+            os.unlink(path)
+            self.notify(to_filepath(path), self.inotify.IN_DELETE)
+            yield up_proc
+            self.assertFalse(os.path.exists(path))
+
+            # ensure we still have a DB entry, and that the version is 1
+            node, metadata = yield self.magicfolder.downloader._get_collective_latest_file(u'foo')
+            self.assertTrue(node is not None, "Failed to find '{}' in DMD".format(path))
+            self.failUnlessEqual(metadata['version'], 1)
+
+        finally:
+            yield self.cleanup(None)
+
+    @defer.inlineCallbacks
+    def test_delete_and_restore(self):
+        self.set_up_grid()
+        self.local_dir = os.path.join(self.basedir, u"local_dir")
+        self.mkdir_nonascii(self.local_dir)
+
+        yield self.create_invite_join_magic_folder(u"Alice\u0101", self.local_dir)
+        yield self._restart_client(None)
+
+        try:
+            # create a file
+            up_proc = self.magicfolder.uploader.set_hook('processed')
+            # down_proc = self.magicfolder.downloader.set_hook('processed')
+            path = os.path.join(self.local_dir, u'foo')
+            with open(path, 'w') as f:
+                f.write('foo\n')
+            self.notify(to_filepath(path), self.inotify.IN_CLOSE_WRITE)
+            yield up_proc
+            self.assertTrue(os.path.exists(path))
+
+            # delete the file
+            up_proc = self.magicfolder.uploader.set_hook('processed')
+            os.unlink(path)
+            self.notify(to_filepath(path), self.inotify.IN_DELETE)
+            yield up_proc
+            self.assertFalse(os.path.exists(path))
+
+            # ensure we still have a DB entry, and that the version is 1
+            node, metadata = yield self.magicfolder.downloader._get_collective_latest_file(u'foo')
+            self.assertTrue(node is not None, "Failed to find '{}' in DMD".format(path))
+            self.failUnlessEqual(metadata['version'], 1)
+
+            # restore the file, with different contents
+            up_proc = self.magicfolder.uploader.set_hook('processed')
+            path = os.path.join(self.local_dir, u'foo')
+            with open(path, 'w') as f:
+                f.write('bar\n')
+            self.notify(to_filepath(path), self.inotify.IN_CLOSE_WRITE)
+            yield up_proc
+
+            # ensure we still have a DB entry, and that the version is 2
+            node, metadata = yield self.magicfolder.downloader._get_collective_latest_file(u'foo')
+            self.assertTrue(node is not None, "Failed to find '{}' in DMD".format(path))
+            self.failUnlessEqual(metadata['version'], 2)
+
+        finally:
+            yield self.cleanup(None)
+
+
+    @defer.inlineCallbacks
+    def test_alice_delete_bob_restore(self):
+        alice_clock = task.Clock()
+        bob_clock = task.Clock()
+        yield self.setup_alice_and_bob(alice_clock, bob_clock)
+        alice_dir = self.alice_magicfolder.uploader._local_path_u
+        bob_dir = self.bob_magicfolder.uploader._local_path_u
+        alice_fname = os.path.join(alice_dir, 'blam')
+        bob_fname = os.path.join(bob_dir, 'blam')
+
+        try:
+            # alice creates a file, bob downloads it
+            alice_proc = self.alice_magicfolder.uploader.set_hook('processed')
+            bob_proc = self.bob_magicfolder.downloader.set_hook('processed')
+
+            with open(alice_fname, 'wb') as f:
+                f.write('contents0\n')
+            self.notify(to_filepath(alice_fname), self.inotify.IN_CLOSE_WRITE, magic=self.alice_magicfolder)
+
+            alice_clock.advance(0)
+            yield alice_proc  # alice uploads
+
+            bob_clock.advance(0)
+            yield bob_proc    # bob downloads
+
+            # check the state
+            yield self._check_version_in_dmd(self.alice_magicfolder, u"blam", 1)
+            yield self._check_version_in_local_db(self.alice_magicfolder, u"blam", 0)
+            yield self._check_version_in_dmd(self.bob_magicfolder, u"blam", 1)
+            yield self._check_version_in_local_db(self.bob_magicfolder, u"blam", 0)
+            yield self.failUnlessReallyEqual(
+                self._get_count('downloader.objects_failed', client=self.bob_magicfolder._client),
+                0
+            )
+            yield self.failUnlessReallyEqual(
+                self._get_count('downloader.objects_downloaded', client=self.bob_magicfolder._client),
+                1
+            )
+
+            print("BOB DELETE")
+            # now bob deletes it (bob should upload, alice download)
+            bob_proc = self.bob_magicfolder.uploader.set_hook('processed')
+            alice_proc = self.alice_magicfolder.downloader.set_hook('processed')
+            os.unlink(bob_fname)
+            self.notify(to_filepath(bob_fname), self.inotify.IN_DELETE, magic=self.bob_magicfolder)
+
+            bob_clock.advance(0)
+            yield bob_proc
+            alice_clock.advance(0)
+            yield alice_proc
+
+            # check versions
+            node, metadata = yield self.alice_magicfolder.downloader._get_collective_latest_file(u'blam')
+            self.assertTrue(metadata['deleted'])
+            yield self._check_version_in_dmd(self.bob_magicfolder, u"blam", 1)
+            yield self._check_version_in_local_db(self.bob_magicfolder, u"blam", 1)
+            yield self._check_version_in_dmd(self.alice_magicfolder, u"blam", 1)
+            yield self._check_version_in_local_db(self.alice_magicfolder, u"blam", 1)
+
+            print("ALICE RESTORE")
+            # now alice restores it (alice should upload, bob download)
+            alice_proc = self.alice_magicfolder.uploader.set_hook('processed')
+            bob_proc = self.bob_magicfolder.downloader.set_hook('processed')
+            with open(alice_fname, 'wb') as f:
+                f.write('new contents\n')
+            self.notify(to_filepath(alice_fname), self.inotify.IN_CLOSE_WRITE, magic=self.alice_magicfolder)
+
+            alice_clock.advance(0)
+            yield alice_proc
+            bob_clock.advance(0)
+            yield bob_proc
+
+            # check versions
+            node, metadata = yield self.alice_magicfolder.downloader._get_collective_latest_file(u'blam')
+            self.assertTrue('deleted' not in metadata or not metadata['deleted'])
+            yield self._check_version_in_dmd(self.bob_magicfolder, u"blam", 2)
+            yield self._check_version_in_local_db(self.bob_magicfolder, u"blam", 2)
+            yield self._check_version_in_dmd(self.alice_magicfolder, u"blam", 2)
+            yield self._check_version_in_local_db(self.alice_magicfolder, u"blam", 2)
+
+        finally:
+            # cleanup
+            d0 = self.alice_magicfolder.finish()
+            alice_clock.advance(0)
+            yield d0
+
+            d1 = self.bob_magicfolder.finish()
+            bob_clock.advance(0)
+            yield d1
+
+    @defer.inlineCallbacks
+    def test_alice_create_bob_update(self):
+        alice_clock = task.Clock()
+        bob_clock = task.Clock()
+        caps = yield self.setup_alice_and_bob(alice_clock, bob_clock)
+        alice_dir = self.alice_magicfolder.uploader._local_path_u
+        bob_dir = self.bob_magicfolder.uploader._local_path_u
+        alice_fname = os.path.join(alice_dir, 'blam')
+        bob_fname = os.path.join(bob_dir, 'blam')
+
+        try:
+            # alice creates a file, bob downloads it
+            alice_proc = self.alice_magicfolder.uploader.set_hook('processed')
+            bob_proc = self.bob_magicfolder.downloader.set_hook('processed')
+
+            with open(alice_fname, 'wb') as f:
+                f.write('contents0\n')
+            self.notify(to_filepath(alice_fname), self.inotify.IN_CLOSE_WRITE, magic=self.alice_magicfolder)
+
+            alice_clock.advance(0)
+            yield alice_proc  # alice uploads
+
+            bob_clock.advance(0)
+            yield bob_proc    # bob downloads
+
+            # check the state
+            yield self._check_version_in_dmd(self.alice_magicfolder, u"blam", 1)
+            yield self._check_version_in_local_db(self.alice_magicfolder, u"blam", 0)
+            yield self._check_version_in_dmd(self.bob_magicfolder, u"blam", 1)
+            yield self._check_version_in_local_db(self.bob_magicfolder, u"blam", 0)
+            yield self.failUnlessReallyEqual(
+                self._get_count('downloader.objects_failed', client=self.bob_magicfolder._client),
+                0
+            )
+            yield self.failUnlessReallyEqual(
+                self._get_count('downloader.objects_downloaded', client=self.bob_magicfolder._client),
+                1
+            )
+
+            # now bob updates it (bob should upload, alice download)
+            bob_proc = self.bob_magicfolder.uploader.set_hook('processed')
+            alice_proc = self.alice_magicfolder.downloader.set_hook('processed')
+            with open(bob_fname, 'wb') as f:
+                f.write('bob wuz here\n')
+            self.notify(to_filepath(bob_fname), self.inotify.IN_CLOSE_WRITE, magic=self.bob_magicfolder)
+
+            bob_clock.advance(0)
+            yield bob_proc
+            alice_clock.advance(0)
+            yield alice_proc
+
+            # check the state
+            yield self._check_version_in_dmd(self.bob_magicfolder, u"blam", 1)
+            yield self._check_version_in_local_db(self.bob_magicfolder, u"blam", 1)
+            yield self._check_version_in_dmd(self.alice_magicfolder, u"blam", 1)
+            yield self._check_version_in_local_db(self.alice_magicfolder, u"blam", 1)
+
+        finally:
+            # cleanup
+            d0 = self.alice_magicfolder.finish()
+            alice_clock.advance(0)
+            yield d0
+
+            d1 = self.bob_magicfolder.finish()
+            bob_clock.advance(0)
+            yield d1
+
+    @defer.inlineCallbacks
+    def test_alice_delete_and_restore(self):
+        alice_clock = task.Clock()
+        bob_clock = task.Clock()
+        yield self.setup_alice_and_bob(alice_clock, bob_clock)
+        alice_dir = self.alice_magicfolder.uploader._local_path_u
+        bob_dir = self.bob_magicfolder.uploader._local_path_u
+        alice_fname = os.path.join(alice_dir, 'blam')
+        bob_fname = os.path.join(bob_dir, 'blam')
+
+        try:
+            # alice creates a file, bob downloads it
+            alice_proc = self.alice_magicfolder.uploader.set_hook('processed')
+            bob_proc = self.bob_magicfolder.downloader.set_hook('processed')
+
+            with open(alice_fname, 'wb') as f:
+                f.write('contents0\n')
+            self.notify(to_filepath(alice_fname), self.inotify.IN_CLOSE_WRITE, magic=self.alice_magicfolder)
+
+            alice_clock.advance(0)
+            yield alice_proc  # alice uploads
+
+            bob_clock.advance(0)
+            yield bob_proc    # bob downloads
+
+            # check the state
+            yield self._check_version_in_dmd(self.alice_magicfolder, u"blam", 1)
+            yield self._check_version_in_local_db(self.alice_magicfolder, u"blam", 0)
+            yield self._check_version_in_dmd(self.bob_magicfolder, u"blam", 1)
+            yield self._check_version_in_local_db(self.bob_magicfolder, u"blam", 0)
+            yield self.failUnlessReallyEqual(
+                self._get_count('downloader.objects_failed', client=self.bob_magicfolder._client),
+                0
+            )
+            yield self.failUnlessReallyEqual(
+                self._get_count('downloader.objects_downloaded', client=self.bob_magicfolder._client),
+                1
+            )
+
+            # now alice deletes it (alice should upload, bob download)
+            alice_proc = self.alice_magicfolder.uploader.set_hook('processed')
+            bob_proc = self.bob_magicfolder.downloader.set_hook('processed')
+            os.unlink(alice_fname)
+            self.notify(to_filepath(alice_fname), self.inotify.IN_DELETE, magic=self.alice_magicfolder)
+
+            alice_clock.advance(0)
+            yield alice_proc
+            bob_clock.advance(0)
+            yield bob_proc
+
+            # check the state
+            yield self._check_version_in_dmd(self.bob_magicfolder, u"blam", 1)
+            yield self._check_version_in_local_db(self.bob_magicfolder, u"blam", 1)
+            yield self._check_version_in_dmd(self.alice_magicfolder, u"blam", 1)
+            yield self._check_version_in_local_db(self.alice_magicfolder, u"blam", 1)
+
+            # now alice restores the file (with new contents)
+            alice_proc = self.alice_magicfolder.uploader.set_hook('processed')
+            bob_proc = self.bob_magicfolder.downloader.set_hook('processed')
+            with open(alice_fname, 'wb') as f:
+                f.write('alice wuz here\n')
+            self.notify(to_filepath(alice_fname), self.inotify.IN_CLOSE_WRITE, magic=self.alice_magicfolder)
+
+            alice_clock.advance(0)
+            yield alice_proc
+            bob_clock.advance(0)
+            yield bob_proc
+
+            # check the state
+            yield self._check_version_in_dmd(self.bob_magicfolder, u"blam", 2)
+            yield self._check_version_in_local_db(self.bob_magicfolder, u"blam", 2)
+            yield self._check_version_in_dmd(self.alice_magicfolder, u"blam", 2)
+            yield self._check_version_in_local_db(self.alice_magicfolder, u"blam", 2)
+
+        finally:
+            # cleanup
+            d0 = self.alice_magicfolder.finish()
+            alice_clock.advance(0)
+            yield d0
+
+            d1 = self.bob_magicfolder.finish()
+            bob_clock.advance(0)
+            yield d1
+
     def test_magic_folder(self):
         self.set_up_grid()
         self.local_dir = os.path.join(self.basedir, self.unicode_or_fallback(u"loc\u0101l_dir", u"local_dir"))
@@ -336,6 +661,10 @@ class MagicFolderTestMixin(MagicFolderCLITestMixin, ShouldFailMixin, ReallyEqual
         #print "_check_version_in_local_db: %r has version %s" % (relpath_u, version)
         self.failUnlessEqual(version, expected_version)
 
+    def _check_file_gone(self, magicfolder, relpath_u):
+        path = os.path.join(magicfolder.uploader._local_path_u, relpath_u)
+        self.assertTrue(not os.path.exists(path))
+
     def test_alice_bob(self):
         alice_clock = task.Clock()
         bob_clock = task.Clock()
@@ -396,6 +725,7 @@ class MagicFolderTestMixin(MagicFolderCLITestMixin, ShouldFailMixin, ReallyEqual
 
         d.addCallback(lambda ign: self._check_version_in_local_db(self.bob_magicfolder, u"file1", 1))
         d.addCallback(lambda ign: self._check_version_in_dmd(self.bob_magicfolder, u"file1", 1))
+        d.addCallback(lambda ign: self._check_file_gone(self.bob_magicfolder, u"file1"))
         d.addCallback(_check_downloader_count, 'objects_failed', 0)
         d.addCallback(_check_downloader_count, 'objects_downloaded', 2)
 
@@ -466,8 +796,10 @@ class MockTest(MagicFolderTestMixin, unittest.TestCase):
         self.inotify = fake_inotify
         self.patch(magic_folder, 'get_inotify_module', lambda: self.inotify)
 
-    def notify(self, path, mask):
-        self.magicfolder.uploader._notifier.event(path, mask)
+    def notify(self, path, mask, magic=None):
+        if magic is None:
+            magic = self.magicfolder
+        magic.uploader._notifier.event(path, mask)
 
     def test_errors(self):
         self.set_up_grid()
@@ -554,7 +886,7 @@ class RealTest(MagicFolderTestMixin, unittest.TestCase):
         MagicFolderTestMixin.setUp(self)
         self.inotify = magic_folder.get_inotify_module()
 
-    def notify(self, path, mask):
+    def notify(self, path, mask, **kw):
         # Writing to the filesystem causes the notification.
         pass