diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/notebook/repo/VFSNotebookRepo.java b/zeppelin-server/src/main/java/org/apache/zeppelin/notebook/repo/VFSNotebookRepo.java index 420a1c148a9..8861aa50784 100644 --- a/zeppelin-server/src/main/java/org/apache/zeppelin/notebook/repo/VFSNotebookRepo.java +++ b/zeppelin-server/src/main/java/org/apache/zeppelin/notebook/repo/VFSNotebookRepo.java @@ -70,19 +70,16 @@ 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()) { @@ -90,19 +87,29 @@ protected void setNotebookDirectory(String notebookDirPath) throws IOException { 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 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 listFolder(FileObject fileObject) throws IOException { Map noteInfos = new HashMap<>(); diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/notebook/repo/GitNotebookRepoTest.java b/zeppelin-server/src/test/java/org/apache/zeppelin/notebook/repo/GitNotebookRepoTest.java index 3996ed2b03e..78a2bc6d6e9 100644 --- a/zeppelin-server/src/test/java/org/apache/zeppelin/notebook/repo/GitNotebookRepoTest.java +++ b/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 diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/notebook/repo/VFSNotebookRepoTest.java b/zeppelin-server/src/test/java/org/apache/zeppelin/notebook/repo/VFSNotebookRepoTest.java index c7dcc9bdc44..fb8d7d25270 100644 --- a/zeppelin-server/src/test/java/org/apache/zeppelin/notebook/repo/VFSNotebookRepoTest.java +++ b/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 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());