Merge lp:~mandel/ubuntuone-client/add-watch-deferred into lp:ubuntuone-client

[Manuel de la Peña]

Changed add_watch to return a deferred so that we will be waiting for a successful callback before we assume that the watch is running.

Changed add_watch to return a deferred so that we will be waiting for a successful callback before we assume that the watch is running.

[lp:~mandel/ubuntuone-client/add-watch-deferred]

1117. By Manuel de la Peña on 2011-07-29

Merged fix-fsm into add-watch-deferred.

1118. By Manuel de la Peña on 2011-07-29

Merged fix-fsm into add-watch-deferred.

1119. By Manuel de la Peña on 2011-07-29

Merged fix-fsm into add-watch-deferred.

[Natalia Bidart (nataliabidart)]

Looks good!

review: Approve

[Alejandro J. Cura (alecu)]

Code looks fine, and all tests pass!

review: Approve

=== modified file 'tests/platform/test_filesystem_notifications.py'
--- tests/platform/test_filesystem_notifications.py	2011-07-28 14:51:42 +0000
+++ tests/platform/test_filesystem_notifications.py	2011-07-29 17:35:26 +0000
@@ -239,6 +239,7 @@
 
     @skipIfOS('win32', 'There is no reasonable way to know when a file is '
               + 'opened on windows.')
+    @defer.inlineCallbacks
     def test_file_open(self):
         """Test receiving the open signal on files."""
         testfile = os.path.join(self.root_dir, "foo")
@@ -246,15 +247,17 @@
         self.monitor.add_to_mute_filter("FS_FILE_OPEN", path=testfile)
         self.monitor.add_to_mute_filter("FS_FILE_CLOSE_NOWRITE", path=testfile)
         self.assertEqual(self._how_many_muted(), 2)
-        self.monitor.add_watch(self.root_dir)
+        yield self.monitor.add_watch(self.root_dir)
 
         # generate the event
         open(testfile)
         reactor.callLater(self.timeout - 0.2, self.check_filter)
-        return self._deferred
+        test_result = yield self._deferred
+        defer.returnValue(test_result)
 
     @skipIfOS('win32', 'There is no reasonable way to know when a file was '
               'opened on windows')
+    @defer.inlineCallbacks
     def test_file_close_nowrite(self):
         """Test receiving the close_nowrite signal on files."""
         testfile = os.path.join(self.root_dir, "foo")
@@ -262,13 +265,15 @@
         fh = open(testfile)
         self.monitor.add_to_mute_filter("FS_FILE_CLOSE_NOWRITE", path=testfile)
         self.assertEqual(self._how_many_muted(), 1)
-        self.monitor.add_watch(self.root_dir)
+        yield self.monitor.add_watch(self.root_dir)
 
         # generate the event
         fh.close()
         reactor.callLater(self.timeout - 0.2, self.check_filter)
-        return self._deferred
+        test_result = yield self._deferred
+        defer.returnValue(test_result)
 
+    @defer.inlineCallbacks
     def test_file_create_close_write(self):
         """Test receiving the create and close_write signals on files."""
         testfile = os.path.join(self.root_dir, "foo")
@@ -276,53 +281,61 @@
         self.monitor.add_to_mute_filter("FS_FILE_OPEN", path=testfile)
         self.monitor.add_to_mute_filter("FS_FILE_CLOSE_WRITE", path=testfile)
         self.assertEqual(self._how_many_muted(), 3)
-        self.monitor.add_watch(self.root_dir)
+        yield self.monitor.add_watch(self.root_dir)
 
         # generate the event
         fd = open(testfile, "w")
         fd.write('test')
         fd.close()
         reactor.callLater(self.timeout - 0.2, self.check_filter)
-        return self._deferred
+        test_result = yield self._deferred
+        defer.returnValue(test_result)
 
+    @defer.inlineCallbacks
     def test_dir_create(self):
         """Test receiving the create signal on dirs."""
         testdir = os.path.join(self.root_dir, "foo")
         self.monitor.add_to_mute_filter("FS_DIR_CREATE", path=testdir)
         self.assertEqual(self._how_many_muted(), 1)
-        self.monitor.add_watch(self.root_dir)
+        yield self.monitor.add_watch(self.root_dir)
 
         # generate the event
         os.mkdir(testdir)
         reactor.callLater(self.timeout - 0.2, self.check_filter)
-        return self._deferred
+        test_result = yield self._deferred
+        defer.returnValue(test_result)
 
+    @defer.inlineCallbacks
     def test_file_delete(self):
         """Test the delete signal on a file."""
         testfile = os.path.join(self.root_dir, "foo")
         open(testfile, "w").close()
         self.monitor.add_to_mute_filter("FS_FILE_DELETE", path=testfile)
         self.assertEqual(self._how_many_muted(), 1)
-        self.monitor.add_watch(self.root_dir)
+        yield self.monitor.add_watch(self.root_dir)
 
         # generate the event
         os.remove(testfile)
         reactor.callLater(self.timeout - 0.2, self.check_filter)
-        return self._deferred
+        test_result = yield self._deferred
+        defer.returnValue(test_result)
 
+    @defer.inlineCallbacks
     def test_dir_delete(self):
         """Test the delete signal on a dir."""
         testdir = os.path.join(self.root_dir, "foo")
         os.mkdir(testdir)
         self.monitor.add_to_mute_filter("FS_DIR_DELETE", path=testdir)
         self.assertEqual(self._how_many_muted(), 1)
-        self.monitor.add_watch(self.root_dir)
+        yield self.monitor.add_watch(self.root_dir)
 
         # generate the event
         os.rmdir(testdir)
         reactor.callLater(self.timeout - 0.2, self.check_filter)
-        return self._deferred
+        test_result = yield self._deferred
+        defer.returnValue(test_result)
 
+    @defer.inlineCallbacks
     def test_file_moved_inside(self):
         """Test the synthesis of the FILE_MOVE event."""
         fromfile = os.path.join(self.root_dir, "foo")
@@ -335,13 +348,15 @@
         self.monitor.add_to_mute_filter("FS_FILE_MOVE",
                                         path_from=fromfile, path_to=tofile)
         self.assertEqual(self._how_many_muted(), 1)
-        self.monitor.add_watch(self.root_dir)
+        yield self.monitor.add_watch(self.root_dir)
 
         # generate the event
         os.rename(fromfile, tofile)
         reactor.callLater(self.timeout - 0.2, self.check_filter)
-        return self._deferred
+        test_result = yield self._deferred
+        defer.returnValue(test_result)
 
+    @defer.inlineCallbacks
     def test_dir_moved_inside(self):
         """Test the synthesis of the DIR_MOVE event."""
         fromdir = os.path.join(self.root_dir, "foo")
@@ -354,13 +369,15 @@
         self.monitor.add_to_mute_filter("FS_DIR_MOVE",
                                         path_from=fromdir, path_to=todir)
         self.assertEqual(self._how_many_muted(), 1)
-        self.monitor.add_watch(self.root_dir)
+        yield self.monitor.add_watch(self.root_dir)
 
         # generate the event
         os.rename(fromdir, todir)
         reactor.callLater(self.timeout - 0.2, self.check_filter)
-        return self._deferred
+        test_result = yield self._deferred
+        defer.returnValue(test_result)
 
+    @defer.inlineCallbacks
     def test_file_moved_from_conflict(self):
         """Test the handling of the FILE_MOVE event when source is conflict."""
         fromfile = os.path.join(self.root_dir, "foo.u1conflict")
@@ -373,13 +390,15 @@
         self.monitor.add_to_mute_filter("FS_FILE_MOVE",
                                         path_from=fromfile, path_to=tofile)
         self.assertEqual(self._how_many_muted(), 2)
-        self.monitor.add_watch(self.root_dir)
+        yield self.monitor.add_watch(self.root_dir)
 
         # generate the event
         os.rename(fromfile, tofile)
         reactor.callLater(self.timeout - 0.2, self.check_filter)
-        return self._deferred
+        test_result = yield self._deferred
+        defer.returnValue(test_result)
 
+    @defer.inlineCallbacks
     def test_file_moved_from_partial(self):
         """Test the handling of the FILE_MOVE event when source is partial."""
         fromfile = os.path.join(self.root_dir, "mdid.u1partial.foo")
@@ -390,9 +409,10 @@
         self.monitor.add_to_mute_filter("FS_FILE_CREATE", path=tofile)
         self.monitor.add_to_mute_filter("FS_FILE_CLOSE_WRITE", path=tofile)
         self.assertEqual(self._how_many_muted(), 2)
-        self.monitor.add_watch(root_dir)
+        yield self.monitor.add_watch(root_dir)
 
         # generate the event
         os.rename(fromfile, tofile)
         reactor.callLater(self.timeout - 0.2, self.check_filter)
-        return self._deferred
+        test_result = yield self._deferred
+        defer.returnValue(test_result)
 
=== modified file 'tests/syncdaemon/test_vm.py'
--- tests/syncdaemon/test_vm.py	2011-07-28 15:43:05 +0000
+++ tests/syncdaemon/test_vm.py	2011-07-29 17:35:26 +0000
@@ -421,7 +421,7 @@
             self.main.fs.create(path, share.volume_id, is_dir=True)
             self.main.fs.set_node_id(path, 'dir_node_id'+str(i))
             # add a inotify watch to the dir
-            self.main.event_q.add_watch(path)
+            yield self.main.event_q.add_watch(path)
         files = ['a_file', os.path.join('dir', 'file'),
                  os.path.join('dir','subdir','file')]
         for i, file in enumerate(files):
@@ -824,15 +824,17 @@
         self.assertEquals('fake_share_uuid', share.node_id)
         self.assertEquals(None, self.vm.shares.get('share_id_1'))
 
+    @defer.inlineCallbacks
     def test_remove_watch(self):
         """Test for VolumeManager._remove_watch"""
         path = os.path.join(self.root_dir, 'dir')
         make_dir(path, recursive=True)
-        self.main.event_q.add_watch(path)
+        yield self.main.event_q.add_watch(path)
         self.assertIn(path, self.main.event_q.monitor._general_watchs)
         self.vm._remove_watch(path)
         self.assertNotIn(path, self.main.event_q.monitor._general_watchs)
 
+    @defer.inlineCallbacks
     def test_remove_watches(self):
         """Test for VolumeManager._remove_watches"""
         dirs = ['dir', os.path.join('dir', 'subdir'), 'emptydir']
@@ -843,9 +845,9 @@
                 make_dir(path, recursive=True)
             self.main.fs.create(path, "", is_dir=True)
             self.main.fs.set_node_id(path, 'dir_node_id'+str(i))
-            self.main.event_q.add_watch(path)
+            yield self.main.event_q.add_watch(path)
         # insert the root_dir in the list
-        self.main.event_q.add_watch(self.root_dir)
+        yield self.main.event_q.add_watch(self.root_dir)
         paths.insert(0, self.root_dir)
         for path in paths:
             self.assertIn(path, self.main.event_q.monitor._general_watchs)
@@ -854,13 +856,14 @@
         for path in paths:
             self.assertNotIn(path, self.main.event_q.monitor._general_watchs)
 
+    @defer.inlineCallbacks
     def test_remove_watches_after_dir_rename(self):
         """Test for VolumeManager._remove_watches after dir rename."""
         path = os.path.join(self.root_dir, 'testit')
         os.mkdir(path)
         self.main.fs.create(path, "", is_dir=True)
         self.main.fs.set_node_id(path, 'dir_node_id')
-        self.main.event_q.add_watch(path)
+        yield self.main.event_q.add_watch(path)
 
         os.rename(path, path+'.old')
         # remove the watches
@@ -869,13 +872,14 @@
         self.assertNotIn(path, self.main.event_q.monitor._general_watchs,
                          'watch should not be present')
 
+    @defer.inlineCallbacks
     def test_delete_fsm_object(self):
         """Test for VolumeManager._delete_fsm_object"""
         path = os.path.abspath(os.path.join(self.root_dir, 'dir'))
         make_dir(path, recursive=True)
         self.main.fs.create(path, "", is_dir=True)
         self.main.fs.set_node_id(path, 'dir_node_id')
-        self.main.event_q.add_watch(path)
+        yield self.main.event_q.add_watch(path)
         self.assertIn(path, self.main.event_q.monitor._general_watchs)
         self.assertTrue(self.main.fs.get_by_path(path), path)
         # remove the watch
@@ -1082,7 +1086,7 @@
             self.main.fs.create(path, share.volume_id, is_dir=True)
             self.main.fs.set_node_id(path, 'dir_node_id'+str(i))
             # add a inotify watch to the dir
-            self.main.event_q.add_watch(path)
+            yield self.main.event_q.add_watch(path)
         files = ['a_file', os.path.join('dir', 'file'),
                  os.path.join('dir','subdir','file')]
         for i, file in enumerate(files):
@@ -1207,7 +1211,7 @@
         # add a inotify watch to the dirs
         for path, is_dir in paths:
             if is_dir:
-                self.main.event_q.add_watch(path)
+                yield self.main.event_q.add_watch(path)
         self.assertEquals(len(paths), len(dirs+files)+1, paths)
 
         # unsubscribe from it
@@ -1420,7 +1424,7 @@
             self.main.fs.create(path, udf.volume_id, is_dir=True)
             self.main.fs.set_node_id(path, 'dir_node_id'+str(i))
             # add a inotify watch to the dir
-            self.main.event_q.add_watch(path)
+            yield self.main.event_q.add_watch(path)
         files = ['a_file', os.path.join('dir','file'),
                  os.path.join('dir','subdir','file')]
         for i, file in enumerate(files):
@@ -2038,7 +2042,7 @@
             self.main.fs.create(path, udf.volume_id, is_dir=True)
             self.main.fs.set_node_id(path, 'dir_node_id'+str(i))
             # add a inotify watch to the dir
-            self.main.event_q.add_watch(path)
+            yield self.main.event_q.add_watch(path)
         files = ['a_file', os.path.join('dir','file'),
                   os.path.join('dir','subdir','file')]
         for i, file in enumerate(files):
@@ -2095,7 +2099,7 @@
         # add a inotify watch to the dirs
         for path, is_dir in paths:
             if is_dir:
-                self.main.event_q.add_watch(path)
+                yield self.main.event_q.add_watch(path)
         self.assertEquals(len(paths), len(dirs+files)+1, paths)
 
         # unsubscribe from it
 
=== modified file 'ubuntuone/platform/linux/filesystem_notifications.py'
--- ubuntuone/platform/linux/filesystem_notifications.py	2011-07-27 17:10:57 +0000
+++ ubuntuone/platform/linux/filesystem_notifications.py	2011-07-29 17:35:26 +0000
@@ -457,13 +457,13 @@
         # not add it if already there
         if dirpath in w_dict:
             self.log.debug("Watch already there for %r", dirpath)
-            return False
+            return defer.succeed(False)
 
         # add the watch!
         self.log.debug("Adding %s inotify watch to %r", w_type, dirpath)
         result = w_manager.add_watch(dirpath, events)
         w_dict[dirpath] = result[dirpath]
-        return True
+        return defer.succeed(True)
 
     def inotify_watch_fix(self, pathfrom, pathto):
         """Fix the path in inotify structures."""
 
=== modified file 'ubuntuone/platform/windows/filesystem_notifications.py'
--- ubuntuone/platform/windows/filesystem_notifications.py	2011-07-29 17:35:26 +0000
+++ ubuntuone/platform/windows/filesystem_notifications.py	2011-07-29 17:35:26 +0000
@@ -695,8 +695,10 @@
         # is also watching it kids
         if not self._watch_manager.get_wd(dirpath):
             # we need to add a watch which will also watch its kids
-            self._watch_manager.add_watch(dirpath, FILESYSTEM_MONITOR_MASK,
-                auto_add=True)
+            return self._watch_manager.add_watch(dirpath,
+                                                 FILESYSTEM_MONITOR_MASK,
+                                                 auto_add=True)
+        return defer.succeed(True)
 
     def is_frozen(self):
         """Checks if there's something frozen."""
 
=== modified file 'ubuntuone/syncdaemon/local_rescan.py'
--- ubuntuone/syncdaemon/local_rescan.py	2011-07-27 17:10:57 +0000
+++ ubuntuone/syncdaemon/local_rescan.py	2011-07-29 17:35:26 +0000
@@ -235,6 +235,7 @@
             self._process_next_queue(None)
         return self._previous_deferred
 
+    @defer.inlineCallbacks
     def _add_watches_to_udf_ancestors(self, volume):
         """Add a inotify watch to volume's ancestors if it's an UDF."""
         added_watches = []
@@ -252,10 +253,10 @@
             if not access(ancestor):
                 log_info("Tree broken at path: %r", volume.path)
                 revert_watches()
-                return False
+                defer.returnValue(False)
 
             log_debug("Adding watch to UDF's ancestor %r", ancestor)
-            really_added = self.eq.add_watch(ancestor)
+            really_added = yield self.eq.add_watch(ancestor)
             # only note it for the revert if the watch was not there before
             if really_added:
                 added_watches.append(ancestor)
@@ -263,10 +264,10 @@
         # finally, check that UDF is ok in disk
         if not access(volume.path):
             revert_watches()
-            return False
+            defer.returnValue(False)
 
         # all is ok
-        return True
+        defer.returnValue(True)
 
     def _process_next_queue(self, _):
         """Process the next item in the queue, if any."""
@@ -280,13 +281,14 @@
         # more to scan
         scan_info = self._queue.pop()
 
+        @defer.inlineCallbacks
         def safe_scan():
             """Scan safely"""
             try:
                 # add watches to UDF ancestors and check UDF is ok
                 volume = scan_info[0]
                 if isinstance(volume, volume_manager.UDF):
-                    udf_ok = self._add_watches_to_udf_ancestors(volume)
+                    udf_ok = yield self._add_watches_to_udf_ancestors(volume)
                     if not udf_ok:
                         self._process_next_queue(None)
                         return
@@ -615,12 +617,13 @@
                 filesdirs[fname] = is_dir, statinfo, changed
         return filesdirs
 
+    @defer.inlineCallbacks
     def _scan_one_dir(self, scan_info):
         """Gets one dir and compares with fsm."""
         share, dirpath, udf_mode = scan_info
 
         log_debug("Adding watch to %r", dirpath)
-        self.eq.add_watch(dirpath)
+        yield self.eq.add_watch(dirpath)
 
         to_later = []
         self.eq.freeze_begin(dirpath)
@@ -698,4 +701,5 @@
             d.addCallback(filter_delete_events)
         d.addCallback(self.eq.freeze_commit)
         d.addCallback(control)
-        return d
+        result = yield d
+        defer.returnValue(result)
 
=== modified file 'ubuntuone/syncdaemon/volume_manager.py'
--- ubuntuone/syncdaemon/volume_manager.py	2011-07-28 15:43:05 +0000
+++ ubuntuone/syncdaemon/volume_manager.py	2011-07-29 17:35:26 +0000
@@ -943,6 +943,7 @@
             self.shares[share.volume_id] = share
             self.m.event_q.push('VM_SHARE_CHANGED', share_id=share.volume_id)
 
+    @defer.inlineCallbacks
     def _create_share_dir(self, share):
         """ Creates the share root dir, and set the permissions. """
         # XXX: verterok: This is going to be moved into fsm
@@ -953,7 +954,7 @@
         # add the watch after the mkdir
         if share.can_write():
              self.log.debug('adding inotify watch to: %s', share.path)
-             self.m.event_q.add_watch(share.path)
+             yield self.m.event_q.add_watch(share.path)
         # if it's a ro share, change the perms
         if not share.can_write():
             set_dir_readonly(share.path)
@@ -1949,4 +1950,3 @@
     def _serialize(self, value):
         """Serialize value to string using protocol."""
         return cPickle.dumps(value.__dict__, protocol=2)
-