zeppelin-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From m...@apache.org
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 GMT
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 <moon@apache.org>
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<String> userAndRoles, Notebook
notebook,
+  private void runAllParagraphs(NotebookSocket conn, HashSet<String> 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<Map<String, Object>> paragraphs =
+        gson.fromJson(String.valueOf(fromMessage.data.get("paragraphs")),
+            new TypeToken<List<Map<String, Object>>>() {}.getType());
+
+    for (Map<String, Object> 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<String, Object> params = (Map<String, Object>) raw.get("params");
+      Map<String, Object> config = (Map<String, Object>) raw.get("config");
+
+      Paragraph p = setParagraphUsingMessage(note, fromMessage,
+          paragraphId, text, title, params, config);
+
+      persistAndExecuteSingleParagraph(conn, note, p);
+    }
+  }
+
+  private void runParagraph(NotebookSocket conn, HashSet<String> 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<String, Object> params = (Map<String, Object>) fromMessage.get("params");
-    p.settings.setParams(params);
     Map<String, Object> config = (Map<String, Object>) 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<String, Object>
params,
+                                             Map<String, Object> 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<String> 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.
       <span class="labelBtn btn-group">
       <button type="button"
               class="btn btn-default btn-xs"
-              ng-click="runNote()"
+              ng-click="runAllParagraphs(note.id)"
               ng-class="{'disabled':isNoteRunning()}"
               tooltip-placement="bottom" tooltip="Run all paragraphs"
               ng-disabled="revisionView">

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/a89cb104/zeppelin-web/src/app/notebook/notebook.controller.js
----------------------------------------------------------------------
diff --git a/zeppelin-web/src/app/notebook/notebook.controller.js b/zeppelin-web/src/app/notebook/notebook.controller.js
index 82b070a..b1b5447 100644
--- a/zeppelin-web/src/app/notebook/notebook.controller.js
+++ b/zeppelin-web/src/app/notebook/notebook.controller.js
@@ -286,16 +286,23 @@
       }
     };
 
-    $scope.runNote = function() {
+    $scope.runAllParagraphs = function(noteId) {
       BootstrapDialog.confirm({
         closable: true,
         title: '',
         message: 'Run all paragraphs?',
         callback: function(result) {
           if (result) {
-            _.forEach($scope.note.paragraphs, function(n, key) {
-              angular.element('#' + n.id + '_paragraphColumn_main').scope().runParagraph(n.text);
+            const paragraphs = $scope.note.paragraphs.map(p => {
+              return {
+                id: p.id,
+                title: p.title,
+                paragraph: p.text,
+                config: p.config,
+                params: p.settings.params
+              };
             });
+            websocketMsgSrv.runAllParagraphs(noteId, paragraphs);
           }
         }
       });

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/a89cb104/zeppelin-web/src/components/websocketEvents/websocketMsg.service.js
----------------------------------------------------------------------
diff --git a/zeppelin-web/src/components/websocketEvents/websocketMsg.service.js b/zeppelin-web/src/components/websocketEvents/websocketMsg.service.js
index 24bbc17..1fb5f74 100644
--- a/zeppelin-web/src/components/websocketEvents/websocketMsg.service.js
+++ b/zeppelin-web/src/components/websocketEvents/websocketMsg.service.js
@@ -173,6 +173,16 @@
         });
       },
 
+      runAllParagraphs: function(noteId, paragraphs) {
+        websocketEvents.sendNewEvent({
+          op: 'RUN_ALL_PARAGRAPHS',
+          data: {
+            noteId: noteId,
+            paragraphs: JSON.stringify(paragraphs)
+          }
+        });
+      },
+
       removeParagraph: function(paragraphId) {
         websocketEvents.sendNewEvent({op: 'PARAGRAPH_REMOVE', data: {id: paragraphId}});
       },

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/a89cb104/zeppelin-web/test/spec/controllers/notebook.js
----------------------------------------------------------------------
diff --git a/zeppelin-web/test/spec/controllers/notebook.js b/zeppelin-web/test/spec/controllers/notebook.js
index ec8ec97..f4a420b 100644
--- a/zeppelin-web/test/spec/controllers/notebook.js
+++ b/zeppelin-web/test/spec/controllers/notebook.js
@@ -35,7 +35,7 @@ describe('Controller: NotebookCtrl', function() {
     scope.note = noteMock;
   });
 
-  var functions = ['getCronOptionNameFromValue', 'removeNote', 'runNote', 'saveNote', 'toggleAllEditor',
+  var functions = ['getCronOptionNameFromValue', 'removeNote', 'runAllParagraphs', 'saveNote',
'toggleAllEditor',
     'showAllEditor', 'hideAllEditor', 'toggleAllTable', 'hideAllTable', 'showAllTable', 'isNoteRunning',
     'killSaveTimer', 'startSaveTimer', 'setLookAndFeel', 'setCronScheduler', 'setConfig',
'updateNoteName',
     'openSetting', 'closeSetting', 'saveSetting', 'toggleSetting'];

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/a89cb104/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/socket/Message.java
----------------------------------------------------------------------
diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/socket/Message.java
b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/socket/Message.java
index d123020..162baf8 100644
--- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/socket/Message.java
+++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/socket/Message.java
@@ -171,7 +171,8 @@ public class Message {
     PARAGRAPH_ADDED,              // [s-c] paragraph is added
     PARAGRAPH_REMOVED,            // [s-c] paragraph deleted
     PARAGRAPH_MOVED,              // [s-c] paragraph moved
-    NOTE_UPDATED                  // [s-c] paragraph updated(name, config)
+    NOTE_UPDATED,                 // [s-c] paragraph updated(name, config)
+    RUN_ALL_PARAGRAPHS            // [c-s] run all paragraphs
   }
 
   public static final Message EMPTY = new Message(null);


Mime
View raw message