ENH: Support for moving files and directories

.gitignore

@@ -40,6 +40,7 @@ htmlcov/
 .noseids
 nosetests.xml
 coverage.xml
+/.pytest_cache/*
 
 # Translations
 *.mo

pgcontents/hybridmanager.py

@@ -221,6 +221,7 @@ def _extra_root_dirs(self):
 
     save = path_dispatch2('save', 'model', True)
     rename = path_dispatch_old_new('rename', False)
+    rename_file = path_dispatch_old_new('rename_file', False)
 
     __get = path_dispatch1('get', True)
     __delete = path_dispatch1('delete', False)

pgcontents/pgmanager.py

@@ -18,10 +18,11 @@
 from __future__ import unicode_literals
 from itertools import chain
 from tornado import web
+from traitlets import default
 
 from .api_utils import (
-    base_model,
     base_directory_model,
+    base_model,
     from_b64,
     outside_root_to_404,
     reads_base64,
@@ -57,7 +58,6 @@
     save_file,
 )
 from .utils.ipycompat import Bool, ContentsManager, from_dict
-from traitlets import default
 
 
 class PostgresContentsManager(PostgresManagerMixin, ContentsManager):
@@ -376,26 +376,89 @@ def save(self, model, path):
             model['message'] = validation_message
         return model
 
+    @outside_root_to_404
+    def rename_files(self, old_paths, new_paths):
+        """
+        Rename multiple objects at once. This function is specific to this
+        implementation of ContentsManager and is not in the base class.
+
+        Parameters
+        ----------
+        old_paths : list
+            List of paths for existing files or directories to be renamed. The
+            index position of each path should align with the index of its new
+            path in the `new_paths` parameter.
+        new_paths : list
+            List of new paths to which the files or directories should be
+            named. The index position of each path should align with the index
+            of its existing path in the `old_paths` parameter.
+
+        Returns
+        -------
+        renamed : int
+            The count of paths that were successfully renamed.
+
+        Raises
+        ------
+        FileExists
+            If one of the new paths already exists as a file.
+        DirectoryExists
+            If one of the new paths already exists as a directory.
+        RenameRoot
+            If one of the old paths given is the root path.
+        PathOutsideRoot
+            If one of the new paths given is outside of the root path.
+        """
+        if len(set(old_paths)) != len(old_paths):
+            self.do_409(
+                'The list of paths to rename cannot contain duplicates.',
+            )
+        if len(set(new_paths)) != len(new_paths):
+            self.do_409(
+                'The list of new path names cannot contain duplicates.',
+            )
+
+        renamed = 0
+
+        with self.engine.begin() as db:
+            for old_path, new_path in zip(old_paths, new_paths):
+                try:
+                    if self.file_exists(old_path):
+                        rename_file(db, self.user_id, old_path, new_path)
+                    elif self.dir_exists(old_path):
+                        rename_directory(db, self.user_id, old_path, new_path)
+                    else:
+                        self.no_such_entity(old_path)
+                except (FileExists, DirectoryExists):
+                    self.already_exists(new_path)
+                except RenameRoot as e:
+                    self.do_409(str(e))
+                except (web.HTTPError, PathOutsideRoot):
+                    raise
+                except Exception as e:
+                    self.log.exception(
+                        'Error renaming file/directory from %s to %s',
+                        old_path,
+                        new_path,
+                    )
+                    self.do_500(
+                        u'Unexpected error while renaming %s: %s'
+                        % (old_path, e)
+                    )
+                renamed += 1
+
+        self.log.info('Successfully renamed %d paths.', renamed)
+        return renamed
+
     @outside_root_to_404
     def rename_file(self, old_path, path):
         """
         Rename object from old_path to path.
 
-        NOTE: This method is unfortunately named on the base class.  It
-        actually moves a file or a directory.
+        NOTE: This method is unfortunately named on the base class. It actually
+              moves files and directories as well.
         """
-        with self.engine.begin() as db:
-            try:
-                if self.file_exists(old_path):
-                    rename_file(db, self.user_id, old_path, path)
-                elif self.dir_exists(old_path):
-                    rename_directory(db, self.user_id, old_path, path)
-                else:
-                    self.no_such_entity(path)
-            except (FileExists, DirectoryExists):
-                self.already_exists(path)
-            except RenameRoot as e:
-                self.do_409(str(e))
+        return self.rename_files([old_path], [path])
 
     def _delete_non_directory(self, path):
         with self.engine.begin() as db:

pgcontents/query.py

@@ -1,8 +1,6 @@
 """
 Database Queries for PostgresContentsManager.
 """
-from textwrap import dedent
-
 from sqlalchemy import (
     and_,
     cast,
@@ -420,32 +418,18 @@ def rename_file(db, user_id, old_api_path, new_api_path):
     """
     Rename a file.
     """
-
     # Overwriting existing files is disallowed.
     if file_exists(db, user_id, new_api_path):
         raise FileExists(new_api_path)
 
-    old_dir, old_name = split_api_filepath(old_api_path)
     new_dir, new_name = split_api_filepath(new_api_path)
-    if old_dir != new_dir:
-        raise ValueError(
-            dedent(
-                """
-                Can't rename object to new directory.
-                Old Path: {old_api_path}
-                New Path: {new_api_path}
-                """.format(
-                    old_api_path=old_api_path,
-                    new_api_path=new_api_path
-                )
-            )
-        )
 
     db.execute(
         files.update().where(
             _file_where(user_id, old_api_path),
         ).values(
             name=new_name,
+            parent_name=new_dir,
             created_at=func.now(),
         )
     )
@@ -470,7 +454,13 @@ def rename_directory(db, user_id, old_api_path, new_api_path):
     db.execute('SET CONSTRAINTS '
                'pgcontents.directories_parent_user_id_fkey DEFERRED')
 
-    # Update name column for the directory that's being renamed
+    old_api_dir, old_name = split_api_filepath(old_api_path)
+    new_api_dir, new_name = split_api_filepath(new_api_path)
+    new_db_dir = from_api_dirname(new_api_dir)
+
+    # Update the name and parent_name columns for the directory that is being
+    # renamed. The parent_name column will not change for a simple rename, but
+    # will if the directory is moving.
     db.execute(
         directories.update().where(
             and_(
@@ -479,34 +469,36 @@ def rename_directory(db, user_id, old_api_path, new_api_path):
             )
         ).values(
             name=new_db_path,
+            parent_name=new_db_dir,
         )
     )
 
-    # Update the name and parent_name of any descendant directories.  Do
-    # this in a single statement so the non-deferrable check constraint
-    # is satisfied.
+    # Update the name and parent_name of any descendant directories. Do this in
+    # a single statement so the non-deferrable check constraint is satisfied.
     db.execute(
         directories.update().where(
             and_(
                 directories.c.user_id == user_id,
                 directories.c.name.startswith(old_db_path),
                 directories.c.parent_name.startswith(old_db_path),
-            )
+            ),
         ).values(
             name=func.concat(
                 new_db_path,
-                func.right(directories.c.name, -func.length(old_db_path))
+                func.right(directories.c.name, -func.length(old_db_path)),
             ),
             parent_name=func.concat(
                 new_db_path,
                 func.right(
                     directories.c.parent_name,
-                    -func.length(old_db_path)
-                )
+                    -func.length(old_db_path),
+                ),
             ),
         )
     )
 
+    return True
+
 
 def save_file(db, user_id, path, content, encrypt_func, max_size_bytes):
     """

pgcontents/tests/test_hybrid_manager.py

@@ -11,6 +11,8 @@
     join as osjoin,
 )
 from posixpath import join as pjoin
+
+from mock import Mock
 from six import (
     iteritems,
     itervalues,
@@ -88,6 +90,21 @@ def setUp(self):
     def test_get_file_id(self):
         pass
 
+    # This test also uses `get_file_id`, but it is not pertinent to the crucial
+    # parts of the test so just mock out.
+    def test_rename_file(self):
+        HybridContentsManager.get_file_id = Mock()
+        try:
+            super(PostgresTestCase, self).test_rename_file()
+        finally:
+            del HybridContentsManager.get_file_id
+
+    # This test calls `rename_files` which HybridContentsManager is also not
+    # expected to dispatch to as PostgresContentsManager is the only contents
+    # manager that implements it.
+    def test_move_multiple_objects(self):
+        pass
+
     def set_pgmgr_attribute(self, name, value):
         setattr(self._pgmanager, name, value)