You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zeppelin.apache.org by mo...@apache.org on 2017/01/10 18:33:51 UTC
zeppelin git commit: [ZEPPELIN-1910] DON'T show the same dialogs
multiple times when don't have permission for run all paragraphs (BUG)
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
#### 2. After
### Questions:
* Does the licenses files need update? - NO
* Is there breaking changes for older versions? - NO
* Does this needs documentation? - NO
Author: 1ambda <1a...@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 <1a...@gmail.com>
Authored: Sun Jan 8 18:58:52 2017 +0900
Committer: Lee moon soo <mo...@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);