From 536059b142329338d35bff17a6c7bd705e9fe1b3 Mon Sep 17 00:00:00 2001 From: Jongyoul Lee Date: Wed, 29 Jun 2016 01:37:22 +0900 Subject: [PATCH 1/5] Duplicated all method in NoteInterpreterLoader to InterpreterFactory --- .../zeppelin/socket/NotebookServer.java | 20 +-- .../zeppelin/rest/InterpreterRestApiTest.java | 2 +- .../zeppelin/socket/NotebookServerTest.java | 2 +- .../interpreter/InterpreterFactory.java | 161 ++++++++++++++++++ .../org/apache/zeppelin/notebook/Note.java | 11 +- .../notebook/NoteInterpreterLoader.java | 12 +- .../apache/zeppelin/notebook/Notebook.java | 15 +- .../apache/zeppelin/notebook/Paragraph.java | 7 +- .../notebook/NoteInterpreterLoaderTest.java | 95 +++++------ .../apache/zeppelin/notebook/NoteTest.java | 16 +- .../zeppelin/notebook/NotebookTest.java | 53 +++--- .../zeppelin/notebook/ParagraphTest.java | 5 +- .../notebook/repo/VFSNotebookRepoTest.java | 2 +- .../zeppelin/search/LuceneSearchTest.java | 4 +- 14 files changed, 287 insertions(+), 118 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 f4cf9d9efb1..42edb08ccef 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 @@ -307,7 +307,7 @@ private void broadcastToNoteBindedInterpreter(String interpreterGroupId, Notebook notebook = notebook(); List notes = notebook.getAllNotes(); for (Note note : notes) { - List ids = note.getNoteReplLoader().getInterpreters(); + List ids = notebook.getInterpreterFactory().getInterpreters(note.getId()); for (String id : ids) { if (id.equals(interpreterGroupId)) { broadcast(note.id(), m); @@ -750,8 +750,8 @@ private void angularObjectUpdated(NotebookSocket conn, HashSet userAndRo // propagate change to (Remote) AngularObjectRegistry Note note = notebook.getNote(noteId); if (note != null) { - List settings = note.getNoteReplLoader() - .getInterpreterSettings(); + List settings = notebook.getInterpreterFactory() + .getInterpreterSettings(note.getId()); for (InterpreterSetting setting : settings) { if (setting.getInterpreterGroup(note.id()) == null) { continue; @@ -791,8 +791,8 @@ private void angularObjectUpdated(NotebookSocket conn, HashSet userAndRo if (global) { // broadcast change to all web session that uses related // interpreter. for (Note n : notebook.getAllNotes()) { - List settings = note.getNoteReplLoader() - .getInterpreterSettings(); + List settings = notebook.getInterpreterFactory() + .getInterpreterSettings(note.getId()); for (InterpreterSetting setting : settings) { if (setting.getInterpreterGroup(n.id()) == null) { continue; @@ -1239,8 +1239,8 @@ public ParagraphJobListener getParagraphJobListener(Note note) { } private void sendAllAngularObjects(Note note, NotebookSocket conn) throws IOException { - List settings = note.getNoteReplLoader() - .getInterpreterSettings(); + List settings = + notebook().getInterpreterFactory().getInterpreterSettings(note.getId()); if (settings == null || settings.size() == 0) { return; } @@ -1279,8 +1279,8 @@ public void onUpdate(String interpreterGroupId, AngularObject object) { continue; } - List intpSettings = note.getNoteReplLoader() - .getInterpreterSettings(); + List intpSettings = notebook.getInterpreterFactory() + .getInterpreterSettings(note.getId()); if (intpSettings.isEmpty()) continue; for (InterpreterSetting setting : intpSettings) { @@ -1306,7 +1306,7 @@ public void onRemove(String interpreterGroupId, String name, String noteId, Stri continue; } - List ids = note.getNoteReplLoader().getInterpreters(); + List ids = notebook.getInterpreterFactory().getInterpreters(note.getId()); for (String id : ids) { if (id.equals(interpreterGroupId)) { broadcast( 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 f03d87b32f9..4d7caf16c9e 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 @@ -159,7 +159,7 @@ public void testInterpreterRestart() throws IOException, InterruptedException { // restart interpreter - for (InterpreterSetting setting : note.getNoteReplLoader().getInterpreterSettings()) { + for (InterpreterSetting setting : ZeppelinServer.notebook.getInterpreterFactory().getInterpreterSettings(note.getId())) { if (setting.getName().equals("md")) { // Call Restart Interpreter REST API PutMethod put = httpPut("/interpreter/setting/restart/" + setting.id(), ""); diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/socket/NotebookServerTest.java b/zeppelin-server/src/test/java/org/apache/zeppelin/socket/NotebookServerTest.java index ee2c7c6aa94..bc131131ea9 100644 --- a/zeppelin-server/src/test/java/org/apache/zeppelin/socket/NotebookServerTest.java +++ b/zeppelin-server/src/test/java/org/apache/zeppelin/socket/NotebookServerTest.java @@ -93,7 +93,7 @@ public void testMakeSureNoAngularObjectBroadcastToWebsocketWhoFireTheEvent() thr // get reference to interpreterGroup InterpreterGroup interpreterGroup = null; - List settings = note1.getNoteReplLoader().getInterpreterSettings(); + List settings = notebook.getInterpreterFactory().getInterpreterSettings(note1.getId()); for (InterpreterSetting setting : settings) { if (setting.getName().equals("md")) { interpreterGroup = setting.getInterpreterGroup("sharedProcess"); diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterFactory.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterFactory.java index ca9e471487d..0e26b565647 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterFactory.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterFactory.java @@ -17,6 +17,7 @@ package org.apache.zeppelin.interpreter; +import com.google.common.base.*; import com.google.gson.Gson; import com.google.gson.GsonBuilder; @@ -62,6 +63,8 @@ public class InterpreterFactory implements InterpreterGroupFactory { Logger logger = LoggerFactory.getLogger(InterpreterFactory.class); + private static String SHARED_SESSION = "shared_session"; + private Map cleanCl = Collections .synchronizedMap(new HashMap()); @@ -923,6 +926,164 @@ private Properties updatePropertiesFromRegisteredInterpreter(Properties properti return properties; } + /** + * map interpreter ids into noteId + * @param noteId note id + * @param ids InterpreterSetting id list + * @throws IOException + */ + public void setInterpreters(String noteId, List ids) throws IOException { + putNoteInterpreterSettingBinding(noteId, ids); + } + + public List getInterpreters(String noteId) { + return getNoteInterpreterSettingBinding(noteId); + } + + public List getInterpreterSettings(String noteId) { + List interpreterSettingIds = getNoteInterpreterSettingBinding(noteId); + LinkedList settings = new LinkedList(); + synchronized (interpreterSettingIds) { + for (String id : interpreterSettingIds) { + InterpreterSetting setting = get(id); + if (setting == null) { + // interpreter setting is removed from factory. remove id from here, too + interpreterSettingIds.remove(id); + } else { + settings.add(setting); + } + } + } + return settings; + } + + public void closeNote(String noteId) { + // close interpreters in this note session + List settings = getInterpreterSettings(noteId); + if (settings == null || settings.size() == 0) { + return; + } + + System.err.println("close"); + for (InterpreterSetting setting : settings) { + removeInterpretersForNote(setting, noteId); + } + } + + private String getInterpreterInstanceKey(String noteId, InterpreterSetting setting) { + if (setting.getOption().isExistingProcess()) { + return Constants.EXISTING_PROCESS; + } else if (setting.getOption().isPerNoteSession() || setting.getOption().isPerNoteProcess()) { + return noteId; + } else { + return SHARED_SESSION; + } + } + + private List createOrGetInterpreterList(String noteId, InterpreterSetting setting) { + InterpreterGroup interpreterGroup = + setting.getInterpreterGroup(noteId); + synchronized (interpreterGroup) { + String key = getInterpreterInstanceKey(noteId, setting); + if (!interpreterGroup.containsKey(key)) { + createInterpretersForNote(setting, noteId, key); + } + return interpreterGroup.get(getInterpreterInstanceKey(noteId, setting)); + } + } + + private com.google.common.base.Optional + getDefaultInterpreterSetting(List settings) { + if (settings == null || settings.isEmpty()) { + return com.google.common.base.Optional.absent(); + } + return com.google.common.base.Optional.of(settings.get(0)); + } + + com.google.common.base.Optional getDefaultInterpreterSetting(String noteId) { + return getDefaultInterpreterSetting(getInterpreterSettings(noteId)); + } + + public Interpreter getInterpreter(String noteId, String replName) { + List settings = getInterpreterSettings(noteId); + + if (settings == null || settings.size() == 0) { + return null; + } + + if (replName == null || replName.trim().length() == 0) { + // get default settings (first available) + InterpreterSetting defaultSettings = getDefaultInterpreterSetting(settings).get(); + return createOrGetInterpreterList(noteId, defaultSettings).get(0); + } + + if (Interpreter.registeredInterpreters == null) { + return null; + } + + String[] replNameSplit = replName.split("\\."); + String group = null; + String name = null; + if (replNameSplit.length == 2) { + group = replNameSplit[0]; + name = replNameSplit[1]; + + Interpreter.RegisteredInterpreter registeredInterpreter = Interpreter.registeredInterpreters + .get(group + "." + name); + if (registeredInterpreter == null + || registeredInterpreter.getClassName() == null) { + throw new InterpreterException(replName + " interpreter not found"); + } + String interpreterClassName = registeredInterpreter.getClassName(); + + for (InterpreterSetting setting : settings) { + if (registeredInterpreter.getGroup().equals(setting.getGroup())) { + List intpGroup = createOrGetInterpreterList(noteId, setting); + for (Interpreter interpreter : intpGroup) { + if (interpreterClassName.equals(interpreter.getClassName())) { + return interpreter; + } + } + } + } + throw new InterpreterException(replName + " interpreter not found"); + } else { + // first assume replName is 'name' of interpreter. ('groupName' is ommitted) + // search 'name' from first (default) interpreter group + InterpreterSetting defaultSetting = getDefaultInterpreterSetting(settings).get(); + Interpreter.RegisteredInterpreter registeredInterpreter = + Interpreter.registeredInterpreters.get(defaultSetting.getGroup() + "." + replName); + if (registeredInterpreter != null) { + List interpreters = createOrGetInterpreterList(noteId, defaultSetting); + for (Interpreter interpreter : interpreters) { + + RegisteredInterpreter intp = + Interpreter.findRegisteredInterpreterByClassName(interpreter.getClassName()); + if (intp == null) { + continue; + } + + if (intp.getName().equals(replName)) { + return interpreter; + } + } + + throw new InterpreterException( + defaultSetting.getGroup() + "." + replName + " interpreter not found"); + } + + // next, assume replName is 'group' of interpreter ('name' is ommitted) + // search interpreter group and return first interpreter. + for (InterpreterSetting setting : settings) { + if (setting.getGroup().equals(replName)) { + List interpreters = createOrGetInterpreterList(noteId, setting); + return interpreters.get(0); + } + } + } + + return null; + } private URL[] recursiveBuildLibList(File path) throws MalformedURLException { URL[] urls = new URL[0]; 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 1f4a50813d3..7bb0c8d1757 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 @@ -73,6 +73,7 @@ public class Note implements Serializable, JobListener { @SuppressWarnings("rawtypes") Map> angularObjects = new HashMap<>(); + private transient InterpreterFactory factory; private transient NoteInterpreterLoader replLoader; private transient JobListenerFactory jobListenerFactory; private transient NotebookRepo repo; @@ -437,7 +438,7 @@ public void runAll() { p.setAuthenticationInfo(authenticationInfo); p.setNoteReplLoader(replLoader); p.setListener(jobListenerFactory.getParagraphJobListener(this)); - Interpreter intp = replLoader.get(p.getRequiredReplName()); + Interpreter intp = factory.get(getId(), p.getRequiredReplName()); intp.getScheduler().submit(p); } } @@ -453,10 +454,10 @@ public void run(String paragraphId) { p.setNoteReplLoader(replLoader); p.setListener(jobListenerFactory.getParagraphJobListener(this)); String requiredReplName = p.getRequiredReplName(); - Interpreter intp = replLoader.get(requiredReplName); + Interpreter intp = factory.getInterpreter(getId(), requiredReplName); if (intp == null) { // TODO(jongyoul): Make "%jdbc" configurable from JdbcInterpreter - if (conf.getUseJdbcAlias() && null != (intp = replLoader.get("jdbc"))) { + if (conf.getUseJdbcAlias() && null != (intp = factory.getInterpreter(getId(), "jdbc"))) { String pText = p.getText().replaceFirst(requiredReplName, "jdbc(" + requiredReplName + ")"); logger.debug("New paragraph: {}", pText); p.setEffectiveText(pText); @@ -503,7 +504,7 @@ public List getParagraphs() { private void snapshotAngularObjectRegistry() { angularObjects = new HashMap<>(); - List settings = replLoader.getInterpreterSettings(); + List settings = factory.getInterpreterSettings(getId()); if (settings == null || settings.size() == 0) { return; } @@ -518,7 +519,7 @@ private void snapshotAngularObjectRegistry() { private void removeAllAngularObjectInParagraph(String paragraphId) { angularObjects = new HashMap>(); - List settings = replLoader.getInterpreterSettings(); + List settings = factory.getInterpreterSettings(getId()); if (settings == null || settings.size() == 0) { return; } diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NoteInterpreterLoader.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NoteInterpreterLoader.java index 3c432cbb12e..a04e7b55a20 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NoteInterpreterLoader.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NoteInterpreterLoader.java @@ -52,14 +52,17 @@ public void setNoteId(String noteId) { * @param ids InterpreterSetting id list * @throws IOException */ + @Deprecated public void setInterpreters(List ids) throws IOException { factory.putNoteInterpreterSettingBinding(noteId, ids); } + @Deprecated public List getInterpreters() { return factory.getNoteInterpreterSettingBinding(noteId); } + @Deprecated public List getInterpreterSettings() { List interpreterSettingIds = factory.getNoteInterpreterSettingBinding(noteId); LinkedList settings = new LinkedList(); @@ -77,6 +80,7 @@ public List getInterpreterSettings() { return settings; } + private String getInterpreterInstanceKey(InterpreterSetting setting) { if (setting.getOption().isExistingProcess()) { return Constants.EXISTING_PROCESS; @@ -99,9 +103,10 @@ private List createOrGetInterpreterList(InterpreterSetting setting) } } + @Deprecated public void close() { // close interpreters in this note session - List settings = this.getInterpreterSettings(); + List settings = factory.getInterpreterSettings(noteId); if (settings == null || settings.size() == 0) { return; } @@ -112,8 +117,9 @@ public void close() { } } + @Deprecated public Interpreter get(String replName) { - List settings = getInterpreterSettings(); + List settings = factory.getInterpreterSettings(noteId); if (settings == null || settings.size() == 0) { return null; @@ -202,6 +208,6 @@ public Interpreter get(String replName) { } Optional getDefaultInterpreterSetting() { - return getDefaultInterpreterSetting(getInterpreterSettings()); + return getDefaultInterpreterSetting(factory.getInterpreterSettings(noteId)); } } 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 d590223b2e0..13de4c47339 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 @@ -258,7 +258,7 @@ public void bindInterpretersToNote(String id, List interpreterSettingIds) throws IOException { Note note = getNote(id); if (note != null) { - note.getNoteReplLoader().setInterpreters(interpreterSettingIds); + replFactory.setInterpreters(note.getId(), interpreterSettingIds); // comment out while note.getNoteReplLoader().setInterpreters(...) do the same // replFactory.putNoteInterpreterSettingBinding(id, interpreterSettingIds); } @@ -267,7 +267,7 @@ public void bindInterpretersToNote(String id, public List getBindedInterpreterSettingsIds(String id) { Note note = getNote(id); if (note != null) { - return note.getNoteReplLoader().getInterpreters(); + return getInterpreterFactory().getInterpreters(note.getId()); } else { return new LinkedList(); } @@ -276,7 +276,7 @@ public List getBindedInterpreterSettingsIds(String id) { public List getBindedInterpreterSettings(String id) { Note note = getNote(id); if (note != null) { - return note.getNoteReplLoader().getInterpreterSettings(); + return replFactory.getInterpreterSettings(note.getId()); } else { return new LinkedList(); } @@ -607,9 +607,9 @@ public List> getJobListforNotebook(boolean needsReload, // set interpreter bind type String interpreterGroupName = null; - if (note.getNoteReplLoader().getInterpreterSettings() != null && - note.getNoteReplLoader().getInterpreterSettings().size() >= 1) { - interpreterGroupName = note.getNoteReplLoader().getInterpreterSettings().get(0).getGroup(); + if (replFactory.getInterpreterSettings(note.getId()) != null && + replFactory.getInterpreterSettings(note.getId()).size() >= 1) { + interpreterGroupName = replFactory.getInterpreterSettings(note.getId()).get(0).getGroup(); } // not update and not running -> pass @@ -659,7 +659,8 @@ public void execute(JobExecutionContext context) throws JobExecutionException { logger.error(e.getMessage(), e); } if (releaseResource) { - for (InterpreterSetting setting : note.getNoteReplLoader().getInterpreterSettings()) { + for (InterpreterSetting setting : + notebook.getInterpreterFactory().getInterpreterSettings(note.getId())) { notebook.getInterpreterFactory().restart(setting.id()); } } 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 bc53887f0fc..9c2648c2355 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 @@ -49,6 +49,7 @@ public class Paragraph extends Job implements Serializable, Cloneable { private static final long serialVersionUID = -6328572073497992016L; + private transient InterpreterFactory factory; private transient NoteInterpreterLoader replLoader; private transient Note note; private transient AuthenticationInfo authenticationInfo; @@ -199,7 +200,7 @@ public NoteInterpreterLoader getNoteReplLoader() { } public Interpreter getRepl(String name) { - return replLoader.get(name); + return factory.getInterpreter(note.getId(), name); } public Interpreter getCurrentRepl() { @@ -336,8 +337,8 @@ private InterpreterContext getInterpreterContext() { AngularObjectRegistry registry = null; ResourcePool resourcePool = null; - if (!getNoteReplLoader().getInterpreterSettings().isEmpty()) { - InterpreterSetting intpGroup = getNoteReplLoader().getInterpreterSettings().get(0); + 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(); } diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NoteInterpreterLoaderTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NoteInterpreterLoaderTest.java index 147d431873b..c2e79e0f70a 100644 --- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NoteInterpreterLoaderTest.java +++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NoteInterpreterLoaderTest.java @@ -73,100 +73,93 @@ public void tearDown() throws Exception { public void testGetInterpreter() throws IOException { NoteInterpreterLoader loader = new NoteInterpreterLoader(factory); loader.setNoteId("note"); - loader.setInterpreters(factory.getDefaultInterpreterSettingList()); +// loader.setInterpreters(factory.getDefaultInterpreterSettingList()); + factory.setInterpreters("note", factory.getDefaultInterpreterSettingList()); // when there're no interpreter selection directive - assertEquals("org.apache.zeppelin.interpreter.mock.MockInterpreter1", loader.get(null).getClassName()); - assertEquals("org.apache.zeppelin.interpreter.mock.MockInterpreter1", loader.get("").getClassName()); - assertEquals("org.apache.zeppelin.interpreter.mock.MockInterpreter1", loader.get(" ").getClassName()); + assertEquals("org.apache.zeppelin.interpreter.mock.MockInterpreter1", factory.getInterpreter("note", null).getClassName()); + assertEquals("org.apache.zeppelin.interpreter.mock.MockInterpreter1", factory.getInterpreter("note", "").getClassName()); + assertEquals("org.apache.zeppelin.interpreter.mock.MockInterpreter1", factory.getInterpreter("note", " ").getClassName()); // when group name is omitted - assertEquals("org.apache.zeppelin.interpreter.mock.MockInterpreter11", loader.get("mock11").getClassName()); + assertEquals("org.apache.zeppelin.interpreter.mock.MockInterpreter11", factory.getInterpreter("note", "mock11").getClassName()); // when 'name' is ommitted - assertEquals("org.apache.zeppelin.interpreter.mock.MockInterpreter1", loader.get("group1").getClassName()); - assertEquals("org.apache.zeppelin.interpreter.mock.MockInterpreter2", loader.get("group2").getClassName()); + assertEquals("org.apache.zeppelin.interpreter.mock.MockInterpreter1", factory.getInterpreter("note", "group1").getClassName()); + assertEquals("org.apache.zeppelin.interpreter.mock.MockInterpreter2", factory.getInterpreter("note", "group2").getClassName()); // when nothing is ommitted - assertEquals("org.apache.zeppelin.interpreter.mock.MockInterpreter1", loader.get("group1.mock1").getClassName()); - assertEquals("org.apache.zeppelin.interpreter.mock.MockInterpreter11", loader.get("group1.mock11").getClassName()); - assertEquals("org.apache.zeppelin.interpreter.mock.MockInterpreter2", loader.get("group2.mock2").getClassName()); + assertEquals("org.apache.zeppelin.interpreter.mock.MockInterpreter1", factory.getInterpreter("note", "group1.mock1").getClassName()); + assertEquals("org.apache.zeppelin.interpreter.mock.MockInterpreter11", factory.getInterpreter("note", "group1.mock11").getClassName()); + assertEquals("org.apache.zeppelin.interpreter.mock.MockInterpreter2", factory.getInterpreter("note", "group2.mock2").getClassName()); - loader.close(); + factory.closeNote("note"); } @Test public void testNoteSession() throws IOException { - NoteInterpreterLoader loaderA = new NoteInterpreterLoader(factory); - loaderA.setNoteId("noteA"); - loaderA.setInterpreters(factory.getDefaultInterpreterSettingList()); - loaderA.getInterpreterSettings().get(0).getOption().setPerNoteSession(true); + factory.setInterpreters("noteA", factory.getDefaultInterpreterSettingList()); + factory.getInterpreterSettings("noteA").get(0).getOption().setPerNoteSession(true); - NoteInterpreterLoader loaderB = new NoteInterpreterLoader(factory); - loaderB.setNoteId("noteB"); - loaderB.setInterpreters(factory.getDefaultInterpreterSettingList()); - loaderB.getInterpreterSettings().get(0).getOption().setPerNoteSession(true); + factory.setInterpreters("noteB", factory.getDefaultInterpreterSettingList()); + factory.getInterpreterSettings("noteB").get(0).getOption().setPerNoteSession(true); // interpreters are not created before accessing it - assertNull(loaderA.getInterpreterSettings().get(0).getInterpreterGroup("shared_process").get("noteA")); - assertNull(loaderB.getInterpreterSettings().get(0).getInterpreterGroup("shared_process").get("noteB")); + assertNull(factory.getInterpreterSettings("noteA").get(0).getInterpreterGroup("shared_process").get("noteA")); + assertNull(factory.getInterpreterSettings("noteB").get(0).getInterpreterGroup("shared_process").get("noteB")); - loaderA.get(null).open(); - loaderB.get(null).open(); + factory.getInterpreter("noteA", null).open(); + factory.getInterpreter("noteB", null).open(); assertTrue( - loaderA.get(null).getInterpreterGroup().getId().equals( - loaderB.get(null).getInterpreterGroup().getId())); + factory.getInterpreter("noteA", null).getInterpreterGroup().getId().equals( + factory.getInterpreter("noteB", null).getInterpreterGroup().getId())); // interpreters are created after accessing it - assertNotNull(loaderA.getInterpreterSettings().get(0).getInterpreterGroup("shared_process").get("noteA")); - assertNotNull(loaderB.getInterpreterSettings().get(0).getInterpreterGroup("shared_process").get("noteB")); + assertNotNull(factory.getInterpreterSettings("noteA").get(0).getInterpreterGroup("shared_process").get("noteA")); + assertNotNull(factory.getInterpreterSettings("noteB").get(0).getInterpreterGroup("shared_process").get("noteB")); // when - loaderA.close(); - loaderB.close(); + factory.closeNote("noteB"); + factory.closeNote("noteB"); // interpreters are destroyed after close - assertNull(loaderA.getInterpreterSettings().get(0).getInterpreterGroup("shared_process").get("noteA")); - assertNull(loaderB.getInterpreterSettings().get(0).getInterpreterGroup("shared_process").get("noteB")); + assertNull(factory.getInterpreterSettings("noteA").get(0).getInterpreterGroup("shared_process").get("noteA")); + assertNull(factory.getInterpreterSettings("noteB").get(0).getInterpreterGroup("shared_process").get("noteB")); } @Test public void testNotePerInterpreterProcess() throws IOException { - NoteInterpreterLoader loaderA = new NoteInterpreterLoader(factory); - loaderA.setNoteId("noteA"); - loaderA.setInterpreters(factory.getDefaultInterpreterSettingList()); - loaderA.getInterpreterSettings().get(0).getOption().setPerNoteProcess(true); + factory.setInterpreters("noteA", factory.getDefaultInterpreterSettingList()); + factory.getInterpreterSettings("noteA").get(0).getOption().setPerNoteProcess(true); - NoteInterpreterLoader loaderB = new NoteInterpreterLoader(factory); - loaderB.setNoteId("noteB"); - loaderB.setInterpreters(factory.getDefaultInterpreterSettingList()); - loaderB.getInterpreterSettings().get(0).getOption().setPerNoteProcess(true); + factory.setInterpreters("noteB", factory.getDefaultInterpreterSettingList()); + factory.getInterpreterSettings("noteB").get(0).getOption().setPerNoteProcess(true); // interpreters are not created before accessing it - assertNull(loaderA.getInterpreterSettings().get(0).getInterpreterGroup("noteA").get("noteA")); - assertNull(loaderB.getInterpreterSettings().get(0).getInterpreterGroup("noteB").get("noteB")); + assertNull(factory.getInterpreterSettings("noteA").get(0).getInterpreterGroup("noteA").get("noteA")); + assertNull(factory.getInterpreterSettings("noteB").get(0).getInterpreterGroup("noteB").get("noteB")); - loaderA.get(null).open(); - loaderB.get(null).open(); + factory.getInterpreter("noteA", null).open(); + factory.getInterpreter("noteB", null).open(); // per note interpreter process assertFalse( - loaderA.get(null).getInterpreterGroup().getId().equals( - loaderB.get(null).getInterpreterGroup().getId())); + factory.getInterpreter("noteA", null).getInterpreterGroup().getId().equals( + factory.getInterpreter("noteB", null).getInterpreterGroup().getId())); // interpreters are created after accessing it - assertNotNull(loaderA.getInterpreterSettings().get(0).getInterpreterGroup("noteA").get("noteA")); - assertNotNull(loaderB.getInterpreterSettings().get(0).getInterpreterGroup("noteB").get("noteB")); + assertNotNull(factory.getInterpreterSettings("noteA").get(0).getInterpreterGroup("noteA").get("noteA")); + assertNotNull(factory.getInterpreterSettings("noteB").get(0).getInterpreterGroup("noteB").get("noteB")); // when - loaderA.close(); - loaderB.close(); + factory.closeNote("noteA"); + factory.closeNote("noteB"); // interpreters are destroyed after close - assertNull(loaderA.getInterpreterSettings().get(0).getInterpreterGroup("noteA").get("noteA")); - assertNull(loaderB.getInterpreterSettings().get(0).getInterpreterGroup("noteB").get("noteB")); + assertNull(factory.getInterpreterSettings("noteA").get(0).getInterpreterGroup("noteA").get("noteA")); + assertNull(factory.getInterpreterSettings("noteB").get(0).getInterpreterGroup("noteB").get("noteB")); } diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NoteTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NoteTest.java index 3e323efd5cd..1dae66dae7d 100644 --- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NoteTest.java +++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NoteTest.java @@ -21,6 +21,7 @@ import org.apache.commons.lang.StringUtils; import org.apache.zeppelin.interpreter.Interpreter; +import org.apache.zeppelin.interpreter.InterpreterFactory; import org.apache.zeppelin.interpreter.InterpreterSetting; import org.apache.zeppelin.notebook.repo.NotebookRepo; import org.apache.zeppelin.scheduler.Scheduler; @@ -59,9 +60,12 @@ public class NoteTest { @Mock Scheduler scheduler; + @Mock + InterpreterFactory interpreterFactory; + @Test public void runNormalTest() { - when(replLoader.get("spark")).thenReturn(interpreter); + when(interpreterFactory.getInterpreter(anyString(), "spark")).thenReturn(interpreter); when(interpreter.getScheduler()).thenReturn(scheduler); String pText = "%spark sc.version"; @@ -72,15 +76,15 @@ public void runNormalTest() { ArgumentCaptor pCaptor = ArgumentCaptor.forClass(Paragraph.class); verify(scheduler, only()).submit(pCaptor.capture()); - verify(replLoader, only()).get("spark"); + verify(interpreterFactory, only()).getInterpreter(anyString(), "spark"); assertEquals("Paragraph text", pText, pCaptor.getValue().getText()); } @Test public void runJdbcTest() { - when(replLoader.get("mysql")).thenReturn(null); - when(replLoader.get("jdbc")).thenReturn(interpreter); + when(interpreterFactory.getInterpreter(anyString(), "mysql")).thenReturn(null); + when(interpreterFactory.getInterpreter(anyString(), "jdbc")).thenReturn(interpreter); when(interpreter.getScheduler()).thenReturn(scheduler); String pText = "%mysql show databases"; @@ -91,7 +95,7 @@ public void runJdbcTest() { ArgumentCaptor pCaptor = ArgumentCaptor.forClass(Paragraph.class); verify(scheduler, only()).submit(pCaptor.capture()); - verify(replLoader, times(2)).get(anyString()); + verify(interpreterFactory, times(2)).getInterpreter(anyString(), anyString()); assertEquals("Change paragraph text", "%jdbc(mysql) show databases", pCaptor.getValue().getEffectiveText()); assertEquals("Change paragraph text", pText, pCaptor.getValue().getText()); @@ -99,7 +103,7 @@ public void runJdbcTest() { @Test public void putDefaultReplNameIfInterpreterSettingAbsent() { - when(replLoader.getDefaultInterpreterSetting()) + when(interpreterFactory.getDefaultInterpreterSetting(anyListOf(InterpreterSetting.class))) .thenReturn(Optional.absent()); Note note = new Note(repo, replLoader, jobListenerFactory, index, credentials); 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 0305b21bba7..13771b70718 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 @@ -105,7 +105,7 @@ public void tearDown() throws Exception { @Test public void testSelectingReplImplementation() throws IOException { Note note = notebook.createNote(null); - note.getNoteReplLoader().setInterpreters(factory.getDefaultInterpreterSettingList()); + factory.setInterpreters(note.getId(), factory.getDefaultInterpreterSettingList()); // run with defatul repl Paragraph p1 = note.addParagraph(); @@ -203,7 +203,7 @@ public void testClearParagraphOutput() throws IOException, SchedulerException{ @Test public void testRunAll() throws IOException { Note note = notebook.createNote(null); - note.getNoteReplLoader().setInterpreters(factory.getDefaultInterpreterSettingList()); + factory.setInterpreters(note.getId(), factory.getDefaultInterpreterSettingList()); // p1 Paragraph p1 = note.addParagraph(); @@ -242,7 +242,7 @@ public void testRunAll() throws IOException { public void testSchedule() throws InterruptedException, IOException{ // create a note and a paragraph Note note = notebook.createNote(null); - note.getNoteReplLoader().setInterpreters(factory.getDefaultInterpreterSettingList()); + factory.setInterpreters(note.getId(), factory.getDefaultInterpreterSettingList()); Paragraph p = note.addParagraph(); Map config = new HashMap(); @@ -274,7 +274,7 @@ public void testSchedule() throws InterruptedException, IOException{ public void testAutoRestartInterpreterAfterSchedule() throws InterruptedException, IOException{ // create a note and a paragraph Note note = notebook.createNote(null); - note.getNoteReplLoader().setInterpreters(factory.getDefaultInterpreterSettingList()); + factory.setInterpreters(note.getId(), factory.getDefaultInterpreterSettingList()); Paragraph p = note.addParagraph(); Map config = new HashMap(); @@ -295,11 +295,11 @@ public void testAutoRestartInterpreterAfterSchedule() throws InterruptedExceptio MockInterpreter1 mock1 = ((MockInterpreter1) (((ClassloaderInterpreter) - ((LazyOpenInterpreter) note.getNoteReplLoader().get("mock1")).getInnerInterpreter()) + ((LazyOpenInterpreter) factory.getInterpreter(note.getId(), "mock1")).getInnerInterpreter()) .getInnerInterpreter())); MockInterpreter2 mock2 = ((MockInterpreter2) (((ClassloaderInterpreter) - ((LazyOpenInterpreter) note.getNoteReplLoader().get("mock2")).getInnerInterpreter()) + ((LazyOpenInterpreter) factory.getInterpreter(note.getId(), "mock2")).getInnerInterpreter()) .getInnerInterpreter())); // wait until interpreters are started @@ -326,7 +326,7 @@ public void testAutoRestartInterpreterAfterSchedule() throws InterruptedExceptio public void testExportAndImportNote() throws IOException, CloneNotSupportedException, InterruptedException { Note note = notebook.createNote(null); - note.getNoteReplLoader().setInterpreters(factory.getDefaultInterpreterSettingList()); + factory.setInterpreters(note.getId(), factory.getDefaultInterpreterSettingList()); final Paragraph p = note.addParagraph(); String simpleText = "hello world"; @@ -353,7 +353,7 @@ public void testExportAndImportNote() throws IOException, CloneNotSupportedExcep public void testCloneNote() throws IOException, CloneNotSupportedException, InterruptedException { Note note = notebook.createNote(null); - note.getNoteReplLoader().setInterpreters(factory.getDefaultInterpreterSettingList()); + factory.setInterpreters(note.getId(), factory.getDefaultInterpreterSettingList()); final Paragraph p = note.addParagraph(); p.setText("hello world"); @@ -375,7 +375,7 @@ public void testCloneNote() throws IOException, CloneNotSupportedException, public void testCloneNoteWithExceptionResult() throws IOException, CloneNotSupportedException, InterruptedException { Note note = notebook.createNote(null); - note.getNoteReplLoader().setInterpreters(factory.getDefaultInterpreterSettingList()); + factory.setInterpreters(note.getId(), factory.getDefaultInterpreterSettingList()); final Paragraph p = note.addParagraph(); p.setText("hello world"); @@ -398,7 +398,7 @@ public void testCloneNoteWithExceptionResult() throws IOException, CloneNotSuppo @Test public void testResourceRemovealOnParagraphNoteRemove() throws IOException { Note note = notebook.createNote(null); - note.getNoteReplLoader().setInterpreters(factory.getDefaultInterpreterSettingList()); + factory.setInterpreters(note.getId(), factory.getDefaultInterpreterSettingList()); for (InterpreterGroup intpGroup : InterpreterGroup.getAll()) { intpGroup.setResourcePool(new LocalResourcePool(intpGroup.getId())); } @@ -427,10 +427,10 @@ public void testAngularObjectRemovalOnNotebookRemove() throws InterruptedExcepti IOException { // create a note and a paragraph Note note = notebook.createNote(null); - note.getNoteReplLoader().setInterpreters(factory.getDefaultInterpreterSettingList()); + factory.setInterpreters(note.getId(), factory.getDefaultInterpreterSettingList()); - AngularObjectRegistry registry = note.getNoteReplLoader() - .getInterpreterSettings().get(0).getInterpreterGroup("sharedProcess") + AngularObjectRegistry registry = factory + .getInterpreterSettings(note.getId()).get(0).getInterpreterGroup("sharedProcess") .getAngularObjectRegistry(); Paragraph p1 = note.addParagraph(); @@ -460,10 +460,10 @@ public void testAngularObjectRemovalOnParagraphRemove() throws InterruptedExcept IOException { // create a note and a paragraph Note note = notebook.createNote(null); - note.getNoteReplLoader().setInterpreters(factory.getDefaultInterpreterSettingList()); + factory.setInterpreters(note.getId(), factory.getDefaultInterpreterSettingList()); - AngularObjectRegistry registry = note.getNoteReplLoader() - .getInterpreterSettings().get(0).getInterpreterGroup("sharedProcess") + AngularObjectRegistry registry = factory + .getInterpreterSettings(note.getId()).get(0).getInterpreterGroup("sharedProcess") .getAngularObjectRegistry(); Paragraph p1 = note.addParagraph(); @@ -493,10 +493,10 @@ public void testAngularObjectRemovalOnInterpreterRestart() throws InterruptedExc IOException { // create a note and a paragraph Note note = notebook.createNote(null); - note.getNoteReplLoader().setInterpreters(factory.getDefaultInterpreterSettingList()); + factory.setInterpreters(note.getId(), factory.getDefaultInterpreterSettingList()); - AngularObjectRegistry registry = note.getNoteReplLoader() - .getInterpreterSettings().get(0).getInterpreterGroup("sharedProcess") + AngularObjectRegistry registry = factory + .getInterpreterSettings(note.getId()).get(0).getInterpreterGroup("sharedProcess") .getAngularObjectRegistry(); // add local scope object @@ -505,9 +505,8 @@ public void testAngularObjectRemovalOnInterpreterRestart() throws InterruptedExc registry.add("o2", "object2", null, null); // restart interpreter - factory.restart(note.getNoteReplLoader().getInterpreterSettings().get(0).id()); - registry = note.getNoteReplLoader() - .getInterpreterSettings().get(0).getInterpreterGroup("sharedProcess") + factory.restart(factory.getInterpreterSettings(note.getId()).get(0).id()); + registry = factory.getInterpreterSettings(note.getId()).get(0).getInterpreterGroup("sharedProcess") .getAngularObjectRegistry(); // local and global scope object should be removed @@ -565,7 +564,7 @@ public void testPermissions() throws IOException { public void testAbortParagraphStatusOnInterpreterRestart() throws InterruptedException, IOException { Note note = notebook.createNote(null); - note.getNoteReplLoader().setInterpreters(factory.getDefaultInterpreterSettingList()); + factory.setInterpreters(note.getId(), factory.getDefaultInterpreterSettingList()); ArrayList paragraphs = new ArrayList<>(); for (int i = 0; i < 100; i++) { @@ -582,7 +581,7 @@ public void testAbortParagraphStatusOnInterpreterRestart() throws InterruptedExc while (paragraphs.get(0).getStatus() != Status.FINISHED) Thread.yield(); - factory.restart(note.getNoteReplLoader().getInterpreterSettings().get(0).id()); + factory.restart(factory.getInterpreterSettings(note.getId()).get(0).id()); boolean isAborted = false; for (Paragraph p : paragraphs) { @@ -606,7 +605,7 @@ public void testPerSessionInterpreterCloseOnNoteRemoval() throws IOException { p1.setText("getId"); // restart interpreter with per note session enabled - for (InterpreterSetting setting : note1.getNoteReplLoader().getInterpreterSettings()) { + for (InterpreterSetting setting : factory.getInterpreterSettings(note1.getId())) { setting.getOption().setPerNoteSession(true); notebook.getInterpreterFactory().restart(setting.id()); } @@ -651,7 +650,7 @@ public void testPerSessionInterpreter() throws IOException { // restart interpreter with per note session enabled - for (InterpreterSetting setting : note1.getNoteReplLoader().getInterpreterSettings()) { + for (InterpreterSetting setting : notebook.getInterpreterFactory().getInterpreterSettings(note1.getId())) { setting.getOption().setPerNoteSession(true); notebook.getInterpreterFactory().restart(setting.id()); } @@ -677,7 +676,7 @@ public void testPerSessionInterpreterCloseOnUnbindInterpreterSetting() throws IO p1.setText("getId"); // restart interpreter with per note session enabled - for (InterpreterSetting setting : note1.getNoteReplLoader().getInterpreterSettings()) { + for (InterpreterSetting setting : factory.getInterpreterSettings(note1.getId())) { setting.getOption().setPerNoteSession(true); notebook.getInterpreterFactory().restart(setting.id()); } diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/ParagraphTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/ParagraphTest.java index 833eef341f8..3644b23a84c 100644 --- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/ParagraphTest.java +++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/ParagraphTest.java @@ -19,6 +19,7 @@ import static org.junit.Assert.assertEquals; +import static org.mockito.Matchers.anyString; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -28,6 +29,7 @@ import org.apache.zeppelin.display.AngularObjectRegistry; import org.apache.zeppelin.display.Input; import org.apache.zeppelin.interpreter.Interpreter; +import org.apache.zeppelin.interpreter.InterpreterFactory; import org.junit.Test; import java.util.HashMap; @@ -72,6 +74,7 @@ public void replNameEndsWithWhitespace() { @Test public void effectiveTextTest() { + InterpreterFactory interpreterFactory = mock(InterpreterFactory.class); NoteInterpreterLoader noteInterpreterLoader = mock(NoteInterpreterLoader.class); Interpreter interpreter = mock(Interpreter.class); @@ -81,7 +84,7 @@ public void effectiveTextTest() { assertEquals("Get right replName", "jdbc", p.getRequiredReplName()); assertEquals("Get right scriptBody", "(h2) show databases", p.getScriptBody()); - when(noteInterpreterLoader.get("jdbc")).thenReturn(interpreter); + when(interpreterFactory.getInterpreter(anyString(), "jdbc")).thenReturn(interpreter); when(interpreter.getFormType()).thenReturn(Interpreter.FormType.NATIVE); try { diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/VFSNotebookRepoTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/VFSNotebookRepoTest.java index 0550e901a20..9fb0360f603 100644 --- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/VFSNotebookRepoTest.java +++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/VFSNotebookRepoTest.java @@ -107,7 +107,7 @@ public void testInvalidJsonFile() throws IOException { @Test public void testSaveNotebook() throws IOException, InterruptedException { Note note = notebook.createNote(null); - note.getNoteReplLoader().setInterpreters(factory.getDefaultInterpreterSettingList()); + factory.setInterpreters(note.getId(), factory.getDefaultInterpreterSettingList()); Paragraph p1 = note.addParagraph(); Map config = p1.getConfig(); diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/search/LuceneSearchTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/search/LuceneSearchTest.java index 36419294b83..49ecab5b1c1 100644 --- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/search/LuceneSearchTest.java +++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/search/LuceneSearchTest.java @@ -49,8 +49,8 @@ public static void beforeStartUp() { notebookRepoMock = mock(NotebookRepo.class); replLoaderMock = mock(NoteInterpreterLoader.class); - when(replLoaderMock.getInterpreterSettings()) - .thenReturn(ImmutableList.of()); +// when(replLoaderMock.getInterpreterSettings()) +// .thenReturn(ImmutableList.of()); } @Before From 600de98138d14649cefb3402e0dd88224f52f2e0 Mon Sep 17 00:00:00 2001 From: Jongyoul Lee Date: Wed, 29 Jun 2016 02:00:36 +0900 Subject: [PATCH 2/5] Removed NoteInterpreterLoader Fixed some tests --- .../interpreter/InterpreterFactory.java | 4 +- .../org/apache/zeppelin/notebook/Note.java | 29 +-- .../notebook/NoteInterpreterLoader.java | 213 ------------------ .../apache/zeppelin/notebook/Notebook.java | 8 +- .../apache/zeppelin/notebook/Paragraph.java | 17 +- .../notebook/NoteInterpreterLoaderTest.java | 5 +- .../apache/zeppelin/notebook/NoteTest.java | 33 ++- .../zeppelin/notebook/ParagraphTest.java | 8 +- .../zeppelin/search/LuceneSearchTest.java | 10 +- 9 files changed, 46 insertions(+), 281 deletions(-) delete mode 100644 zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NoteInterpreterLoader.java diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterFactory.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterFactory.java index 0e26b565647..ab15444f9eb 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterFactory.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterFactory.java @@ -17,7 +17,6 @@ package org.apache.zeppelin.interpreter; -import com.google.common.base.*; import com.google.gson.Gson; import com.google.gson.GsonBuilder; @@ -35,7 +34,6 @@ import org.apache.zeppelin.interpreter.remote.RemoteAngularObjectRegistry; import org.apache.zeppelin.interpreter.remote.RemoteInterpreter; import org.apache.zeppelin.interpreter.remote.RemoteInterpreterProcessListener; -import org.apache.zeppelin.notebook.NoteInterpreterLoader; import org.apache.zeppelin.scheduler.Job; import org.apache.zeppelin.scheduler.Job.Status; import org.slf4j.Logger; @@ -1000,7 +998,7 @@ private List createOrGetInterpreterList(String noteId, InterpreterS return com.google.common.base.Optional.of(settings.get(0)); } - com.google.common.base.Optional getDefaultInterpreterSetting(String noteId) { + public com.google.common.base.Optional getDefaultInterpreterSetting(String noteId) { return getDefaultInterpreterSetting(getInterpreterSettings(noteId)); } 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 7bb0c8d1757..2b691f3d4d1 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 @@ -74,7 +74,6 @@ public class Note implements Serializable, JobListener { Map> angularObjects = new HashMap<>(); private transient InterpreterFactory factory; - private transient NoteInterpreterLoader replLoader; private transient JobListenerFactory jobListenerFactory; private transient NotebookRepo repo; private transient SearchService index; @@ -98,10 +97,10 @@ public class Note implements Serializable, JobListener { public Note() {} - public Note(NotebookRepo repo, NoteInterpreterLoader replLoader, + public Note(NotebookRepo repo, InterpreterFactory factory, JobListenerFactory jlFactory, SearchService noteIndex, Credentials credentials) { this.repo = repo; - this.replLoader = replLoader; + this.factory = factory; this.jobListenerFactory = jlFactory; this.index = noteIndex; this.credentials = credentials; @@ -113,7 +112,7 @@ private void generateId() { } private String getDefaultInterpreterName() { - Optional settingOptional = replLoader.getDefaultInterpreterSetting(); + Optional settingOptional = factory.getDefaultInterpreterSetting(getId()); return settingOptional.isPresent() ? settingOptional.get().getGroup() : StringUtils.EMPTY; } @@ -155,12 +154,8 @@ public void setName(String name) { this.name = name; } - public NoteInterpreterLoader getNoteReplLoader() { - return replLoader; - } - - public void setReplLoader(NoteInterpreterLoader replLoader) { - this.replLoader = replLoader; + public void setInterpreterFactory(InterpreterFactory factory) { + this.factory = factory; } public JobListenerFactory getJobListenerFactory() { @@ -202,7 +197,7 @@ public Map> getAngularObjects() { */ public Paragraph addParagraph() { - Paragraph p = new Paragraph(this, this, replLoader); + Paragraph p = new Paragraph(this, this, factory); addLastReplNameIfEmptyText(p); synchronized (paragraphs) { paragraphs.add(p); @@ -218,7 +213,7 @@ public Paragraph addParagraph() { public void addCloneParagraph(Paragraph srcParagraph) { // Keep paragraph original ID - final Paragraph newParagraph = new Paragraph(srcParagraph.getId(), this, this, replLoader); + final Paragraph newParagraph = new Paragraph(srcParagraph.getId(), this, this, factory); Map config = new HashMap<>(srcParagraph.getConfig()); Map param = new HashMap<>(srcParagraph.settings.getParams()); @@ -252,7 +247,7 @@ public void addCloneParagraph(Paragraph srcParagraph) { * @param index */ public Paragraph insertParagraph(int index) { - Paragraph p = new Paragraph(this, this, replLoader); + Paragraph p = new Paragraph(this, this, factory); addLastReplNameIfEmptyText(p); synchronized (paragraphs) { paragraphs.add(index, p); @@ -436,9 +431,9 @@ public void runAll() { AuthenticationInfo authenticationInfo = new AuthenticationInfo(); authenticationInfo.setUser(cronExecutingUser); p.setAuthenticationInfo(authenticationInfo); - p.setNoteReplLoader(replLoader); + p.setInterpreterFactory(factory); p.setListener(jobListenerFactory.getParagraphJobListener(this)); - Interpreter intp = factory.get(getId(), p.getRequiredReplName()); + Interpreter intp = factory.getInterpreter(getId(), p.getRequiredReplName()); intp.getScheduler().submit(p); } } @@ -451,7 +446,7 @@ public void runAll() { */ public void run(String paragraphId) { Paragraph p = getParagraph(paragraphId); - p.setNoteReplLoader(replLoader); + p.setInterpreterFactory(factory); p.setListener(jobListenerFactory.getParagraphJobListener(this)); String requiredReplName = p.getRequiredReplName(); Interpreter intp = factory.getInterpreter(getId(), requiredReplName); @@ -488,7 +483,7 @@ public boolean isTerminated() { public List completion(String paragraphId, String buffer, int cursor) { Paragraph p = getParagraph(paragraphId); - p.setNoteReplLoader(replLoader); + p.setInterpreterFactory(factory); p.setListener(jobListenerFactory.getParagraphJobListener(this)); List completion = p.completion(buffer, cursor); diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NoteInterpreterLoader.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NoteInterpreterLoader.java deleted file mode 100644 index a04e7b55a20..00000000000 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NoteInterpreterLoader.java +++ /dev/null @@ -1,213 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.apache.zeppelin.notebook; - -import com.google.common.base.Optional; - -import java.io.IOException; -import java.util.LinkedList; -import java.util.List; - -import org.apache.zeppelin.interpreter.Constants; -import org.apache.zeppelin.interpreter.Interpreter; -import org.apache.zeppelin.interpreter.Interpreter.RegisteredInterpreter; -import org.apache.zeppelin.interpreter.InterpreterException; -import org.apache.zeppelin.interpreter.InterpreterFactory; -import org.apache.zeppelin.interpreter.InterpreterGroup; -import org.apache.zeppelin.interpreter.InterpreterSetting; - -/** - * Interpreter loader per note. - */ -public class NoteInterpreterLoader { - private transient InterpreterFactory factory; - private static String SHARED_SESSION = "shared_session"; - String noteId; - - public NoteInterpreterLoader(InterpreterFactory factory) { - this.factory = factory; - } - - public void setNoteId(String noteId) { - this.noteId = noteId; - } - - /** - * set interpreter ids - * @param ids InterpreterSetting id list - * @throws IOException - */ - @Deprecated - public void setInterpreters(List ids) throws IOException { - factory.putNoteInterpreterSettingBinding(noteId, ids); - } - - @Deprecated - public List getInterpreters() { - return factory.getNoteInterpreterSettingBinding(noteId); - } - - @Deprecated - public List getInterpreterSettings() { - List interpreterSettingIds = factory.getNoteInterpreterSettingBinding(noteId); - LinkedList settings = new LinkedList(); - synchronized (interpreterSettingIds) { - for (String id : interpreterSettingIds) { - InterpreterSetting setting = factory.get(id); - if (setting == null) { - // interpreter setting is removed from factory. remove id from here, too - interpreterSettingIds.remove(id); - } else { - settings.add(setting); - } - } - } - return settings; - } - - - private String getInterpreterInstanceKey(InterpreterSetting setting) { - if (setting.getOption().isExistingProcess()) { - return Constants.EXISTING_PROCESS; - } else if (setting.getOption().isPerNoteSession() || setting.getOption().isPerNoteProcess()) { - return noteId; - } else { - return SHARED_SESSION; - } - } - - private List createOrGetInterpreterList(InterpreterSetting setting) { - InterpreterGroup interpreterGroup = - setting.getInterpreterGroup(noteId); - synchronized (interpreterGroup) { - String key = getInterpreterInstanceKey(setting); - if (!interpreterGroup.containsKey(key)) { - factory.createInterpretersForNote(setting, noteId, key); - } - return interpreterGroup.get(getInterpreterInstanceKey(setting)); - } - } - - @Deprecated - public void close() { - // close interpreters in this note session - List settings = factory.getInterpreterSettings(noteId); - if (settings == null || settings.size() == 0) { - return; - } - - System.err.println("close"); - for (InterpreterSetting setting : settings) { - factory.removeInterpretersForNote(setting, noteId); - } - } - - @Deprecated - public Interpreter get(String replName) { - List settings = factory.getInterpreterSettings(noteId); - - if (settings == null || settings.size() == 0) { - return null; - } - - if (replName == null || replName.trim().length() == 0) { - // get default settings (first available) - InterpreterSetting defaultSettings = getDefaultInterpreterSetting(settings).get(); - return createOrGetInterpreterList(defaultSettings).get(0); - } - - if (Interpreter.registeredInterpreters == null) { - return null; - } - - String[] replNameSplit = replName.split("\\."); - String group = null; - String name = null; - if (replNameSplit.length == 2) { - group = replNameSplit[0]; - name = replNameSplit[1]; - - Interpreter.RegisteredInterpreter registeredInterpreter = Interpreter.registeredInterpreters - .get(group + "." + name); - if (registeredInterpreter == null - || registeredInterpreter.getClassName() == null) { - throw new InterpreterException(replName + " interpreter not found"); - } - String interpreterClassName = registeredInterpreter.getClassName(); - - for (InterpreterSetting setting : settings) { - if (registeredInterpreter.getGroup().equals(setting.getGroup())) { - List intpGroup = createOrGetInterpreterList(setting); - for (Interpreter interpreter : intpGroup) { - if (interpreterClassName.equals(interpreter.getClassName())) { - return interpreter; - } - } - } - } - throw new InterpreterException(replName + " interpreter not found"); - } else { - // first assume replName is 'name' of interpreter. ('groupName' is ommitted) - // search 'name' from first (default) interpreter group - InterpreterSetting defaultSetting = getDefaultInterpreterSetting(settings).get(); - Interpreter.RegisteredInterpreter registeredInterpreter = - Interpreter.registeredInterpreters.get(defaultSetting.getGroup() + "." + replName); - if (registeredInterpreter != null) { - List interpreters = createOrGetInterpreterList(defaultSetting); - for (Interpreter interpreter : interpreters) { - - RegisteredInterpreter intp = - Interpreter.findRegisteredInterpreterByClassName(interpreter.getClassName()); - if (intp == null) { - continue; - } - - if (intp.getName().equals(replName)) { - return interpreter; - } - } - - throw new InterpreterException( - defaultSetting.getGroup() + "." + replName + " interpreter not found"); - } - - // next, assume replName is 'group' of interpreter ('name' is ommitted) - // search interpreter group and return first interpreter. - for (InterpreterSetting setting : settings) { - if (setting.getGroup().equals(replName)) { - List interpreters = createOrGetInterpreterList(setting); - return interpreters.get(0); - } - } - } - - return null; - } - - private Optional - getDefaultInterpreterSetting(List settings) { - if (settings == null || settings.isEmpty()) { - return Optional.absent(); - } - return Optional.of(settings.get(0)); - } - - Optional getDefaultInterpreterSetting() { - return getDefaultInterpreterSetting(factory.getInterpreterSettings(noteId)); - } -} 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 13de4c47339..3243ba774ab 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,9 +151,7 @@ public Note createNote(AuthenticationInfo subject) throws IOException { */ public Note createNote(List interpreterIds, AuthenticationInfo subject) throws IOException { - NoteInterpreterLoader intpLoader = new NoteInterpreterLoader(replFactory); - Note note = new Note(notebookRepo, intpLoader, jobListenerFactory, notebookIndex, credentials); - intpLoader.setNoteId(note.id()); + Note note = new Note(notebookRepo, replFactory, jobListenerFactory, notebookIndex, credentials); synchronized (notes) { notes.put(note.id(), note); } @@ -348,9 +346,7 @@ private Note loadNoteFromRepo(String id, AuthenticationInfo subject) { note.setIndex(this.notebookIndex); note.setCredentials(this.credentials); - NoteInterpreterLoader replLoader = new NoteInterpreterLoader(replFactory); - note.setReplLoader(replLoader); - replLoader.setNoteId(note.id()); + note.setInterpreterFactory(replFactory); note.setJobListenerFactory(jobListenerFactory); note.setNotebookRepo(notebookRepo); 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 9c2648c2355..df4765d8cee 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 @@ -50,7 +50,6 @@ public class Paragraph extends Job implements Serializable, Cloneable { private static final long serialVersionUID = -6328572073497992016L; private transient InterpreterFactory factory; - private transient NoteInterpreterLoader replLoader; private transient Note note; private transient AuthenticationInfo authenticationInfo; private transient String effectiveText; @@ -70,10 +69,10 @@ public class Paragraph extends Job implements Serializable, Cloneable { } public Paragraph(String paragraphId, Note note, JobListener listener, - NoteInterpreterLoader replLoader) { + InterpreterFactory factory) { super(paragraphId, generateId(), listener); this.note = note; - this.replLoader = replLoader; + this.factory = factory; title = null; text = null; authenticationInfo = null; @@ -83,10 +82,10 @@ public Paragraph(String paragraphId, Note note, JobListener listener, config = new HashMap(); } - public Paragraph(Note note, JobListener listener, NoteInterpreterLoader replLoader) { + public Paragraph(Note note, JobListener listener, InterpreterFactory factory) { super(generateId(), listener); this.note = note; - this.replLoader = replLoader; + this.factory = factory; title = null; text = null; authenticationInfo = null; @@ -195,10 +194,6 @@ public static String getScriptBody(String text) { return text.substring(magic.length() + 1).trim(); } - public NoteInterpreterLoader getNoteReplLoader() { - return replLoader; - } - public Interpreter getRepl(String name) { return factory.getInterpreter(note.getId(), name); } @@ -222,8 +217,8 @@ public List completion(String buffer, int cursor) { return completion; } - public void setNoteReplLoader(NoteInterpreterLoader repls) { - this.replLoader = repls; + public void setInterpreterFactory(InterpreterFactory factory) { + this.factory = factory; } public InterpreterResult getResult() { diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NoteInterpreterLoaderTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NoteInterpreterLoaderTest.java index c2e79e0f70a..7d9071c07cd 100644 --- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NoteInterpreterLoaderTest.java +++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NoteInterpreterLoaderTest.java @@ -71,9 +71,6 @@ public void tearDown() throws Exception { @Test public void testGetInterpreter() throws IOException { - NoteInterpreterLoader loader = new NoteInterpreterLoader(factory); - loader.setNoteId("note"); -// loader.setInterpreters(factory.getDefaultInterpreterSettingList()); factory.setInterpreters("note", factory.getDefaultInterpreterSettingList()); // when there're no interpreter selection directive @@ -120,7 +117,7 @@ public void testNoteSession() throws IOException { assertNotNull(factory.getInterpreterSettings("noteB").get(0).getInterpreterGroup("shared_process").get("noteB")); // when - factory.closeNote("noteB"); + factory.closeNote("noteA"); factory.closeNote("noteB"); // interpreters are destroyed after close diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NoteTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NoteTest.java index 1dae66dae7d..d8a8c2f9342 100644 --- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NoteTest.java +++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NoteTest.java @@ -42,9 +42,6 @@ public class NoteTest { @Mock NotebookRepo repo; - @Mock - NoteInterpreterLoader replLoader; - @Mock JobListenerFactory jobListenerFactory; @@ -65,30 +62,30 @@ public class NoteTest { @Test public void runNormalTest() { - when(interpreterFactory.getInterpreter(anyString(), "spark")).thenReturn(interpreter); + when(interpreterFactory.getInterpreter(anyString(), eq("spark"))).thenReturn(interpreter); when(interpreter.getScheduler()).thenReturn(scheduler); String pText = "%spark sc.version"; - Note note = new Note(repo, replLoader, jobListenerFactory, index, credentials); + Note note = new Note(repo, interpreterFactory, jobListenerFactory, index, credentials); Paragraph p = note.addParagraph(); p.setText(pText); note.run(p.getId()); ArgumentCaptor pCaptor = ArgumentCaptor.forClass(Paragraph.class); verify(scheduler, only()).submit(pCaptor.capture()); - verify(interpreterFactory, only()).getInterpreter(anyString(), "spark"); + verify(interpreterFactory, only()).getInterpreter(anyString(), eq("spark")); assertEquals("Paragraph text", pText, pCaptor.getValue().getText()); } @Test public void runJdbcTest() { - when(interpreterFactory.getInterpreter(anyString(), "mysql")).thenReturn(null); - when(interpreterFactory.getInterpreter(anyString(), "jdbc")).thenReturn(interpreter); + when(interpreterFactory.getInterpreter(anyString(), eq("mysql"))).thenReturn(null); + when(interpreterFactory.getInterpreter(anyString(), eq("jdbc"))).thenReturn(interpreter); when(interpreter.getScheduler()).thenReturn(scheduler); String pText = "%mysql show databases"; - Note note = new Note(repo, replLoader, jobListenerFactory, index, credentials); + Note note = new Note(repo, interpreterFactory, jobListenerFactory, index, credentials); Paragraph p = note.addParagraph(); p.setText(pText); note.run(p.getId()); @@ -103,10 +100,10 @@ public void runJdbcTest() { @Test public void putDefaultReplNameIfInterpreterSettingAbsent() { - when(interpreterFactory.getDefaultInterpreterSetting(anyListOf(InterpreterSetting.class))) + when(interpreterFactory.getDefaultInterpreterSetting(anyString())) .thenReturn(Optional.absent()); - Note note = new Note(repo, replLoader, jobListenerFactory, index, credentials); + Note note = new Note(repo, interpreterFactory, jobListenerFactory, index, credentials); note.putDefaultReplName(); assertEquals(StringUtils.EMPTY, note.getLastReplName()); @@ -117,10 +114,10 @@ public void putDefaultReplNameIfInterpreterSettingAbsent() { public void putDefaultReplNameIfInterpreterSettingPresent() { InterpreterSetting interpreterSetting = Mockito.mock(InterpreterSetting.class); when(interpreterSetting.getGroup()).thenReturn("spark"); - when(replLoader.getDefaultInterpreterSetting()) + when(interpreterFactory.getDefaultInterpreterSetting(anyString())) .thenReturn(Optional.of(interpreterSetting)); - Note note = new Note(repo, replLoader, jobListenerFactory, index, credentials); + Note note = new Note(repo, interpreterFactory, jobListenerFactory, index, credentials); note.putDefaultReplName(); assertEquals("spark", note.getLastReplName()); @@ -131,10 +128,10 @@ public void putDefaultReplNameIfInterpreterSettingPresent() { public void addParagraphWithLastReplName() { InterpreterSetting interpreterSetting = Mockito.mock(InterpreterSetting.class); when(interpreterSetting.getGroup()).thenReturn("spark"); - when(replLoader.getDefaultInterpreterSetting()) + when(interpreterFactory.getDefaultInterpreterSetting(anyString())) .thenReturn(Optional.of(interpreterSetting)); - Note note = new Note(repo, replLoader, jobListenerFactory, index, credentials); + Note note = new Note(repo, interpreterFactory, jobListenerFactory, index, credentials); note.putDefaultReplName(); //set lastReplName Paragraph p = note.addParagraph(); @@ -146,10 +143,10 @@ public void addParagraphWithLastReplName() { public void insertParagraphWithLastReplName() { InterpreterSetting interpreterSetting = Mockito.mock(InterpreterSetting.class); when(interpreterSetting.getGroup()).thenReturn("spark"); - when(replLoader.getDefaultInterpreterSetting()) + when(interpreterFactory.getDefaultInterpreterSetting(anyString())) .thenReturn(Optional.of(interpreterSetting)); - Note note = new Note(repo, replLoader, jobListenerFactory, index, credentials); + Note note = new Note(repo, interpreterFactory, jobListenerFactory, index, credentials); note.putDefaultReplName(); //set lastReplName Paragraph p = note.insertParagraph(note.getParagraphs().size()); @@ -160,7 +157,7 @@ public void insertParagraphWithLastReplName() { @Test public void setLastReplName() { String paragraphId = "HelloWorld"; - Note note = Mockito.spy(new Note(repo, replLoader, jobListenerFactory, index, credentials)); + Note note = Mockito.spy(new Note(repo, interpreterFactory, jobListenerFactory, index, credentials)); Paragraph mockParagraph = Mockito.mock(Paragraph.class); when(note.getParagraph(paragraphId)).thenReturn(mockParagraph); when(mockParagraph.getRequiredReplName()).thenReturn("spark"); diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/ParagraphTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/ParagraphTest.java index 3644b23a84c..1f8519cb399 100644 --- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/ParagraphTest.java +++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/ParagraphTest.java @@ -20,6 +20,7 @@ import static org.junit.Assert.assertEquals; import static org.mockito.Matchers.anyString; +import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -75,17 +76,18 @@ public void replNameEndsWithWhitespace() { @Test public void effectiveTextTest() { InterpreterFactory interpreterFactory = mock(InterpreterFactory.class); - NoteInterpreterLoader noteInterpreterLoader = mock(NoteInterpreterLoader.class); Interpreter interpreter = mock(Interpreter.class); + Note note = mock(Note.class); - Paragraph p = new Paragraph(null, null, null, noteInterpreterLoader); + Paragraph p = new Paragraph("paragraph", note, null, interpreterFactory); p.setText("%h2 show databases"); p.setEffectiveText("%jdbc(h2) show databases"); assertEquals("Get right replName", "jdbc", p.getRequiredReplName()); assertEquals("Get right scriptBody", "(h2) show databases", p.getScriptBody()); - when(interpreterFactory.getInterpreter(anyString(), "jdbc")).thenReturn(interpreter); + when(interpreterFactory.getInterpreter(anyString(), eq("jdbc"))).thenReturn(interpreter); when(interpreter.getFormType()).thenReturn(Interpreter.FormType.NATIVE); + when(note.getId()).thenReturn("noteId"); try { p.jobRun(); diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/search/LuceneSearchTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/search/LuceneSearchTest.java index 49ecab5b1c1..b7fa2df6c87 100644 --- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/search/LuceneSearchTest.java +++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/search/LuceneSearchTest.java @@ -25,9 +25,8 @@ import java.util.List; import java.util.Map; -import org.apache.zeppelin.interpreter.InterpreterSetting; +import org.apache.zeppelin.interpreter.InterpreterFactory; import org.apache.zeppelin.notebook.Note; -import org.apache.zeppelin.notebook.NoteInterpreterLoader; import org.apache.zeppelin.notebook.Paragraph; import org.apache.zeppelin.notebook.repo.NotebookRepo; import org.junit.After; @@ -36,18 +35,17 @@ import org.junit.Test; import com.google.common.base.Splitter; -import com.google.common.collect.ImmutableList; public class LuceneSearchTest { - private static NoteInterpreterLoader replLoaderMock; private static NotebookRepo notebookRepoMock; + private static InterpreterFactory interpreterFactory; private SearchService notebookIndex; @BeforeClass public static void beforeStartUp() { notebookRepoMock = mock(NotebookRepo.class); - replLoaderMock = mock(NoteInterpreterLoader.class); + interpreterFactory = mock(InterpreterFactory.class); // when(replLoaderMock.getInterpreterSettings()) // .thenReturn(ImmutableList.of()); @@ -286,7 +284,7 @@ private Paragraph addParagraphWithTextAndTitle(Note note, String text, String ti } private Note newNote(String name) { - Note note = new Note(notebookRepoMock, replLoaderMock, null, notebookIndex, null); + Note note = new Note(notebookRepoMock, interpreterFactory, null, notebookIndex, null); note.setName(name); return note; } From 28bf520e262f18488951377d5ddbb9a6a684c6ae Mon Sep 17 00:00:00 2001 From: Jongyoul Lee Date: Wed, 29 Jun 2016 10:20:09 +0900 Subject: [PATCH 3/5] Fixed style --- .../org/apache/zeppelin/interpreter/InterpreterFactory.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterFactory.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterFactory.java index ab15444f9eb..af48eed3ee8 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterFactory.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterFactory.java @@ -998,7 +998,8 @@ private List createOrGetInterpreterList(String noteId, InterpreterS return com.google.common.base.Optional.of(settings.get(0)); } - public com.google.common.base.Optional getDefaultInterpreterSetting(String noteId) { + public com.google.common.base.Optional + getDefaultInterpreterSetting(String noteId) { return getDefaultInterpreterSetting(getInterpreterSettings(noteId)); } From a2d81040e04f23ef6819c069fb62f3967b252b0a Mon Sep 17 00:00:00 2001 From: Jongyoul Lee Date: Thu, 30 Jun 2016 23:29:21 +0900 Subject: [PATCH 4/5] Fixed some style and removed unused codes --- .../interpreter/InterpreterFactory.java | 186 +++++++----------- 1 file changed, 71 insertions(+), 115 deletions(-) diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterFactory.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterFactory.java index af48eed3ee8..5595c148c29 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterFactory.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterFactory.java @@ -59,29 +59,28 @@ * Manage interpreters. */ public class InterpreterFactory implements InterpreterGroupFactory { - Logger logger = LoggerFactory.getLogger(InterpreterFactory.class); + private static Logger logger = LoggerFactory.getLogger(InterpreterFactory.class); - private static String SHARED_SESSION = "shared_session"; + private static final String SHARED_SESSION = "shared_session"; - private Map cleanCl = Collections - .synchronizedMap(new HashMap()); + private Map cleanCl = + Collections.synchronizedMap(new HashMap()); private ZeppelinConfiguration conf; @Deprecated - String[] interpreterClassList; - String[] interpreterGroupOrderList; + private String[] interpreterClassList; + private String[] interpreterGroupOrderList; - private Map interpreterSettings = - new HashMap(); + private Map interpreterSettings = new HashMap<>(); - private Map> interpreterBindings = new HashMap>(); + private Map> interpreterBindings = new HashMap<>(); private List interpreterRepositories; private Gson gson; private InterpreterOption defaultOption; - AngularObjectRegistryListener angularObjectRegistryListener; + private AngularObjectRegistryListener angularObjectRegistryListener; private final RemoteInterpreterProcessListener remoteInterpreterProcessListener; private DependencyResolver depResolver; @@ -92,7 +91,7 @@ public InterpreterFactory(ZeppelinConfiguration conf, DependencyResolver depResolver) throws InterpreterException, IOException, RepositoryException { this(conf, new InterpreterOption(true), angularObjectRegistryListener, - remoteInterpreterProcessListener, depResolver); + remoteInterpreterProcessListener, depResolver); } @@ -231,7 +230,7 @@ public boolean accept(Path entry) throws IOException { } private void registerInterpreterFromResource(ClassLoader cl, String interpreterDir, - String interpreterJson) + String interpreterJson) throws MalformedURLException { URL[] urls = recursiveBuildLibList(new File(interpreterDir)); ClassLoader tempClassLoader = new URLClassLoader(urls, cl); @@ -247,7 +246,7 @@ private void registerInterpreterFromResource(ClassLoader cl, String interpreterD } private void registerInterpreterFromPath(String interpreterDir, - String interpreterJson) throws IOException { + String interpreterJson) throws IOException { Path interpreterJsonPath = Paths.get(interpreterDir, interpreterJson); if (Files.exists(interpreterJsonPath)) { @@ -270,11 +269,11 @@ private List getInterpreterListFromJson(InputStream strea } private void registerInterpreters(List registeredInterpreters, - String absolutePath) { + String absolutePath) { for (RegisteredInterpreter registeredInterpreter : registeredInterpreters) { String className = registeredInterpreter.getClassName(); if (validateRegisterInterpreter(registeredInterpreter) && - null == Interpreter.findRegisteredInterpreterByClassName(className)) { + null == Interpreter.findRegisteredInterpreterByClassName(className)) { registeredInterpreter.setPath(absolutePath); Interpreter.register(registeredInterpreter); logger.debug("Registered. key: {}, className: {}, path: {}", @@ -286,7 +285,7 @@ private void registerInterpreters(List registeredInterpre private boolean validateRegisterInterpreter(RegisteredInterpreter registeredInterpreter) { return null != registeredInterpreter.getGroup() && null != registeredInterpreter.getName() && - null != registeredInterpreter.getClassName(); + null != registeredInterpreter.getClassName(); } private void loadFromFile() throws IOException { @@ -324,14 +323,9 @@ private void loadFromFile() throws IOException { // previously created setting should turn this feature on here. setting.getOption().setRemote(true); - InterpreterSetting intpSetting = new InterpreterSetting( - setting.id(), - setting.getName(), - setting.getGroup(), - setting.getInterpreterInfos(), - setting.getProperties(), - setting.getDependencies(), - setting.getOption()); + InterpreterSetting intpSetting = new InterpreterSetting(setting.id(), setting.getName(), + setting.getGroup(), setting.getInterpreterInfos(), setting.getProperties(), + setting.getDependencies(), setting.getOption()); intpSetting.setInterpreterGroupFactory(this); interpreterSettings.put(k, intpSetting); @@ -359,18 +353,14 @@ private void loadInterpreterDependencies(InterpreterSetting intSetting) // load dependencies List deps = intSetting.getDependencies(); if (deps != null) { - for (Dependency d: deps) { + for (Dependency d : deps) { File destDir = new File(conf.getRelativeDir(ConfVars.ZEPPELIN_DEP_LOCALREPO)); if (d.getExclusions() != null) { - depResolver.load( - d.getGroupArtifactVersion(), - d.getExclusions(), + depResolver.load(d.getGroupArtifactVersion(), d.getExclusions(), new File(destDir, intSetting.id())); } else { - depResolver.load( - d.getGroupArtifactVersion(), - new File(destDir, intSetting.id())); + depResolver.load(d.getGroupArtifactVersion(), new File(destDir, intSetting.id())); } } } @@ -400,29 +390,19 @@ private void saveToFile() throws IOException { fos.close(); } - private RegisteredInterpreter getRegisteredReplInfoFromClassName(String clsName) { - Set keys = Interpreter.registeredInterpreters.keySet(); - for (String intName : keys) { - RegisteredInterpreter info = Interpreter.registeredInterpreters.get(intName); - if (clsName.equals(info.getClassName())) { - return info; - } - } - return null; - } - /** * Return ordered interpreter setting list. * The list does not contain more than one setting from the same interpreter class. * Order by InterpreterClass (order defined by ZEPPELIN_INTERPRETERS), Interpreter setting name + * * @return */ public List getDefaultInterpreterSettingList() { // this list will contain default interpreter setting list - List defaultSettings = new LinkedList(); + List defaultSettings = new LinkedList<>(); // to ignore the same interpreter group - Map interpreterGroupCheck = new HashMap(); + Map interpreterGroupCheck = new HashMap<>(); List sortedSettings = get(); @@ -444,15 +424,14 @@ public List getRegisteredInterpreterList() { } /** - * @param name user defined name - * @param groupName interpreter group name to instantiate + * @param name user defined name + * @param groupName interpreter group name to instantiate * @param properties * @return * @throws InterpreterException * @throws IOException */ - public InterpreterSetting add(String name, String groupName, - List dependencies, + public InterpreterSetting add(String name, String groupName, List dependencies, InterpreterOption option, Properties properties) throws InterpreterException, IOException, RepositoryException { synchronized (interpreterSettings) { @@ -472,13 +451,8 @@ public InterpreterSetting add(String name, String groupName, } } - InterpreterSetting intpSetting = new InterpreterSetting( - name, - groupName, - interpreterInfos, - properties, - dependencies, - option); + InterpreterSetting intpSetting = new InterpreterSetting(name, groupName, interpreterInfos, + properties, dependencies, option); if (dependencies.size() > 0) { loadInterpreterDependencies(intpSetting); @@ -503,15 +477,10 @@ public InterpreterGroup createInterpreterGroup(String id, InterpreterOption opti InterpreterGroup interpreterGroup = new InterpreterGroup(id); if (option.isRemote()) { - angularObjectRegistry = new RemoteAngularObjectRegistry( - id, - angularObjectRegistryListener, - interpreterGroup - ); + angularObjectRegistry = new RemoteAngularObjectRegistry(id, angularObjectRegistryListener, + interpreterGroup); } else { - angularObjectRegistry = new AngularObjectRegistry( - id, - angularObjectRegistryListener); + angularObjectRegistry = new AngularObjectRegistry(id, angularObjectRegistryListener); // TODO(moon) : create distributed resource pool for local interpreters and set } @@ -520,8 +489,7 @@ public InterpreterGroup createInterpreterGroup(String id, InterpreterOption opti return interpreterGroup; } - public void removeInterpretersForNote(InterpreterSetting interpreterSetting, - String noteId) { + public void removeInterpretersForNote(InterpreterSetting interpreterSetting, String noteId) { if (interpreterSetting.getOption().isPerNoteProcess()) { interpreterSetting.closeAndRemoveInterpreterGroup(noteId); } else if (interpreterSetting.getOption().isPerNoteSession()) { @@ -539,9 +507,7 @@ public void removeInterpretersForNote(InterpreterSetting interpreterSetting, } } - public void createInterpretersForNote( - InterpreterSetting interpreterSetting, - String noteId, + public void createInterpretersForNote(InterpreterSetting interpreterSetting, String noteId, String key) { InterpreterGroup interpreterGroup = interpreterSetting.getInterpreterGroup(noteId); String groupName = interpreterSetting.getGroup(); @@ -558,10 +524,8 @@ public void createInterpretersForNote( // in ZEPPELIN_INTERPRETER_CONNECT_TIMEOUT msec. However, if termination of the process and // removal from interpreter group take too long, throw an error. long minTimeout = 10L * 1000 * 1000000; // 10 sec - long interpreterRemovalWaitTimeout = - Math.max( - minTimeout, - conf.getInt(ConfVars.ZEPPELIN_INTERPRETER_CONNECT_TIMEOUT) * 1000000L * 2); + long interpreterRemovalWaitTimeout = Math.max(minTimeout, + conf.getInt(ConfVars.ZEPPELIN_INTERPRETER_CONNECT_TIMEOUT) * 1000000L * 2); while (interpreterGroup.containsKey(key)) { if (System.nanoTime() - interpreterRemovalWaitStart > interpreterRemovalWaitTimeout) { throw new InterpreterException("Can not create interpreter"); @@ -583,21 +547,16 @@ public void createInterpretersForNote( Interpreter intp; if (option.isRemote()) { - intp = createRemoteRepl(info.getPath(), - key, - info.getClassName(), - properties, + intp = createRemoteRepl(info.getPath(), key, info.getClassName(), properties, interpreterSetting.id()); } else { - intp = createRepl(info.getPath(), - info.getClassName(), - properties); + intp = createRepl(info.getPath(), info.getClassName(), properties); } synchronized (interpreterGroup) { List interpreters = interpreterGroup.get(key); if (interpreters == null) { - interpreters = new LinkedList(); + interpreters = new LinkedList<>(); interpreterGroup.put(key, interpreters); } if (info.isDefaultInterpreter()) { @@ -613,7 +572,6 @@ public void createInterpretersForNote( } - public void remove(String id) throws IOException { synchronized (interpreterSettings) { if (interpreterSettings.containsKey(id)) { @@ -640,11 +598,12 @@ public void remove(String id) throws IOException { /** * Get interpreter settings + * * @return */ public List get() { synchronized (interpreterSettings) { - List orderedSettings = new LinkedList(); + List orderedSettings = new LinkedList<>(); Map> groupNameInterpreterSettingMap = new HashMap<>(); for (InterpreterSetting interpreterSetting : interpreterSettings.values()) { @@ -693,9 +652,9 @@ public InterpreterSetting get(String name) { } } - public void putNoteInterpreterSettingBinding(String noteId, - List settingList) throws IOException { - List unBindedSettings = new LinkedList(); + private void putNoteInterpreterSettingBinding(String noteId, List settingList) + throws IOException { + List unBindedSettings = new LinkedList<>(); synchronized (interpreterSettings) { List oldSettings = interpreterBindings.get(noteId); @@ -726,8 +685,8 @@ public void removeNoteInterpreterSettingBinding(String noteId) { } } - public List getNoteInterpreterSettingBinding(String noteId) { - LinkedList bindings = new LinkedList(); + private List getNoteInterpreterSettingBinding(String noteId) { + LinkedList bindings = new LinkedList<>(); synchronized (interpreterSettings) { List settingIds = interpreterBindings.get(noteId); if (settingIds != null) { @@ -739,14 +698,13 @@ public List getNoteInterpreterSettingBinding(String noteId) { /** * Change interpreter property and restart + * * @param id * @param option * @param properties * @throws IOException */ - public void setPropertyAndRestart(String id, - InterpreterOption option, - Properties properties, + public void setPropertyAndRestart(String id, InterpreterOption option, Properties properties, List dependencies) throws IOException, RepositoryException { synchronized (interpreterSettings) { InterpreterSetting intpsetting = interpreterSettings.get(id); @@ -807,7 +765,7 @@ private void stopJobAllInterpreter(InterpreterSetting intpsetting) { } public void close() { - List closeThreads = new LinkedList(); + List closeThreads = new LinkedList<>(); synchronized (interpreterSettings) { Collection intpsettings = interpreterSettings.values(); for (final InterpreterSetting intpsetting : intpsettings) { @@ -843,7 +801,7 @@ private Interpreter createRepl(String dirName, String className, URLClassLoader ccl = cleanCl.get(dirName); if (ccl == null) { // classloader fallback - ccl = URLClassLoader.newInstance(new URL[] {}, oldcl); + ccl = URLClassLoader.newInstance(new URL[]{}, oldcl); } boolean separateCL = true; @@ -853,13 +811,13 @@ private Interpreter createRepl(String dirName, String className, separateCL = false; } } catch (Exception e) { - logger.error("exception checking server classloader driver" , e); + logger.error("exception checking server classloader driver", e); } URLClassLoader cl; if (separateCL == true) { - cl = URLClassLoader.newInstance(new URL[] {}, ccl); + cl = URLClassLoader.newInstance(new URL[]{}, ccl); } else { cl = ccl; } @@ -867,7 +825,7 @@ private Interpreter createRepl(String dirName, String className, Class replClass = (Class) cl.loadClass(className); Constructor constructor = - replClass.getConstructor(new Class[] {Properties.class}); + replClass.getConstructor(new Class[]{Properties.class}); Interpreter repl = constructor.newInstance(property); repl.setClassloaderUrls(ccl.getURLs()); LazyOpenInterpreter intp = new LazyOpenInterpreter( @@ -901,15 +859,14 @@ private Interpreter createRemoteRepl(String interpreterPath, String noteId, Stri updatePropertiesFromRegisteredInterpreter(property, className); - LazyOpenInterpreter intp = new LazyOpenInterpreter(new RemoteInterpreter( - property, noteId, className, conf.getInterpreterRemoteRunnerPath(), - interpreterPath, localRepoPath, connectTimeout, - maxPoolSize, remoteInterpreterProcessListener)); + LazyOpenInterpreter intp = new LazyOpenInterpreter(new RemoteInterpreter(property, noteId, + className, conf.getInterpreterRemoteRunnerPath(), interpreterPath, localRepoPath, + connectTimeout, maxPoolSize, remoteInterpreterProcessListener)); return intp; } private Properties updatePropertiesFromRegisteredInterpreter(Properties properties, - String className) { + String className) { RegisteredInterpreter registeredInterpreter = Interpreter.findRegisteredInterpreterByClassName( className); if (null != registeredInterpreter) { @@ -926,8 +883,9 @@ private Properties updatePropertiesFromRegisteredInterpreter(Properties properti /** * map interpreter ids into noteId + * * @param noteId note id - * @param ids InterpreterSetting id list + * @param ids InterpreterSetting id list * @throws IOException */ public void setInterpreters(String noteId, List ids) throws IOException { @@ -940,7 +898,7 @@ public List getInterpreters(String noteId) { public List getInterpreterSettings(String noteId) { List interpreterSettingIds = getNoteInterpreterSettingBinding(noteId); - LinkedList settings = new LinkedList(); + LinkedList settings = new LinkedList<>(); synchronized (interpreterSettingIds) { for (String id : interpreterSettingIds) { InterpreterSetting setting = get(id); @@ -962,7 +920,7 @@ public void closeNote(String noteId) { return; } - System.err.println("close"); + logger.info("closeNote: {}", noteId); for (InterpreterSetting setting : settings) { removeInterpretersForNote(setting, noteId); } @@ -979,8 +937,7 @@ private String getInterpreterInstanceKey(String noteId, InterpreterSetting setti } private List createOrGetInterpreterList(String noteId, InterpreterSetting setting) { - InterpreterGroup interpreterGroup = - setting.getInterpreterGroup(noteId); + InterpreterGroup interpreterGroup = setting.getInterpreterGroup(noteId); synchronized (interpreterGroup) { String key = getInterpreterInstanceKey(noteId, setting); if (!interpreterGroup.containsKey(key)) { @@ -990,16 +947,14 @@ private List createOrGetInterpreterList(String noteId, InterpreterS } } - private com.google.common.base.Optional - getDefaultInterpreterSetting(List settings) { + private InterpreterSetting getDefaultInterpreterSetting(List settings) { if (settings == null || settings.isEmpty()) { - return com.google.common.base.Optional.absent(); + return null; } - return com.google.common.base.Optional.of(settings.get(0)); + return settings.get(0); } - public com.google.common.base.Optional - getDefaultInterpreterSetting(String noteId) { + public InterpreterSetting getDefaultInterpreterSetting(String noteId) { return getDefaultInterpreterSetting(getInterpreterSettings(noteId)); } @@ -1012,7 +967,8 @@ public Interpreter getInterpreter(String noteId, String replName) { if (replName == null || replName.trim().length() == 0) { // get default settings (first available) - InterpreterSetting defaultSettings = getDefaultInterpreterSetting(settings).get(); + // TODO(jl): Fix it in case of returning null + InterpreterSetting defaultSettings = getDefaultInterpreterSetting(settings); return createOrGetInterpreterList(noteId, defaultSettings).get(0); } @@ -1049,7 +1005,7 @@ public Interpreter getInterpreter(String noteId, String replName) { } else { // first assume replName is 'name' of interpreter. ('groupName' is ommitted) // search 'name' from first (default) interpreter group - InterpreterSetting defaultSetting = getDefaultInterpreterSetting(settings).get(); + InterpreterSetting defaultSetting = getDefaultInterpreterSetting(settings); Interpreter.RegisteredInterpreter registeredInterpreter = Interpreter.registeredInterpreters.get(defaultSetting.getGroup() + "." + replName); if (registeredInterpreter != null) { @@ -1086,7 +1042,7 @@ public Interpreter getInterpreter(String noteId, String replName) { private URL[] recursiveBuildLibList(File path) throws MalformedURLException { URL[] urls = new URL[0]; - if (path == null || path.exists() == false) { + if (path == null || !path.exists()) { return urls; } else if (path.getName().startsWith(".")) { return urls; @@ -1099,7 +1055,7 @@ private URL[] recursiveBuildLibList(File path) throws MalformedURLException { } return urls; } else { - return new URL[] {path.toURI().toURL()}; + return new URL[]{path.toURI().toURL()}; } } From d9816f1291d7d81f60d0678aeb2b4c2bcc0fc9b9 Mon Sep 17 00:00:00 2001 From: Jongyoul Lee Date: Thu, 30 Jun 2016 23:46:56 +0900 Subject: [PATCH 5/5] Fixed related codes --- .../src/main/java/org/apache/zeppelin/notebook/Note.java | 4 ++-- .../test/java/org/apache/zeppelin/notebook/NoteTest.java | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) 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 2b691f3d4d1..e57ed9bf545 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 @@ -112,8 +112,8 @@ private void generateId() { } private String getDefaultInterpreterName() { - Optional settingOptional = factory.getDefaultInterpreterSetting(getId()); - return settingOptional.isPresent() ? settingOptional.get().getGroup() : StringUtils.EMPTY; + InterpreterSetting setting = factory.getDefaultInterpreterSetting(getId()); + return null != setting ? setting.getGroup() : StringUtils.EMPTY; } void putDefaultReplName() { diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NoteTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NoteTest.java index d8a8c2f9342..2ca817fb2e0 100644 --- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NoteTest.java +++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NoteTest.java @@ -101,7 +101,7 @@ public void runJdbcTest() { @Test public void putDefaultReplNameIfInterpreterSettingAbsent() { when(interpreterFactory.getDefaultInterpreterSetting(anyString())) - .thenReturn(Optional.absent()); + .thenReturn(null); Note note = new Note(repo, interpreterFactory, jobListenerFactory, index, credentials); note.putDefaultReplName(); @@ -115,7 +115,7 @@ public void putDefaultReplNameIfInterpreterSettingPresent() { InterpreterSetting interpreterSetting = Mockito.mock(InterpreterSetting.class); when(interpreterSetting.getGroup()).thenReturn("spark"); when(interpreterFactory.getDefaultInterpreterSetting(anyString())) - .thenReturn(Optional.of(interpreterSetting)); + .thenReturn(interpreterSetting); Note note = new Note(repo, interpreterFactory, jobListenerFactory, index, credentials); note.putDefaultReplName(); @@ -129,7 +129,7 @@ public void addParagraphWithLastReplName() { InterpreterSetting interpreterSetting = Mockito.mock(InterpreterSetting.class); when(interpreterSetting.getGroup()).thenReturn("spark"); when(interpreterFactory.getDefaultInterpreterSetting(anyString())) - .thenReturn(Optional.of(interpreterSetting)); + .thenReturn(interpreterSetting); Note note = new Note(repo, interpreterFactory, jobListenerFactory, index, credentials); note.putDefaultReplName(); //set lastReplName @@ -144,7 +144,7 @@ public void insertParagraphWithLastReplName() { InterpreterSetting interpreterSetting = Mockito.mock(InterpreterSetting.class); when(interpreterSetting.getGroup()).thenReturn("spark"); when(interpreterFactory.getDefaultInterpreterSetting(anyString())) - .thenReturn(Optional.of(interpreterSetting)); + .thenReturn(interpreterSetting); Note note = new Note(repo, interpreterFactory, jobListenerFactory, index, credentials); note.putDefaultReplName(); //set lastReplName