Repository: zeppelin Updated Branches: refs/heads/master f1a247130 -> 83469be5b
[ZEPPELIN-1313] NullPointerException when using Clone notebook REST API ### What is this PR for? This fixes when the input json is empty for the clone notebook REST API and for this case the default name will be populated. ### What type of PR is it? Bug Fix ### Todos NA ### What is the Jira issue? https://issues.apache.org/jira/browse/ZEPPELIN-1313 ### How should this be tested? When the input json is empty for the clone notebook REST API, the response should be 201 with the default name populated. ### Screenshots (if appropriate) ### Questions: * Does the licenses files need update? No * Is there breaking changes for older versions? No * Does this needs documentation? No Author: Kavin <kavin.ku...@imaginea.com> Author: Kavin Kumar <junoka...@gmail.com> Closes #1348 from kavinkumarks/zeppelin-1313-fix-NPE-clone-notebook and squashes the following commits: 941e13b [Kavin] Removed note.id() and replaced the references with note.getId() 11a2903 [Kavin Kumar] Fixed NPE when the input json is empty for clone notebook REST API and set the default name for the case.Added test cases too. Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/83469be5 Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/83469be5 Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/83469be5 Branch: refs/heads/master Commit: 83469be5bac23dbde81f511afb5bb014fce50489 Parents: f1a2471 Author: Kavin <kavin.ku...@imaginea.com> Authored: Tue Aug 23 12:33:56 2016 +0530 Committer: AhyoungRyu <ahyoung...@apache.org> Committed: Thu Sep 1 11:56:23 2016 +0900 ---------------------------------------------------------------------- .../apache/zeppelin/rest/NotebookRestApi.java | 9 ++- .../apache/zeppelin/socket/NotebookServer.java | 62 +++++++-------- .../zeppelin/rest/InterpreterRestApiTest.java | 2 +- .../zeppelin/rest/NotebookRestApiTest.java | 27 +++++++ .../zeppelin/rest/ZeppelinSparkClusterTest.java | 14 ++-- .../java/org/apache/zeppelin/notebook/Note.java | 8 +- .../org/apache/zeppelin/notebook/NoteInfo.java | 2 +- .../org/apache/zeppelin/notebook/Notebook.java | 24 +++--- .../org/apache/zeppelin/notebook/Paragraph.java | 8 +- .../zeppelin/notebook/repo/S3NotebookRepo.java | 2 +- .../zeppelin/notebook/repo/VFSNotebookRepo.java | 2 +- .../repo/zeppelinhub/ZeppelinHubRepo.java | 2 +- .../helium/HeliumApplicationFactoryTest.java | 12 +-- .../apache/zeppelin/notebook/NotebookTest.java | 82 +++++++++++--------- .../repo/zeppelinhub/ZeppelinHubRepoTest.java | 2 +- 15 files changed, 148 insertions(+), 110 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/zeppelin/blob/83469be5/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 6a286a4..db0cbec 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 @@ -319,7 +319,10 @@ public class NotebookRestApi { 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); @@ -669,7 +672,7 @@ public class NotebookRestApi { 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(); } @@ -697,7 +700,7 @@ public class NotebookRestApi { 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(); } http://git-wip-us.apache.org/repos/asf/zeppelin/blob/83469be5/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 694f081..19bfba1 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 @@ -336,7 +336,7 @@ public class NotebookServer extends WebSocketServlet implements List<String> ids = notebook.getInterpreterFactory().getInterpreters(note.getId()); for (String id : ids) { if (id.equals(interpreterGroupId)) { - broadcast(note.id(), m); + broadcast(note.getId(), m); } } } @@ -473,11 +473,11 @@ public class NotebookServer extends WebSocketServlet implements 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); } @@ -486,7 +486,7 @@ public class NotebookServer extends WebSocketServlet implements } 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, @@ -544,7 +544,7 @@ public class NotebookServer extends WebSocketServlet implements 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 { @@ -568,7 +568,7 @@ public class NotebookServer extends WebSocketServlet implements 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 { @@ -604,7 +604,7 @@ public class NotebookServer extends WebSocketServlet implements note.setName(name); note.setConfig(config); if (cronUpdated) { - notebook.refreshCron(note.id()); + notebook.refreshCron(note.getId()); } AuthenticationInfo subject = new AuthenticationInfo(fromMessage.principal); @@ -643,7 +643,7 @@ public class NotebookServer extends WebSocketServlet implements } 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); } @@ -697,7 +697,7 @@ public class NotebookServer extends WebSocketServlet implements 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, @@ -707,7 +707,7 @@ public class NotebookServer extends WebSocketServlet implements 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); } @@ -810,12 +810,12 @@ public class NotebookServer extends WebSocketServlet implements 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); @@ -852,17 +852,17 @@ public class NotebookServer extends WebSocketServlet implements 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); } @@ -870,10 +870,10 @@ public class NotebookServer extends WebSocketServlet implements } } 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); } @@ -1149,7 +1149,7 @@ public class NotebookServer extends WebSocketServlet implements 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)); } } } @@ -1309,7 +1309,7 @@ public class NotebookServer extends WebSocketServlet implements @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())); } @@ -1384,15 +1384,15 @@ public class NotebookServer extends WebSocketServlet implements } 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()) )); } @@ -1413,7 +1413,7 @@ public class NotebookServer extends WebSocketServlet implements 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; } @@ -1424,11 +1424,11 @@ public class NotebookServer extends WebSocketServlet implements } 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())); } } @@ -1438,7 +1438,7 @@ public class NotebookServer extends WebSocketServlet implements 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; } @@ -1446,7 +1446,7 @@ public class NotebookServer extends WebSocketServlet implements 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)); } http://git-wip-us.apache.org/repos/asf/zeppelin/blob/83469be5/zeppelin-server/src/test/java/org/apache/zeppelin/rest/InterpreterRestApiTest.java ---------------------------------------------------------------------- diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/InterpreterRestApiTest.java b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/InterpreterRestApiTest.java index 1ed3567..c767eb0 100644 --- a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/InterpreterRestApiTest.java +++ b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/InterpreterRestApiTest.java @@ -134,7 +134,7 @@ public class InterpreterRestApiTest extends AbstractTestRestApi { 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>>() { http://git-wip-us.apache.org/repos/asf/zeppelin/blob/83469be5/zeppelin-server/src/test/java/org/apache/zeppelin/rest/NotebookRestApiTest.java ---------------------------------------------------------------------- diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/NotebookRestApiTest.java b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/NotebookRestApiTest.java index b06c7ca..d7f55f5 100644 --- a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/NotebookRestApiTest.java +++ b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/NotebookRestApiTest.java @@ -22,6 +22,7 @@ import com.google.common.collect.Sets; 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; @@ -144,6 +145,32 @@ public class NotebookRestApiTest extends AbstractTestRestApi { 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); + + } } http://git-wip-us.apache.org/repos/asf/zeppelin/blob/83469be5/zeppelin-server/src/test/java/org/apache/zeppelin/rest/ZeppelinSparkClusterTest.java ---------------------------------------------------------------------- diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/ZeppelinSparkClusterTest.java b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/ZeppelinSparkClusterTest.java index 61dc6d1..1250f9c 100644 --- a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/ZeppelinSparkClusterTest.java +++ b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/ZeppelinSparkClusterTest.java @@ -79,7 +79,7 @@ public class ZeppelinSparkClusterTest extends AbstractTestRestApi { 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 @@ -91,7 +91,7 @@ public class ZeppelinSparkClusterTest extends AbstractTestRestApi { 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")) { @@ -118,7 +118,7 @@ public class ZeppelinSparkClusterTest extends AbstractTestRestApi { assertEquals(Status.FINISHED, p.getStatus()); assertEquals("[1] 3", p.getResult().message()); } - ZeppelinServer.notebook.removeNote(note.id(), null); + ZeppelinServer.notebook.removeNote(note.getId(), null); } @Test @@ -141,7 +141,7 @@ public class ZeppelinSparkClusterTest extends AbstractTestRestApi { assertEquals(Status.FINISHED, p.getStatus()); assertEquals("55\n", p.getResult().message()); } - ZeppelinServer.notebook.removeNote(note.id(), null); + ZeppelinServer.notebook.removeNote(note.getId(), null); } @Test @@ -172,7 +172,7 @@ public class ZeppelinSparkClusterTest extends AbstractTestRestApi { assertEquals(Status.FINISHED, p.getStatus()); assertEquals("10\n", p.getResult().message()); } - ZeppelinServer.notebook.removeNote(note.id(), null); + ZeppelinServer.notebook.removeNote(note.getId(), null); } @Test @@ -204,7 +204,7 @@ public class ZeppelinSparkClusterTest extends AbstractTestRestApi { assertEquals(Status.FINISHED, p2.getStatus()); assertEquals("10", p2.getResult().message()); - ZeppelinServer.notebook.removeNote(note.id(), null); + ZeppelinServer.notebook.removeNote(note.getId(), null); } @Test @@ -216,7 +216,7 @@ public class ZeppelinSparkClusterTest extends AbstractTestRestApi { 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")) { http://git-wip-us.apache.org/repos/asf/zeppelin/blob/83469be5/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Note.java ---------------------------------------------------------------------- diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Note.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Note.java index e2194fd..2b89524 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Note.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Note.java @@ -131,10 +131,6 @@ public class Note implements Serializable, ParagraphJobListener { lastReplName.set(defaultInterpreterName); } - public String id() { - return id; - } - public String getId() { return id; } @@ -292,7 +288,7 @@ public class Note implements Serializable, ParagraphJobListener { */ 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()) { @@ -609,7 +605,7 @@ public class Note implements Serializable, ParagraphJobListener { } void unpersist(AuthenticationInfo subject) throws IOException { - repo.remove(id(), subject); + repo.remove(getId(), subject); } http://git-wip-us.apache.org/repos/asf/zeppelin/blob/83469be5/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NoteInfo.java ---------------------------------------------------------------------- diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NoteInfo.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NoteInfo.java index f75a107..9783c76 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NoteInfo.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NoteInfo.java @@ -36,7 +36,7 @@ public class NoteInfo { } public NoteInfo(Note note) { - id = note.id(); + id = note.getId(); name = note.getName(); config = note.getConfig(); } http://git-wip-us.apache.org/repos/asf/zeppelin/blob/83469be5/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 025accd..9acb156 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 @@ -151,10 +151,10 @@ public class Notebook implements NoteEventListener { Note note = new Note(notebookRepo, replFactory, jobListenerFactory, notebookIndex, credentials, this); synchronized (notes) { - notes.put(note.id(), note); + notes.put(note.getId(), note); } if (interpreterIds != null) { - bindInterpretersToNote(note.id(), interpreterIds); + bindInterpretersToNote(note.getId(), interpreterIds); note.putDefaultReplName(); } @@ -239,10 +239,12 @@ public class Notebook implements NoteEventListener { Note newNote = createNote(subject); if (newNoteName != null) { newNote.setName(newNoteName); + } else { + newNote.setName("Note " + newNote.getId()); } // Copy the interpreter bindings - List<String> boundInterpreterSettingsIds = getBindedInterpreterSettingsIds(sourceNote.id()); - bindInterpretersToNote(newNote.id(), boundInterpreterSettingsIds); + List<String> boundInterpreterSettingsIds = getBindedInterpreterSettingsIds(sourceNote.getId()); + bindInterpretersToNote(newNote.getId(), boundInterpreterSettingsIds); List<Paragraph> paragraphs = sourceNote.getParagraphs(); for (Paragraph p : paragraphs) { @@ -419,15 +421,15 @@ public class Notebook implements NoteEventListener { note.setNoteEventListener(this); synchronized (notes) { - notes.put(note.id(), note); - refreshCron(note.id()); + notes.put(note.getId(), note); + refreshCron(note.getId()); } for (String name : angularObjectSnapshot.keySet()) { SnapshotAngularObject snapshot = angularObjectSnapshot.get(name); List<InterpreterSetting> settings = replFactory.get(); for (InterpreterSetting setting : settings) { - InterpreterGroup intpGroup = setting.getInterpreterGroup(note.id()); + InterpreterGroup intpGroup = setting.getInterpreterGroup(note.getId()); if (intpGroup.getId().equals(snapshot.getIntpGroupId())) { AngularObjectRegistry registry = intpGroup.getAngularObjectRegistry(); String noteId = snapshot.getAngularObject().getNoteId(); @@ -508,11 +510,11 @@ public class Notebook implements NoteEventListener { Collections.sort(noteList, new Comparator<Note>() { @Override public int compare(Note note1, Note note2) { - String name1 = note1.id(); + String name1 = note1.getId(); if (note1.getName() != null) { name1 = note1.getName(); } - String name2 = note2.id(); + String name2 = note2.getId(); if (note2.getName() != null) { name2 = note2.getName(); } @@ -593,14 +595,14 @@ public class Notebook implements NoteEventListener { Map<String, Object> info = new HashMap<>(); // set notebook ID - info.put("notebookId", note.id()); + info.put("notebookId", note.getId()); // set notebook Name String notebookName = note.getName(); if (notebookName != null && !notebookName.equals("")) { info.put("notebookName", note.getName()); } else { - info.put("notebookName", "Note " + note.id()); + info.put("notebookName", "Note " + note.getId()); } // set notebook type ( cron or normal ) http://git-wip-us.apache.org/repos/asf/zeppelin/blob/83469be5/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Paragraph.java ---------------------------------------------------------------------- diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Paragraph.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Paragraph.java index 079216c..60f3161 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Paragraph.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Paragraph.java @@ -451,13 +451,13 @@ public class Paragraph extends Job implements Serializable, Cloneable { if (!factory.getInterpreterSettings(note.getId()).isEmpty()) { InterpreterSetting intpGroup = factory.getInterpreterSettings(note.getId()).get(0); - registry = intpGroup.getInterpreterGroup(note.id()).getAngularObjectRegistry(); - resourcePool = intpGroup.getInterpreterGroup(note.id()).getResourcePool(); + registry = intpGroup.getInterpreterGroup(note.getId()).getAngularObjectRegistry(); + resourcePool = intpGroup.getInterpreterGroup(note.getId()).getResourcePool(); } List<InterpreterContextRunner> runners = new LinkedList<InterpreterContextRunner>(); for (Paragraph p : note.getParagraphs()) { - runners.add(new ParagraphRunner(note, note.id(), p.getId())); + runners.add(new ParagraphRunner(note, note.getId(), p.getId())); } final Paragraph self = this; @@ -470,7 +470,7 @@ public class Paragraph extends Job implements Serializable, Cloneable { } InterpreterContext interpreterContext = new InterpreterContext( - note.id(), + note.getId(), getId(), this.getTitle(), this.getText(), http://git-wip-us.apache.org/repos/asf/zeppelin/blob/83469be5/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/S3NotebookRepo.java ---------------------------------------------------------------------- diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/S3NotebookRepo.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/S3NotebookRepo.java index 8f4dd31..0163fc4 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/S3NotebookRepo.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/S3NotebookRepo.java @@ -210,7 +210,7 @@ public class S3NotebookRepo implements NotebookRepo { gsonBuilder.setPrettyPrinting(); Gson gson = gsonBuilder.create(); String json = gson.toJson(note); - String key = user + "/" + "notebook" + "/" + note.id() + "/" + "note.json"; + String key = user + "/" + "notebook" + "/" + note.getId() + "/" + "note.json"; File file = File.createTempFile("note", "json"); try { http://git-wip-us.apache.org/repos/asf/zeppelin/blob/83469be5/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/VFSNotebookRepo.java ---------------------------------------------------------------------- diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/VFSNotebookRepo.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/VFSNotebookRepo.java index 4a93dec..213fdf8 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/VFSNotebookRepo.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/VFSNotebookRepo.java @@ -226,7 +226,7 @@ public class VFSNotebookRepo implements NotebookRepo { FileObject rootDir = getRootDir(); - FileObject noteDir = rootDir.resolveFile(note.id(), NameScope.CHILD); + FileObject noteDir = rootDir.resolveFile(note.getId(), NameScope.CHILD); if (!noteDir.exists()) { noteDir.createFolder(); http://git-wip-us.apache.org/repos/asf/zeppelin/blob/83469be5/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/zeppelinhub/ZeppelinHubRepo.java ---------------------------------------------------------------------- diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/zeppelinhub/ZeppelinHubRepo.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/zeppelinhub/ZeppelinHubRepo.java index 960bcde..d1864c5 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/zeppelinhub/ZeppelinHubRepo.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/zeppelinhub/ZeppelinHubRepo.java @@ -176,7 +176,7 @@ public class ZeppelinHubRepo implements NotebookRepo { } String notebook = GSON.toJson(note); restApiClient.asyncPut(notebook); - LOG.info("ZeppelinHub REST API saving note {} ", note.id()); + LOG.info("ZeppelinHub REST API saving note {} ", note.getId()); } @Override http://git-wip-us.apache.org/repos/asf/zeppelin/blob/83469be5/zeppelin-zengine/src/test/java/org/apache/zeppelin/helium/HeliumApplicationFactoryTest.java ---------------------------------------------------------------------- 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 0af6aca..b32b3d8 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 @@ -176,7 +176,7 @@ public class HeliumApplicationFactoryTest implements JobListenerFactory { new String[][]{}); Note note1 = notebook.createNote(null); - factory.setInterpreters(note1.id(), factory.getDefaultInterpreterSettingList()); + factory.setInterpreters(note1.getId(), factory.getDefaultInterpreterSettingList()); Paragraph p1 = note1.addParagraph(); @@ -214,7 +214,7 @@ public class HeliumApplicationFactoryTest implements JobListenerFactory { new String[][]{}); Note note1 = notebook.createNote(null); - notebook.bindInterpretersToNote(note1.id(), factory.getDefaultInterpreterSettingList()); + notebook.bindInterpretersToNote(note1.getId(), factory.getDefaultInterpreterSettingList()); Paragraph p1 = note1.addParagraph(); @@ -231,7 +231,7 @@ public class HeliumApplicationFactoryTest implements JobListenerFactory { } // when unbind interpreter - notebook.bindInterpretersToNote(note1.id(), new LinkedList<String>()); + notebook.bindInterpretersToNote(note1.getId(), new LinkedList<String>()); // then assertEquals(ApplicationState.Status.UNLOADED, app.getStatus()); @@ -255,7 +255,7 @@ public class HeliumApplicationFactoryTest implements JobListenerFactory { // Unbind all interpreter from note // NullPointerException shouldn't occur here - notebook.bindInterpretersToNote(note1.id(), new LinkedList<String>()); + notebook.bindInterpretersToNote(note1.getId(), new LinkedList<String>()); // remove note notebook.removeNote(note1.getId(), null); @@ -273,9 +273,9 @@ public class HeliumApplicationFactoryTest implements JobListenerFactory { new String[][]{}); Note note1 = notebook.createNote(null); - notebook.bindInterpretersToNote(note1.id(), factory.getDefaultInterpreterSettingList()); + notebook.bindInterpretersToNote(note1.getId(), factory.getDefaultInterpreterSettingList()); String mock1IntpSettingId = null; - for (InterpreterSetting setting : notebook.getBindedInterpreterSettings(note1.id())) { + for (InterpreterSetting setting : notebook.getBindedInterpreterSettings(note1.getId())) { if (setting.getName().equals("mock1")) { mock1IntpSettingId = setting.getId(); break; http://git-wip-us.apache.org/repos/asf/zeppelin/blob/83469be5/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 bfa97e0..648062e 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 @@ -171,7 +171,7 @@ public class NotebookTest implements JobListenerFactory{ notebook.reloadAllNotes(null); notes = notebook.getAllNotes(); assertEquals(notes.size(), 2); - assertEquals(notes.get(1).id(), copiedNote.id()); + assertEquals(notes.get(1).getId(), copiedNote.getId()); assertEquals(notes.get(1).getName(), copiedNote.getName()); assertEquals(notes.get(1).getParagraphs(), copiedNote.getParagraphs()); @@ -283,13 +283,13 @@ public class NotebookTest implements JobListenerFactory{ config.put("enabled", true); config.put("cron", "* * * * * ?"); note.setConfig(config); - notebook.refreshCron(note.id()); + notebook.refreshCron(note.getId()); Thread.sleep(1*1000); // remove cron scheduler. config.put("cron", null); note.setConfig(config); - notebook.refreshCron(note.id()); + notebook.refreshCron(note.getId()); Thread.sleep(1000); dateFinished = p.getDateFinished(); assertNotNull(dateFinished); @@ -318,7 +318,7 @@ public class NotebookTest implements JobListenerFactory{ config.put("cron", "1/3 * * * * ?"); config.put("releaseresource", true); note.setConfig(config); - notebook.refreshCron(note.id()); + notebook.refreshCron(note.getId()); MockInterpreter1 mock1 = ((MockInterpreter1) (((ClassloaderInterpreter) @@ -342,7 +342,7 @@ public class NotebookTest implements JobListenerFactory{ // remove cron scheduler. config.put("cron", null); note.setConfig(config); - notebook.refreshCron(note.id()); + notebook.refreshCron(note.getId()); // make sure all paragraph has been executed assertNotNull(p.getDateFinished()); @@ -399,6 +399,16 @@ public class NotebookTest implements JobListenerFactory{ } @Test + public void testCloneNoteWithNoName() throws IOException, CloneNotSupportedException, + InterruptedException { + Note note = notebook.createNote(null); + factory.setInterpreters(note.getId(), factory.getDefaultInterpreterSettingList()); + + Note cloneNote = notebook.cloneNote(note.getId(), null, null); + assertEquals(cloneNote.getName(), "Note " + cloneNote.getId()); + } + + @Test public void testCloneNoteWithExceptionResult() throws IOException, CloneNotSupportedException, InterruptedException { Note note = notebook.createNote(null); @@ -445,7 +455,7 @@ public class NotebookTest implements JobListenerFactory{ assertEquals(1, ResourcePoolUtils.getAllResources().size()); // remove note - notebook.removeNote(note.id(), null); + notebook.removeNote(note.getId(), null); assertEquals(0, ResourcePoolUtils.getAllResources().size()); } @@ -463,20 +473,20 @@ public class NotebookTest implements JobListenerFactory{ Paragraph p1 = note.addParagraph(); // add paragraph scope object - registry.add("o1", "object1", note.id(), p1.getId()); + registry.add("o1", "object1", note.getId(), p1.getId()); // add notebook scope object - registry.add("o2", "object2", note.id(), null); + registry.add("o2", "object2", note.getId(), null); // add global scope object registry.add("o3", "object3", null, null); // remove notebook - notebook.removeNote(note.id(), null); + notebook.removeNote(note.getId(), null); // notebook scope or paragraph scope object should be removed - assertNull(registry.get("o1", note.id(), null)); - assertNull(registry.get("o2", note.id(), p1.getId())); + assertNull(registry.get("o1", note.getId(), null)); + assertNull(registry.get("o2", note.getId(), p1.getId())); // global object sould be remained assertNotNull(registry.get("o3", null, null)); @@ -496,10 +506,10 @@ public class NotebookTest implements JobListenerFactory{ Paragraph p1 = note.addParagraph(); // add paragraph scope object - registry.add("o1", "object1", note.id(), p1.getId()); + registry.add("o1", "object1", note.getId(), p1.getId()); // add notebook scope object - registry.add("o2", "object2", note.id(), null); + registry.add("o2", "object2", note.getId(), null); // add global scope object registry.add("o3", "object3", null, null); @@ -508,10 +518,10 @@ public class NotebookTest implements JobListenerFactory{ note.removeParagraph(p1.getId()); // paragraph scope should be removed - assertNull(registry.get("o1", note.id(), null)); + assertNull(registry.get("o1", note.getId(), null)); // notebook scope and global object sould be remained - assertNotNull(registry.get("o2", note.id(), null)); + assertNotNull(registry.get("o2", note.getId(), null)); assertNotNull(registry.get("o3", null, null)); } @@ -527,7 +537,7 @@ public class NotebookTest implements JobListenerFactory{ .getAngularObjectRegistry(); // add local scope object - registry.add("o1", "object1", note.id(), null); + registry.add("o1", "object1", note.getId(), null); // add global scope object registry.add("o2", "object2", null, null); @@ -537,9 +547,9 @@ public class NotebookTest implements JobListenerFactory{ .getAngularObjectRegistry(); // local and global scope object should be removed - assertNull(registry.get("o1", note.id(), null)); + assertNull(registry.get("o1", note.getId(), null)); assertNull(registry.get("o2", null, null)); - notebook.removeNote(note.id(), null); + notebook.removeNote(note.getId(), null); } @Test @@ -548,43 +558,43 @@ public class NotebookTest implements JobListenerFactory{ Note note = notebook.createNote(null); NotebookAuthorization notebookAuthorization = notebook.getNotebookAuthorization(); // empty owners, readers and writers means note is public - assertEquals(notebookAuthorization.isOwner(note.id(), + assertEquals(notebookAuthorization.isOwner(note.getId(), new HashSet<String>(Arrays.asList("user2"))), true); - assertEquals(notebookAuthorization.isReader(note.id(), + assertEquals(notebookAuthorization.isReader(note.getId(), new HashSet<String>(Arrays.asList("user2"))), true); - assertEquals(notebookAuthorization.isWriter(note.id(), + assertEquals(notebookAuthorization.isWriter(note.getId(), new HashSet<String>(Arrays.asList("user2"))), true); - notebookAuthorization.setOwners(note.id(), + notebookAuthorization.setOwners(note.getId(), new HashSet<String>(Arrays.asList("user1"))); - notebookAuthorization.setReaders(note.id(), + notebookAuthorization.setReaders(note.getId(), new HashSet<String>(Arrays.asList("user1", "user2"))); - notebookAuthorization.setWriters(note.id(), + notebookAuthorization.setWriters(note.getId(), new HashSet<String>(Arrays.asList("user1"))); - assertEquals(notebookAuthorization.isOwner(note.id(), + assertEquals(notebookAuthorization.isOwner(note.getId(), new HashSet<String>(Arrays.asList("user2"))), false); - assertEquals(notebookAuthorization.isOwner(note.id(), + assertEquals(notebookAuthorization.isOwner(note.getId(), new HashSet<String>(Arrays.asList("user1"))), true); - assertEquals(notebookAuthorization.isReader(note.id(), + assertEquals(notebookAuthorization.isReader(note.getId(), new HashSet<String>(Arrays.asList("user3"))), false); - assertEquals(notebookAuthorization.isReader(note.id(), + assertEquals(notebookAuthorization.isReader(note.getId(), new HashSet<String>(Arrays.asList("user2"))), true); - assertEquals(notebookAuthorization.isWriter(note.id(), + assertEquals(notebookAuthorization.isWriter(note.getId(), new HashSet<String>(Arrays.asList("user2"))), false); - assertEquals(notebookAuthorization.isWriter(note.id(), + assertEquals(notebookAuthorization.isWriter(note.getId(), new HashSet<String>(Arrays.asList("user1"))), true); // Test clearing of permssions - notebookAuthorization.setReaders(note.id(), Sets.<String>newHashSet()); - assertEquals(notebookAuthorization.isReader(note.id(), + notebookAuthorization.setReaders(note.getId(), Sets.<String>newHashSet()); + assertEquals(notebookAuthorization.isReader(note.getId(), new HashSet<String>(Arrays.asList("user2"))), true); - assertEquals(notebookAuthorization.isReader(note.id(), + assertEquals(notebookAuthorization.isReader(note.getId(), new HashSet<String>(Arrays.asList("user3"))), true); - notebook.removeNote(note.id(), null); + notebook.removeNote(note.getId(), null); } @Test @@ -777,8 +787,8 @@ public class NotebookTest implements JobListenerFactory{ note1.removeParagraph(p1.getId()); assertEquals(1, onParagraphRemove.get()); - List<String> settings = notebook.getBindedInterpreterSettingsIds(note1.id()); - notebook.bindInterpretersToNote(note1.id(), new LinkedList<String>()); + List<String> settings = notebook.getBindedInterpreterSettingsIds(note1.getId()); + notebook.bindInterpretersToNote(note1.getId(), new LinkedList<String>()); assertEquals(settings.size(), unbindInterpreter.get()); notebook.removeNote(note1.getId(), null); http://git-wip-us.apache.org/repos/asf/zeppelin/blob/83469be5/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/zeppelinhub/ZeppelinHubRepoTest.java ---------------------------------------------------------------------- diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/zeppelinhub/ZeppelinHubRepoTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/zeppelinhub/ZeppelinHubRepoTest.java index 938521a..720dd70 100644 --- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/zeppelinhub/ZeppelinHubRepoTest.java +++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/zeppelinhub/ZeppelinHubRepoTest.java @@ -132,7 +132,7 @@ public class ZeppelinHubRepoTest { public void testGetNote() throws IOException { Note notebook = repo.get("AAAAA", null); assertThat(notebook).isNotNull(); - assertThat(notebook.id()).isEqualTo("2A94M5J1Z"); + assertThat(notebook.getId()).isEqualTo("2A94M5J1Z"); } @Test