Repository: zeppelin Updated Branches: refs/heads/master 09f0ebda9 -> ab9e114c5
Get note revision of note - git repo ### What is this PR for? This PR implements the backend of git storage layer for `get(noteId, revision)`, so that you can get the note of certain revision. It doesn't provide frontend api yet, which is future work. ### What type of PR is it? Improvement ### Todos * [x] - implement `get` of GitNotebookRepo * [x] - add tests ### What is the Jira issue? N/A ### How should this be tested? two tests for `get` function should 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: Khalid Huseynov <[email protected]> Closes #1041 from khalidhuseynov/feat/git-get-note-revision-of-note and squashes the following commits: 93b90c9 [Khalid Huseynov] add javadoc bdc17c4 [Khalid Huseynov] add edge case fail test b87e145 [Khalid Huseynov] basic impl of get specific version of note 2dcca8d [Khalid Huseynov] change field name: revId -> id Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/ab9e114c Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/ab9e114c Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/ab9e114c Branch: refs/heads/master Commit: ab9e114c542159b828a511a0577a30f6f02ec48f Parents: 09f0ebd Author: Khalid Huseynov <[email protected]> Authored: Tue Jun 21 14:43:14 2016 -0700 Committer: Alexander Bezzubov <[email protected]> Committed: Mon Jun 27 16:26:29 2016 +0900 ---------------------------------------------------------------------- .../zeppelin/notebook/repo/GitNotebookRepo.java | 46 ++++++++- .../zeppelin/notebook/repo/NotebookRepo.java | 4 +- .../notebook/repo/GitNotebookRepoTest.java | 99 +++++++++++++++++++- 3 files changed, 143 insertions(+), 6 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/zeppelin/blob/ab9e114c/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/GitNotebookRepo.java ---------------------------------------------------------------------- diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/GitNotebookRepo.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/GitNotebookRepo.java index 243e4d0..4705980 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/GitNotebookRepo.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/GitNotebookRepo.java @@ -19,6 +19,7 @@ package org.apache.zeppelin.notebook.repo; import java.io.File; import java.io.IOException; +import java.util.Collection; import java.util.List; import org.apache.zeppelin.conf.ZeppelinConfiguration; @@ -30,8 +31,11 @@ import org.eclipse.jgit.api.errors.NoHeadException; import org.eclipse.jgit.diff.DiffEntry; import org.eclipse.jgit.dircache.DirCache; import org.eclipse.jgit.internal.storage.file.FileRepository; +import org.eclipse.jgit.lib.Constants; +import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.treewalk.filter.PathFilter; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -96,10 +100,46 @@ public class GitNotebookRepo extends VFSNotebookRepo { return revision; } + /** + * the idea is to: + * 1. stash current changes + * 2. remember head commit and checkout to the desired revision + * 3. get note and checkout back to the head + * 4. apply stash on top and remove it + */ @Override - public Note get(String noteId, Revision rev, AuthenticationInfo subject) throws IOException { - //TODO(bzz): something like 'git checkout rev', that will not change-the-world though - return super.get(noteId, subject); + public synchronized Note get(String noteId, Revision rev, AuthenticationInfo subject) + throws IOException { + Note note = null; + RevCommit stash = null; + try { + List<DiffEntry> gitDiff = git.diff().setPathFilter(PathFilter.create(noteId)).call(); + boolean modified = !gitDiff.isEmpty(); + if (modified) { + // stash changes + stash = git.stashCreate().call(); + Collection<RevCommit> stashes = git.stashList().call(); + LOG.debug("Created stash : {}, stash size : {}", stash, stashes.size()); + } + ObjectId head = git.getRepository().resolve(Constants.HEAD); + // checkout to target revision + git.checkout().setStartPoint(rev.id).addPath(noteId).call(); + // get the note + note = super.get(noteId, subject); + // checkout back to head + git.checkout().setStartPoint(head.getName()).addPath(noteId).call(); + if (modified && stash != null) { + // unstash changes + ObjectId applied = git.stashApply().setStashRef(stash.getName()).call(); + ObjectId dropped = git.stashDrop().setStashRef(0).call(); + Collection<RevCommit> stashes = git.stashList().call(); + LOG.debug("Stash applied as : {}, and dropped : {}, stash size: {}", applied, dropped, + stashes.size()); + } + } catch (GitAPIException e) { + LOG.error("Failed to return note from revision \"{}\"", rev.message, e); + } + return note; } @Override http://git-wip-us.apache.org/repos/asf/zeppelin/blob/ab9e114c/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/NotebookRepo.java ---------------------------------------------------------------------- diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/NotebookRepo.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/NotebookRepo.java index 8624792..85df81f 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/NotebookRepo.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/NotebookRepo.java @@ -105,11 +105,11 @@ public interface NotebookRepo { */ static class Revision { public Revision(String revId, String message, int time) { - this.revId = revId; + this.id = revId; this.message = message; this.time = time; } - public String revId; + public String id; public String message; public int time; } http://git-wip-us.apache.org/repos/asf/zeppelin/blob/ab9e114c/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/GitNotebookRepoTest.java ---------------------------------------------------------------------- diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/GitNotebookRepoTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/GitNotebookRepoTest.java index 39db0fe..8e787a2 100644 --- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/GitNotebookRepoTest.java +++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/GitNotebookRepoTest.java @@ -25,6 +25,7 @@ import java.util.List; import java.util.Map; import org.apache.commons.io.FileUtils; +import org.apache.commons.lang.StringUtils; import org.apache.zeppelin.conf.ZeppelinConfiguration; import org.apache.zeppelin.conf.ZeppelinConfiguration.ConfVars; import org.apache.zeppelin.interpreter.mock.MockInterpreter1; @@ -80,7 +81,7 @@ public class GitNotebookRepoTest { @After public void tearDown() throws Exception { - NotebookRepoSyncTest.delete(zeppelinDir); + //NotebookRepoSyncTest.delete(zeppelinDir); } @Test @@ -156,4 +157,100 @@ public class GitNotebookRepoTest { } return false; } + + @Test + public void getRevisionTest() throws IOException { + // initial checks + notebookRepo = new GitNotebookRepo(conf); + assertThat(notebookRepo.list(null)).isNotEmpty(); + assertThat(containsNote(notebookRepo.list(null), TEST_NOTE_ID)).isTrue(); + assertThat(notebookRepo.revisionHistory(TEST_NOTE_ID, null)).isEmpty(); + + // add first checkpoint + Revision revision_1 = notebookRepo.checkpoint(TEST_NOTE_ID, "first commit", null); + assertThat(notebookRepo.revisionHistory(TEST_NOTE_ID, null).size()).isEqualTo(1); + int paragraphCount_1 = notebookRepo.get(TEST_NOTE_ID, null).getParagraphs().size(); + + // add paragraph and save + Note note = notebookRepo.get(TEST_NOTE_ID, null); + Paragraph p1 = note.addParagraph(); + Map<String, Object> config = p1.getConfig(); + config.put("enabled", true); + p1.setConfig(config); + p1.setText("%md checkpoint test text"); + notebookRepo.save(note, null); + + // second checkpoint + notebookRepo.checkpoint(TEST_NOTE_ID, "second commit", null); + assertThat(notebookRepo.revisionHistory(TEST_NOTE_ID, null).size()).isEqualTo(2); + int paragraphCount_2 = notebookRepo.get(TEST_NOTE_ID, null).getParagraphs().size(); + assertThat(paragraphCount_2).isEqualTo(paragraphCount_1 + 1); + + // get note from revision 1 + Note noteRevision_1 = notebookRepo.get(TEST_NOTE_ID, revision_1, null); + assertThat(noteRevision_1.getParagraphs().size()).isEqualTo(paragraphCount_1); + + // get current note + note = notebookRepo.get(TEST_NOTE_ID, null); + assertThat(note.getParagraphs().size()).isEqualTo(paragraphCount_2); + + // add one more paragraph and save + Paragraph p2 = note.addParagraph(); + config.put("enabled", false); + p2.setConfig(config); + p2.setText("%md get revision when modified note test text"); + notebookRepo.save(note, null); + note = notebookRepo.get(TEST_NOTE_ID, null); + int paragraphCount_3 = note.getParagraphs().size(); + assertThat(paragraphCount_3).isEqualTo(paragraphCount_2 + 1); + + // get revision 1 again + noteRevision_1 = notebookRepo.get(TEST_NOTE_ID, revision_1, null); + assertThat(noteRevision_1.getParagraphs().size()).isEqualTo(paragraphCount_1); + + // check that note is unchanged + note = notebookRepo.get(TEST_NOTE_ID, null); + assertThat(note.getParagraphs().size()).isEqualTo(paragraphCount_3); + } + + @Test + public void getRevisionFailTest() throws IOException { + // initial checks + notebookRepo = new GitNotebookRepo(conf); + assertThat(notebookRepo.list(null)).isNotEmpty(); + assertThat(containsNote(notebookRepo.list(null), TEST_NOTE_ID)).isTrue(); + assertThat(notebookRepo.revisionHistory(TEST_NOTE_ID, null)).isEmpty(); + + // add first checkpoint + Revision revision_1 = notebookRepo.checkpoint(TEST_NOTE_ID, "first commit", null); + assertThat(notebookRepo.revisionHistory(TEST_NOTE_ID, null).size()).isEqualTo(1); + int paragraphCount_1 = notebookRepo.get(TEST_NOTE_ID, null).getParagraphs().size(); + + // get current note + Note note = notebookRepo.get(TEST_NOTE_ID, null); + assertThat(note.getParagraphs().size()).isEqualTo(paragraphCount_1); + + // add one more paragraph and save + Paragraph p1 = note.addParagraph(); + Map<String, Object> config = p1.getConfig(); + config.put("enabled", true); + p1.setConfig(config); + p1.setText("%md get revision when modified note test text"); + notebookRepo.save(note, null); + int paragraphCount_2 = note.getParagraphs().size(); + + // get note from revision 1 + Note noteRevision_1 = notebookRepo.get(TEST_NOTE_ID, revision_1, null); + assertThat(noteRevision_1.getParagraphs().size()).isEqualTo(paragraphCount_1); + + // get current note + note = notebookRepo.get(TEST_NOTE_ID, null); + assertThat(note.getParagraphs().size()).isEqualTo(paragraphCount_2); + + // test for absent revision + Revision absentRevision = new Revision("absentId", StringUtils.EMPTY, 0); + note = notebookRepo.get(TEST_NOTE_ID, absentRevision, null); + assertThat(note).isNull(); + } + }
