Skip to content

Commit

Permalink
Allow non-empty directory deletion for windows through settings
Browse files Browse the repository at this point in the history 
Fixes #570

More robust unit tests
  • Loading branch information
@fcollonval
fcollonval committed Aug 20, 2021
1 parent 90f619c commit 6428ebc
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 10 deletions.
46 changes: 36 additions & 10 deletions jupyter_server/services/contents/filemanager.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,16 @@ def _checkpoints_class_default(self):
deleting files really deletes them.""",
)

always_delete_dir = Bool(
False,
config=True,
help="""If True, deleting non-empty directory will always be allowed.
WARNING this may result in files being definitely removed; e.g. on Windows
if the data size is too big for the trash/recycle bin they will be really
deleted. If False (default), non-empty directory will be send to trash only
if safe. And if ``delete_to_trash`` is True, they won't be deleted.""",
)

@default("files_handler_class")
def _files_handler_class_default(self):
return AuthenticatedFileHandler
Expand Down Expand Up @@ -331,7 +341,10 @@ def _file_model(self, path, content=True, format=None):
if content:
content, format = self._read_file(os_path, format)
if model["mimetype"] is None:
default_mime = {"text": "text/plain", "base64": "application/octet-stream"}[format]
default_mime = {
"text": "text/plain",
"base64": "application/octet-stream",
}[format]
model["mimetype"] = default_mime

model.update(
Expand Down Expand Up @@ -391,7 +404,9 @@ def get(self, path, content=True, type=None, format=None):
if os.path.isdir(os_path):
if type not in (None, "directory"):
raise web.HTTPError(
400, u"%s is a directory, not a %s" % (path, type), reason="bad type"
400,
u"%s is a directory, not a %s" % (path, type),
reason="bad type",
)
model = self._dir_model(path, content=content)
elif type == "notebook" or (type is None and path.endswith(".ipynb")):
Expand Down Expand Up @@ -494,7 +509,7 @@ def is_non_empty_dir(os_path):
return False

if self.delete_to_trash:
if sys.platform == "win32" and is_non_empty_dir(os_path):
if not self.always_delete_dir and sys.platform == "win32" and is_non_empty_dir(os_path):
# send2trash can really delete files on Windows, so disallow
# deleting non-empty files. See Github issue 3631.
raise web.HTTPError(400, u"Directory %s not empty" % os_path)
Expand All @@ -507,12 +522,13 @@ def is_non_empty_dir(os_path):
return
else:
self.log.warning(
"Skipping trash for %s, on different device " "to home directory", os_path
"Skipping trash for %s, on different device " "to home directory",
os_path,
)

if os.path.isdir(os_path):
# Don't permanently delete non-empty directories.
if is_non_empty_dir(os_path):
if not self.always_delete_dir and is_non_empty_dir(os_path):
raise web.HTTPError(400, u"Directory %s not empty" % os_path)
self.log.debug("Removing directory %s", os_path)
with self.perm_to_403():
Expand Down Expand Up @@ -649,7 +665,10 @@ async def _file_model(self, path, content=True, format=None):
if content:
content, format = await self._read_file(os_path, format)
if model["mimetype"] is None:
default_mime = {"text": "text/plain", "base64": "application/octet-stream"}[format]
default_mime = {
"text": "text/plain",
"base64": "application/octet-stream",
}[format]
model["mimetype"] = default_mime

model.update(
Expand Down Expand Up @@ -709,7 +728,9 @@ async def get(self, path, content=True, type=None, format=None):
if os.path.isdir(os_path):
if type not in (None, "directory"):
raise web.HTTPError(
400, u"%s is a directory, not a %s" % (path, type), reason="bad type"
400,
u"%s is a directory, not a %s" % (path, type),
reason="bad type",
)
model = await self._dir_model(path, content=content)
elif type == "notebook" or (type is None and path.endswith(".ipynb")):
Expand Down Expand Up @@ -813,7 +834,11 @@ async def is_non_empty_dir(os_path):
return False

if self.delete_to_trash:
if sys.platform == "win32" and await is_non_empty_dir(os_path):
if (
not self.always_delete_dir
and sys.platform == "win32"
and await is_non_empty_dir(os_path)
):
# send2trash can really delete files on Windows, so disallow
# deleting non-empty files. See Github issue 3631.
raise web.HTTPError(400, u"Directory %s not empty" % os_path)
Expand All @@ -826,12 +851,13 @@ async def is_non_empty_dir(os_path):
return
else:
self.log.warning(
"Skipping trash for %s, on different device " "to home directory", os_path
"Skipping trash for %s, on different device " "to home directory",
os_path,
)

if os.path.isdir(os_path):
# Don't permanently delete non-empty directories.
if await is_non_empty_dir(os_path):
if not self.always_delete_dir and await is_non_empty_dir(os_path):
raise web.HTTPError(400, u"Directory %s not empty" % os_path)
self.log.debug("Removing directory %s", os_path)
with self.perm_to_403():
Expand Down
41 changes: 41 additions & 0 deletions jupyter_server/tests/services/contents/test_manager.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,47 @@ async def test_delete(jp_contents_manager):
await ensure_async(cm.get(path))


@pytest.mark.parametrize(
"delete_to_trash, always_delete, error",
(
[True, True, False],
# on linux test folder may not be on home folder drive
# => if this is the case, _check_trash will be False
[True, False, None],
[False, True, False],
[False, False, True],
),
)
async def test_delete_non_empty_folder(delete_to_trash, always_delete, error, jp_contents_manager):
cm = jp_contents_manager
cm.delete_to_trash = delete_to_trash
cm.always_delete_dir = always_delete

dir = "to_delete"

await make_populated_dir(cm, dir)
await check_populated_dir_files(cm, dir)

if error is None:
error = False
if sys.platform == "win32":
error = True
elif sys.platform == "linux":
file_dev = os.stat(cm.root_dir).st_dev
home_dev = os.stat(os.path.expanduser("~")).st_dev
error = file_dev != home_dev

if error:
with pytest.raises(
HTTPError,
match=r"HTTP 400: Bad Request \(Directory .*?to_delete not empty\)",
):
await ensure_async(cm.delete_file(dir))
else:
await ensure_async(cm.delete_file(dir))
assert cm.dir_exists(dir) == False


async def test_rename(jp_contents_manager):
cm = jp_contents_manager
# Create a new notebook
Expand Down

0 comments on commit 6428ebc

Please sign in to comment.