From 57c0676cc1dc1a65c136ef92e41188f056f3882f Mon Sep 17 00:00:00 2001 From: Josh Hamet Date: Tue, 29 Mar 2022 17:28:34 -0400 Subject: [PATCH] Remove Redundant Dir_Exists Invocation When Creating New Files with ContentsManager (#720) Co-authored-by: Josh Hamet Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- jupyter_server/services/contents/handlers.py | 4 ---- jupyter_server/services/contents/manager.py | 23 ++++++++++++++++++-- tests/services/contents/test_manager.py | 9 ++++++++ 3 files changed, 30 insertions(+), 6 deletions(-) diff --git a/jupyter_server/services/contents/handlers.py b/jupyter_server/services/contents/handlers.py index ff68f4ddd6..e3f9c19a9e 100644 --- a/jupyter_server/services/contents/handlers.py +++ b/jupyter_server/services/contents/handlers.py @@ -190,10 +190,6 @@ async def post(self, path=""): if file_exists: raise web.HTTPError(400, "Cannot POST to files, use PUT instead.") - dir_exists = await ensure_async(cm.dir_exists(path)) - if not dir_exists: - raise web.HTTPError(404, "No such directory: %s" % path) - model = self.get_json_body() if model is not None: diff --git a/jupyter_server/services/contents/manager.py b/jupyter_server/services/contents/manager.py index 90b9efafb8..8bb9c0c9a4 100644 --- a/jupyter_server/services/contents/manager.py +++ b/jupyter_server/services/contents/manager.py @@ -584,6 +584,7 @@ def copy(self, from_path, to_path=None): from_path must be a full path to a file. """ path = from_path.strip("/") + if to_path is not None: to_path = to_path.strip("/") @@ -599,12 +600,20 @@ def copy(self, from_path, to_path=None): if model["type"] == "directory": raise HTTPError(400, "Can't copy directories") - if to_path is None: + is_destination_specified = to_path is not None + if not is_destination_specified: to_path = from_dir if self.dir_exists(to_path): name = copy_pat.sub(".", from_name) to_name = self.increment_filename(name, to_path, insert="-Copy") to_path = "{0}/{1}".format(to_path, to_name) + elif is_destination_specified: + if "/" in to_path: + to_dir, to_name = to_path.rsplit("/", 1) + if not self.dir_exists(to_dir): + raise HTTPError(404, "No such parent directory: %s to copy file in" % to_dir) + else: + raise HTTPError(404, "No such directory: %s" % to_path) model = self.save(model, to_path) return model @@ -944,6 +953,7 @@ async def copy(self, from_path, to_path=None): from_path must be a full path to a file. """ path = from_path.strip("/") + if to_path is not None: to_path = to_path.strip("/") @@ -958,12 +968,21 @@ async def copy(self, from_path, to_path=None): model.pop("name", None) if model["type"] == "directory": raise HTTPError(400, "Can't copy directories") - if to_path is None: + + is_destination_specified = to_path is not None + if not is_destination_specified: to_path = from_dir if await ensure_async(self.dir_exists(to_path)): name = copy_pat.sub(".", from_name) to_name = await self.increment_filename(name, to_path, insert="-Copy") to_path = "{0}/{1}".format(to_path, to_name) + elif is_destination_specified: + if "/" in to_path: + to_dir, to_name = to_path.rsplit("/", 1) + if not await ensure_async(self.dir_exists(to_dir)): + raise HTTPError(404, "No such parent directory: %s to copy file in" % to_dir) + else: + raise HTTPError(404, "No such directory: %s" % to_path) model = await self.save(model, to_path) return model diff --git a/tests/services/contents/test_manager.py b/tests/services/contents/test_manager.py index 2542488162..dfcee4a1c5 100644 --- a/tests/services/contents/test_manager.py +++ b/tests/services/contents/test_manager.py @@ -658,11 +658,20 @@ async def test_copy(jp_contents_manager): copy2 = await ensure_async(cm.copy(path, "å b/copy 2.ipynb")) assert copy2["name"] == "copy 2.ipynb" assert copy2["path"] == "å b/copy 2.ipynb" + # copy with specified path copy2 = await ensure_async(cm.copy(path, "/")) assert copy2["name"] == name assert copy2["path"] == name + # copy to destination whose parent dir does not exist + with pytest.raises(HTTPError) as e: + await ensure_async(cm.copy(path, "å x/copy 2.ipynb")) + + copy3 = await ensure_async(cm.copy(path, "/copy 3.ipynb")) + assert copy3["name"] == "copy 3.ipynb" + assert copy3["path"] == "copy 3.ipynb" + async def test_mark_trusted_cells(jp_contents_manager): cm = jp_contents_manager