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-4028]. Pop up error message if user try to create duplicated note
Date Wed, 13 Mar 2019 01:41:16 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 5e933be  [ZEPPELIN-4028]. Pop up error message if user try to create duplicated note
5e933be is described below

commit 5e933befbf762eef201e6d9f2ca89d03e1f1221f
Author: Jeff Zhang <zjffdu@apache.org>
AuthorDate: Tue Mar 5 12:02:31 2019 +0800

    [ZEPPELIN-4028]. Pop up error message if user try to create duplicated note
    
    ### What is this PR for?
    This PR fix the backend by disallowing create duplicated note. If user try to create duplicate
note, it throw pop up one error message in fronend.
    
    ### What type of PR is it?
    [Bug Fix]
    
    ### Todos
    * [ ] - Task
    
    ### What is the Jira issue?
    * https://jira.apache.org/jira/browse/ZEPPELIN-4028
    
    ### How should this be tested?
    * Unit test is added, and also manually verify it
    
    ### Screenshots (if appropriate)
    ![image](https://user-images.githubusercontent.com/164491/54021230-9e818b80-41ca-11e9-9c72-e811a01a5ada.png)
    
    ### 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 #3326 from zjffdu/ZEPPELIN-4028 and squashes the following commits:
    
    bbe9bd77f [Jeff Zhang] ZEPPELIN-4028. Same note name should not be allowed
---
 bin/interpreter.sh                                           |  4 +---
 zeppelin-interpreter-integration/pom.xml                     |  3 ---
 .../java/org/apache/zeppelin/service/NotebookService.java    |  2 +-
 .../org/apache/zeppelin/service/NotebookServiceTest.java     | 11 ++++++++++-
 .../java/org/apache/zeppelin/socket/NotebookServerTest.java  |  2 +-
 zeppelin-zengine/pom.xml                                     |  3 ---
 .../main/java/org/apache/zeppelin/notebook/NoteManager.java  |  8 +++++++-
 .../src/main/java/org/apache/zeppelin/notebook/Notebook.java |  2 +-
 .../test/java/org/apache/zeppelin/notebook/NotebookTest.java | 12 ++++++++++++
 9 files changed, 33 insertions(+), 14 deletions(-)

diff --git a/bin/interpreter.sh b/bin/interpreter.sh
index 46e25f9..0bbfd0c 100755
--- a/bin/interpreter.sh
+++ b/bin/interpreter.sh
@@ -79,9 +79,7 @@ fi
 # add test classes for unittest
 if [[ -d "${ZEPPELIN_HOME}/zeppelin-zengine/target/test-classes" ]]; then
   ZEPPELIN_INTP_CLASSPATH+=":${ZEPPELIN_HOME}/zeppelin-zengine/target/test-classes"
-  if [[ -n "${ZEPPELIN_ZENGINE_TEST}" ]]; then
-    addJarInDirForIntp "${ZEPPELIN_HOME}/zeppelin-zengine/target/test-classes"
-  fi
+  addJarInDirForIntp "${ZEPPELIN_HOME}/zeppelin-zengine/target/test-classes"
 fi
 
 addJarInDirForIntp "${ZEPPELIN_HOME}/zeppelin-interpreter-api/target"
diff --git a/zeppelin-interpreter-integration/pom.xml b/zeppelin-interpreter-integration/pom.xml
index 6140925..d66b279 100644
--- a/zeppelin-interpreter-integration/pom.xml
+++ b/zeppelin-interpreter-integration/pom.xml
@@ -143,9 +143,6 @@
         <artifactId>maven-surefire-plugin</artifactId>
         <configuration>
           <forkMode>always</forkMode>
-          <environmentVariables>
-            <ZEPPELIN_ZENGINE_TEST>1</ZEPPELIN_ZENGINE_TEST>
-          </environmentVariables>
         </configuration>
       </plugin>
       <plugin>
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 ea4df53..36eaa15 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
@@ -149,7 +149,7 @@ public class NotebookService {
       callback.onSuccess(note, context);
       return note;
     } catch (IOException e) {
-      callback.onFailure(new IOException("Fail to create note", e), context);
+      callback.onFailure(e, context);
       return null;
     }
   }
diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/service/NotebookServiceTest.java
b/zeppelin-server/src/test/java/org/apache/zeppelin/service/NotebookServiceTest.java
index 8962482..801cfeb 100644
--- a/zeppelin-server/src/test/java/org/apache/zeppelin/service/NotebookServiceTest.java
+++ b/zeppelin-server/src/test/java/org/apache/zeppelin/service/NotebookServiceTest.java
@@ -64,6 +64,7 @@ import org.apache.zeppelin.user.AuthenticationInfo;
 import org.apache.zeppelin.user.Credentials;
 import org.junit.Before;
 import org.junit.Test;
+import org.mockito.ArgumentCaptor;
 
 public class NotebookServiceTest {
 
@@ -129,6 +130,14 @@ public class NotebookServiceTest {
     assertEquals(1, note1.getParagraphCount());
     verify(callback).onSuccess(note1, context);
 
+    // create duplicated note
+    reset(callback);
+    Note note2 = notebookService.createNote("/folder_1/note1", "test", context, callback);
+    assertNull(note2);
+    ArgumentCaptor<Exception> exception = ArgumentCaptor.forClass(Exception.class);
+    verify(callback).onFailure(exception.capture(), any(ServiceContext.class));
+    assertTrue(exception.getValue().getCause().getMessage().equals("Note /folder_1/note1
existed"));
+
     // list note
     reset(callback);
     List<NoteInfo> notesInfo = notebookService.listNotesInfo(false, context, callback);
@@ -157,7 +166,7 @@ public class NotebookServiceTest {
     assertEquals("/folder_3/new_name", notesInfo.get(0).getPath());
 
     // create another note
-    Note note2 = notebookService.createNote("/note2", "test", context, callback);
+    note2 = notebookService.createNote("/note2", "test", context, callback);
     assertEquals("note2", note2.getName());
     verify(callback).onSuccess(note2, context);
 
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 1e9071d..4c69a42 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
@@ -595,7 +595,7 @@ public class NotebookServerTest extends AbstractTestRestApi {
         "{\"note\":{\"paragraphs\": [{\"text\": \"Test " +
         "paragraphs import\"," + "\"progressUpdateIntervalMs\":500," +
         "\"config\":{},\"settings\":{}}]," +
-        "\"name\": \"Test Zeppelin notebook import\",\"config\": " +
+        "\"name\": \"Test RuntimeInfos\",\"config\": " +
         "{}}}}";
     Message messageReceived = notebookServer.deserializeMessage(msg);
     Note note = null;
diff --git a/zeppelin-zengine/pom.xml b/zeppelin-zengine/pom.xml
index efcb57f..0b38c58 100644
--- a/zeppelin-zengine/pom.xml
+++ b/zeppelin-zengine/pom.xml
@@ -252,9 +252,6 @@
           <systemProperties>
             <java.io.tmpdir>${project.build.directory}/tmp</java.io.tmpdir>
           </systemProperties>
-          <environmentVariables>
-            <ZEPPELIN_ZENGINE_TEST>1</ZEPPELIN_ZENGINE_TEST>
-          </environmentVariables>
         </configuration>
       </plugin>
 
diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NoteManager.java
b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NoteManager.java
index edaabfe..4b6ba48 100644
--- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NoteManager.java
+++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NoteManager.java
@@ -114,7 +114,7 @@ public class NoteManager {
       }
     }
     if (checkDuplicates && curFolder.containsNote(tokens[tokens.length - 1])) {
-      throw new IOException("Note " + note.getPath() + " existed");
+      throw new IOException("Note '" + note.getPath() + "' existed");
     }
     curFolder.addNote(tokens[tokens.length -1], note);
     this.notesInfo.put(note.getId(), note.getPath());
@@ -167,6 +167,12 @@ public class NoteManager {
     note.setLoaded(true);
   }
 
+  public void addNote(Note note, AuthenticationInfo subject) throws IOException {
+    addOrUpdateNoteNode(note, true);
+    this.notebookRepo.save(note, subject);
+    note.setLoaded(true);
+  }
+
   /**
    * Add or update Note
    *
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 e7f5ff0..39c9b9b 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
@@ -183,7 +183,7 @@ public class Notebook {
     Note note =
         new Note(notePath, defaultInterpreterGroup, replFactory, interpreterSettingManager,
             paragraphJobListener, credentials, noteEventListeners);
-    saveNote(note, subject);
+    noteManager.addNote(note, subject);
     fireNoteCreateEvent(note, subject);
     return note;
   }
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 7feb1e8..e8cbf3c 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
@@ -1379,6 +1379,18 @@ public class NotebookTest extends AbstractInterpreterTest implements
ParagraphJo
     notebook.removeNote(note2.getId(), anonymous);
   }
 
+  @Test
+  public void testCreateDuplicateNote() throws Exception {
+    Note note1 = notebook.createNote("note1", anonymous);
+    try {
+      notebook.createNote("note1", anonymous);
+      fail("Should not be able to create same note 'note1'");
+    } catch (Exception e) {
+      assertTrue(e.getMessage().contains("Note '/note1' existed"));
+    } finally {
+      notebook.removeNote(note1.getId(), anonymous);
+    }
+  }
 
   @Test
   public void testGetAllNotesWithDifferentPermissions() throws IOException {


Mime
View raw message