[ZEPPELIN-5989] Resolve a local notebook dir as a file path instead of parsing it as a URI

zeppelin-server/src/main/java/org/apache/zeppelin/notebook/repo/VFSNotebookRepo.java

@@ -70,39 +70,46 @@ protected void setNotebookDirectory(String notebookDirPath) throws IOException {
     URI filesystemRoot = null;
     try {
       LOGGER.info("Using notebookDir: {}", notebookDirPath);
-      if (zConf.isWindowsPath(notebookDirPath)) {
-        filesystemRoot = new File(notebookDirPath).toURI();
+      if (zConf.isWindowsPath(notebookDirPath) || !zConf.isPathWithScheme(notebookDirPath)) {
+        // A local path is not parsed as a URI: it can contain characters that a URI does not
+        // allow, such as spaces or the backslashes of a Windows path like the default ./\notebook.
+        filesystemRoot = new File(zConf.getAbsoluteDir(notebookDirPath)).toURI();
       } else {
         filesystemRoot = new URI(notebookDirPath);
       }
     } catch (URISyntaxException e) {
       throw new IOException(e);
     }
-
-    if (filesystemRoot.getScheme() == null) { // it is local path
-      File f = new File(zConf.getAbsoluteDir(filesystemRoot.getPath()));
-      filesystemRoot = f.toURI();
-    }
     this.fsManager = VFS.getManager();
     this.rootNotebookFileObject = fsManager.resolveFile(filesystemRoot);
     if (!this.rootNotebookFileObject.exists()) {
       this.rootNotebookFileObject.createFolder();
       LOGGER.info("Notebook dir doesn't exist: {}, creating it.",
           rootNotebookFileObject.getName().getPath());
     }
-    // getPath() method returns a string without root directory in windows, so we use getURI() instead
-    // windows does not support paths with "file:///" prepended, so we replace it by "/"
-    this.rootNotebookFolder = rootNotebookFileObject.getName().getURI().replace("file:///", "/");
+    this.rootNotebookFolder = toRootNotebookFolder(rootNotebookFileObject.getName().getURI());
   }
 
   @Override
   public Map<String, NoteInfo> list(AuthenticationInfo subject) throws IOException {
     // Must to create rootNotebookFileObject each time when call method list, otherwise we can not
     // get the updated data under this folder.
-    this.rootNotebookFileObject = fsManager.resolveFile(this.rootNotebookFolder);
+    this.rootNotebookFileObject = fsManager.resolveFile(rootNotebookFileObject.getName().getURI());
     return listFolder(rootNotebookFileObject);
   }
 
+  /**
+   * A local root is kept as a decoded path, like the note file names in {@link #listFolder}, so
+   * that {@link #getNotePath} can strip it from them and GitNotebookRepo can open the repository
+   * in it. Windows does not support paths with "file:///" prepended, so it starts with "/".
+   */
+  private static String toRootNotebookFolder(String rootUri) {
+    if (rootUri.startsWith("file:")) {
+      return URI.create(rootUri).getPath();
+    }
+    return rootUri;
+  }
+
   private Map<String, NoteInfo> listFolder(FileObject fileObject) throws IOException {
     Map<String, NoteInfo> noteInfos = new HashMap<>();
 

zeppelin-server/src/test/java/org/apache/zeppelin/notebook/repo/GitNotebookRepoTest.java

@@ -96,6 +96,22 @@ public void tearDown() throws Exception {
     FileUtils.deleteDirectory(zeppelinDir);
   }
 
+  @Test
+  void notebookDirWithSpace() throws IOException {
+    File dirWithSpace = new File(zeppelinDir, "my notebooks");
+    FileUtils.moveDirectory(notebooksDir, dirWithSpace);
+    zConf.setProperty(ConfVars.ZEPPELIN_NOTEBOOK_DIR.getVarName(), dirWithSpace.getAbsolutePath());
+
+    notebookRepo = new GitNotebookRepo();
+    notebookRepo.init(zConf, noteParser);
+
+    // The repository is opened in the notebook dir, not in a sibling named after its URI form.
+    assertTrue(new File(dirWithSpace, ".git").isDirectory());
+    assertEquals(TEST_NOTE_PATH, notebookRepo.list(null).get(TEST_NOTE_ID).getPath());
+    notebookRepo.checkpoint(TEST_NOTE_ID, TEST_NOTE_PATH, "first commit", null);
+    assertEquals(1, notebookRepo.revisionHistory(TEST_NOTE_ID, TEST_NOTE_PATH, null).size());
+  }
+
   @Test
   void initNonemptyNotebookDir() throws IOException, GitAPIException {
     //given - .git does not exit

zeppelin-server/src/test/java/org/apache/zeppelin/notebook/repo/VFSNotebookRepoTest.java

@@ -30,16 +30,19 @@
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
 
 import java.io.File;
 import java.nio.file.Files;
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
+import java.nio.file.Path;
 import java.util.List;
 import java.util.Map;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
 class VFSNotebookRepoTest {
 
@@ -115,6 +118,28 @@ void testBasics() throws IOException {
     assertEquals(1, notebookRepo.list(AuthenticationInfo.ANONYMOUS).size());
   }
 
+  @Test
+  void testNotebookDirWithCharactersThatAUriDoesNotAllow(@TempDir Path tempDir) throws IOException {
+    // A space anywhere, or the backslashes of a Windows path such as the default "./\\notebook".
+    // Commons VFS treats a backslash as a separator, so only check that the repo works.
+    for (String dirName : new String[] {"my notebooks", "back\\slash"}) {
+      zConf.setProperty(ZeppelinConfiguration.ConfVars.ZEPPELIN_NOTEBOOK_DIR.getVarName(),
+          tempDir.resolve(dirName).toString());
+      VFSNotebookRepo repo = new VFSNotebookRepo();
+      repo.init(zConf, noteParser);
+
+      assertTrue(new File(repo.rootNotebookFolder).isDirectory(), dirName);
+      Note note = new Note();
+      note.setPath("/note1");
+      note.setNoteParser(noteParser);
+      repo.save(note, AuthenticationInfo.ANONYMOUS);
+      Map<String, NoteInfo> noteInfos = repo.list(AuthenticationInfo.ANONYMOUS);
+      assertEquals(1, noteInfos.size(), dirName);
+      assertEquals("/note1", noteInfos.get(note.getId()).getPath(), dirName);
+    }
+    assertTrue(tempDir.resolve("my notebooks").toFile().isDirectory());
+  }
+
   @Test
   void testNoteNameWithColon() throws IOException {
     assertEquals(0, notebookRepo.list(AuthenticationInfo.ANONYMOUS).size());