Skip to content

[ZEPPELIN-6415] Fix FileSystemNotebookRepo folder move() creating parent of source instead of destination#5283

Open
Coen90 wants to merge 1 commit into
apache:masterfrom
Coen90:ZEPPELIN-6415
Open

[ZEPPELIN-6415] Fix FileSystemNotebookRepo folder move() creating parent of source instead of destination#5283
Coen90 wants to merge 1 commit into
apache:masterfrom
Coen90:ZEPPELIN-6415

Conversation

@Coen90

@Coen90 Coen90 commented Jul 7, 2026

Copy link
Copy Markdown

What is this PR for?

The folder move(String folderPath, String newFolderPath, AuthenticationInfo subject) method in FileSystemNotebookRepo creates the parent directory of the source folder instead of the destination folder before performing the move.

Since the source folder must already exist for the move to succeed, creating its parent is redundant. Meanwhile the destination's parent directory may not exist, causing the subsequent move to fail — e.g. moving /A/X to /B/X when /B does not exist fails because the code creates /A (already present) instead of /B.

This PR changes the tryMkDir call to use newFolderPath (the destination) instead of folderPath (the source), mirroring the existing behavior already present in the note move() overload.

-    this.fs.tryMkDir(new Path(notebookDir, folderPath.substring(1)).getParent());
+    this.fs.tryMkDir(new Path(notebookDir, newFolderPath.substring(1)).getParent());

What type of PR is it?

Bug Fix

Todos

  • - Fix parent directory creation to target the destination folder

What is the Jira issue?

How should this be tested?

  • Existing tests in FileSystemNotebookRepoTest cover the folder move flow.
  • Manual: move a folder into a destination whose parent directory does not yet exist (e.g. /folder1/folder2/folder3) and verify the move succeeds and the parent (/folder2) is created.

Screenshots (if appropriate)

N/A

Questions:

  • Does the license files need to update? No
  • Is there breaking changes for older versions? No
  • Does this needs documentation? No

…ent of source instead of destination

The folder move() method called tryMkDir on the source folder's parent
instead of the destination folder's parent. Since the source folder must
already exist for the move to succeed, creating its parent is redundant,
while the destination's parent may not exist, causing the move to fail
(e.g. moving /A/X to /B/X when /B does not exist).

Create the parent of newFolderPath (the destination) before moving, so
missing destination parent directories are created. This mirrors the
existing behavior in the note move() overload.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@ParkGyeongTae ParkGyeongTae left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. +1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants