You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zeppelin.apache.org by pr...@apache.org on 2016/10/20 08:55:19 UTC
zeppelin git commit: [ZEPPELIN-1483] Zeppelin home page list
notebooks doesn't show notebook with group permission
Repository: zeppelin
Updated Branches:
refs/heads/master 908b2a74f -> 9e9ea3aea
[ZEPPELIN-1483] Zeppelin home page list notebooks doesn't show notebook with group permission
### What is this PR for?
Zeppelin home page list notebooks doesn't show notebook with group permission
### What type of PR is it?
[Bug Fix]
### Todos
* [x] - consume userAndRole instead of AuthenticationInfo
### What is the Jira issue?
* [ZEPPELIN-1483](https://issues.apache.org/jira/browse/ZEPPELIN-1483)
### How should this be tested?
In current scenario only those notebook lists that have direct user permission, those with group does not list up, but if user have link to those notebook, it can still be accessed.
IMO the notebook with group permission should also be listed in the home screen.
### Screenshots (if appropriate)
### Questions:
* Does the licenses files need update? n/a
* Is there breaking changes for older versions? n/a
* Does this needs documentation? n/a
Author: Prabhjyot Singh <pr...@gmail.com>
Author: Prabhjyot Singh <pr...@gmail.org>
Closes #1454 from prabhjyotsingh/ZEPPELIN-1483 and squashes the following commits:
2484833 [Prabhjyot Singh] Merge remote-tracking branch 'origin/master' into ZEPPELIN-1483
c8d810e [Prabhjyot Singh] organise imports
d3261c4 [Prabhjyot Singh] consume userAndRole instead of AuthenticationInfo
Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo
Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/9e9ea3ae
Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/9e9ea3ae
Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/9e9ea3ae
Branch: refs/heads/master
Commit: 9e9ea3aea0a4e4ffaa87e0783fc85fb18cdc9887
Parents: 908b2a7
Author: Prabhjyot Singh <pr...@gmail.com>
Authored: Thu Oct 20 12:15:29 2016 +0530
Committer: Prabhjyot Singh <pr...@gmail.org>
Committed: Thu Oct 20 14:25:06 2016 +0530
----------------------------------------------------------------------
.../apache/zeppelin/rest/NotebookRestApi.java | 12 +++----
.../apache/zeppelin/socket/NotebookServer.java | 36 ++++++++++----------
.../zeppelin/integration/AuthenticationIT.java | 2 +-
.../zeppelin/rest/ZeppelinRestApiTest.java | 6 ++--
.../org/apache/zeppelin/notebook/Notebook.java | 6 ++--
.../apache/zeppelin/notebook/NotebookTest.java | 20 +++++------
6 files changed, 42 insertions(+), 40 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/9e9ea3ae/zeppelin-server/src/main/java/org/apache/zeppelin/rest/NotebookRestApi.java
----------------------------------------------------------------------
diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/rest/NotebookRestApi.java b/zeppelin-server/src/main/java/org/apache/zeppelin/rest/NotebookRestApi.java
index d9af812..d581404 100644
--- a/zeppelin-server/src/main/java/org/apache/zeppelin/rest/NotebookRestApi.java
+++ b/zeppelin-server/src/main/java/org/apache/zeppelin/rest/NotebookRestApi.java
@@ -39,7 +39,6 @@ import com.google.common.reflect.TypeToken;
import com.google.gson.Gson;
import org.apache.commons.lang3.StringUtils;
import org.apache.zeppelin.interpreter.InterpreterResult;
-import org.apache.zeppelin.scheduler.Job;
import org.apache.zeppelin.utils.InterpreterBindingUtils;
import org.quartz.CronExpression;
import org.slf4j.Logger;
@@ -162,7 +161,7 @@ public class NotebookRestApi {
AuthenticationInfo subject = new AuthenticationInfo(SecurityUtils.getPrincipal());
note.persist(subject);
notebookServer.broadcastNote(note);
- notebookServer.broadcastNoteList(subject);
+ notebookServer.broadcastNoteList(subject, userAndRoles);
return new JsonResponse<>(Status.OK).build();
}
@@ -199,7 +198,8 @@ public class NotebookRestApi {
@ZeppelinApi
public Response getNotebookList() throws IOException {
AuthenticationInfo subject = new AuthenticationInfo(SecurityUtils.getPrincipal());
- List<Map<String, String>> notesInfo = notebookServer.generateNotebooksInfo(false, subject);
+ List<Map<String, String>> notesInfo = notebookServer.generateNotebooksInfo(false, subject,
+ SecurityUtils.getRoles());
return new JsonResponse<>(Status.OK, "", notesInfo).build();
}
@@ -278,7 +278,7 @@ public class NotebookRestApi {
note.setName(noteName);
note.persist(subject);
notebookServer.broadcastNote(note);
- notebookServer.broadcastNoteList(subject);
+ notebookServer.broadcastNoteList(subject, SecurityUtils.getRoles());
return new JsonResponse<>(Status.CREATED, "", note.getId()).build();
}
@@ -302,7 +302,7 @@ public class NotebookRestApi {
}
}
- notebookServer.broadcastNoteList(subject);
+ notebookServer.broadcastNoteList(subject, SecurityUtils.getRoles());
return new JsonResponse<>(Status.OK, "").build();
}
@@ -327,7 +327,7 @@ public class NotebookRestApi {
AuthenticationInfo subject = new AuthenticationInfo(SecurityUtils.getPrincipal());
Note newNote = notebook.cloneNote(notebookId, newNoteName, subject);
notebookServer.broadcastNote(newNote);
- notebookServer.broadcastNoteList(subject);
+ notebookServer.broadcastNoteList(subject, SecurityUtils.getRoles());
return new JsonResponse<>(Status.CREATED, "", newNote.getId()).build();
}
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/9e9ea3ae/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 969bdf9..2f9faba 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
@@ -20,7 +20,6 @@ import com.google.common.base.Strings;
import com.google.gson.Gson;
import com.google.gson.GsonBuilder;
import com.google.gson.reflect.TypeToken;
-
import org.apache.commons.lang.StringUtils;
import org.apache.commons.vfs2.FileSystemException;
import org.apache.zeppelin.conf.ZeppelinConfiguration;
@@ -172,10 +171,10 @@ public class NotebookServer extends WebSocketServlet implements
/** Lets be elegant here */
switch (messagereceived.op) {
case LIST_NOTES:
- unicastNoteList(conn, subject);
+ unicastNoteList(conn, subject, userAndRoles);
break;
case RELOAD_NOTES_FROM_REPO:
- broadcastReloadedNoteList(subject);
+ broadcastReloadedNoteList(subject, userAndRoles);
break;
case GET_HOME_NOTE:
sendHomeNote(conn, userAndRoles, notebook, messagereceived);
@@ -491,7 +490,7 @@ public class NotebookServer extends WebSocketServlet implements
}
public List<Map<String, String>> generateNotebooksInfo(boolean needsReload,
- AuthenticationInfo subject) {
+ AuthenticationInfo subject, HashSet<String> userAndRoles) {
Notebook notebook = notebook();
@@ -508,7 +507,7 @@ public class NotebookServer extends WebSocketServlet implements
}
}
- List<Note> notes = notebook.getAllNotes(subject);
+ List<Note> notes = notebook.getAllNotes(userAndRoles);
List<Map<String, String>> notesInfo = new LinkedList<>();
for (Note note : notes) {
Map<String, String> info = new HashMap<>();
@@ -535,34 +534,35 @@ public class NotebookServer extends WebSocketServlet implements
.put("interpreterBindings", settingList));
}
- public void broadcastNoteList(AuthenticationInfo subject) {
+ public void broadcastNoteList(AuthenticationInfo subject, HashSet userAndRoles) {
if (subject == null) {
subject = new AuthenticationInfo(StringUtils.EMPTY);
}
//send first to requesting user
- List<Map<String, String>> notesInfo = generateNotebooksInfo(false, subject);
+ List<Map<String, String>> notesInfo = generateNotebooksInfo(false, subject, userAndRoles);
multicastToUser(subject.getUser(), new Message(OP.NOTES_INFO).put("notes", notesInfo));
//to others afterwards
for (String user: userConnectedSockets.keySet()) {
if (subject.getUser() == user) {
continue;
}
- notesInfo = generateNotebooksInfo(false, new AuthenticationInfo(user));
+ notesInfo = generateNotebooksInfo(false, new AuthenticationInfo(user), userAndRoles);
multicastToUser(user, new Message(OP.NOTES_INFO).put("notes", notesInfo));
}
}
- public void unicastNoteList(NotebookSocket conn, AuthenticationInfo subject) {
- List<Map<String, String>> notesInfo = generateNotebooksInfo(false, subject);
+ public void unicastNoteList(NotebookSocket conn, AuthenticationInfo subject,
+ HashSet<String> userAndRoles) {
+ List<Map<String, String>> notesInfo = generateNotebooksInfo(false, subject, userAndRoles);
unicast(new Message(OP.NOTES_INFO).put("notes", notesInfo), conn);
}
- public void broadcastReloadedNoteList(AuthenticationInfo subject) {
+ public void broadcastReloadedNoteList(AuthenticationInfo subject, HashSet userAndRoles) {
if (subject == null) {
subject = new AuthenticationInfo(StringUtils.EMPTY);
}
//reload and reply first to requesting user
- List<Map<String, String>> notesInfo = generateNotebooksInfo(true, subject);
+ List<Map<String, String>> notesInfo = generateNotebooksInfo(true, subject, userAndRoles);
multicastToUser(subject.getUser(), new Message(OP.NOTES_INFO).put("notes", notesInfo));
//to others afterwards
for (String user: userConnectedSockets.keySet()) {
@@ -570,7 +570,7 @@ public class NotebookServer extends WebSocketServlet implements
continue;
}
//reloaded already above; parameter - false
- notesInfo = generateNotebooksInfo(false, new AuthenticationInfo(user));
+ notesInfo = generateNotebooksInfo(false, new AuthenticationInfo(user), userAndRoles);
multicastToUser(user, new Message(OP.NOTES_INFO).put("notes", notesInfo));
}
}
@@ -678,7 +678,7 @@ public class NotebookServer extends WebSocketServlet implements
AuthenticationInfo subject = new AuthenticationInfo(fromMessage.principal);
note.persist(subject);
broadcastNote(note);
- broadcastNoteList(subject);
+ broadcastNoteList(subject, userAndRoles);
}
}
@@ -713,7 +713,7 @@ public class NotebookServer extends WebSocketServlet implements
note.persist(subject);
addConnectionToNote(note.getId(), (NotebookSocket) conn);
conn.send(serializeMessage(new Message(OP.NEW_NOTE).put("note", note)));
- broadcastNoteList(subject);
+ broadcastNoteList(subject, userAndRoles);
}
private void removeNote(NotebookSocket conn, HashSet<String> userAndRoles,
@@ -735,7 +735,7 @@ public class NotebookServer extends WebSocketServlet implements
AuthenticationInfo subject = new AuthenticationInfo(fromMessage.principal);
notebook.removeNote(noteId, subject);
removeNote(noteId);
- broadcastNoteList(subject);
+ broadcastNoteList(subject, userAndRoles);
}
private void updateParagraph(NotebookSocket conn, HashSet<String> userAndRoles,
@@ -777,7 +777,7 @@ public class NotebookServer extends WebSocketServlet implements
AuthenticationInfo subject = new AuthenticationInfo(fromMessage.principal);
addConnectionToNote(newNote.getId(), (NotebookSocket) conn);
conn.send(serializeMessage(new Message(OP.NEW_NOTE).put("note", newNote)));
- broadcastNoteList(subject);
+ broadcastNoteList(subject, userAndRoles);
}
protected Note importNote(NotebookSocket conn, HashSet<String> userAndRoles,
@@ -796,7 +796,7 @@ public class NotebookServer extends WebSocketServlet implements
note = notebook.importNote(noteJson, noteName, subject);
note.persist(subject);
broadcastNote(note);
- broadcastNoteList(subject);
+ broadcastNoteList(subject, userAndRoles);
}
return note;
}
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/9e9ea3ae/zeppelin-server/src/test/java/org/apache/zeppelin/integration/AuthenticationIT.java
----------------------------------------------------------------------
diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/integration/AuthenticationIT.java b/zeppelin-server/src/test/java/org/apache/zeppelin/integration/AuthenticationIT.java
index 6001429..e1a6f9e 100644
--- a/zeppelin-server/src/test/java/org/apache/zeppelin/integration/AuthenticationIT.java
+++ b/zeppelin-server/src/test/java/org/apache/zeppelin/integration/AuthenticationIT.java
@@ -202,7 +202,7 @@ public class AuthenticationIT extends AbstractZeppelinIT {
try {
WebElement element = pollingWait(By.xpath("//*[@id='notebook-names']//a[contains(@href, '" + noteId + "')]"),
MAX_BROWSER_TIMEOUT_SEC);
- collector.checkThat("Check is user has permission to view this notebook link", false,
+ collector.checkThat("Check is user has permission to view this notebook link", true,
CoreMatchers.equalTo(element.isDisplayed()));
} catch (Exception e) {
//This should have failed, nothing to worry.
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/9e9ea3ae/zeppelin-server/src/test/java/org/apache/zeppelin/rest/ZeppelinRestApiTest.java
----------------------------------------------------------------------
diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/ZeppelinRestApiTest.java b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/ZeppelinRestApiTest.java
index c2606f8..3b92c14 100644
--- a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/ZeppelinRestApiTest.java
+++ b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/ZeppelinRestApiTest.java
@@ -19,9 +19,11 @@ package org.apache.zeppelin.rest;
import java.io.IOException;
import java.util.ArrayList;
+import java.util.HashSet;
import java.util.List;
import java.util.Map;
+import com.google.common.collect.Sets;
import org.apache.commons.httpclient.methods.DeleteMethod;
import org.apache.commons.httpclient.methods.GetMethod;
import org.apache.commons.httpclient.methods.PostMethod;
@@ -352,8 +354,8 @@ public class ZeppelinRestApiTest extends AbstractTestRestApi {
}.getType());
List<Map<String, String>> body = (List<Map<String, String>>) resp.get("body");
//TODO(khalid): anonymous or specific user notes?
- AuthenticationInfo subject = new AuthenticationInfo("anonymous");
- assertEquals("List notebooks are equal", ZeppelinServer.notebook.getAllNotes(subject).size(), body.size());
+ HashSet<String> anonymous = Sets.newHashSet("anonymous");
+ assertEquals("List notebooks are equal", ZeppelinServer.notebook.getAllNotes(anonymous).size(), body.size());
get.releaseConnection();
}
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/9e9ea3ae/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Notebook.java
----------------------------------------------------------------------
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 d996488..9ad6d4c 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
@@ -540,10 +540,10 @@ public class Notebook implements NoteEventListener {
}
}
- public List<Note> getAllNotes(AuthenticationInfo subject) {
+ public List<Note> getAllNotes(HashSet<String> userAndRoles) {
final Set<String> entities = Sets.newHashSet();
- if (subject != null) {
- entities.add(subject.getUser());
+ if (userAndRoles != null) {
+ entities.addAll(userAndRoles);
}
synchronized (notes) {
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/9e9ea3ae/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java
----------------------------------------------------------------------
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 d0af2c9..87e2415 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
@@ -885,20 +885,20 @@ public class NotebookTest implements JobListenerFactory{
public void testGetAllNotes() throws Exception {
Note note1 = notebook.createNote(anonymous);
Note note2 = notebook.createNote(anonymous);
- assertEquals(2, notebook.getAllNotes(anonymous).size());
+ assertEquals(2, notebook.getAllNotes(Sets.newHashSet("anonymous")).size());
notebook.getNotebookAuthorization().setOwners(note1.getId(), Sets.newHashSet("user1"));
notebook.getNotebookAuthorization().setWriters(note1.getId(), Sets.newHashSet("user1"));
notebook.getNotebookAuthorization().setReaders(note1.getId(), Sets.newHashSet("user1"));
- assertEquals(1, notebook.getAllNotes(anonymous).size());
- assertEquals(2, notebook.getAllNotes(new AuthenticationInfo("user1")).size());
+ assertEquals(1, notebook.getAllNotes(Sets.newHashSet("anonymous")).size());
+ assertEquals(2, notebook.getAllNotes(Sets.newHashSet("user1")).size());
notebook.getNotebookAuthorization().setOwners(note2.getId(), Sets.newHashSet("user2"));
notebook.getNotebookAuthorization().setWriters(note2.getId(), Sets.newHashSet("user2"));
notebook.getNotebookAuthorization().setReaders(note2.getId(), Sets.newHashSet("user2"));
- assertEquals(0, notebook.getAllNotes(anonymous).size());
- assertEquals(1, notebook.getAllNotes(new AuthenticationInfo("user1")).size());
- assertEquals(1, notebook.getAllNotes(new AuthenticationInfo("user2")).size());
+ assertEquals(0, notebook.getAllNotes(Sets.newHashSet("anonymous")).size());
+ assertEquals(1, notebook.getAllNotes(Sets.newHashSet("user1")).size());
+ assertEquals(1, notebook.getAllNotes(Sets.newHashSet("user2")).size());
notebook.removeNote(note1.getId(), anonymous);
notebook.removeNote(note2.getId(), anonymous);
}
@@ -906,15 +906,15 @@ public class NotebookTest implements JobListenerFactory{
@Test
public void testGetAllNotesWithDifferentPermissions() throws IOException {
- AuthenticationInfo user1 = new AuthenticationInfo("user1");
- AuthenticationInfo user2 = new AuthenticationInfo("user2");
+ HashSet<String> user1 = Sets.newHashSet("user1");
+ HashSet<String> user2 = Sets.newHashSet("user1");
List<Note> notes1 = notebook.getAllNotes(user1);
List<Note> notes2 = notebook.getAllNotes(user2);
assertEquals(notes1.size(), 0);
assertEquals(notes2.size(), 0);
//creates note and sets user1 owner
- Note note = notebook.createNote(user1);
+ Note note = notebook.createNote(new AuthenticationInfo("user1"));
// note is public since readers and writers empty
notes1 = notebook.getAllNotes(user1);
@@ -933,7 +933,7 @@ public class NotebookTest implements JobListenerFactory{
notes1 = notebook.getAllNotes(user1);
notes2 = notebook.getAllNotes(user2);
assertEquals(notes1.size(), 1);
- assertEquals(notes2.size(), 0);
+ assertEquals(notes2.size(), 1);
}
private void delete(File file){