From 17e2d4c0c5e5528031b36218339305300a69a543 Mon Sep 17 00:00:00 2001 From: Khalid Huseynov Date: Thu, 1 Sep 2016 16:29:57 +0900 Subject: [PATCH 01/13] first draft --- .../zeppelin/socket/NotebookServer.java | 20 +++-- .../zeppelin/rest/ZeppelinRestApiTest.java | 5 +- .../apache/zeppelin/notebook/Notebook.java | 76 +++++++++++++++++-- .../zeppelin/notebook/NotebookTest.java | 59 ++++++++++++-- 4 files changed, 141 insertions(+), 19 deletions(-) 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 8b6329a4073..e2d509bf78c 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 @@ -168,7 +168,8 @@ public void onMessage(NotebookSocket conn, String msg) { unicastNoteList(conn, subject); break; case RELOAD_NOTES_FROM_REPO: - broadcastReloadedNoteList(subject); + //broadcastReloadedNoteList(subject); + unicastNoteList(conn, subject); break; case GET_HOME_NOTE: sendHomeNote(conn, userAndRoles, notebook, messagereceived); @@ -331,7 +332,9 @@ private String getOpenNoteId(NotebookSocket socket) { private void broadcastToNoteBindedInterpreter(String interpreterGroupId, Message m) { Notebook notebook = notebook(); - List notes = notebook.getAllNotes(); + //TODO(khalid): anonymous or specific user notes? + AuthenticationInfo subject = new AuthenticationInfo("anonymous"); + List notes = notebook.getAllNotes(subject); for (Note note : notes) { List ids = notebook.getInterpreterFactory().getInterpreters(note.getId()); for (String id : ids) { @@ -472,6 +475,7 @@ public List> generateNotebooksInfo(boolean needsReload, LOG.error("Fail to reload notes from repository", e); } } + List notes = notebook.getAllNotes(subject); List> notesInfo = new LinkedList<>(); for (Note note : notes) { @@ -855,7 +859,9 @@ private void angularObjectUpdated(NotebookSocket conn, HashSet userAndRo if (global) { // broadcast change to all web session that uses related // interpreter. - for (Note n : notebook.getAllNotes()) { + //TODO(khalid): anonymous or specific user notes? + AuthenticationInfo subject = new AuthenticationInfo("anonymous"); + for (Note n : notebook.getAllNotes(subject)) { List settings = notebook.getInterpreterFactory() .getInterpreterSettings(note.getId()); for (InterpreterSetting setting : settings) { @@ -1533,7 +1539,9 @@ public void onUpdate(String interpreterGroupId, AngularObject object) { return; } - List notes = notebook.getAllNotes(); + //TODO(khalid): anonymous or specific user notes? + AuthenticationInfo subject = new AuthenticationInfo("anonymous"); + List notes = notebook.getAllNotes(subject ); for (Note note : notes) { if (object.getNoteId() != null && !note.getId().equals(object.getNoteId())) { continue; @@ -1558,7 +1566,9 @@ public void onUpdate(String interpreterGroupId, AngularObject object) { @Override public void onRemove(String interpreterGroupId, String name, String noteId, String paragraphId) { Notebook notebook = notebook(); - List notes = notebook.getAllNotes(); + //TODO(khalid): anonymous or specific user notes? + AuthenticationInfo subject = new AuthenticationInfo("anonymous"); + List notes = notebook.getAllNotes(subject); for (Note note : notes) { if (noteId != null && !note.getId().equals(noteId)) { continue; 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 4390d74b495..ad48b50786f 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 @@ -32,6 +32,7 @@ import org.apache.zeppelin.notebook.Paragraph; import org.apache.zeppelin.scheduler.Job.Status; import org.apache.zeppelin.server.ZeppelinServer; +import org.apache.zeppelin.user.AuthenticationInfo; import org.junit.AfterClass; import org.junit.BeforeClass; import org.junit.FixMethodOrder; @@ -341,7 +342,9 @@ public void testListNotebooks() throws IOException { Map resp = gson.fromJson(get.getResponseBodyAsString(), new TypeToken>() { }.getType()); List> body = (List>) resp.get("body"); - assertEquals("List notebooks are equal", ZeppelinServer.notebook.getAllNotes().size(), body.size()); + //TODO(khalid): anonymous or specific user notes? + AuthenticationInfo subject = new AuthenticationInfo("anonymous"); + assertEquals("List notebooks are equal", ZeppelinServer.notebook.getAllNotes(subject).size(), body.size()); get.releaseConnection(); } 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 d961ac033b0..e4616f05972 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 @@ -20,6 +20,7 @@ import java.io.IOException; import java.io.StringReader; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.Comparator; import java.util.Date; @@ -31,9 +32,9 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.TimeUnit; - import com.google.common.base.Predicate; import com.google.common.collect.FluentIterable; +import com.google.common.collect.Lists; import com.google.common.collect.Sets; import com.google.gson.Gson; import com.google.gson.GsonBuilder; @@ -484,11 +485,74 @@ public void reloadAllNotes(AuthenticationInfo subject) throws IOException { } List noteInfos = notebookRepo.list(subject); + noteInfos = getAuthorizedNoteInfos(noteInfos, subject); + for (NoteInfo info : noteInfos) { loadNoteFromRepo(info.getId(), subject); } } + private List getAuthorizedNoteInfos(List notes, AuthenticationInfo subject) { + Set allIds = Sets.newHashSet(); + if (subject == null) { + logger.warn("Subject for retrieving notes is null"); + return notes; + } + if ("anonymous".equals(subject.getUser())) { + return notes; + } + for (NoteInfo note: notes) { + allIds.add(note.getId()); + } + Set filteredIds = applyAuthorizationFilter(allIds, subject.getUser()); + List filteredNotes = Lists.newArrayList(); + for (NoteInfo note: notes) { + if (filteredIds.contains(note.getId())){ + filteredNotes.add(note); + } + } + return filteredNotes; + } + + //TODO(khalid): to change from processing everytime to keeping in memory view of each user + public List getAuthorizedNotes(AuthenticationInfo subject) { + List allNotes = new ArrayList<>(notes.values()); + Set allIds = Sets.newHashSet(); + + if (subject == null) { + logger.warn("Subject for retrieving notes is null"); + return allNotes; + } + if ("anonymous".equals(subject.getUser())) { + return allNotes; + } + for (Note note: allNotes) { + allIds.add(note.getId()); + } + Set filteredIds = applyAuthorizationFilter(allIds, subject.getUser()); + List filteredNotes = Lists.newArrayList(); + for (Note note: allNotes) { + if (filteredIds.contains(note.getId())){ + filteredNotes.add(note); + } + } + return filteredNotes; + } + + public Set applyAuthorizationFilter(Set ids, String user) { + logger.info("applying filter for {}", user); + Set filteredIds = Sets.newHashSet(); + Set userEntity = Sets.newHashSet((Arrays.asList(user))); + NotebookAuthorization auth = getNotebookAuthorization(); + for (String id: ids) { + if (auth.isOwner(id, userEntity) || auth.isReader(id, userEntity) + || auth.isWriter(id, userEntity)) { + filteredIds.add(id); + } + } + return filteredIds; + } + private class SnapshotAngularObject { String intpGroupId; AngularObject angularObject; @@ -514,9 +578,9 @@ Date getLastUpdate() { } } - public List getAllNotes() { + public List getAllNotes(AuthenticationInfo subject) { synchronized (notes) { - List noteList = new ArrayList<>(notes.values()); + List noteList = getAuthorizedNotes(subject); Collections.sort(noteList, new Comparator() { @Override public int compare(Note note1, Note note2) { @@ -534,7 +598,7 @@ public int compare(Note note1, Note note2) { return noteList; } } - +/* public List getAllNotes(AuthenticationInfo subject) { final Set entities = Sets.newHashSet(); if (subject != null) { @@ -563,7 +627,7 @@ public int compare(Note note1, Note note2) { }); } } - +*/ private Map getParagraphForJobManagerItem(Paragraph paragraph) { Map paragraphItem = new HashMap<>(); @@ -696,7 +760,7 @@ public List> getJobListByUnixTime(boolean needsReload, } } - List notes = getAllNotes(); + List notes = getAllNotes(subject); List> notesInfo = new LinkedList<>(); for (Note note : notes) { boolean isNotebookRunning = false; 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 0ec8e7cfdd1..8651283ab34 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 @@ -135,12 +135,15 @@ public void testReloadAndSetInterpreter() throws IOException { File destDir = new File(notebookDir.getAbsolutePath() + "/2A94M5J1Z"); FileUtils.copyDirectory(srcDir, destDir); + //TODO(khalid): anonymous or specific user notes? + AuthenticationInfo subject = new AuthenticationInfo("anonymous"); + // when load notebook.reloadAllNotes(null); - assertEquals(1, notebook.getAllNotes().size()); + assertEquals(1, notebook.getAllNotes(subject).size()); // then interpreter factory should be injected into all the paragraphs - Note note = notebook.getAllNotes().get(0); + Note note = notebook.getAllNotes(subject).get(0); assertNull(note.getParagraphs().get(0).getRepl(null)); } @@ -163,14 +166,17 @@ public void testReloadAllNotes() throws IOException { logger.error(e.toString(), e); } + //TODO(khalid): anonymous or specific user notes? + AuthenticationInfo subject = new AuthenticationInfo("anonymous"); + // doesn't have copied notebook in memory before reloading - List notes = notebook.getAllNotes(); + List notes = notebook.getAllNotes(subject); assertEquals(notes.size(), 0); // load copied notebook on memory when reloadAllNotes() is called Note copiedNote = notebookRepo.get("2A94M5J1Z", null); notebook.reloadAllNotes(null); - notes = notebook.getAllNotes(); + notes = notebook.getAllNotes(subject); assertEquals(notes.size(), 2); assertEquals(notes.get(1).getId(), copiedNote.getId()); assertEquals(notes.get(1).getName(), copiedNote.getName()); @@ -183,15 +189,52 @@ public void testReloadAllNotes() throws IOException { } // keep notebook in memory before reloading - notes = notebook.getAllNotes(); + notes = notebook.getAllNotes(subject); assertEquals(notes.size(), 2); // delete notebook from notebook list when reloadAllNotes() is called notebook.reloadAllNotes(null); - notes = notebook.getAllNotes(); + notes = notebook.getAllNotes(subject); assertEquals(notes.size(), 0); } + @Test + public void testReloadAuthorizedNotes() throws IOException { + AuthenticationInfo user1 = new AuthenticationInfo("user1"); + AuthenticationInfo user2 = new AuthenticationInfo("user2"); + notebook.reloadAllNotes(user1); + List notes1 = notebook.getAllNotes(user1); + notebook.reloadAllNotes(user2); + List notes2 = notebook.getAllNotes(user2); + assertEquals(notes1.size(), 0); + assertEquals(notes2.size(), 0); + logger.info("Loaded {} notes", notes1.size()); + + Note note = notebook.createNote(user1); + setNotePermissions(note.id(), user1.getUser(), true, true, true); + notebook.reloadAllNotes(user1); + notes1 = notebook.getAllNotes(user1); + notebook.reloadAllNotes(user2); + notes2 = notebook.getAllNotes(user2); + assertEquals(notes1.size(), 1); + assertEquals(notes2.size(), 0); + } + + //fails if all failed, otherwise true + private boolean setNotePermissions(String noteId, String user, boolean isOwner, boolean isReader, boolean isWriter) { + NotebookAuthorization notebookAuthorization = notebook.getNotebookAuthorization(); + Set owners = notebookAuthorization.getOwners(noteId); + Set readers = notebookAuthorization.getReaders(noteId); + Set writers = notebookAuthorization.getWriters(noteId); + boolean ownerSet = isOwner ? owners.add(user) : owners.remove(user); + boolean readerSet = isReader ? readers.add(user) : readers.remove(user); + boolean writerSet = isWriter ? writers.add(user) : writers.remove(user); + notebookAuthorization.setOwners(noteId, owners); + notebookAuthorization.setReaders(noteId, readers); + notebookAuthorization.setWriters(noteId, writers); + return ownerSet || readerSet || writerSet; + } + @Test public void testPersist() throws IOException, SchedulerException, RepositoryException { Note note = notebook.createNote(null); @@ -207,7 +250,9 @@ public void testPersist() throws IOException, SchedulerException, RepositoryExce Notebook notebook2 = new Notebook( conf, notebookRepo, schedulerFactory, new InterpreterFactory(conf, null, null, null, depResolver), this, null, null, null); - assertEquals(1, notebook2.getAllNotes().size()); + //TODO(khalid): anonymous or specific user notes? + AuthenticationInfo subject = new AuthenticationInfo("anonymous"); + assertEquals(1, notebook2.getAllNotes(subject).size()); } @Test From b7f19c918b015867aefce5bd5b3a08fd65d720ff Mon Sep 17 00:00:00 2001 From: Khalid Huseynov Date: Sat, 17 Sep 2016 15:11:40 +0900 Subject: [PATCH 02/13] separate getAllNotes() and getAllNotes(subject) --- .../zeppelin/socket/NotebookServer.java | 16 +++--------- .../apache/zeppelin/notebook/Notebook.java | 23 ++++++++++++++++- .../zeppelin/notebook/NotebookTest.java | 25 +++++++------------ 3 files changed, 35 insertions(+), 29 deletions(-) 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 e2d509bf78c..a99c06c48b5 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 @@ -332,9 +332,7 @@ private String getOpenNoteId(NotebookSocket socket) { private void broadcastToNoteBindedInterpreter(String interpreterGroupId, Message m) { Notebook notebook = notebook(); - //TODO(khalid): anonymous or specific user notes? - AuthenticationInfo subject = new AuthenticationInfo("anonymous"); - List notes = notebook.getAllNotes(subject); + List notes = notebook.getAllNotes(); for (Note note : notes) { List ids = notebook.getInterpreterFactory().getInterpreters(note.getId()); for (String id : ids) { @@ -859,9 +857,7 @@ private void angularObjectUpdated(NotebookSocket conn, HashSet userAndRo if (global) { // broadcast change to all web session that uses related // interpreter. - //TODO(khalid): anonymous or specific user notes? - AuthenticationInfo subject = new AuthenticationInfo("anonymous"); - for (Note n : notebook.getAllNotes(subject)) { + for (Note n : notebook.getAllNotes()) { List settings = notebook.getInterpreterFactory() .getInterpreterSettings(note.getId()); for (InterpreterSetting setting : settings) { @@ -1539,9 +1535,7 @@ public void onUpdate(String interpreterGroupId, AngularObject object) { return; } - //TODO(khalid): anonymous or specific user notes? - AuthenticationInfo subject = new AuthenticationInfo("anonymous"); - List notes = notebook.getAllNotes(subject ); + List notes = notebook.getAllNotes(); for (Note note : notes) { if (object.getNoteId() != null && !note.getId().equals(object.getNoteId())) { continue; @@ -1566,9 +1560,7 @@ public void onUpdate(String interpreterGroupId, AngularObject object) { @Override public void onRemove(String interpreterGroupId, String name, String noteId, String paragraphId) { Notebook notebook = notebook(); - //TODO(khalid): anonymous or specific user notes? - AuthenticationInfo subject = new AuthenticationInfo("anonymous"); - List notes = notebook.getAllNotes(subject); + List notes = notebook.getAllNotes(); for (Note note : notes) { if (noteId != null && !note.getId().equals(noteId)) { continue; 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 e4616f05972..b314f72e9fb 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 @@ -578,6 +578,27 @@ Date getLastUpdate() { } } + public List getAllNotes() { + synchronized (notes) { + List noteList = new ArrayList<>(notes.values()); + Collections.sort(noteList, new Comparator() { + @Override + public int compare(Note note1, Note note2) { + String name1 = note1.getId(); + if (note1.getName() != null) { + name1 = note1.getName(); + } + String name2 = note2.getId(); + if (note2.getName() != null) { + name2 = note2.getName(); + } + return name1.compareTo(name2); + } + }); + return noteList; + } + } + public List getAllNotes(AuthenticationInfo subject) { synchronized (notes) { List noteList = getAuthorizedNotes(subject); @@ -760,7 +781,7 @@ public List> getJobListByUnixTime(boolean needsReload, } } - List notes = getAllNotes(subject); + List notes = getAllNotes(); List> notesInfo = new LinkedList<>(); for (Note note : notes) { boolean isNotebookRunning = false; 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 8651283ab34..cd57d1c36dc 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 @@ -135,15 +135,12 @@ public void testReloadAndSetInterpreter() throws IOException { File destDir = new File(notebookDir.getAbsolutePath() + "/2A94M5J1Z"); FileUtils.copyDirectory(srcDir, destDir); - //TODO(khalid): anonymous or specific user notes? - AuthenticationInfo subject = new AuthenticationInfo("anonymous"); - // when load notebook.reloadAllNotes(null); - assertEquals(1, notebook.getAllNotes(subject).size()); + assertEquals(1, notebook.getAllNotes().size()); // then interpreter factory should be injected into all the paragraphs - Note note = notebook.getAllNotes(subject).get(0); + Note note = notebook.getAllNotes().get(0); assertNull(note.getParagraphs().get(0).getRepl(null)); } @@ -166,17 +163,14 @@ public void testReloadAllNotes() throws IOException { logger.error(e.toString(), e); } - //TODO(khalid): anonymous or specific user notes? - AuthenticationInfo subject = new AuthenticationInfo("anonymous"); - // doesn't have copied notebook in memory before reloading - List notes = notebook.getAllNotes(subject); + List notes = notebook.getAllNotes(); assertEquals(notes.size(), 0); // load copied notebook on memory when reloadAllNotes() is called Note copiedNote = notebookRepo.get("2A94M5J1Z", null); notebook.reloadAllNotes(null); - notes = notebook.getAllNotes(subject); + notes = notebook.getAllNotes(); assertEquals(notes.size(), 2); assertEquals(notes.get(1).getId(), copiedNote.getId()); assertEquals(notes.get(1).getName(), copiedNote.getName()); @@ -189,12 +183,12 @@ public void testReloadAllNotes() throws IOException { } // keep notebook in memory before reloading - notes = notebook.getAllNotes(subject); + notes = notebook.getAllNotes(); assertEquals(notes.size(), 2); // delete notebook from notebook list when reloadAllNotes() is called notebook.reloadAllNotes(null); - notes = notebook.getAllNotes(subject); + notes = notebook.getAllNotes(); assertEquals(notes.size(), 0); } @@ -211,7 +205,7 @@ public void testReloadAuthorizedNotes() throws IOException { logger.info("Loaded {} notes", notes1.size()); Note note = notebook.createNote(user1); - setNotePermissions(note.id(), user1.getUser(), true, true, true); + setNotePermissions(note.getId(), user1.getUser(), true, true, true); notebook.reloadAllNotes(user1); notes1 = notebook.getAllNotes(user1); notebook.reloadAllNotes(user2); @@ -250,9 +244,8 @@ public void testPersist() throws IOException, SchedulerException, RepositoryExce Notebook notebook2 = new Notebook( conf, notebookRepo, schedulerFactory, new InterpreterFactory(conf, null, null, null, depResolver), this, null, null, null); - //TODO(khalid): anonymous or specific user notes? - AuthenticationInfo subject = new AuthenticationInfo("anonymous"); - assertEquals(1, notebook2.getAllNotes(subject).size()); + + assertEquals(1, notebook2.getAllNotes().size()); } @Test From 92f37f50c3205d1a231693131d25e31b339d9deb Mon Sep 17 00:00:00 2001 From: Khalid Huseynov Date: Sat, 17 Sep 2016 16:36:02 +0900 Subject: [PATCH 03/13] substitute old getAllNotes(subject) with new implementation --- .../apache/zeppelin/notebook/Notebook.java | 48 +------------------ 1 file changed, 1 insertion(+), 47 deletions(-) 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 b314f72e9fb..068188b0e6b 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 @@ -514,31 +514,6 @@ private List getAuthorizedNoteInfos(List notes, Authenticati return filteredNotes; } - //TODO(khalid): to change from processing everytime to keeping in memory view of each user - public List getAuthorizedNotes(AuthenticationInfo subject) { - List allNotes = new ArrayList<>(notes.values()); - Set allIds = Sets.newHashSet(); - - if (subject == null) { - logger.warn("Subject for retrieving notes is null"); - return allNotes; - } - if ("anonymous".equals(subject.getUser())) { - return allNotes; - } - for (Note note: allNotes) { - allIds.add(note.getId()); - } - Set filteredIds = applyAuthorizationFilter(allIds, subject.getUser()); - List filteredNotes = Lists.newArrayList(); - for (Note note: allNotes) { - if (filteredIds.contains(note.getId())){ - filteredNotes.add(note); - } - } - return filteredNotes; - } - public Set applyAuthorizationFilter(Set ids, String user) { logger.info("applying filter for {}", user); Set filteredIds = Sets.newHashSet(); @@ -599,27 +574,6 @@ public int compare(Note note1, Note note2) { } } - public List getAllNotes(AuthenticationInfo subject) { - synchronized (notes) { - List noteList = getAuthorizedNotes(subject); - Collections.sort(noteList, new Comparator() { - @Override - public int compare(Note note1, Note note2) { - String name1 = note1.getId(); - if (note1.getName() != null) { - name1 = note1.getName(); - } - String name2 = note2.getId(); - if (note2.getName() != null) { - name2 = note2.getName(); - } - return name1.compareTo(name2); - } - }); - return noteList; - } - } -/* public List getAllNotes(AuthenticationInfo subject) { final Set entities = Sets.newHashSet(); if (subject != null) { @@ -648,7 +602,7 @@ public int compare(Note note1, Note note2) { }); } } -*/ + private Map getParagraphForJobManagerItem(Paragraph paragraph) { Map paragraphItem = new HashMap<>(); From d9c3bc98cb799526d343ac7151877089de5d2f54 Mon Sep 17 00:00:00 2001 From: Khalid Huseynov Date: Sat, 17 Sep 2016 17:22:48 +0900 Subject: [PATCH 04/13] filter reload using predicates --- .../apache/zeppelin/notebook/Notebook.java | 45 +++++-------------- 1 file changed, 11 insertions(+), 34 deletions(-) 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 068188b0e6b..77fe78f097d 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 @@ -485,47 +485,24 @@ public void reloadAllNotes(AuthenticationInfo subject) throws IOException { } List noteInfos = notebookRepo.list(subject); - noteInfos = getAuthorizedNoteInfos(noteInfos, subject); + noteInfos = filterByUser(noteInfos, subject); for (NoteInfo info : noteInfos) { loadNoteFromRepo(info.getId(), subject); } } - - private List getAuthorizedNoteInfos(List notes, AuthenticationInfo subject) { - Set allIds = Sets.newHashSet(); - if (subject == null) { - logger.warn("Subject for retrieving notes is null"); - return notes; - } - if ("anonymous".equals(subject.getUser())) { - return notes; - } - for (NoteInfo note: notes) { - allIds.add(note.getId()); - } - Set filteredIds = applyAuthorizationFilter(allIds, subject.getUser()); - List filteredNotes = Lists.newArrayList(); - for (NoteInfo note: notes) { - if (filteredIds.contains(note.getId())){ - filteredNotes.add(note); - } + + private List filterByUser(List notes, AuthenticationInfo subject) { + final Set entities = Sets.newHashSet(); + if (subject != null) { + entities.add(subject.getUser()); } - return filteredNotes; - } - - public Set applyAuthorizationFilter(Set ids, String user) { - logger.info("applying filter for {}", user); - Set filteredIds = Sets.newHashSet(); - Set userEntity = Sets.newHashSet((Arrays.asList(user))); - NotebookAuthorization auth = getNotebookAuthorization(); - for (String id: ids) { - if (auth.isOwner(id, userEntity) || auth.isReader(id, userEntity) - || auth.isWriter(id, userEntity)) { - filteredIds.add(id); + return FluentIterable.from(notes).filter(new Predicate() { + @Override + public boolean apply(NoteInfo input) { + return input != null && notebookAuthorization.isReader(input.getId(), entities); } - } - return filteredIds; + }).toList(); } private class SnapshotAngularObject { From 139940743f73692c4e3febff56a342960dd80185 Mon Sep 17 00:00:00 2001 From: Khalid Huseynov Date: Mon, 19 Sep 2016 09:23:43 +0900 Subject: [PATCH 05/13] remove unused imports --- .../src/main/java/org/apache/zeppelin/notebook/Notebook.java | 3 --- 1 file changed, 3 deletions(-) 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 77fe78f097d..823d8e50bc0 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 @@ -20,7 +20,6 @@ import java.io.IOException; import java.io.StringReader; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collections; import java.util.Comparator; import java.util.Date; @@ -34,12 +33,10 @@ import java.util.concurrent.TimeUnit; import com.google.common.base.Predicate; import com.google.common.collect.FluentIterable; -import com.google.common.collect.Lists; import com.google.common.collect.Sets; import com.google.gson.Gson; import com.google.gson.GsonBuilder; import com.google.gson.stream.JsonReader; -import org.apache.commons.codec.binary.StringUtils; import org.quartz.CronScheduleBuilder; import org.quartz.CronTrigger; import org.quartz.JobBuilder; From 6614e2bb5224a4f408473cf0cbbd1f5fcb5b119c Mon Sep 17 00:00:00 2001 From: Khalid Huseynov Date: Tue, 20 Sep 2016 13:14:13 +0900 Subject: [PATCH 06/13] improve tests --- .../zeppelin/notebook/NotebookTest.java | 72 +++++++++---------- 1 file changed, 34 insertions(+), 38 deletions(-) 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 cd57d1c36dc..e2567467fb8 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 @@ -192,43 +192,6 @@ public void testReloadAllNotes() throws IOException { assertEquals(notes.size(), 0); } - @Test - public void testReloadAuthorizedNotes() throws IOException { - AuthenticationInfo user1 = new AuthenticationInfo("user1"); - AuthenticationInfo user2 = new AuthenticationInfo("user2"); - notebook.reloadAllNotes(user1); - List notes1 = notebook.getAllNotes(user1); - notebook.reloadAllNotes(user2); - List notes2 = notebook.getAllNotes(user2); - assertEquals(notes1.size(), 0); - assertEquals(notes2.size(), 0); - logger.info("Loaded {} notes", notes1.size()); - - Note note = notebook.createNote(user1); - setNotePermissions(note.getId(), user1.getUser(), true, true, true); - notebook.reloadAllNotes(user1); - notes1 = notebook.getAllNotes(user1); - notebook.reloadAllNotes(user2); - notes2 = notebook.getAllNotes(user2); - assertEquals(notes1.size(), 1); - assertEquals(notes2.size(), 0); - } - - //fails if all failed, otherwise true - private boolean setNotePermissions(String noteId, String user, boolean isOwner, boolean isReader, boolean isWriter) { - NotebookAuthorization notebookAuthorization = notebook.getNotebookAuthorization(); - Set owners = notebookAuthorization.getOwners(noteId); - Set readers = notebookAuthorization.getReaders(noteId); - Set writers = notebookAuthorization.getWriters(noteId); - boolean ownerSet = isOwner ? owners.add(user) : owners.remove(user); - boolean readerSet = isReader ? readers.add(user) : readers.remove(user); - boolean writerSet = isWriter ? writers.add(user) : writers.remove(user); - notebookAuthorization.setOwners(noteId, owners); - notebookAuthorization.setReaders(noteId, readers); - notebookAuthorization.setWriters(noteId, writers); - return ownerSet || readerSet || writerSet; - } - @Test public void testPersist() throws IOException, SchedulerException, RepositoryException { Note note = notebook.createNote(null); @@ -626,7 +589,7 @@ public void testPermissions() throws IOException { // create a note and a paragraph Note note = notebook.createNote(null); NotebookAuthorization notebookAuthorization = notebook.getNotebookAuthorization(); - // empty owners, readers and writers means note is public + // empty owners, readers or writers means note is public assertEquals(notebookAuthorization.isOwner(note.getId(), new HashSet(Arrays.asList("user2"))), true); assertEquals(notebookAuthorization.isReader(note.getId(), @@ -911,6 +874,39 @@ public void testGetAllNotes() throws Exception { assertEquals(1, notebook.getAllNotes(new AuthenticationInfo("user2")).size()); } + + @Test + public void testGetAllNotesWithDifferentPermissions() throws IOException { + AuthenticationInfo user1 = new AuthenticationInfo("user1"); + AuthenticationInfo user2 = new AuthenticationInfo("user2"); + List notes1 = notebook.getAllNotes(user1); + List notes2 = notebook.getAllNotes(user2); + assertEquals(notes1.size(), 0); + assertEquals(notes2.size(), 0); + + //creates note and sets user1 owner + Note note = notebook.createNote(user1); + + // note is public since readers and writers empty + notes1 = notebook.getAllNotes(user1); + notes2 = notebook.getAllNotes(user2); + assertEquals(notes1.size(), 1); + assertEquals(notes2.size(), 1); + + notebook.getNotebookAuthorization().setReaders(note.getId(), Sets.newHashSet("user1")); + //note is public since writers empty + notes1 = notebook.getAllNotes(user1); + notes2 = notebook.getAllNotes(user2); + assertEquals(notes1.size(), 1); + assertEquals(notes2.size(), 1); + + notebook.getNotebookAuthorization().setWriters(note.getId(), Sets.newHashSet("user1")); + notes1 = notebook.getAllNotes(user1); + notes2 = notebook.getAllNotes(user2); + assertEquals(notes1.size(), 1); + assertEquals(notes2.size(), 0); + } + private void delete(File file){ if(file.isFile()) file.delete(); else if(file.isDirectory()){ From 9427e6260ab66665ed3cf8a606152f5361d7ec79 Mon Sep 17 00:00:00 2001 From: Khalid Huseynov Date: Tue, 20 Sep 2016 19:04:25 +0900 Subject: [PATCH 07/13] multicast fine grained note lists to users instead of broadcast --- .../zeppelin/socket/NotebookServer.java | 59 +++++++++++++++++-- .../zeppelin/socket/NotebookSocket.java | 10 ++++ 2 files changed, 63 insertions(+), 6 deletions(-) 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 a99c06c48b5..5fbb43329af 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 @@ -59,6 +59,7 @@ import java.net.URISyntaxException; import java.net.UnknownHostException; import java.util.*; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentLinkedQueue; /** @@ -85,6 +86,8 @@ String getKey() { Gson gson = new GsonBuilder().setDateFormat("yyyy-MM-dd'T'HH:mm:ssZ").create(); final Map> noteSocketMap = new HashMap<>(); final Queue connectedSockets = new ConcurrentLinkedQueue<>(); + final Map> userConnectedSockets = + new ConcurrentHashMap>(); private Notebook notebook() { return ZeppelinServer.notebook; @@ -160,6 +163,9 @@ public void onMessage(NotebookSocket conn, String msg) { userAndRoles.addAll(roles); } } + if (StringUtils.isEmpty(conn.getUser())) { + addUserConnection(messagereceived.principal, conn); + } AuthenticationInfo subject = new AuthenticationInfo(messagereceived.principal); /** Lets be elegant here */ @@ -168,8 +174,7 @@ public void onMessage(NotebookSocket conn, String msg) { unicastNoteList(conn, subject); break; case RELOAD_NOTES_FROM_REPO: - //broadcastReloadedNoteList(subject); - unicastNoteList(conn, subject); + broadcastReloadedNoteList(subject); break; case GET_HOME_NOTE: sendHomeNote(conn, userAndRoles, notebook, messagereceived); @@ -265,6 +270,26 @@ public void onClose(NotebookSocket conn, int code, String reason) { .getRemoteAddr(), conn.getRequest().getRemotePort(), code, reason); removeConnectionFromAllNote(conn); connectedSockets.remove(conn); + removeUserConnection(conn.getUser(), conn); + } + + private void removeUserConnection(String user, NotebookSocket conn) { + if (userConnectedSockets.containsKey(user)) { + userConnectedSockets.get(user).remove(conn); + } else { + LOG.warn("Closing connection that is absent in user connections"); + } + } + + private void addUserConnection(String user, NotebookSocket conn) { + conn.setUser(user); + if (userConnectedSockets.containsKey(user)) { + userConnectedSockets.get(user).add(conn); + } else { + Queue socketQueue = new ConcurrentLinkedQueue<>(); + socketQueue.add(conn); + userConnectedSockets.put(user, socketQueue); + } } protected Message deserializeMessage(String msg) { @@ -380,8 +405,12 @@ private void broadcastExcept(String noteId, Message m, NotebookSocket exclude) { } } - private void broadcastAll(Message m) { - for (NotebookSocket conn : connectedSockets) { + private void multicastToUser(String user, Message m) { + if (!userConnectedSockets.containsKey(user)) { + LOG.warn("Broadcasting to user that is not in connections map"); + return; + } + for (NotebookSocket conn: userConnectedSockets.get(user)) { try { conn.send(serializeMessage(m)); } catch (IOException e) { @@ -502,8 +531,17 @@ public void broadcastInterpreterBindings(String noteId, } public void broadcastNoteList(AuthenticationInfo subject) { + //send first to requesting user List> notesInfo = generateNotebooksInfo(false, subject); - broadcastAll(new Message(OP.NOTES_INFO).put("notes", notesInfo)); + 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)); + multicastToUser(user, new Message(OP.NOTES_INFO).put("notes", notesInfo)); + } } public void unicastNoteList(NotebookSocket conn, AuthenticationInfo subject) { @@ -512,8 +550,17 @@ public void unicastNoteList(NotebookSocket conn, AuthenticationInfo subject) { } public void broadcastReloadedNoteList(AuthenticationInfo subject) { + //send first to requesting user List> notesInfo = generateNotebooksInfo(true, subject); - broadcastAll(new Message(OP.NOTES_INFO).put("notes", notesInfo)); + 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(true, new AuthenticationInfo(user)); + multicastToUser(user, new Message(OP.NOTES_INFO).put("notes", notesInfo)); + } } void permissionError(NotebookSocket conn, String op, diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookSocket.java b/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookSocket.java index 5d68bf5ec2d..2bae36b3b75 100644 --- a/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookSocket.java +++ b/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookSocket.java @@ -20,6 +20,7 @@ import javax.servlet.http.HttpServletRequest; +import org.apache.commons.lang.StringUtils; import org.eclipse.jetty.websocket.api.Session; import org.eclipse.jetty.websocket.api.WebSocketAdapter; @@ -32,12 +33,14 @@ public class NotebookSocket extends WebSocketAdapter { private NotebookSocketListener listener; private HttpServletRequest request; private String protocol; + private String user; public NotebookSocket(HttpServletRequest req, String protocol, NotebookSocketListener listener) { this.listener = listener; this.request = req; this.protocol = protocol; + this.user = StringUtils.EMPTY; } @Override @@ -69,4 +72,11 @@ public void send(String serializeMessage) throws IOException { connection.getRemote().sendString(serializeMessage); } + public String getUser() { + return user; + } + + public void setUser(String user) { + this.user = user; + } } From 09e67234821823f1e5cf2f4acf2329b36abdbed9 Mon Sep 17 00:00:00 2001 From: Khalid Huseynov Date: Mon, 19 Sep 2016 14:46:06 +0900 Subject: [PATCH 08/13] notebookAuthorization as singleton --- .../zeppelin/server/ZeppelinServer.java | 2 +- .../notebook/NotebookAuthorization.java | 33 ++++++++++++++----- .../helium/HeliumApplicationFactoryTest.java | 2 +- .../zeppelin/notebook/NotebookTest.java | 2 +- .../notebook/repo/NotebookRepoSyncTest.java | 2 +- 5 files changed, 28 insertions(+), 13 deletions(-) diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/server/ZeppelinServer.java b/zeppelin-server/src/main/java/org/apache/zeppelin/server/ZeppelinServer.java index d352c080176..48956c912c9 100644 --- a/zeppelin-server/src/main/java/org/apache/zeppelin/server/ZeppelinServer.java +++ b/zeppelin-server/src/main/java/org/apache/zeppelin/server/ZeppelinServer.java @@ -92,7 +92,7 @@ public ZeppelinServer() throws Exception { notebookWsServer, heliumApplicationFactory, depResolver); this.notebookRepo = new NotebookRepoSync(conf); this.notebookIndex = new LuceneSearch(); - this.notebookAuthorization = new NotebookAuthorization(conf); + this.notebookAuthorization = NotebookAuthorization.init(conf); this.credentials = new Credentials(conf.credentialsPersist(), conf.getCredentialsPath()); notebook = new Notebook(conf, notebookRepo, schedulerFactory, replFactory, notebookWsServer, diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NotebookAuthorization.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NotebookAuthorization.java index 0633906d110..8fed6f0d845 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NotebookAuthorization.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NotebookAuthorization.java @@ -31,17 +31,22 @@ */ public class NotebookAuthorization { private static final Logger LOG = LoggerFactory.getLogger(NotebookAuthorization.class); - + private static NotebookAuthorization instance = null; /* * { "note1": { "owners": ["u1"], "readers": ["u1", "u2"], "writers": ["u1"] }, "note2": ... } } */ - private Map>> authInfo = new HashMap<>(); - private ZeppelinConfiguration conf; - private Gson gson; - private String filePath; + private static Map>> authInfo = new HashMap<>(); + private static ZeppelinConfiguration conf; + private static Gson gson; + private static String filePath; + + private NotebookAuthorization() {} - public NotebookAuthorization(ZeppelinConfiguration conf) { - this.conf = conf; + public static NotebookAuthorization init(ZeppelinConfiguration config) { + if (instance == null) { + instance = new NotebookAuthorization(); + } + conf = config; filePath = conf.getNotebookAuthorizationPath(); GsonBuilder builder = new GsonBuilder(); builder.setPrettyPrinting(); @@ -51,9 +56,19 @@ public NotebookAuthorization(ZeppelinConfiguration conf) { } catch (IOException e) { LOG.error("Error loading NotebookAuthorization", e); } + return instance; + } + + public static NotebookAuthorization getInstance() { + if (instance == null) { + LOG.warn("Notebook authorization module was called without initialization," + + " initializing with default configuration"); + init(ZeppelinConfiguration.create()); + } + return instance; } - private void loadFromFile() throws IOException { + private static void loadFromFile() throws IOException { File settingFile = new File(filePath); LOG.info(settingFile.getAbsolutePath()); if (!settingFile.exists()) { @@ -74,7 +89,7 @@ private void loadFromFile() throws IOException { String json = sb.toString(); NotebookAuthorizationInfoSaving info = gson.fromJson(json, NotebookAuthorizationInfoSaving.class); - this.authInfo = info.authInfo; + authInfo = info.authInfo; } private void saveToFile() { diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/helium/HeliumApplicationFactoryTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/helium/HeliumApplicationFactoryTest.java index b32b3d8feb2..29cdf554de6 100644 --- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/helium/HeliumApplicationFactoryTest.java +++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/helium/HeliumApplicationFactoryTest.java @@ -89,7 +89,7 @@ public void setUp() throws Exception { SearchService search = mock(SearchService.class); notebookRepo = new VFSNotebookRepo(conf); - NotebookAuthorization notebookAuthorization = new NotebookAuthorization(conf); + NotebookAuthorization notebookAuthorization = NotebookAuthorization.init(conf); notebook = new Notebook( conf, notebookRepo, 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 e2567467fb8..9e395c83220 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 @@ -91,7 +91,7 @@ public void setUp() throws Exception { SearchService search = mock(SearchService.class); notebookRepo = new VFSNotebookRepo(conf); - notebookAuthorization = new NotebookAuthorization(conf); + notebookAuthorization = NotebookAuthorization.init(conf); credentials = new Credentials(conf.credentialsPersist(), conf.getCredentialsPath()); notebook = new Notebook(conf, notebookRepo, schedulerFactory, factory, this, search, diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/NotebookRepoSyncTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/NotebookRepoSyncTest.java index c768df80435..95b9209dc52 100644 --- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/NotebookRepoSyncTest.java +++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/NotebookRepoSyncTest.java @@ -99,7 +99,7 @@ public void setUp() throws Exception { search = mock(SearchService.class); notebookRepoSync = new NotebookRepoSync(conf); - notebookAuthorization = new NotebookAuthorization(conf); + notebookAuthorization = NotebookAuthorization.init(conf); credentials = new Credentials(conf.credentialsPersist(), conf.getCredentialsPath()); notebookSync = new Notebook(conf, notebookRepoSync, schedulerFactory, factory, this, search, notebookAuthorization, credentials); From 537cc0ebadac7bce9a9eb00fe76cd3d8da17f220 Mon Sep 17 00:00:00 2001 From: Khalid Huseynov Date: Tue, 20 Sep 2016 01:21:03 +0900 Subject: [PATCH 09/13] apply filter from authorization in sync that's to sync only user workbench notes --- .../org/apache/zeppelin/notebook/Notebook.java | 15 +-------------- .../zeppelin/notebook/NotebookAuthorization.java | 16 ++++++++++++++++ .../zeppelin/notebook/repo/NotebookRepoSync.java | 5 ++++- 3 files changed, 21 insertions(+), 15 deletions(-) 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 823d8e50bc0..568f193a5b5 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 @@ -482,25 +482,12 @@ public void reloadAllNotes(AuthenticationInfo subject) throws IOException { } List noteInfos = notebookRepo.list(subject); - noteInfos = filterByUser(noteInfos, subject); + noteInfos = notebookAuthorization.filterByUser(noteInfos, subject); for (NoteInfo info : noteInfos) { loadNoteFromRepo(info.getId(), subject); } } - - private List filterByUser(List notes, AuthenticationInfo subject) { - final Set entities = Sets.newHashSet(); - if (subject != null) { - entities.add(subject.getUser()); - } - return FluentIterable.from(notes).filter(new Predicate() { - @Override - public boolean apply(NoteInfo input) { - return input != null && notebookAuthorization.isReader(input.getId(), entities); - } - }).toList(); - } private class SnapshotAngularObject { String intpGroupId; diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NotebookAuthorization.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NotebookAuthorization.java index 8fed6f0d845..0724dfa9e4a 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NotebookAuthorization.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NotebookAuthorization.java @@ -17,9 +17,13 @@ package org.apache.zeppelin.notebook; +import com.google.common.base.Predicate; +import com.google.common.collect.FluentIterable; +import com.google.common.collect.Sets; import com.google.gson.Gson; import com.google.gson.GsonBuilder; import org.apache.zeppelin.conf.ZeppelinConfiguration; +import org.apache.zeppelin.user.AuthenticationInfo; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -240,4 +244,16 @@ public void removeNote(String noteId) { saveToFile(); } + public List filterByUser(List notes, AuthenticationInfo subject) { + final Set entities = Sets.newHashSet(); + if (subject != null) { + entities.add(subject.getUser()); + } + return FluentIterable.from(notes).filter(new Predicate() { + @Override + public boolean apply(NoteInfo input) { + return input != null && isReader(input.getId(), entities); + } + }).toList(); + } } diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/NotebookRepoSync.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/NotebookRepoSync.java index f67b71f5d6c..21a1d7a25d9 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/NotebookRepoSync.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/NotebookRepoSync.java @@ -31,6 +31,7 @@ import org.apache.zeppelin.conf.ZeppelinConfiguration.ConfVars; import org.apache.zeppelin.notebook.Note; import org.apache.zeppelin.notebook.NoteInfo; +import org.apache.zeppelin.notebook.NotebookAuthorization; import org.apache.zeppelin.notebook.Paragraph; import org.apache.zeppelin.user.AuthenticationInfo; import org.slf4j.Logger; @@ -178,9 +179,11 @@ public void remove(String noteId, AuthenticationInfo subject) throws IOException */ void sync(int sourceRepoIndex, int destRepoIndex, AuthenticationInfo subject) throws IOException { LOG.info("Sync started"); + NotebookAuthorization auth = NotebookAuthorization.getInstance(); NotebookRepo srcRepo = getRepo(sourceRepoIndex); NotebookRepo dstRepo = getRepo(destRepoIndex); - List srcNotes = srcRepo.list(subject); + List allSrcNotes = srcRepo.list(subject); + List srcNotes = auth.filterByUser(allSrcNotes, subject); List dstNotes = dstRepo.list(subject); Map> noteIDs = notesCheckDiff(srcNotes, srcRepo, dstNotes, dstRepo); From 781207e15b44dd90506910e8eceab2f805250a15 Mon Sep 17 00:00:00 2001 From: Khalid Huseynov Date: Wed, 21 Sep 2016 01:25:55 +0900 Subject: [PATCH 10/13] bugfix: reload only once --- .../main/java/org/apache/zeppelin/socket/NotebookServer.java | 5 +++-- .../src/main/java/org/apache/zeppelin/notebook/Notebook.java | 1 - 2 files changed, 3 insertions(+), 3 deletions(-) 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 5fbb43329af..b5da147d27a 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 @@ -550,7 +550,7 @@ public void unicastNoteList(NotebookSocket conn, AuthenticationInfo subject) { } public void broadcastReloadedNoteList(AuthenticationInfo subject) { - //send first to requesting user + //reload and reply first to requesting user List> notesInfo = generateNotebooksInfo(true, subject); multicastToUser(subject.getUser(), new Message(OP.NOTES_INFO).put("notes", notesInfo)); //to others afterwards @@ -558,7 +558,8 @@ public void broadcastReloadedNoteList(AuthenticationInfo subject) { if (subject.getUser() == user) { continue; } - notesInfo = generateNotebooksInfo(true, new AuthenticationInfo(user)); + //reloaded already above; parameter - false + notesInfo = generateNotebooksInfo(false, new AuthenticationInfo(user)); multicastToUser(user, new Message(OP.NOTES_INFO).put("notes", notesInfo)); } } 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 568f193a5b5..1e65a86e21a 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 @@ -482,7 +482,6 @@ public void reloadAllNotes(AuthenticationInfo subject) throws IOException { } List noteInfos = notebookRepo.list(subject); - noteInfos = notebookAuthorization.filterByUser(noteInfos, subject); for (NoteInfo info : noteInfos) { loadNoteFromRepo(info.getId(), subject); From 17eae84c0c259c6181df2ac49a63349adced2ed7 Mon Sep 17 00:00:00 2001 From: Khalid Huseynov Date: Wed, 21 Sep 2016 13:20:13 +0900 Subject: [PATCH 11/13] bugfix: add precondition for NP --- .../java/org/apache/zeppelin/socket/NotebookServer.java | 6 ++++++ 1 file changed, 6 insertions(+) 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 b5da147d27a..63764c1620e 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 @@ -531,6 +531,9 @@ public void broadcastInterpreterBindings(String noteId, } public void broadcastNoteList(AuthenticationInfo subject) { + if (subject == null) { + subject = new AuthenticationInfo(StringUtils.EMPTY); + } //send first to requesting user List> notesInfo = generateNotebooksInfo(false, subject); multicastToUser(subject.getUser(), new Message(OP.NOTES_INFO).put("notes", notesInfo)); @@ -550,6 +553,9 @@ public void unicastNoteList(NotebookSocket conn, AuthenticationInfo subject) { } public void broadcastReloadedNoteList(AuthenticationInfo subject) { + if (subject == null) { + subject = new AuthenticationInfo(StringUtils.EMPTY); + } //reload and reply first to requesting user List> notesInfo = generateNotebooksInfo(true, subject); multicastToUser(subject.getUser(), new Message(OP.NOTES_INFO).put("notes", notesInfo)); From 9cf1d8848ee02b5fafda1afd538b71befd3eb412 Mon Sep 17 00:00:00 2001 From: Khalid Huseynov Date: Fri, 23 Sep 2016 18:13:51 +0900 Subject: [PATCH 12/13] fix init not to initialize every time --- .../notebook/NotebookAuthorization.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NotebookAuthorization.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NotebookAuthorization.java index 0724dfa9e4a..75dc61b30f6 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NotebookAuthorization.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NotebookAuthorization.java @@ -49,16 +49,16 @@ private NotebookAuthorization() {} public static NotebookAuthorization init(ZeppelinConfiguration config) { if (instance == null) { instance = new NotebookAuthorization(); - } - conf = config; - filePath = conf.getNotebookAuthorizationPath(); - GsonBuilder builder = new GsonBuilder(); - builder.setPrettyPrinting(); - gson = builder.create(); - try { - loadFromFile(); - } catch (IOException e) { - LOG.error("Error loading NotebookAuthorization", e); + conf = config; + filePath = conf.getNotebookAuthorizationPath(); + GsonBuilder builder = new GsonBuilder(); + builder.setPrettyPrinting(); + gson = builder.create(); + try { + loadFromFile(); + } catch (IOException e) { + LOG.error("Error loading NotebookAuthorization", e); + } } return instance; } From a2ce26889117f364f9d2a5242f6c4daa1c25e265 Mon Sep 17 00:00:00 2001 From: Khalid Huseynov Date: Tue, 27 Sep 2016 12:56:29 +0900 Subject: [PATCH 13/13] broadcast note list on perm update - zeppelin-1438 --- .../src/main/java/org/apache/zeppelin/rest/NotebookRestApi.java | 1 + 1 file changed, 1 insertion(+) 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 727211292b2..b83a8891d0b 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 @@ -162,6 +162,7 @@ public Response putNotePermissions(@PathParam("noteId") String noteId, String re AuthenticationInfo subject = new AuthenticationInfo(SecurityUtils.getPrincipal()); note.persist(subject); notebookServer.broadcastNote(note); + notebookServer.broadcastNoteList(subject); return new JsonResponse<>(Status.OK).build(); }