diff --git a/notebook/services/contents/filemanager.py b/notebook/services/contents/filemanager.py index ef397d4e33..7d0c7be5d3 100644 --- a/notebook/services/contents/filemanager.py +++ b/notebook/services/contents/filemanager.py @@ -489,17 +489,8 @@ def delete_file(self, path): path = path.strip('/') os_path = self._get_os_path(path) rm = os.unlink - if os.path.isdir(os_path): - listing = os.listdir(os_path) - # Don't delete non-empty directories. - # A directory containing only leftover checkpoints is - # considered empty. - cp_dir = getattr(self.checkpoints, 'checkpoint_dir', None) - for entry in listing: - if entry != cp_dir: - 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) + if not os.path.exists(os_path): + raise web.HTTPError(404, u'File or directory does not exist: %s' % os_path) if self.delete_to_trash: self.log.debug("Sending %s to trash", os_path) @@ -510,6 +501,14 @@ def delete_file(self, path): return if os.path.isdir(os_path): + listing = os.listdir(os_path) + # Don't permanently delete non-empty directories. + # A directory containing only leftover checkpoints is + # considered empty. + cp_dir = getattr(self.checkpoints, 'checkpoint_dir', None) + for entry in listing: + if entry != cp_dir: + raise web.HTTPError(400, u'Directory %s not empty' % os_path) self.log.debug("Removing directory %s", os_path) with self.perm_to_403(): shutil.rmtree(os_path) diff --git a/notebook/services/contents/tests/test_contents_api.py b/notebook/services/contents/tests/test_contents_api.py index 990b6ca519..cfea8cb5fc 100644 --- a/notebook/services/contents/tests/test_contents_api.py +++ b/notebook/services/contents/tests/test_contents_api.py @@ -523,9 +523,11 @@ def test_delete_dirs(self): 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') + # Test that non empty directory can be deleted + self.api.delete(u'å b') + # Check if directory has actually been deleted + with assert_http_error(404): + self.api.list(u'å b') def test_rename(self): resp = self.api.rename('foo/a.ipynb', 'foo/z.ipynb')