zeppelin git commit: Get note revision of note - git repo

[bz...@apache.org]

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 <kh...@nflabs.com>

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 <kh...@nflabs.com>
Authored: Tue Jun 21 14:43:14 2016 -0700
Committer: Alexander Bezzubov <bz...@apache.org>
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();
+  }
+
 }