Skip to content

Commit

Permalink
Remove Redundant Dir_Exists Invocation When Creating New Files with C…
Browse files Browse the repository at this point in the history
…ontentsManager (#720)

Co-authored-by: Josh Hamet <jhamet@twitter.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
  • Loading branch information
3 people authored Mar 29, 2022
1 parent 78521ab commit 57c0676
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 6 deletions.
4 changes: 0 additions & 4 deletions jupyter_server/services/contents/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
23 changes: 21 additions & 2 deletions jupyter_server/services/contents/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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("/")

Expand All @@ -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
Expand Down Expand Up @@ -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("/")

Expand All @@ -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
Expand Down
9 changes: 9 additions & 0 deletions tests/services/contents/test_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 57c0676

Please sign in to comment.