[zeppelin] branch master updated: [ZEPPELIN-4147]. Fail to rename folder

[zj...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

zjffdu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/zeppelin.git


The following commit(s) were added to refs/heads/master by this push:
     new 8babd76  [ZEPPELIN-4147]. Fail to rename folder
8babd76 is described below

commit 8babd76f3f2093695e045e4b47fa54c7d5104757
Author: Jeff Zhang <zj...@apache.org>
AuthorDate: Thu May 9 17:35:04 2019 +0800

    [ZEPPELIN-4147]. Fail to rename folder
    
    ### What is this PR for?
    
    It is a trivial bug fox for rename folder failure. The root cause is that the folder path is not including leading `/` which is required by NotebookRepo.
    
    ### What type of PR is it?
    [Bug Fix]
    
    ### Todos
    * [ ] - Task
    
    ### What is the Jira issue?
    * https://jira.apache.org/jira/browse/ZEPPELIN-4147
    
    ### How should this be tested?
    * CI Pass
    
    ### Screenshots (if appropriate)
    
    ### Questions:
    * Does the licenses files need update? No
    * Is there breaking changes for older versions? No
    * Does this needs documentation? No
    
    Author: Jeff Zhang <zj...@apache.org>
    
    Closes #3367 from zjffdu/ZEPPELIN-4147 and squashes the following commits:
    
    51c05c168 [Jeff Zhang] [ZEPPELIN-4147]. Fail to rename folder
---
 .../main/java/org/apache/zeppelin/service/NotebookService.java | 10 +++++++++-
 .../java/org/apache/zeppelin/service/NotebookServiceTest.java  |  9 ++++++++-
 .../apache/zeppelin/notebook/repo/InMemoryNotebookRepo.java    |  6 ++++++
 3 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java b/zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java
index 2b829fd..bb42ce2 100644
--- a/zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java
+++ b/zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java
@@ -156,6 +156,13 @@ public class NotebookService {
     }
   }
 
+  /**
+   * normalize both note name and note folder
+   *
+   * @param notePath
+   * @return
+   * @throws IOException
+   */
   String normalizeNotePath(String notePath) throws IOException {
     if (StringUtils.isBlank(notePath)) {
       notePath = "/Untitled Note";
@@ -949,7 +956,8 @@ public class NotebookService {
     //TODO(zjffdu) folder permission check
 
     try {
-      notebook.moveFolder(folderPath, newFolderPath, context.getAutheInfo());
+      notebook.moveFolder(normalizeNotePath(folderPath),
+              normalizeNotePath(newFolderPath), context.getAutheInfo());
       List<NoteInfo> notesInfo = notebook.getNotesInfo(
               noteId -> authorizationService.isReader(noteId, context.getUserAndRoles()));
       callback.onSuccess(notesInfo, context);
diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/service/NotebookServiceTest.java b/zeppelin-server/src/test/java/org/apache/zeppelin/service/NotebookServiceTest.java
index f37418e..e4df8c3 100644
--- a/zeppelin-server/src/test/java/org/apache/zeppelin/service/NotebookServiceTest.java
+++ b/zeppelin-server/src/test/java/org/apache/zeppelin/service/NotebookServiceTest.java
@@ -175,6 +175,13 @@ public class NotebookServiceTest {
     assertEquals(1, notesInfo.size());
     assertEquals("/folder_3/new_name", notesInfo.get(0).getPath());
 
+    // move folder in case of folder path without prefix '/'
+    reset(callback);
+    notesInfo = notebookService.renameFolder("folder_3", "folder_4", context, callback);
+    verify(callback).onSuccess(notesInfo, context);
+    assertEquals(1, notesInfo.size());
+    assertEquals("/folder_4/new_name", notesInfo.get(0).getPath());
+
     // create another note
     note2 = notebookService.createNote("/note2", "test", context, callback);
     assertEquals("note2", note2.getName());
@@ -204,7 +211,7 @@ public class NotebookServiceTest {
     verify(callback).onSuccess(notesInfo, context);
 
     // delete folder
-    notesInfo = notebookService.removeFolder("/folder_3", context, callback);
+    notesInfo = notebookService.removeFolder("/folder_4", context, callback);
     verify(callback).onSuccess(notesInfo, context);
 
     // list note again
diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/InMemoryNotebookRepo.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/InMemoryNotebookRepo.java
index 1793f9c..187cfc0 100644
--- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/InMemoryNotebookRepo.java
+++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/InMemoryNotebookRepo.java
@@ -71,6 +71,12 @@ public class InMemoryNotebookRepo implements NotebookRepo {
 
   @Override
   public void move(String folderPath, String newFolderPath, AuthenticationInfo subject) {
+    if (!folderPath.startsWith("/")) {
+      throw new RuntimeException(String.format("folderPath '%s' is not started with '/'", folderPath));
+    }
+    if (folderPath.startsWith("//")) {
+      throw new RuntimeException(String.format("folderPath '%s' is started with '//'", folderPath));
+    }
     if (!newFolderPath.startsWith("/")) {
       throw new RuntimeException(String.format("newFolderPath '%s' is not started with '/'", newFolderPath));
     }