Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id 83BA1200C09 for ; Tue, 10 Jan 2017 19:33:53 +0100 (CET) Received: by cust-asf.ponee.io (Postfix) id 824E1160B3D; Tue, 10 Jan 2017 18:33:53 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id 82CB7160B2C for ; Tue, 10 Jan 2017 19:33:52 +0100 (CET) Received: (qmail 96198 invoked by uid 500); 10 Jan 2017 18:33:51 -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 96188 invoked by uid 99); 10 Jan 2017 18:33:51 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 10 Jan 2017 18:33:51 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 86CF3DFA98; Tue, 10 Jan 2017 18:33:51 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: moon@apache.org To: commits@zeppelin.apache.org Message-Id: <7d3d26736d184adebf735279527598ca@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: zeppelin git commit: [ZEPPELIN-1910] DON'T show the same dialogs multiple times when don't have permission for run all paragraphs (BUG) Date: Tue, 10 Jan 2017 18:33:51 +0000 (UTC) archived-at: Tue, 10 Jan 2017 18:33:53 -0000 Repository: zeppelin Updated Branches: refs/heads/master d96a14913 -> a89cb1047 [ZEPPELIN-1910] DON'T show the same dialogs multiple times when don't have permission for run all paragraphs (BUG) ### What is this PR for? DON't show the multiple same dialog when user doesn't have permission for **Run all paragraphs** inside a note #### Implementation details - Introduce new websocket message `RUN_ALL_PARAGRAPHS` since we need to broadcast *bringing dialog* message only once (We did same thing about `CLEAR_ALL_PARAGRAPHS`) - Refactor `NotebookServer.runParagraph` to avoid duplication - Add necessary functions to backend and frontend ### What type of PR is it? [Bug Fix] ### Todos Fixed at once ### What is the Jira issue? [ZEPPELIN-1910](https://issues.apache.org/jira/browse/ZEPPELIN-1910) ### How should this be tested? 1. Set permission to a note 2. Enable shiro and login as an user **who doens't have write permission** 3. Click *Run all paragraphs* button ### Screenshots (if appropriate) #### 1. Before ![bug-multiple-dialog](https://cloud.githubusercontent.com/assets/4968473/21704503/440a9774-d3fd-11e6-8e41-fcad71c5c9e7.gif) #### 2. After ![run-all-after-fixed](https://cloud.githubusercontent.com/assets/4968473/21704488/304fd578-d3fd-11e6-9f6e-d64c82c508df.gif) ### Questions: * Does the licenses files need update? - NO * Is there breaking changes for older versions? - NO * Does this needs documentation? - NO Author: 1ambda <1amb4a@gmail.com> Closes #1852 from 1ambda/ZEPPELIN-1910/run-all-para-bring-multi-dialog and squashes the following commits: 837b7be [1ambda] fix: func name in ParagraphActionsIT c200585 [1ambda] fix: zeppeiln-web test for runNote 284e8e6 [1ambda] fix: Multiple dialog when don't have permission for run all para Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/a89cb104 Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/a89cb104 Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/a89cb104 Branch: refs/heads/master Commit: a89cb1047059a3eb01a9f2b9f5caae4aee6835a1 Parents: d96a149 Author: 1ambda <1amb4a@gmail.com> Authored: Sun Jan 8 18:58:52 2017 +0900 Committer: Lee moon soo Committed: Tue Jan 10 10:33:44 2017 -0800 ---------------------------------------------------------------------- .../apache/zeppelin/socket/NotebookServer.java | 88 ++++++++++++++++---- .../integration/ParagraphActionsIT.java | 2 +- .../src/app/notebook/notebook-actionBar.html | 2 +- .../src/app/notebook/notebook.controller.js | 13 ++- .../websocketEvents/websocketMsg.service.js | 10 +++ zeppelin-web/test/spec/controllers/notebook.js | 2 +- .../zeppelin/notebook/socket/Message.java | 3 +- 7 files changed, 96 insertions(+), 24 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/zeppelin/blob/a89cb104/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java ---------------------------------------------------------------------- diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java b/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java index 878bad8..6e58e3d 100644 --- a/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java +++ b/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java @@ -36,6 +36,7 @@ import javax.servlet.http.HttpServletRequest; import com.google.common.base.Strings; import com.google.common.collect.Sets; +import com.google.gson.*; import org.apache.commons.lang.StringUtils; import org.apache.commons.vfs2.FileSystemException; import org.apache.zeppelin.conf.ZeppelinConfiguration; @@ -85,8 +86,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import com.google.common.collect.Queues; -import com.google.gson.Gson; -import com.google.gson.GsonBuilder; import com.google.gson.reflect.TypeToken; /** @@ -263,6 +262,9 @@ public class NotebookServer extends WebSocketServlet case RUN_PARAGRAPH: runParagraph(conn, userAndRoles, notebook, messagereceived); break; + case RUN_ALL_PARAGRAPHS: + runAllParagraphs(conn, userAndRoles, notebook, messagereceived); + break; case CANCEL_PARAGRAPH: cancelParagraph(conn, userAndRoles, notebook, messagereceived); break; @@ -1534,8 +1536,46 @@ public class NotebookServer extends WebSocketServlet p.abort(); } - private void runParagraph(NotebookSocket conn, HashSet userAndRoles, Notebook notebook, + private void runAllParagraphs(NotebookSocket conn, HashSet userAndRoles, + Notebook notebook, Message fromMessage) throws IOException { + final String noteId = (String) fromMessage.get("noteId"); + if (StringUtils.isBlank(noteId)) { + return; + } + + Note note = notebook.getNote(noteId); + NotebookAuthorization notebookAuthorization = notebook.getNotebookAuthorization(); + if (!notebookAuthorization.isWriter(noteId, userAndRoles)) { + permissionError(conn, "run all paragraphs", fromMessage.principal, userAndRoles, + notebookAuthorization.getOwners(noteId)); + return; + } + + List> paragraphs = + gson.fromJson(String.valueOf(fromMessage.data.get("paragraphs")), + new TypeToken>>() {}.getType()); + + for (Map raw : paragraphs) { + String paragraphId = (String) raw.get("id"); + if (paragraphId == null) { + continue; + } + + String text = (String) raw.get("paragraph"); + String title = (String) raw.get("title"); + Map params = (Map) raw.get("params"); + Map config = (Map) raw.get("config"); + + Paragraph p = setParagraphUsingMessage(note, fromMessage, + paragraphId, text, title, params, config); + + persistAndExecuteSingleParagraph(conn, note, p); + } + } + + private void runParagraph(NotebookSocket conn, HashSet userAndRoles, Notebook notebook, + Message fromMessage) throws IOException { final String paragraphId = (String) fromMessage.get("id"); if (paragraphId == null) { return; @@ -1550,30 +1590,29 @@ public class NotebookServer extends WebSocketServlet return; } - Paragraph p = note.getParagraph(paragraphId); String text = (String) fromMessage.get("paragraph"); - p.setText(text); - p.setTitle((String) fromMessage.get("title")); - AuthenticationInfo subject = - new AuthenticationInfo(fromMessage.principal, fromMessage.ticket); - p.setAuthenticationInfo(subject); - + String title = (String) fromMessage.get("title"); Map params = (Map) fromMessage.get("params"); - p.settings.setParams(params); Map config = (Map) fromMessage.get("config"); - p.setConfig(config); + Paragraph p = setParagraphUsingMessage(note, fromMessage, paragraphId, + text, title, params, config); + + persistAndExecuteSingleParagraph(conn, note, p); + } + private void persistAndExecuteSingleParagraph(NotebookSocket conn, + Note note, Paragraph p) throws IOException { // if it's the last paragraph and empty, let's add a new one boolean isTheLastParagraph = note.isLastParagraph(p.getId()); - if (!(text.trim().equals(p.getMagic()) || - Strings.isNullOrEmpty(text)) && + if (!(p.getText().trim().equals(p.getMagic()) || + Strings.isNullOrEmpty(p.getText())) && isTheLastParagraph) { - Paragraph newPara = note.addParagraph(subject); + Paragraph newPara = note.addParagraph(p.getAuthenticationInfo()); broadcastNewParagraph(note, newPara); } try { - note.persist(subject); + note.persist(p.getAuthenticationInfo()); } catch (FileSystemException ex) { LOG.error("Exception from run", ex); conn.send(serializeMessage(new Message(OP.ERROR_INFO).put("info", @@ -1584,7 +1623,7 @@ public class NotebookServer extends WebSocketServlet } try { - note.run(paragraphId); + note.run(p.getId()); } catch (Exception ex) { LOG.error("Exception from run", ex); if (p != null) { @@ -1595,6 +1634,21 @@ public class NotebookServer extends WebSocketServlet } } + private Paragraph setParagraphUsingMessage(Note note, Message fromMessage, String paragraphId, + String text, String title, Map params, + Map config) { + Paragraph p = note.getParagraph(paragraphId); + p.setText(text); + p.setTitle(title); + AuthenticationInfo subject = + new AuthenticationInfo(fromMessage.principal, fromMessage.ticket); + p.setAuthenticationInfo(subject); + p.settings.setParams(params); + p.setConfig(config); + + return p; + } + private void sendAllConfigurations(NotebookSocket conn, HashSet userAndRoles, Notebook notebook) throws IOException { ZeppelinConfiguration conf = notebook.getConf(); http://git-wip-us.apache.org/repos/asf/zeppelin/blob/a89cb104/zeppelin-server/src/test/java/org/apache/zeppelin/integration/ParagraphActionsIT.java ---------------------------------------------------------------------- diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/integration/ParagraphActionsIT.java b/zeppelin-server/src/test/java/org/apache/zeppelin/integration/ParagraphActionsIT.java index f939579..d93a6e5 100644 --- a/zeppelin-server/src/test/java/org/apache/zeppelin/integration/ParagraphActionsIT.java +++ b/zeppelin-server/src/test/java/org/apache/zeppelin/integration/ParagraphActionsIT.java @@ -241,7 +241,7 @@ public class ParagraphActionsIT extends AbstractZeppelinIT { driver.findElement(By.xpath(getParagraphXPath(1) + "//span[@class='icon-control-play shortcut-icon']")).isDisplayed(), CoreMatchers.equalTo(false) ); - driver.findElement(By.xpath(".//*[@id='main']//button[@ng-click='runNote()']")).sendKeys(Keys.ENTER); + driver.findElement(By.xpath(".//*[@id='main']//button[contains(@ng-click, 'runAllParagraphs')]")).sendKeys(Keys.ENTER); ZeppelinITUtils.sleep(1000, true); driver.findElement(By.xpath("//div[@class='modal-dialog'][contains(.,'Run all paragraphs?')]" + "//div[@class='modal-footer']//button[contains(.,'OK')]")).click(); http://git-wip-us.apache.org/repos/asf/zeppelin/blob/a89cb104/zeppelin-web/src/app/notebook/notebook-actionBar.html ---------------------------------------------------------------------- diff --git a/zeppelin-web/src/app/notebook/notebook-actionBar.html b/zeppelin-web/src/app/notebook/notebook-actionBar.html index 4249f21..99d7d7a 100644 --- a/zeppelin-web/src/app/notebook/notebook-actionBar.html +++ b/zeppelin-web/src/app/notebook/notebook-actionBar.html @@ -24,7 +24,7 @@ limitations under the License.