From commits-return-5316-archive-asf-public=cust-asf.ponee.io@zeppelin.apache.org Wed Mar 13 01:41:19 2019 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by mx-eu-01.ponee.io (Postfix) with SMTP id BCFB1180763 for ; Wed, 13 Mar 2019 02:41:18 +0100 (CET) Received: (qmail 70438 invoked by uid 500); 13 Mar 2019 01:41:17 -0000 Mailing-List: contact commits-help@zeppelin.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@zeppelin.apache.org Delivered-To: mailing list commits@zeppelin.apache.org Received: (qmail 70424 invoked by uid 99); 13 Mar 2019 01:41:17 -0000 Received: from ec2-52-202-80-70.compute-1.amazonaws.com (HELO gitbox.apache.org) (52.202.80.70) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 13 Mar 2019 01:41:17 +0000 Received: by gitbox.apache.org (ASF Mail Server at gitbox.apache.org, from userid 33) id 2D2D2879DB; Wed, 13 Mar 2019 01:41:17 +0000 (UTC) Date: Wed, 13 Mar 2019 01:41:16 +0000 To: "commits@zeppelin.apache.org" Subject: [zeppelin] branch master updated: [ZEPPELIN-4028]. Pop up error message if user try to create duplicated note MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Message-ID: <155244127666.27046.2165552215522034764@gitbox.apache.org> From: zjffdu@apache.org X-Git-Host: gitbox.apache.org X-Git-Repo: zeppelin X-Git-Refname: refs/heads/master X-Git-Reftype: branch X-Git-Oldrev: 3c7f0093f4e49f76c9a10a6a759f7c693f6c7eb5 X-Git-Newrev: 5e933befbf762eef201e6d9f2ca89d03e1f1221f X-Git-Rev: 5e933befbf762eef201e6d9f2ca89d03e1f1221f X-Git-NotificationType: ref_changed_plus_diff X-Git-Multimail-Version: 1.5.dev Auto-Submitted: auto-generated 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 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 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 @@ maven-surefire-plugin always - - 1 - 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 = 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 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 @@ ${project.build.directory}/tmp - - 1 - 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 {