zeppelin-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From zjf...@apache.org
Subject [zeppelin] branch master updated: [ZEPPELIN-4051]. Commit note is broken
Date Tue, 12 Mar 2019 07:19:23 GMT
This is an automated email from the ASF dual-hosted git repository.

zjffdu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/zeppelin.git


The following commit(s) were added to refs/heads/master by this push:
     new 4f5297d  [ZEPPELIN-4051]. Commit note is broken
4f5297d is described below

commit 4f5297d0d33abfa588f1ee7aa860a4bec6bffd52
Author: Jeff Zhang <zjffdu@apache.org>
AuthorDate: Mon Mar 11 14:43:50 2019 +0800

    [ZEPPELIN-4051]. Commit note is broken
    
    ### What is this PR for?
    Commit note is broken due to wrong argument is passed (due to ZEPPELIN-2619). The correct
parameter passed to NotebookRepo should be note path instead of note name.
    
    ### What type of PR is it?
    [Bug Fix]
    
    ### Todos
    * [ ] - Task
    
    ### What is the Jira issue?
    * https://jira.apache.org/jira/browse/ZEPPELIN-4051
    
    ### How should this be tested?
    * CI 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: Jeff Zhang <zjffdu@apache.org>
    
    Closes #3328 from zjffdu/ZEPPELIN-4051 and squashes the following commits:
    
    9d35ef105 [Jeff Zhang] [ZEPPELIN-4051]. Commit note is broken
---
 .../zeppelin/notebook/repo/GitNotebookRepo.java    |  4 +--
 .../apache/zeppelin/service/NotebookService.java   |  2 +-
 .../apache/zeppelin/socket/NotebookServerTest.java | 30 ++++++++++++++++++++++
 .../src/test/resources/zeppelin-site.xml           |  2 +-
 .../src/app/notebook/notebook.controller.js        |  2 +-
 .../org/apache/zeppelin/notebook/Notebook.java     | 12 ++++-----
 .../zeppelin/notebook/repo/NotebookRepoSync.java   | 16 ++++++------
 .../repo/NotebookRepoWithVersionControl.java       | 16 ++++++------
 .../org/apache/zeppelin/notebook/NotebookTest.java |  8 +++---
 9 files changed, 61 insertions(+), 31 deletions(-)

diff --git a/zeppelin-plugins/notebookrepo/git/src/main/java/org/apache/zeppelin/notebook/repo/GitNotebookRepo.java
b/zeppelin-plugins/notebookrepo/git/src/main/java/org/apache/zeppelin/notebook/repo/GitNotebookRepo.java
index 2da91fc..322d692 100644
--- a/zeppelin-plugins/notebookrepo/git/src/main/java/org/apache/zeppelin/notebook/repo/GitNotebookRepo.java
+++ b/zeppelin-plugins/notebookrepo/git/src/main/java/org/apache/zeppelin/notebook/repo/GitNotebookRepo.java
@@ -214,10 +214,10 @@ public class GitNotebookRepo extends VFSNotebookRepo implements NotebookRepoWith
   }
 
   @Override
-  public Note setNoteRevision(String noteId, String noteName, String revId,
+  public Note setNoteRevision(String noteId, String notePath, String revId,
                               AuthenticationInfo subject)
       throws IOException {
-    Note revisionNote = get(noteId, noteName, revId, subject);
+    Note revisionNote = get(noteId, notePath, revId, subject);
     if (revisionNote != null) {
       save(revisionNote, subject);
     }
diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java
b/zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java
index 62973ff..ea4df53 100644
--- a/zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java
+++ b/zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java
@@ -700,7 +700,7 @@ public class NotebookService {
     }
 
     NotebookRepoWithVersionControl.Revision revision =
-        notebook.checkpointNote(noteId, note.getName(), commitMessage, context.getAutheInfo());
+        notebook.checkpointNote(noteId, note.getPath(), commitMessage, context.getAutheInfo());
     callback.onSuccess(revision, context);
     return revision;
   }
diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/socket/NotebookServerTest.java
b/zeppelin-server/src/test/java/org/apache/zeppelin/socket/NotebookServerTest.java
index 0760e6a..1e9071d 100644
--- a/zeppelin-server/src/test/java/org/apache/zeppelin/socket/NotebookServerTest.java
+++ b/zeppelin-server/src/test/java/org/apache/zeppelin/socket/NotebookServerTest.java
@@ -55,6 +55,8 @@ import org.apache.zeppelin.notebook.Note;
 import org.apache.zeppelin.notebook.Notebook;
 import org.apache.zeppelin.notebook.NotebookAuthorization;
 import org.apache.zeppelin.notebook.Paragraph;
+import org.apache.zeppelin.notebook.repo.NotebookRepoWithVersionControl;
+import org.apache.zeppelin.notebook.repo.zeppelinhub.security.Authentication;
 import org.apache.zeppelin.notebook.socket.Message;
 import org.apache.zeppelin.notebook.socket.Message.OP;
 import org.apache.zeppelin.rest.AbstractTestRestApi;
@@ -695,6 +697,34 @@ public class NotebookServerTest extends AbstractTestRestApi {
     assertNotNull(user1Id + " can get " + user2Id + "'s shared note", paragraphList2);
   }
 
+  @Test
+  public void testNoteRevision() throws IOException {
+    Note note = notebook.createNote("note1", anonymous);
+    assertEquals(0, note.getParagraphCount());
+    NotebookRepoWithVersionControl.Revision firstRevision = notebook.checkpointNote(note.getId(),
note.getPath(), "first commit", AuthenticationInfo.ANONYMOUS);
+    List<NotebookRepoWithVersionControl.Revision> revisionList = notebook.listRevisionHistory(note.getId(),
note.getPath(), AuthenticationInfo.ANONYMOUS);
+    assertEquals(1, revisionList.size());
+    assertEquals(firstRevision.id, revisionList.get(0).id);
+    assertEquals("first commit", revisionList.get(0).message);
+
+    // add one new paragraph and commit it
+    note.addNewParagraph(AuthenticationInfo.ANONYMOUS);
+    notebook.saveNote(note, AuthenticationInfo.ANONYMOUS);
+    assertEquals(1, note.getParagraphCount());
+    NotebookRepoWithVersionControl.Revision secondRevision = notebook.checkpointNote(note.getId(),
note.getPath(), "second commit", AuthenticationInfo.ANONYMOUS);
+
+    revisionList = notebook.listRevisionHistory(note.getId(), note.getPath(), AuthenticationInfo.ANONYMOUS);
+    assertEquals(2, revisionList.size());
+    assertEquals(secondRevision.id, revisionList.get(0).id);
+    assertEquals("second commit", revisionList.get(0).message);
+    assertEquals(firstRevision.id, revisionList.get(1).id);
+    assertEquals("first commit", revisionList.get(1).message);
+
+    // checkout the first commit
+    note = notebook.getNoteByRevision(note.getId(), note.getPath(), firstRevision.id, AuthenticationInfo.ANONYMOUS);
+    assertEquals(0, note.getParagraphCount());
+  }
+
   private NotebookSocket createWebSocket() {
     NotebookSocket sock = mock(NotebookSocket.class);
     when(sock.getRequest()).thenReturn(mockRequest);
diff --git a/zeppelin-server/src/test/resources/zeppelin-site.xml b/zeppelin-server/src/test/resources/zeppelin-site.xml
index 3215c5b..e46fce7 100644
--- a/zeppelin-server/src/test/resources/zeppelin-site.xml
+++ b/zeppelin-server/src/test/resources/zeppelin-site.xml
@@ -73,7 +73,7 @@
 
 <property>
   <name>zeppelin.notebook.storage</name>
-  <value>org.apache.zeppelin.notebook.repo.VFSNotebookRepo</value>
+  <value>org.apache.zeppelin.notebook.repo.GitNotebookRepo</value>
   <description>notebook persistence layer implementation</description>
 </property>
 
diff --git a/zeppelin-web/src/app/notebook/notebook.controller.js b/zeppelin-web/src/app/notebook/notebook.controller.js
index 426667d..085c94e 100644
--- a/zeppelin-web/src/app/notebook/notebook.controller.js
+++ b/zeppelin-web/src/app/notebook/notebook.controller.js
@@ -289,7 +289,7 @@ function NotebookCtrl($scope, $route, $routeParams, $location, $rootScope,
       message: 'Commit note to current repository?',
       callback: function(result) {
         if (result) {
-          websocketMsgSrv.checkpointNote($routeParams.noteId, $routeParams.name, commitMessage);
+          websocketMsgSrv.checkpointNote($routeParams.noteId, commitMessage);
         }
       },
     });
diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Notebook.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Notebook.java
index ab32059..e7f5ff0 100644
--- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Notebook.java
+++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Notebook.java
@@ -325,32 +325,32 @@ public class Notebook {
     }
   }
 
-  public Revision checkpointNote(String noteId, String noteName, String checkpointMessage,
+  public Revision checkpointNote(String noteId, String notePath, String checkpointMessage,
       AuthenticationInfo subject) throws IOException {
     if (((NotebookRepoSync) notebookRepo).isRevisionSupportedInDefaultRepo()) {
       return ((NotebookRepoWithVersionControl) notebookRepo)
-          .checkpoint(noteId, noteName, checkpointMessage, subject);
+          .checkpoint(noteId, notePath, checkpointMessage, subject);
     } else {
       return null;
     }
   }
 
   public List<Revision> listRevisionHistory(String noteId,
-                                            String noteName,
+                                            String notePath,
                                             AuthenticationInfo subject) throws IOException
{
     if (((NotebookRepoSync) notebookRepo).isRevisionSupportedInDefaultRepo()) {
       return ((NotebookRepoWithVersionControl) notebookRepo)
-          .revisionHistory(noteId, noteName, subject);
+          .revisionHistory(noteId, notePath, subject);
     } else {
       return null;
     }
   }
 
-  public Note setNoteRevision(String noteId, String noteName, String revisionId, AuthenticationInfo
subject)
+  public Note setNoteRevision(String noteId, String notePath, String revisionId, AuthenticationInfo
subject)
       throws IOException {
     if (((NotebookRepoSync) notebookRepo).isRevisionSupportedInDefaultRepo()) {
       return ((NotebookRepoWithVersionControl) notebookRepo)
-          .setNoteRevision(noteId, noteName, revisionId, subject);
+          .setNoteRevision(noteId, notePath, revisionId, subject);
     } else {
       return null;
     }
diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/NotebookRepoSync.java
b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/NotebookRepoSync.java
index be20cfb..b6efdc3 100644
--- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/NotebookRepoSync.java
+++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/NotebookRepoSync.java
@@ -483,7 +483,7 @@ public class NotebookRepoSync implements NotebookRepoWithVersionControl
{
 
   //checkpoint to all available storages
   @Override
-  public Revision checkpoint(String noteId, String noteName, String checkpointMsg, AuthenticationInfo
subject)
+  public Revision checkpoint(String noteId, String notePath, String checkpointMsg, AuthenticationInfo
subject)
       throws IOException {
     int repoCount = getRepoCount();
     int repoBound = Math.min(repoCount, getMaxRepoNum());
@@ -496,7 +496,7 @@ public class NotebookRepoSync implements NotebookRepoWithVersionControl
{
         if (isRevisionSupportedInRepo(i)) {
           allRepoCheckpoints
               .add(((NotebookRepoWithVersionControl) getRepo(i))
-                  .checkpoint(noteId, noteName, checkpointMsg, subject));
+                  .checkpoint(noteId, notePath, checkpointMsg, subject));
         }
       } catch (IOException e) {
         LOGGER.warn("Couldn't checkpoint in {} storage with index {} for note {}",
@@ -521,11 +521,11 @@ public class NotebookRepoSync implements NotebookRepoWithVersionControl
{
   }
 
   @Override
-  public Note get(String noteId, String noteName, String revId, AuthenticationInfo subject)
{
+  public Note get(String noteId, String notePath, String revId, AuthenticationInfo subject)
{
     Note revisionNote = null;
     try {
       if (isRevisionSupportedInDefaultRepo()) {
-        revisionNote = ((NotebookRepoWithVersionControl) getRepo(0)).get(noteId, noteName,
+        revisionNote = ((NotebookRepoWithVersionControl) getRepo(0)).get(noteId, notePath,
             revId, subject);
       }
     } catch (IOException e) {
@@ -535,13 +535,13 @@ public class NotebookRepoSync implements NotebookRepoWithVersionControl
{
   }
 
   @Override
-  public List<Revision> revisionHistory(String noteId, String noteName,
+  public List<Revision> revisionHistory(String noteId, String notePath,
                                         AuthenticationInfo subject) {
     List<Revision> revisions = Collections.emptyList();
     try {
       if (isRevisionSupportedInDefaultRepo()) {
         revisions = ((NotebookRepoWithVersionControl) getRepo(0))
-            .revisionHistory(noteId, noteName, subject);
+            .revisionHistory(noteId, notePath, subject);
       }
     } catch (IOException e) {
       LOGGER.error("Failed to list revision history", e);
@@ -570,7 +570,7 @@ public class NotebookRepoSync implements NotebookRepoWithVersionControl
{
   }
 
   @Override
-  public Note setNoteRevision(String noteId, String noteName, String revId, AuthenticationInfo
subject)
+  public Note setNoteRevision(String noteId, String notePath, String revId, AuthenticationInfo
subject)
       throws IOException {
     int repoCount = getRepoCount();
     int repoBound = Math.min(repoCount, getMaxRepoNum());
@@ -579,7 +579,7 @@ public class NotebookRepoSync implements NotebookRepoWithVersionControl
{
       try {
         if (isRevisionSupportedInRepo(i)) {
           currentNote = ((NotebookRepoWithVersionControl) getRepo(i))
-              .setNoteRevision(noteId, noteName, revId, subject);
+              .setNoteRevision(noteId, notePath, revId, subject);
         }
       } catch (IOException e) {
         // already logged
diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/NotebookRepoWithVersionControl.java
b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/NotebookRepoWithVersionControl.java
index ba5f4cf..a734c1a 100644
--- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/NotebookRepoWithVersionControl.java
+++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/NotebookRepoWithVersionControl.java
@@ -35,13 +35,13 @@ public interface NotebookRepoWithVersionControl extends NotebookRepo {
   /**
    * chekpoint (set revision) for notebook.
    * @param noteId Id of the note
-   * @param noteName name of the note
+   * @param notePath path of the note
    * @param checkpointMsg message description of the checkpoint
    * @return Rev
    * @throws IOException
    */
   @ZeppelinApi Revision checkpoint(String noteId,
-                                   String noteName,
+                                   String notePath,
                                    String checkpointMsg,
                                    AuthenticationInfo subject) throws IOException;
 
@@ -49,36 +49,36 @@ public interface NotebookRepoWithVersionControl extends NotebookRepo {
    * Get particular revision of the Notebook.
    * 
    * @param noteId Id of the note
-   * @param noteName name of the note
+   * @param notePath path of the note
    * @param revId revision of the Notebook
    * @return a Notebook
    * @throws IOException
    */
-  @ZeppelinApi Note get(String noteId, String noteName, String revId, AuthenticationInfo
subject)
+  @ZeppelinApi Note get(String noteId, String notePath, String revId, AuthenticationInfo
subject)
       throws IOException;
 
   /**
    * List of revisions of the given Notebook.
    * 
    * @param noteId id of the note
-   * @param noteName name of the note
+   * @param notePath path of the note
    * @param subject
    * @return list of revisions
    */
   @ZeppelinApi List<Revision> revisionHistory(String noteId,
-                                              String noteName,
+                                              String notePath,
                                               AuthenticationInfo subject) throws IOException;
 
   /**
    * Set note to particular revision.
    * 
    * @param noteId Id of the Notebook
-   * @param noteName name of the note
+   * @param notePath path of the note
    * @param revId revision of the Notebook
    * @return a Notebook
    * @throws IOException
    */
-  @ZeppelinApi Note setNoteRevision(String noteId, String noteName, String revId,
+  @ZeppelinApi Note setNoteRevision(String noteId, String notePath, String revId,
                                     AuthenticationInfo subject) throws IOException;
 
   /**
diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java
b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java
index 8b1527d..7feb1e8 100644
--- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java
+++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java
@@ -187,23 +187,23 @@ public class NotebookTest extends AbstractInterpreterTest implements
ParagraphJo
       NotebookRepoWithVersionControl {
 
     @Override
-    public Revision checkpoint(String noteId, String noteName, String checkpointMsg, AuthenticationInfo
subject)
+    public Revision checkpoint(String noteId, String notePath, String checkpointMsg, AuthenticationInfo
subject)
         throws IOException {
       return null;
     }
 
     @Override
-    public Note get(String noteId, String noteName, String revId, AuthenticationInfo subject)
throws IOException {
+    public Note get(String noteId, String notePath, String revId, AuthenticationInfo subject)
throws IOException {
       return null;
     }
 
     @Override
-    public List<Revision> revisionHistory(String noteId, String noteName, AuthenticationInfo
subject) {
+    public List<Revision> revisionHistory(String noteId, String notePath, AuthenticationInfo
subject) {
       return null;
     }
 
     @Override
-    public Note setNoteRevision(String noteId, String noteName, String revId, AuthenticationInfo
subject) throws
+    public Note setNoteRevision(String noteId, String notePath, String revId, AuthenticationInfo
subject) throws
         IOException {
       return null;
     }


Mime
View raw message