[ZEPPELIN-1257] Decouple revision id handling

zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java

@@ -45,8 +45,8 @@
 import org.apache.zeppelin.interpreter.InterpreterSetting;
 import org.apache.zeppelin.interpreter.remote.RemoteInterpreterProcessListener;
 import org.apache.zeppelin.notebook.*;
-import org.apache.zeppelin.notebook.repo.NotebookRepo;
-import org.apache.zeppelin.notebook.repo.NotebookRepo.Revision;
+import org.apache.zeppelin.notebook.repo.revision.Revision;
+import org.apache.zeppelin.notebook.repo.revision.RevisionId;
 import org.apache.zeppelin.notebook.socket.Message;
 import org.apache.zeppelin.notebook.socket.Message.OP;
 import org.apache.zeppelin.scheduler.Job;
@@ -1136,7 +1136,7 @@ private void checkpointNotebook(NotebookSocket conn, Notebook notebook,
     AuthenticationInfo subject = new AuthenticationInfo(fromMessage.principal);
     Revision revision = notebook.checkpointNote(noteId, commitMessage, subject);
     if (revision != null) {
-      List<NotebookRepo.Revision> revisions = notebook.listRevisionHistory(noteId, subject);
+      List<Revision> revisions = notebook.listRevisionHistory(noteId, subject);
       conn.send(serializeMessage(new Message(OP.LIST_REVISION_HISTORY)
         .put("revisionList", revisions)));
     }
@@ -1146,7 +1146,7 @@ private void listRevisionHistory(NotebookSocket conn, Notebook notebook,
       Message fromMessage) throws IOException {
     String noteId = (String) fromMessage.get("noteId");
     AuthenticationInfo subject = new AuthenticationInfo(fromMessage.principal);
-    List<NotebookRepo.Revision> revisions = notebook.listRevisionHistory(noteId, subject);
+    List<Revision> revisions = notebook.listRevisionHistory(noteId, subject);
 
     conn.send(serializeMessage(new Message(OP.LIST_REVISION_HISTORY)
       .put("revisionList", revisions)));
@@ -1155,7 +1155,7 @@ private void listRevisionHistory(NotebookSocket conn, Notebook notebook,
   private void getNoteRevision(NotebookSocket conn, Notebook notebook, Message fromMessage)
       throws IOException {
     String noteId = (String) fromMessage.get("noteId");
-    Revision revision = (Revision) fromMessage.get("revision");
+    RevisionId<?> revision = (RevisionId<?>) fromMessage.get("revisionId");
     AuthenticationInfo subject = new AuthenticationInfo(fromMessage.principal);
     Note revisionNote = notebook.getNoteRevision(noteId, revision, subject);
     conn.send(serializeMessage(new Message(OP.NOTE_REVISION)

zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Notebook.java

@@ -55,7 +55,8 @@
 import org.apache.zeppelin.interpreter.InterpreterSetting;
 import org.apache.zeppelin.interpreter.remote.RemoteAngularObjectRegistry;
 import org.apache.zeppelin.notebook.repo.NotebookRepo;
-import org.apache.zeppelin.notebook.repo.NotebookRepo.Revision;
+import org.apache.zeppelin.notebook.repo.revision.Revision;
+import org.apache.zeppelin.notebook.repo.revision.RevisionId;
 import org.apache.zeppelin.notebook.repo.NotebookRepoSync;
 import org.apache.zeppelin.resource.ResourcePoolUtils;
 import org.apache.zeppelin.scheduler.Job;
@@ -358,14 +359,14 @@ public Revision checkpointNote(String noteId, String checkpointMessage,
     return notebookRepo.checkpoint(noteId, checkpointMessage, subject);
   }
 
-  public List<NotebookRepo.Revision> listRevisionHistory(String noteId,
+  public List<Revision> listRevisionHistory(String noteId,
       AuthenticationInfo subject) {
     return notebookRepo.revisionHistory(noteId, subject);
   }
 
-  public Note getNoteRevision(String noteId, Revision revision, AuthenticationInfo subject)
+  public Note getNoteRevision(String noteId, RevisionId<?> revisionId, AuthenticationInfo subject)
       throws IOException {
-    return notebookRepo.get(noteId, revision, subject);
+    return notebookRepo.get(noteId, revisionId, subject);
   }
 
   @SuppressWarnings("rawtypes")

zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/AzureNotebookRepo.java

@@ -29,6 +29,8 @@
 import org.apache.zeppelin.notebook.NoteInfo;
 import org.apache.zeppelin.notebook.NotebookImportDeserializer;
 import org.apache.zeppelin.notebook.Paragraph;
+import org.apache.zeppelin.notebook.repo.revision.Revision;
+import org.apache.zeppelin.notebook.repo.revision.RevisionId;
 import org.apache.zeppelin.scheduler.Job;
 import org.apache.zeppelin.user.AuthenticationInfo;
 import org.slf4j.Logger;
@@ -217,7 +219,8 @@ public Revision checkpoint(String noteId, String checkpointMsg, AuthenticationIn
   }
 
   @Override
-  public Note get(String noteId, Revision rev, AuthenticationInfo subject) throws IOException {
+  public Note get(String noteId, RevisionId<?> revId, AuthenticationInfo subject)
+      throws IOException {
     // Auto-generated method stub
     return null;
   }

zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/GitNotebookRepo.java

@@ -24,6 +24,8 @@
 
 import org.apache.zeppelin.conf.ZeppelinConfiguration;
 import org.apache.zeppelin.notebook.Note;
+import org.apache.zeppelin.notebook.repo.revision.Revision;
+import org.apache.zeppelin.notebook.repo.revision.RevisionId;
 import org.apache.zeppelin.user.AuthenticationInfo;
 import org.eclipse.jgit.api.Git;
 import org.eclipse.jgit.api.errors.GitAPIException;
@@ -90,7 +92,8 @@ public Revision checkpoint(String pattern, String commitMessage, AuthenticationI
         DirCache added = git.add().addFilepattern(pattern).call();
         LOG.debug("{} changes are about to be commited", added.getEntryCount());
         RevCommit commit = git.commit().setMessage(commitMessage).call();
-        revision = new Revision(commit.getName(), commit.getShortMessage(), commit.getCommitTime());
+        RevisionId<String> revisionId = new RevisionId<String>(commit.getName());
+        revision = new Revision(revisionId, commit.getShortMessage(), commit.getCommitTime());
       } else {
         LOG.debug("No changes found {}", pattern);
       }
@@ -108,10 +111,16 @@ public Revision checkpoint(String pattern, String commitMessage, AuthenticationI
    * 4. apply stash on top and remove it
    */
   @Override
-  public synchronized Note get(String noteId, Revision rev, AuthenticationInfo subject)
+  public synchronized Note get(String noteId, RevisionId<?> revId, AuthenticationInfo subject)
       throws IOException {
     Note note = null;
     RevCommit stash = null;
+    Object revisionId = revId.getId();
+    if (!(revisionId instanceof String)) {
+      LOG.warn("Failed to get revision {} of note {}: id of "
+          + "revision isn't string type", revisionId, noteId);
+      return note;
+    }
     try {
       List<DiffEntry> gitDiff = git.diff().setPathFilter(PathFilter.create(noteId)).call();
       boolean modified = !gitDiff.isEmpty();
@@ -123,7 +132,7 @@ public synchronized Note get(String noteId, Revision rev, AuthenticationInfo sub
       }
       ObjectId head = git.getRepository().resolve(Constants.HEAD);
       // checkout to target revision
-      git.checkout().setStartPoint(rev.id).addPath(noteId).call();
+      git.checkout().setStartPoint((String) revisionId).addPath(noteId).call();
       // get the note
       note = super.get(noteId, subject);
       // checkout back to head
@@ -137,7 +146,7 @@ public synchronized Note get(String noteId, Revision rev, AuthenticationInfo sub
             stashes.size());
       }
     } catch (GitAPIException e) {
-      LOG.error("Failed to return note from revision \"{}\"", rev.message, e);
+      LOG.error("Failed to return note from revision \"{}\"", revisionId, e);
     }
     return note;
   }
@@ -149,7 +158,8 @@ public List<Revision> revisionHistory(String noteId, AuthenticationInfo subject)
     try {
       Iterable<RevCommit> logs = git.log().addPath(noteId).call();
       for (RevCommit log: logs) {
-        history.add(new Revision(log.getName(), log.getShortMessage(), log.getCommitTime()));
+        RevisionId<String> revisionId = new RevisionId<String>(log.getName());
+        history.add(new Revision(revisionId, log.getShortMessage(), log.getCommitTime()));
         LOG.debug(" - ({},{},{})", log.getName(), log.getCommitTime(), log.getFullMessage());
       }
     } catch (NoHeadException e) {

zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/NotebookRepo.java

@@ -23,6 +23,8 @@
 import org.apache.zeppelin.annotation.ZeppelinApi;
 import org.apache.zeppelin.notebook.Note;
 import org.apache.zeppelin.notebook.NoteInfo;
+import org.apache.zeppelin.notebook.repo.revision.Revision;
+import org.apache.zeppelin.notebook.repo.revision.RevisionId;
 import org.apache.zeppelin.user.AuthenticationInfo;
 
 /**
@@ -89,7 +91,7 @@ public interface NotebookRepo {
    * @return a Notebook
    * @throws IOException
    */
-  @ZeppelinApi public Note get(String noteId, Revision rev, AuthenticationInfo subject)
+  @ZeppelinApi public Note get(String noteId, RevisionId<?> revId, AuthenticationInfo subject)
       throws IOException;
 
   /**
@@ -100,18 +102,4 @@ public interface NotebookRepo {
    */
   @ZeppelinApi public List<Revision> revisionHistory(String noteId, AuthenticationInfo subject);
 
-  /**
-   * Represents the 'Revision' a point in life of the notebook
-   */
-  static class Revision {
-    public Revision(String revId, String message, int time) {
-      this.id = revId;
-      this.message = message;
-      this.time = time;
-    }
-    public String id;
-    public String message;
-    public int time;
-  }
-
 }

zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/NotebookRepoSync.java

@@ -32,6 +32,8 @@
 import org.apache.zeppelin.notebook.Note;
 import org.apache.zeppelin.notebook.NoteInfo;
 import org.apache.zeppelin.notebook.Paragraph;
+import org.apache.zeppelin.notebook.repo.revision.Revision;
+import org.apache.zeppelin.notebook.repo.revision.RevisionId;
 import org.apache.zeppelin.user.AuthenticationInfo;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -355,12 +357,12 @@ public Revision checkpoint(String noteId, String checkpointMsg, AuthenticationIn
   }
 
   @Override
-  public Note get(String noteId, Revision rev, AuthenticationInfo subject) {
+  public Note get(String noteId, RevisionId<?> revId, AuthenticationInfo subject) {
     Note revisionNote = null;
     try {
-      revisionNote = getRepo(0).get(noteId, rev, subject);
+      revisionNote = getRepo(0).get(noteId, revId, subject);
     } catch (IOException e) {
-      LOG.error("Failed to get revision {} of note {}", rev.id, noteId, e);
+      LOG.error("Failed to get revision {} of note {}", revId.getId(), noteId, e);
     }
     return revisionNote;
   }

zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/S3NotebookRepo.java

@@ -39,6 +39,8 @@
 import org.apache.zeppelin.notebook.NoteInfo;
 import org.apache.zeppelin.notebook.NotebookImportDeserializer;
 import org.apache.zeppelin.notebook.Paragraph;
+import org.apache.zeppelin.notebook.repo.revision.Revision;
+import org.apache.zeppelin.notebook.repo.revision.RevisionId;
 import org.apache.zeppelin.scheduler.Job.Status;
 import org.apache.zeppelin.user.AuthenticationInfo;
 import org.slf4j.Logger;
@@ -260,7 +262,8 @@ public Revision checkpoint(String noteId, String checkpointMsg, AuthenticationIn
   }
 
   @Override
-  public Note get(String noteId, Revision rev, AuthenticationInfo subject) throws IOException {
+  public Note get(String noteId, RevisionId<?> revId, AuthenticationInfo subject)
+      throws IOException {
     // Auto-generated method stub
     return null;
   }

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

@@ -41,6 +41,8 @@
 import org.apache.zeppelin.notebook.Note;
 import org.apache.zeppelin.notebook.NoteInfo;
 import org.apache.zeppelin.notebook.Paragraph;
+import org.apache.zeppelin.notebook.repo.revision.Revision;
+import org.apache.zeppelin.notebook.repo.revision.RevisionId;
 import org.apache.zeppelin.notebook.NotebookImportDeserializer;
 import org.apache.zeppelin.scheduler.Job.Status;
 import org.apache.zeppelin.user.AuthenticationInfo;
@@ -274,7 +276,8 @@ public Revision checkpoint(String noteId, String checkpointMsg, AuthenticationIn
   }
 
   @Override
-  public Note get(String noteId, Revision rev, AuthenticationInfo subject) throws IOException {
+  public Note get(String noteId, RevisionId<?> revId, AuthenticationInfo subject)
+      throws IOException {
     // Auto-generated method stub
     return null;
   }

zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/revision/Revision.java

@@ -0,0 +1,42 @@
+package org.apache.zeppelin.notebook.repo.revision;
+
+/**
+ * Represents the 'Revision' a point in life of the notebook
+ */
+
+public class Revision {
+  private RevisionId<?> id;
+  private String message;
+  private int time;
+
+  public Revision(RevisionId<?> revId, String message, int time) {
+    this.setRevisionId(revId);
+    this.setMessage(message);
+    this.setTime(time);
+  }
+
+  public RevisionId<?> getRevisionId() {
+    return id;
+  }
+
+  public void setRevisionId(RevisionId<?> id) {
+    this.id = id;
+  }
+
+  public String getMessage() {
+    return message;
+  }
+
+  public void setMessage(String message) {
+    this.message = message;
+  }
+
+  public int getTime() {
+    return time;
+  }
+
+  public void setTime(int time) {
+    this.time = time;
+  }
+
+}

zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/revision/RevisionId.java

@@ -0,0 +1,25 @@
+package org.apache.zeppelin.notebook.repo.revision;
+
+/**
+ * Represents Revision id element. This is generic class and can be fed any 
+ * revision id class implemented on developers side. Also all the compare and equals methods 
+ * can be provided by the generic class.
+ * @param <T> tag for generic id class.
+ */
+
+public class RevisionId <T> {
+  private T id;
+
+  public RevisionId(T id) {
+    this.setId(id);
+  }
+
+  public T getId() {
+    return id;
+  }
+
+  public void setId(T id) {
+    this.id = id;
+  }
+
+}

zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/zeppelinhub/ZeppelinHubRepo.java

@@ -27,6 +27,8 @@
 import org.apache.zeppelin.notebook.Note;
 import org.apache.zeppelin.notebook.NoteInfo;
 import org.apache.zeppelin.notebook.repo.NotebookRepo;
+import org.apache.zeppelin.notebook.repo.revision.Revision;
+import org.apache.zeppelin.notebook.repo.revision.RevisionId;
 import org.apache.zeppelin.notebook.repo.zeppelinhub.rest.ZeppelinhubRestApiHandler;
 import org.apache.zeppelin.notebook.repo.zeppelinhub.websocket.Client;
 import org.apache.zeppelin.user.AuthenticationInfo;
@@ -196,7 +198,8 @@ public Revision checkpoint(String noteId, String checkpointMsg, AuthenticationIn
   }
 
   @Override
-  public Note get(String noteId, Revision rev, AuthenticationInfo subject) throws IOException {
+  public Note get(String noteId, RevisionId<?> revId, AuthenticationInfo subject)
+      throws IOException {
     // Auto-generated method stub
     return null;
   }