Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove Redundant Dir_Exists Invocation When Creating New Files with ContentsManager #720

Merged
4 changes: 0 additions & 4 deletions jupyter_server/services/contents/handlers.py
Expand Up @@ -194,10 +194,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
Expand Up @@ -586,6 +586,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 @@ -601,12 +602,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 @@ -946,6 +955,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 @@ -960,12 +970,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
Expand Up @@ -657,11 +657,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