From 93b300adda9461c26ae38f6c57a67a3a19e387f5 Mon Sep 17 00:00:00 2001 From: MinRK Date: Mon, 16 Jun 2014 12:47:35 -0700 Subject: [PATCH] support deleting empty directories MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit can’t copy directories --- IPython/html/services/contents/filemanager.py | 16 +++++++++--- IPython/html/services/contents/manager.py | 4 +++ .../contents/tests/test_contents_api.py | 26 ++++++++++++++++--- 3 files changed, 40 insertions(+), 6 deletions(-) diff --git a/IPython/html/services/contents/filemanager.py b/IPython/html/services/contents/filemanager.py index 2c36e05cd..ef4acf413 100644 --- a/IPython/html/services/contents/filemanager.py +++ b/IPython/html/services/contents/filemanager.py @@ -357,7 +357,13 @@ class FileContentsManager(ContentsManager): """Delete file by name and path.""" path = path.strip('/') os_path = self._get_os_path(name, path) - if not os.path.isfile(os_path): + rm = os.unlink + if os.path.isdir(os_path): + listing = os.listdir(os_path) + # don't delete non-empty directories (checkpoints dir doesn't count) + if listing and listing != ['.ipynb_checkpoints']: + raise web.HTTPError(400, u'Directory %s not empty' % os_path) + elif not os.path.isfile(os_path): raise web.HTTPError(404, u'File does not exist: %s' % os_path) # clear checkpoints @@ -368,8 +374,12 @@ class FileContentsManager(ContentsManager): self.log.debug("Unlinking checkpoint %s", cp_path) os.unlink(cp_path) - self.log.debug("Unlinking file %s", os_path) - os.unlink(os_path) + if os.path.isdir(os_path): + self.log.debug("Removing directory %s", os_path) + shutil.rmtree(os_path) + else: + self.log.debug("Unlinking file %s", os_path) + rm(os_path) def rename(self, old_name, old_path, new_name, new_path): """Rename a file.""" diff --git a/IPython/html/services/contents/manager.py b/IPython/html/services/contents/manager.py index cd4231c79..871c3639e 100644 --- a/IPython/html/services/contents/manager.py +++ b/IPython/html/services/contents/manager.py @@ -7,6 +7,8 @@ from fnmatch import fnmatch import itertools import os +from tornado.web import HTTPError + from IPython.config.configurable import LoggingConfigurable from IPython.nbformat import current, sign from IPython.utils.traitlets import Instance, Unicode, List @@ -187,6 +189,8 @@ class ContentsManager(LoggingConfigurable): """ path = path.strip('/') model = self.get_model(from_name, path) + if model['type'] == 'directory': + raise HTTPError(400, "Can't copy directories") if not to_name: base, ext = os.path.splitext(from_name) copy_name = u'{0}-Copy{1}'.format(base, ext) diff --git a/IPython/html/services/contents/tests/test_contents_api.py b/IPython/html/services/contents/tests/test_contents_api.py index 4c73075c4..aadf0c93b 100644 --- a/IPython/html/services/contents/tests/test_contents_api.py +++ b/IPython/html/services/contents/tests/test_contents_api.py @@ -69,6 +69,9 @@ class API(object): def upload(self, name, body, path='/'): return self._req('PUT', url_path_join(path, name), body) + def mkdir(self, name, path='/'): + return self._req('PUT', url_path_join(path, name), json.dumps({'type': 'directory'})) + def copy(self, copy_from, copy_to, path='/'): body = json.dumps({'copy_from':copy_from}) return self._req('PUT', url_path_join(path, copy_to), body) @@ -299,9 +302,7 @@ class APITest(NotebookTestBase): self._check_created(resp, u'Upload tést.ipynb', u'å b') def test_mkdir(self): - model = {'type': 'directory'} - resp = self.api.upload(u'New ∂ir', path=u'å b', - body=json.dumps(model)) + resp = self.api.mkdir(u'New ∂ir', path=u'å b') self._check_created(resp, u'New ∂ir', u'å b', type='directory') def test_upload_txt(self): @@ -362,6 +363,11 @@ class APITest(NotebookTestBase): resp = self.api.copy(u'ç d.ipynb', u'cøpy.ipynb', path=u'å b') self._check_created(resp, u'cøpy.ipynb', u'å b') + def test_copy_dir_400(self): + # can't copy directories + with assert_http_error(400): + resp = self.api.copy(u'å b', u'å c') + def test_delete(self): for d, name in self.dirs_nbs: resp = self.api.delete('%s.ipynb' % name, d) @@ -371,6 +377,20 @@ class APITest(NotebookTestBase): nbs = notebooks_only(self.api.list(d).json()) self.assertEqual(len(nbs), 0) + def test_delete_dirs(self): + # depth-first delete everything, so we don't try to delete empty directories + for name in sorted(self.dirs + ['/'], key=len, reverse=True): + listing = self.api.list(name).json()['content'] + for model in listing: + self.api.delete(model['name'], model['path']) + listing = self.api.list('/').json()['content'] + self.assertEqual(listing, []) + + def test_delete_non_empty_dir(self): + """delete non-empty dir raises 400""" + with assert_http_error(400): + self.api.delete(u'å b') + def test_rename(self): resp = self.api.rename('a.ipynb', 'foo', 'z.ipynb') self.assertEqual(resp.headers['Location'].split('/')[-1], 'z.ipynb')