Skip to content

[ZEPPELIN-1313] NullPointerException when using Clone notebook REST API #1348

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
kavinkumarks wants to merge 2 commits into from
Closed

[ZEPPELIN-1313] NullPointerException when using Clone notebook REST API #1348

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
11a2903
Fixed NPE when the input json is empty for clone notebook REST API and
kavinkumarks Aug 19, 2016
941e13b
Removed note.id() and replaced the references with note.getId()
Aug 23, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions zeppelin-server/src/main/java/org/apache/zeppelin/rest/NotebookRestApi.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,10 @@ public Response cloneNote(@PathParam("notebookId") String notebookId, String mes
throws IOException, CloneNotSupportedException, IllegalArgumentException {
LOG.info("clone notebook by JSON {}", message);
NewNotebookRequest request = gson.fromJson(message, NewNotebookRequest.class);
String newNoteName = request.getName();
String newNoteName = null;
if (request != null) {
newNoteName = request.getName();
}
AuthenticationInfo subject = new AuthenticationInfo(SecurityUtils.getPrincipal());
Note newNote = notebook.cloneNote(notebookId, newNoteName, subject);
notebookServer.broadcastNote(newNote);
Expand Down Expand Up @@ -660,7 +663,7 @@ public Response registerCronJob(@PathParam("notebookId") String notebookId, Stri
Map<String, Object> config = note.getConfig();
config.put("cron", request.getCronString());
note.setConfig(config);
notebook.refreshCron(note.id());
notebook.refreshCron(note.getId());

return new JsonResponse<>(Status.OK).build();
}
Expand Down Expand Up @@ -688,7 +691,7 @@ public Response removeCronJob(@PathParam("notebookId") String notebookId)
Map<String, Object> config = note.getConfig();
config.put("cron", null);
note.setConfig(config);
notebook.refreshCron(note.id());
notebook.refreshCron(note.getId());

return new JsonResponse<>(Status.OK).build();
}
Expand Down
62 changes: 31 additions & 31 deletions zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ private void broadcastToNoteBindedInterpreter(String interpreterGroupId,
List<String> ids = notebook.getInterpreterFactory().getInterpreters(note.getId());
for (String id : ids) {
if (id.equals(interpreterGroupId)) {
broadcast(note.id(), m);
broadcast(note.getId(), m);
}
}
}
Expand Down Expand Up @@ -473,11 +473,11 @@ public List<Map<String, String>> generateNotebooksInfo(boolean needsReload,
for (Note note : notes) {
Map<String, String> info = new HashMap<>();

if (hideHomeScreenNotebookFromList && note.id().equals(homescreenNotebookId)) {
if (hideHomeScreenNotebookFromList && note.getId().equals(homescreenNotebookId)) {
continue;
}

info.put("id", note.id());
info.put("id", note.getId());
info.put("name", note.getName());
notesInfo.add(info);
}
Expand All @@ -486,7 +486,7 @@ public List<Map<String, String>> generateNotebooksInfo(boolean needsReload,
}

public void broadcastNote(Note note) {
broadcast(note.id(), new Message(OP.NOTE).put("note", note));
broadcast(note.getId(), new Message(OP.NOTE).put("note", note));
}

public void broadcastInterpreterBindings(String noteId,
Expand Down Expand Up @@ -544,7 +544,7 @@ private void sendNote(NotebookSocket conn, HashSet<String> userAndRoles, Noteboo
notebookAuthorization.getReaders(noteId));
return;
}
addConnectionToNote(note.id(), conn);
addConnectionToNote(note.getId(), conn);
conn.send(serializeMessage(new Message(OP.NOTE).put("note", note)));
sendAllAngularObjects(note, conn);
} else {
Expand All @@ -568,7 +568,7 @@ private void sendHomeNote(NotebookSocket conn, HashSet<String> userAndRoles,
userAndRoles, notebookAuthorization.getReaders(noteId));
return;
}
addConnectionToNote(note.id(), conn);
addConnectionToNote(note.getId(), conn);
conn.send(serializeMessage(new Message(OP.NOTE).put("note", note)));
sendAllAngularObjects(note, conn);
} else {
Expand Down Expand Up @@ -604,7 +604,7 @@ private void updateNote(NotebookSocket conn, HashSet<String> userAndRoles,
note.setName(name);
note.setConfig(config);
if (cronUpdated) {
notebook.refreshCron(note.id());
notebook.refreshCron(note.getId());
}

AuthenticationInfo subject = new AuthenticationInfo(fromMessage.principal);
Expand Down Expand Up @@ -643,7 +643,7 @@ private void createNote(NotebookSocket conn, HashSet<String> userAndRoles,
}

note.persist(subject);
addConnectionToNote(note.id(), (NotebookSocket) conn);
addConnectionToNote(note.getId(), (NotebookSocket) conn);
conn.send(serializeMessage(new Message(OP.NEW_NOTE).put("note", note)));
broadcastNoteList(subject);
}
Expand Down Expand Up @@ -697,7 +697,7 @@ private void updateParagraph(NotebookSocket conn, HashSet<String> userAndRoles,
p.setTitle((String) fromMessage.get("title"));
p.setText((String) fromMessage.get("paragraph"));
note.persist(subject);
broadcast(note.id(), new Message(OP.PARAGRAPH).put("paragraph", p));
broadcast(note.getId(), new Message(OP.PARAGRAPH).put("paragraph", p));
}

private void cloneNote(NotebookSocket conn, HashSet<String> userAndRoles,
Expand All @@ -707,7 +707,7 @@ private void cloneNote(NotebookSocket conn, HashSet<String> userAndRoles,
String name = (String) fromMessage.get("name");
Note newNote = notebook.cloneNote(noteId, name, new AuthenticationInfo(fromMessage.principal));
AuthenticationInfo subject = new AuthenticationInfo(fromMessage.principal);
addConnectionToNote(newNote.id(), (NotebookSocket) conn);
addConnectionToNote(newNote.getId(), (NotebookSocket) conn);
conn.send(serializeMessage(new Message(OP.NEW_NOTE).put("note", newNote)));
broadcastNoteList(subject);
}
Expand Down Expand Up @@ -810,12 +810,12 @@ private void angularObjectUpdated(NotebookSocket conn, HashSet<String> userAndRo
List<InterpreterSetting> settings = notebook.getInterpreterFactory()
.getInterpreterSettings(note.getId());
for (InterpreterSetting setting : settings) {
if (setting.getInterpreterGroup(note.id()) == null) {
if (setting.getInterpreterGroup(note.getId()) == null) {
continue;
}
if (interpreterGroupId.equals(setting.getInterpreterGroup(note.id()).getId())) {
if (interpreterGroupId.equals(setting.getInterpreterGroup(note.getId()).getId())) {
AngularObjectRegistry angularObjectRegistry = setting
.getInterpreterGroup(note.id()).getAngularObjectRegistry();
.getInterpreterGroup(note.getId()).getAngularObjectRegistry();

// first trying to get local registry
ao = angularObjectRegistry.get(varName, noteId, paragraphId);
Expand Down Expand Up @@ -852,28 +852,28 @@ private void angularObjectUpdated(NotebookSocket conn, HashSet<String> userAndRo
List<InterpreterSetting> settings = notebook.getInterpreterFactory()
.getInterpreterSettings(note.getId());
for (InterpreterSetting setting : settings) {
if (setting.getInterpreterGroup(n.id()) == null) {
if (setting.getInterpreterGroup(n.getId()) == null) {
continue;
}
if (interpreterGroupId.equals(setting.getInterpreterGroup(n.id()).getId())) {
if (interpreterGroupId.equals(setting.getInterpreterGroup(n.getId()).getId())) {
AngularObjectRegistry angularObjectRegistry = setting
.getInterpreterGroup(n.id()).getAngularObjectRegistry();
.getInterpreterGroup(n.getId()).getAngularObjectRegistry();
this.broadcastExcept(
n.id(),
n.getId(),
new Message(OP.ANGULAR_OBJECT_UPDATE).put("angularObject", ao)
.put("interpreterGroupId", interpreterGroupId)
.put("noteId", n.id())
.put("noteId", n.getId())
.put("paragraphId", ao.getParagraphId()),
conn);
}
}
}
} else { // broadcast to all web session for the note
this.broadcastExcept(
note.id(),
note.getId(),
new Message(OP.ANGULAR_OBJECT_UPDATE).put("angularObject", ao)
.put("interpreterGroupId", interpreterGroupId)
.put("noteId", note.id())
.put("noteId", note.getId())
.put("paragraphId", ao.getParagraphId()),
conn);
}
Expand Down Expand Up @@ -1149,7 +1149,7 @@ private void runParagraph(NotebookSocket conn, HashSet<String> userAndRoles, Not
new InterpreterResult(InterpreterResult.Code.ERROR, ex.getMessage()),
ex);
p.setStatus(Status.ERROR);
broadcast(note.id(), new Message(OP.PARAGRAPH).put("paragraph", p));
broadcast(note.getId(), new Message(OP.PARAGRAPH).put("paragraph", p));
}
}
}
Expand Down Expand Up @@ -1309,7 +1309,7 @@ public ParagraphListenerImpl(NotebookServer notebookServer, Note note) {
@Override
public void onProgressUpdate(Job job, int progress) {
notebookServer.broadcast(
note.id(),
note.getId(),
new Message(OP.PROGRESS).put("id", job.getId()).put("progress",
job.progress()));
}
Expand Down Expand Up @@ -1384,15 +1384,15 @@ private void sendAllAngularObjects(Note note, NotebookSocket conn) throws IOExce
}

for (InterpreterSetting intpSetting : settings) {
AngularObjectRegistry registry = intpSetting.getInterpreterGroup(note.id())
AngularObjectRegistry registry = intpSetting.getInterpreterGroup(note.getId())
.getAngularObjectRegistry();
List<AngularObject> objects = registry.getAllWithGlobal(note.id());
List<AngularObject> objects = registry.getAllWithGlobal(note.getId());
for (AngularObject object : objects) {
conn.send(serializeMessage(new Message(OP.ANGULAR_OBJECT_UPDATE)
.put("angularObject", object)
.put("interpreterGroupId",
intpSetting.getInterpreterGroup(note.id()).getId())
.put("noteId", note.id())
intpSetting.getInterpreterGroup(note.getId()).getId())
.put("noteId", note.getId())
.put("paragraphId", object.getParagraphId())
));
}
Expand All @@ -1413,7 +1413,7 @@ public void onUpdate(String interpreterGroupId, AngularObject object) {

List<Note> notes = notebook.getAllNotes();
for (Note note : notes) {
if (object.getNoteId() != null && !note.id().equals(object.getNoteId())) {
if (object.getNoteId() != null && !note.getId().equals(object.getNoteId())) {
continue;
}

Expand All @@ -1424,11 +1424,11 @@ public void onUpdate(String interpreterGroupId, AngularObject object) {
}

broadcast(
note.id(),
note.getId(),
new Message(OP.ANGULAR_OBJECT_UPDATE)
.put("angularObject", object)
.put("interpreterGroupId", interpreterGroupId)
.put("noteId", note.id())
.put("noteId", note.getId())
.put("paragraphId", object.getParagraphId()));
}
}
Expand All @@ -1438,15 +1438,15 @@ public void onRemove(String interpreterGroupId, String name, String noteId, Stri
Notebook notebook = notebook();
List<Note> notes = notebook.getAllNotes();
for (Note note : notes) {
if (noteId != null && !note.id().equals(noteId)) {
if (noteId != null && !note.getId().equals(noteId)) {
continue;
}

List<String> ids = notebook.getInterpreterFactory().getInterpreters(note.getId());
for (String id : ids) {
if (id.equals(interpreterGroupId)) {
broadcast(
note.id(),
note.getId(),
new Message(OP.ANGULAR_OBJECT_REMOVE).put("name", name).put(
"noteId", noteId).put("paragraphId", paragraphId));
}
Expand Down
2 changes: 1 addition & 1 deletion zeppelin-server/src/test/java/org/apache/zeppelin/rest/InterpreterRestApiTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ public void testInterpreterAutoBinding() throws IOException {
Note note = ZeppelinServer.notebook.createNote(null);

// check interpreter is binded
GetMethod get = httpGet("/notebook/interpreter/bind/" + note.id());
GetMethod get = httpGet("/notebook/interpreter/bind/" + note.getId());
assertThat(get, isAllowed());
get.addRequestHeader("Origin", "http://localhost");
Map<String, Object> resp = gson.fromJson(get.getResponseBodyAsString(), new TypeToken<Map<String, Object>>() {
Expand Down
27 changes: 27 additions & 0 deletions zeppelin-server/src/test/java/org/apache/zeppelin/rest/NotebookRestApiTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import com.google.gson.Gson;
import com.google.gson.reflect.TypeToken;
import org.apache.commons.httpclient.methods.GetMethod;
import org.apache.commons.httpclient.methods.PostMethod;
import org.apache.commons.httpclient.methods.PutMethod;
import org.apache.zeppelin.notebook.Note;
import org.apache.zeppelin.notebook.NotebookAuthorization;
Expand Down Expand Up @@ -144,6 +145,32 @@ public void testGetNoteParagraphJobStatus() throws IOException {
ZeppelinServer.notebook.removeNote(note1.getId(), null);

}

@Test
public void testCloneNotebook() throws IOException {
Note note1 = ZeppelinServer.notebook.createNote(null);
PostMethod post = httpPost("/notebook/" + note1.getId(), "");
LOG.info("testCloneNotebook response\n" + post.getResponseBodyAsString());
assertThat(post, isCreated());
Map<String, Object> resp = gson.fromJson(post.getResponseBodyAsString(), new TypeToken<Map<String, Object>>() {
}.getType());
String clonedNotebookId = (String) resp.get("body");
post.releaseConnection();

GetMethod get = httpGet("/notebook/" + clonedNotebookId);
assertThat(get, isAllowed());
Map<String, Object> resp2 = gson.fromJson(get.getResponseBodyAsString(), new TypeToken<Map<String, Object>>() {
}.getType());
Map<String, Object> resp2Body = (Map<String, Object>) resp2.get("body");

assertEquals((String)resp2Body.get("name"), "Note " + clonedNotebookId);
get.releaseConnection();

//cleanup
ZeppelinServer.notebook.removeNote(note1.getId(), null);
ZeppelinServer.notebook.removeNote(clonedNotebookId, null);

}
}


14 changes: 7 additions & 7 deletions zeppelin-server/src/test/java/org/apache/zeppelin/rest/ZeppelinSparkClusterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ public void basicRDDTransformationAndActionTest() throws IOException {
waitForFinish(p);
assertEquals(Status.FINISHED, p.getStatus());
assertEquals("55", p.getResult().message());
ZeppelinServer.notebook.removeNote(note.id(), null);
ZeppelinServer.notebook.removeNote(note.getId(), null);
}

@Test
Expand All @@ -91,7 +91,7 @@ public void sparkRTest() throws IOException {
if (isSparkR() && sparkVersion >= 14) { // sparkr supported from 1.4.0
// restart spark interpreter
List<InterpreterSetting> settings =
ZeppelinServer.notebook.getBindedInterpreterSettings(note.id());
ZeppelinServer.notebook.getBindedInterpreterSettings(note.getId());

for (InterpreterSetting setting : settings) {
if (setting.getName().equals("spark")) {
Expand All @@ -118,7 +118,7 @@ public void sparkRTest() throws IOException {
assertEquals(Status.FINISHED, p.getStatus());
assertEquals("[1] 3", p.getResult().message());
}
ZeppelinServer.notebook.removeNote(note.id(), null);
ZeppelinServer.notebook.removeNote(note.getId(), null);
}

@Test
Expand All @@ -141,7 +141,7 @@ public void pySparkTest() throws IOException {
assertEquals(Status.FINISHED, p.getStatus());
assertEquals("55\n", p.getResult().message());
}
ZeppelinServer.notebook.removeNote(note.id(), null);
ZeppelinServer.notebook.removeNote(note.getId(), null);
}

@Test
Expand Down Expand Up @@ -172,7 +172,7 @@ public void pySparkAutoConvertOptionTest() throws IOException {
assertEquals(Status.FINISHED, p.getStatus());
assertEquals("10\n", p.getResult().message());
}
ZeppelinServer.notebook.removeNote(note.id(), null);
ZeppelinServer.notebook.removeNote(note.getId(), null);
}

@Test
Expand Down Expand Up @@ -204,7 +204,7 @@ public void zRunTest() throws IOException {
assertEquals(Status.FINISHED, p2.getStatus());
assertEquals("10", p2.getResult().message());

ZeppelinServer.notebook.removeNote(note.id(), null);
ZeppelinServer.notebook.removeNote(note.getId(), null);
}

@Test
Expand All @@ -216,7 +216,7 @@ public void pySparkDepLoaderTest() throws IOException {
if (isPyspark() && sparkVersionNumber >= 14) {
// restart spark interpreter
List<InterpreterSetting> settings =
ZeppelinServer.notebook.getBindedInterpreterSettings(note.id());
ZeppelinServer.notebook.getBindedInterpreterSettings(note.getId());

for (InterpreterSetting setting : settings) {
if (setting.getName().equals("spark")) {
Expand Down
8 changes: 2 additions & 6 deletions zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Note.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,10 +131,6 @@ void putDefaultReplName() {
lastReplName.set(defaultInterpreterName);
}

public String id() {
return id;
}

public String getId() {
return id;
}
Expand Down Expand Up @@ -292,7 +288,7 @@ private String getInterpreterName(String replName) {
*/
public Paragraph removeParagraph(String paragraphId) {
removeAllAngularObjectInParagraph(paragraphId);
ResourcePoolUtils.removeResourcesBelongsToParagraph(id(), paragraphId);
ResourcePoolUtils.removeResourcesBelongsToParagraph(getId(), paragraphId);
synchronized (paragraphs) {
Iterator<Paragraph> i = paragraphs.iterator();
while (i.hasNext()) {
Expand Down Expand Up @@ -602,7 +598,7 @@ public void persist(int maxDelaySec, AuthenticationInfo subject) {
}

void unpersist(AuthenticationInfo subject) throws IOException {
repo.remove(id(), subject);
repo.remove(getId(), subject);
}


Expand Down
2 changes: 1 addition & 1 deletion zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NoteInfo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public NoteInfo(String id, String name, Map<String, Object> config) {
}

public NoteInfo(Note note) {
id = note.id();
id = note.getId();
name = note.getName();
config = note.getConfig();
}
Expand Down
Loading