Merge lp:~facundo/ubuntuone-client/plt-fixpath into lp:ubuntuone-client

[Facundo Batista]

Fix the PathLockTree elements on a move operation.

Fix the PathLockTree elements on a move operation.

Tests included.

[Manuel de la Peña (mandel)]

Here is some background for the other reviewers:

<facundobatista> so... the situation is the following, the PathLockTree locks operations on other operations regarding the path
<facundobatista> if you touch a file 'foo', a MakeFile and Upload are queued, the Makefile will be executed, taking the lock on 'foo', the Upload will be locked because of the same path
<facundobatista> when Makefile finishes, it releases the lock, the Upload jumps in, all happy
<facundobatista> so, if you then do something like "ls > foo; mv foo bar"
* pedronis has quit (Ping timeout: 246 seconds)
<facundobatista> syncdaemon will get the FILE_CREATE and CLOSE_WRITE and will queue the Makefile and actually send HQ to hash the file
<facundobatista> the Makefile will be executed, taking the lock on 'foo'
<facundobatista> of course, there's a move really fast after the CREATE and CLOSE
<facundobatista> internal stuff is adjusted because of that move
<facundobatista> milliseconds later, the HQ jumps in, and tries to hash 'foo', it gets an error because the file moved! so after that error the correct path is taken (MOVE adjusted internal stuff!), and finally HQ hashes 'bar'
<facundobatista> when HQ finishes hashing bar, it queues an Upload
<facundobatista> the Upload tries to get the pathlock on 'bar' and it succeeds!!!
<facundobatista> because the Makefile was holding a lock on 'foo'
<facundobatista> so the Upload jumps in, but out of order
<facundobatista> so
<facundobatista> the fix does the following
<facundobatista> when the MOVE is processed by syncdaemon
<facundobatista> it adjusts the path of PathLockTree, changing 'foo' to 'bar', there
<facundobatista> so, the Upload will be locked
<facundobatista> and that's all
<facundobatista> it made me to change a little the internals of PathLockTree, but that's another more complex story

[Manuel de la Peña (mandel)]

Code looks good, here is a small comment regarding the tests but I don't think it should block the landing of the branch. I have noticed the following code repeated several times:

134 + # release
135 + releases.pop(0)()
136 + release = yield d
137 + release()
138 + self.assertEqual(len(self.plt.root['children_nodes']), 0)

It might be a good idea to reuse the code by creating a method, maybe assert_release, and use it in all the diff tests that need it. As I said, this should not block the branch.

review: Approve

[Guillermo Gonzalez (verterok)]

review: Approve

[Ubuntu One Auto Pilot (otto-pilot)]

The attempt to merge lp:~facundo/ubuntuone-client/plt-fixpath into lp:ubuntuone-client failed. Below is the output from the failed tests.

/usr/bin/gnome-autogen.sh
checking for autoconf >= 2.53...
  testing autoconf2.50... not found.
  testing autoconf... found 2.69
checking for automake >= 1.10...
  testing automake-1.12... not found.
  testing automake-1.11... found 1.11.6
checking for libtool >= 1.5...
  testing libtoolize... found 2.4.2
checking for intltool >= 0.30...
  testing intltoolize... found 0.50.2
checking for pkg-config >= 0.14.0...
  testing pkg-config... found 0.26
checking for gtk-doc >= 1.0...
  testing gtkdocize... found 1.18
Checking for required M4 macros...
Checking for forbidden M4 macros...
Processing ./configure.ac
Running libtoolize...
libtoolize: putting auxiliary files in `.'.
libtoolize: copying file `./ltmain.sh'
libtoolize: putting macros in AC_CONFIG_MACRO_DIR, `m4'.
libtoolize: copying file `m4/libtool.m4'
libtoolize: copying file `m4/ltoptions.m4'
libtoolize: copying file `m4/ltsugar.m4'
libtoolize: copying file `m4/ltversion.m4'
libtoolize: copying file `m4/lt~obsolete.m4'
Running intltoolize...
Running gtkdocize...
Running aclocal-1.11...
Running autoconf...
Running autoheader...
Running automake-1.11...
Running ./configure --enable-gtk-doc --enable-debug ...
checking for a BSD-compatible install... /usr/bin/install -c
checking whether build environment is sane... yes
checking for a thread-safe mkdir -p... /bin/mkdir -p
checking for gawk... no
checking for mawk... mawk
checking whether make sets $(MAKE)... yes
checking how to create a ustar tar archive... gnutar
checking whether make supports nested variables... yes
checking for style of include used by make... GNU
checking for gcc... gcc
checking whether the C compiler works... yes
checking for C compiler default output file name... a.out
checking for suffix of executables...
checking whether we are cross compiling... no
checking for suffix of object files... o
checking whether we are using the GNU C compiler... yes
checking whether gcc accepts -g... yes
checking for gcc option to accept ISO C89... none needed
checking dependency style of gcc... gcc3
checking for library containing strerror... none required
checking for gcc... (cached) gcc
checking whether we are using the GNU C compiler... (cached) yes
checking whether gcc accepts -g... (cached) yes
checking for gcc option to accept ISO C89... (cached) none needed
checking dependency style of gcc... (cached) gcc3
checking build system type... x86_64-unknown-linux-gnu
checking host system type... x86_64-unknown-linux-gnu
checking how to print strings... printf
checking for a sed that does not truncate output... /bin/sed
checking for grep that handles long lines and -e... /bin/grep
checking for egrep... /bin/grep -E
checking for fgrep... /bin/grep -F
checking for ld used by gcc... /usr/bin/ld
checking if the linker (/usr/bin/ld) is GNU ld... yes
checking for BSD- or MS-compatible name lister (nm)... /usr/bin/nm -B
checking the name lister (/usr/bin/nm -B) interface... BSD nm
checking whether ln -s works... yes
checking the maximum length of command line arguments... 1572864
checking whether the shell unde...

[lp:~facundo/ubuntuone-client/plt-fixpath]

1311. By Facundo Batista on 2012-09-25

Merged trunk in.

=== modified file 'contrib/testing/testcase.py'
--- contrib/testing/testcase.py	2012-08-16 17:39:19 +0000
+++ contrib/testing/testcase.py	2012-09-25 13:53:21 +0000
@@ -146,6 +146,7 @@
         self.eq = self.event_queue = eq
         self.uuid_map = action_queue.DeferredMap()
         self.queue = action_queue.RequestQueue(self)
+        self.pathlock = action_queue.PathLockingTree()
 
         # throttling attributes
         self.readLimit = None

=== modified file 'tests/syncdaemon/test_pathlockingtree.py'
--- tests/syncdaemon/test_pathlockingtree.py	2012-04-09 20:07:05 +0000
+++ tests/syncdaemon/test_pathlockingtree.py	2012-09-25 13:53:21 +0000
@@ -682,3 +682,276 @@
         self.assertTrue(self.handler.check_debug("releasing",
                                                  "'a', 'b', 'c', 'd', 'e'",
                                                  "remaining: 4"))
+
+
+class PathFixingTests(TwistedTestCase):
+    """Test the path fixing."""
+
+    @defer.inlineCallbacks
+    def setUp(self):
+        """Set up."""
+        yield super(PathFixingTests, self).setUp()
+        self.plt = PathLockingTree()
+
+        self.handler = MementoHandler()
+        self.plt.logger.setLevel(logging.DEBUG)
+        self.plt.logger.propagate = False
+        self.plt.logger.addHandler(self.handler)
+        self.addCleanup(self.plt.logger.removeHandler, self.handler)
+
+    def test_clean_pathlocktree(self):
+        """A fix over nothing stored."""
+        self.plt.fix_path(tuple('abc'), tuple('abX'))
+        self.assertEqual(len(self.plt.root['children_nodes']), 0)
+
+    @defer.inlineCallbacks
+    def test_simple_leaf(self):
+        """Simple change for a leaf."""
+        from_path = tuple('abc')
+        to_path = tuple('abX')
+        release = yield self.plt.acquire(*from_path)
+
+        # get leaf deferred
+        node_a = self.plt.root['children_nodes']['a']
+        node_b = node_a['children_nodes']['b']
+        node_c = node_b['children_nodes']['c']
+        original_deferreds = node_c['node_deferreds']
+
+        # fix path
+        self.plt.fix_path(from_path, to_path)
+
+        # get deferred from new path, assert is the same
+        node_a = self.plt.root['children_nodes']['a']
+        node_b = node_a['children_nodes']['b']
+        node_X = node_b['children_nodes']['X']
+        self.assertEqual(node_X['node_deferreds'], original_deferreds)
+
+        # release, it should be clean now
+        release()
+        self.assertEqual(len(self.plt.root['children_nodes']), 0)
+
+    @defer.inlineCallbacks
+    def test_complex_leaf(self):
+        """Change for a leaf with two items."""
+        from_path = tuple('abc')
+        to_path = tuple('abX')
+
+        releases = []
+        d = self.plt.acquire(*from_path)
+        d.addCallback(releases.append)
+        d = self.plt.acquire(*from_path)
+        d.addCallback(releases.append)
+
+        # get leaf deferred
+        node_a = self.plt.root['children_nodes']['a']
+        node_b = node_a['children_nodes']['b']
+        node_c = node_b['children_nodes']['c']
+        original_deferreds = node_c['node_deferreds']
+
+        # rename
+        self.plt.fix_path(from_path, to_path)
+
+        # get deferred from new path, assert is the same
+        node_a = self.plt.root['children_nodes']['a']
+        node_b = node_a['children_nodes']['b']
+        node_X = node_b['children_nodes']['X']
+        self.assertEqual(node_X['node_deferreds'], original_deferreds)
+
+        # acquire with other one, assert that it's not released
+        d = self.plt.acquire(*to_path)
+        d.addCallback(lambda f: self.assertFalse(releases) or f)
+
+        # release
+        releases.pop(0)()
+        releases.pop(0)()
+        release = yield d
+        release()
+        self.assertEqual(len(self.plt.root['children_nodes']), 0)
+
+    @defer.inlineCallbacks
+    def test_simple_not_leaf(self):
+        """Simple change for not a leaf."""
+        from_path = tuple('abc')
+        to_path = tuple('aXc')
+        releases = []
+        d = self.plt.acquire(*from_path)
+        d.addCallback(releases.append)
+
+        # get leaf deferred
+        node_a = self.plt.root['children_nodes']['a']
+        node_b = node_a['children_nodes']['b']
+        node_c = node_b['children_nodes']['c']
+        original_deferreds = node_c['node_deferreds']
+
+        # rename
+        self.plt.fix_path(from_path, to_path)
+
+        # get deferred from new path, assert is the same
+        node_a = self.plt.root['children_nodes']['a']
+        node_X = node_a['children_nodes']['X']
+        node_c = node_X['children_nodes']['c']
+        self.assertEqual(node_c['node_deferreds'], original_deferreds)
+
+        # acquire with other one, assert that it's not released
+        d = self.plt.acquire(*to_path)
+        d.addCallback(lambda f: self.assertFalse(releases) or f)
+
+        # release
+        releases.pop(0)()
+        release = yield d
+        release()
+        self.assertEqual(len(self.plt.root['children_nodes']), 0)
+
+    @defer.inlineCallbacks
+    def test_same_tree(self):
+        """Move a leaf one level up."""
+        from_path = tuple('abcd')
+        to_path = tuple('abd')
+        releases = []
+        d = self.plt.acquire(*from_path)
+        d.addCallback(releases.append)
+
+        # get leaf deferred
+        node_a = self.plt.root['children_nodes']['a']
+        node_b = node_a['children_nodes']['b']
+        node_c = node_b['children_nodes']['c']
+        node_d = node_c['children_nodes']['d']
+        original_deferreds = node_d['node_deferreds']
+
+        # rename
+        self.plt.fix_path(from_path, to_path)
+
+        # get deferred from new path, assert is the same
+        node_a = self.plt.root['children_nodes']['a']
+        node_b = node_a['children_nodes']['b']
+        node_d = node_b['children_nodes']['d']
+        self.assertEqual(node_d['node_deferreds'], original_deferreds)
+
+        # check also that the 'c' node is gone
+        self.assertNotIn('c', node_b['children_nodes'])
+
+        # acquire with other one, assert that it's not released
+        d = self.plt.acquire(*to_path)
+        d.addCallback(lambda f: self.assertFalse(releases) or f)
+
+        # release
+        releases.pop(0)()
+        release = yield d
+        release()
+        self.assertEqual(len(self.plt.root['children_nodes']), 0)
+
+    @defer.inlineCallbacks
+    def test_parents_move_child(self):
+        """Complex change involving parents, renaming child."""
+        releases = []
+        d = self.plt.acquire('a', 'b')
+        d.addCallback(releases.append)
+        d = self.plt.acquire('a', 'b', 'c', on_parent=True)
+        d.addCallback(releases.append)
+
+        # rename
+        self.plt.fix_path(('a', 'b', 'c'), ('a', 'b', 'X'))
+
+        # acquire with other one, assert that it's not released
+        d = self.plt.acquire('a', 'b', 'X')
+        d.addCallback(lambda f: self.assertFalse(releases) or f)
+
+        # release
+        releases.pop(0)()
+        releases.pop(0)()
+        release = yield d
+        release()
+        self.assertEqual(len(self.plt.root['children_nodes']), 0)
+
+    @defer.inlineCallbacks
+    def test_parents_move_parent(self):
+        """Complex change involving parents, renaming parent."""
+        releases = []
+        d = self.plt.acquire('a', 'b')
+        d.addCallback(releases.append)
+        d = self.plt.acquire('a', 'b', 'c', on_parent=True)
+        d.addCallback(releases.append)
+
+        # rename
+        self.plt.fix_path(('a', 'b'), ('a', 'X'))
+
+        # acquire with other one, assert that there's only one
+        # left to release ('aXC', as releasing 'aX' will trigger
+        # this one)
+        d = self.plt.acquire('a', 'X', 'd', on_parent=True)
+        d.addCallback(lambda f: self.assertEqual(len(releases), 1) and f)
+
+        # release first the parent, then the child
+        releases.pop(0)()
+        releases.pop(0)()
+        release = yield d
+        release()
+        self.assertEqual(len(self.plt.root['children_nodes']), 0)
+
+    @defer.inlineCallbacks
+    def test_very_different_children(self):
+        """Aquire changing the children a lot."""
+        releases = []
+        d = self.plt.acquire('a', 'b', 'c', 'd')
+        d.addCallback(releases.append)
+        d = self.plt.acquire('a', 'b', 'c', on_children=True)
+        d.addCallback(releases.append)
+
+        # rename
+        self.plt.fix_path(('a', 'b', 'c', 'd'), ('a', 'b', 'X', 'Y'))
+
+        # acquire with other one, assert that there's only one
+        # left to release ('aXC', as releasing 'aX' will trigger
+        # this one)
+        d = self.plt.acquire('a', 'b', 'X', on_children=True)
+        d.addCallback(lambda f: self.assertEqual(len(releases), 1) and f)
+
+        # release first the parent, then the child
+        releases.pop(0)()
+        releases.pop(0)()
+        release = yield d
+        release()
+        self.assertEqual(len(self.plt.root['children_nodes']), 0)
+
+    def test_double_simple(self):
+        """Simple but duplicate acquiring."""
+        releases = []
+        d = self.plt.acquire('GetDelta', '')
+        d.addCallback(releases.append)
+        d = self.plt.acquire('GetDelta', '')
+        d.addCallback(releases.append)
+        releases.pop(0)()
+        releases.pop(0)()
+        self.assertEqual(len(self.plt.root['children_nodes']), 0)
+
+    def test_moving_over(self):
+        """Move over something that still exists."""
+        releases = []
+        d = self.plt.acquire('a', 'b', 'c')
+        d.addCallback(releases.append)
+        d = self.plt.acquire('a', 'b', 'd')
+        d.addCallback(releases.append)
+
+        # rename
+        self.plt.fix_path(('a', 'b', 'd'), ('a', 'b', 'c'))
+
+        # release both
+        releases.pop(0)()
+        releases.pop(0)()
+        self.assertEqual(len(self.plt.root['children_nodes']), 0)
+
+    def test_irl_complicated_example(self):
+        """Just a complicated move I found IRL."""
+        releases = []
+        d = self.plt.acquire('temp', 'drizzle', '.bzr', 'checkout', 'limbo',
+                             'new-19', 'handshake.cc')
+        d.addCallback(releases.append)
+
+        # rename
+        fix_from = ('temp', 'drizzle', '.bzr', 'checkout', 'limbo', 'new-19')
+        fix_to = ('temp', 'drizzle', 'libdrizzle')
+        self.plt.fix_path(fix_from, fix_to)
+
+        # release it
+        releases.pop(0)()
+        self.assertEqual(len(self.plt.root['children_nodes']), 0)

=== modified file 'tests/syncdaemon/test_sync.py'
--- tests/syncdaemon/test_sync.py	2012-06-13 21:31:24 +0000
+++ tests/syncdaemon/test_sync.py	2012-09-25 13:53:21 +0000
@@ -1,7 +1,5 @@
 # -*- coding: utf-8 -*-
 #
-# Author: Guillermo Gonzalez <[e-mail]>
-#
 # Copyright 2009-2012 Canonical Ltd.
 #
 # This program is free software: you can redistribute it and/or modify it
@@ -1322,9 +1320,10 @@
         # patch HQ to don't hash the file
         self.main.hash_q.insert = lambda *a: None
 
-        # record the call
+        # record the calls
         called = []
         self.main.fs.add_to_move_limbo = lambda *a: called.append(a)
+        self.main.action_q.pathlock.fix_path = lambda *a: called.append(a)
 
         # create context and call
         key = FSKey(self.main.fs, path=somepath1)
@@ -1332,8 +1331,10 @@
                                       key=key, logger=None)
         parent_id = FSKey(self.main.fs, path=self.root)['node_id']
         ssmr.client_moved('some event', {}, somepath1, somepath2)
-        self.assertEqual(called, [('', 'node_id', parent_id, parent_id,
-                                   'bar', somepath1, somepath2)])
+        self.assertEqual(called[0], (tuple(somepath1.split(os.path.sep)),
+                                     tuple(somepath2.split(os.path.sep))))
+        self.assertEqual(called[1], ('', 'node_id', parent_id, parent_id,
+                                     'bar', somepath1, somepath2))
 
     def test_clean_move_limbo(self):
         """Clean the move limbo with what was called."""

=== modified file 'ubuntuone/syncdaemon/action_queue.py'
--- ubuntuone/syncdaemon/action_queue.py	2012-09-14 22:46:21 +0000
+++ ubuntuone/syncdaemon/action_queue.py	2012-09-25 13:53:21 +0000
@@ -119,6 +119,9 @@
         self.logger = logging.getLogger("ubuntuone.SyncDaemon.PathLockingTree")
         self.root = dict(children_nodes={})
         self.count = 0
+        self.stored_by_id = {}
+        self.stored_by_elements = {}
+        self.id_stored = 0
 
     def acquire(self, *elements, **modifiers):
         """Acquire the lock for the elements.
@@ -175,7 +178,12 @@
                      " wait for: %d", elements, on_parent,
                      on_children, len(wait_for))
 
-        relfunc = partial(self._release, deferred, elements, logger)
+        # store info for later releasing
+        self.id_stored += 1
+        self.stored_by_id[self.id_stored] = (deferred, elements, logger)
+        self.stored_by_elements.setdefault(elements, []).append(self.id_stored)
+        relfunc = partial(self._release, self.id_stored)
+
         if not wait_for:
             return defer.succeed(relfunc)
         if len(wait_for) == 1:
@@ -188,8 +196,15 @@
         deferred_list.addBoth(lambda _: relfunc)
         return deferred_list
 
-    def _release(self, deferred, elements, logger):
+    def _release(self, stored_id):
         """Release the callback and clean the tree."""
+        # get the elements from the stored id
+        deferred, elements, logger = self.stored_by_id.pop(stored_id)
+        stored_ids = self.stored_by_elements[elements]
+        stored_ids.remove(stored_id)
+        if not stored_ids:
+            del self.stored_by_elements[elements]
+
         # clean the tree first!
         # keep here every node and its child element, to backtrack
         branch = []
@@ -200,13 +215,13 @@
         for element in elements[:-1]:
             branch.append((desc, element))
             node = desc['children_nodes'][element]
-            node['children_deferreds'].remove(deferred)
+            node['children_deferreds'].discard(deferred)
             desc = node
 
         # for the final node, remove it from node_deferreds
         branch.append((desc, elements[-1]))
         node = desc['children_nodes'][elements[-1]]
-        node['node_deferreds'].remove(deferred)
+        node['node_deferreds'].discard(deferred)
 
         # backtrack
         while branch:
@@ -223,6 +238,81 @@
                                                              self.count)
         deferred.callback(True)
 
+    def fix_path(self, from_elements, to_elements):
+        """Fix the internal path."""
+        self.logger.debug("Fixing path from %r to %r",
+                          from_elements, to_elements)
+
+        # fix the stored ids and elements
+        something_found = False
+        for key in self.stored_by_elements.keys():
+            if key == from_elements:
+                new_key = to_elements
+            elif key[:len(from_elements)] == from_elements:
+                new_key = to_elements + key[len(from_elements):]
+            else:
+                continue
+
+            # fix the id/elements
+            something_found = True
+            stored_ids = self.stored_by_elements.pop(key)
+            self.stored_by_elements.setdefault(new_key, []).extend(stored_ids)
+            for stored_id in stored_ids:
+                deferred, old_elements, logger = self.stored_by_id[stored_id]
+                self.stored_by_id[stored_id] = deferred, new_key, logger
+
+        # nothing to fix, really
+        if not something_found:
+            return
+
+        # get the node to be moved around
+        branch = []
+        desc = self.root
+        for element in from_elements[:-1]:
+            branch.append((desc, element))
+            node = desc['children_nodes'][element]
+            desc = node
+        node_to_move = desc['children_nodes'].pop(from_elements[-1])
+
+        # backtrack to clean the branch
+        node = desc
+        while branch:
+            if node['node_deferreds'] or node['children_nodes']:
+                # node is not empty, done cleaning the branch!
+                break
+
+            # node is empty! remove it
+            node, element = branch.pop()
+            del node['children_nodes'][element]
+
+        # keep here every node and its child element, to backtrack to add
+        # children deferreds in the branch
+        branch = []
+
+        # get the final parent of the new moved node
+        node = self.root
+        for pos, element in enumerate(to_elements[:-1]):
+            # get previous child or create a new one just empty, not using
+            # setdefault to avoid creating structures if not needed
+            children_nodes = node['children_nodes']
+            if element in children_nodes:
+                node = children_nodes[element]
+            else:
+                node = dict(node_deferreds=set(),
+                            children_nodes={},
+                            children_deferreds={})
+                children_nodes[element] = node
+            branch.append(node)
+
+        node['children_nodes'][to_elements[-1]] = node_to_move
+
+        # fix the children deferreds after the movement
+        all_children_deferreds = (node_to_move['node_deferreds'] |
+                                        node_to_move['children_deferreds'])
+        for node in branch[::-1]:
+            node['children_deferreds'] = set(all_children_deferreds)
+            all_children_deferreds.update(node['node_deferreds'])
+
 
 class NamedTemporaryFile(object):
     """Like tempfile.NamedTemporaryFile, but working in 2.5.
@@ -1099,7 +1189,8 @@
         if len(self.queue) >= self.memory_pool_limit:
             # already in the limit, can't go further as we don't have
             # more room in memory, store it in the offloaded queue
-            logger.debug('offload push: %s %s %s', command_class.__name__, args, kwargs)
+            logger.debug('offload push: %s %s %s',
+                         command_class.__name__, args, kwargs)
             self.disk_queue.push((command_class.__name__, args, kwargs))
             return
 
@@ -1107,9 +1198,11 @@
         yield self._really_execute(command_class, *args, **kwargs)
 
         # command just finished... check to queue more offloaded ones
-        while len(self.queue) < self.memory_pool_limit and len(self.disk_queue) > 0:
+        while (len(self.queue) < self.memory_pool_limit and
+               len(self.disk_queue) > 0):
             command_class_name, args, kwargs = self.disk_queue.pop()
-            logger.debug('offload pop: %s %s %s', command_class_name, args, kwargs)
+            logger.debug('offload pop: %s %s %s',
+                         command_class_name, args, kwargs)
             command_class = self.commands[command_class_name]
             yield self._really_execute(command_class, *args, **kwargs)
 

=== modified file 'ubuntuone/syncdaemon/sync.py'
--- ubuntuone/syncdaemon/sync.py	2012-08-28 14:34:26 +0000
+++ ubuntuone/syncdaemon/sync.py	2012-09-25 13:53:21 +0000
@@ -499,6 +499,11 @@
         share_id = self.key['share_id']
         node_id = self.key['node_id']
 
+        # fix first the PathLockTree, so the move hooks on it's final
+        # path, if any
+        self.m.action_q.pathlock.fix_path(tuple(path_from.split(os.path.sep)),
+                                          tuple(path_to.split(os.path.sep)))
+
         self.m.action_q.move(share_id, node_id, old_parent_id,
                              new_parent_id, new_name, path_from, path_to)
         self.m.fs.add_to_move_limbo(share_id, node_id, old_parent_id,