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 1a6f0be3e2..23d80a41c7 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 @@ -1082,6 +1082,7 @@ public class NotebookServer extends WebSocketServlet if (note != null && !note.isTrash()){ fromMessage.put("name", Folder.TRASH_FOLDER_ID + "/" + note.getName()); renameNote(conn, userAndRoles, notebook, fromMessage, "move"); + notebook.moveNoteToTrash(note.getId()); } } diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSettingManager.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSettingManager.java index 585456f34a..1832564489 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSettingManager.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSettingManager.java @@ -799,20 +799,7 @@ public class InterpreterSettingManager { public void removeInterpretersForNote(InterpreterSetting interpreterSetting, String user, String noteId) { - InterpreterOption option = interpreterSetting.getOption(); - if (option.isProcess()) { - interpreterSetting.closeAndRemoveInterpreterGroupByNoteId(noteId); - } else if (option.isSession()) { - InterpreterGroup interpreterGroup = interpreterSetting.getInterpreterGroup(user, noteId); - String key = getInterpreterSessionKey(user, noteId, interpreterSetting); - interpreterGroup.close(key); - synchronized (interpreterGroup) { - interpreterGroup.remove(key); - interpreterGroup.notifyAll(); // notify createInterpreterForNote() - } - logger.info("Interpreter instance {} for note {} is removed", interpreterSetting.getName(), - noteId); - } + interpreterSetting.closeAndRemoveInterpreterGroup(noteId, ""); } public String getInterpreterSessionKey(String user, String noteId, InterpreterSetting setting) { 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 dfef6f45bb..e7f7e49a43 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 @@ -322,6 +322,13 @@ public class Notebook implements NoteEventListener { } } + public void moveNoteToTrash(String noteId) { + for (InterpreterSetting interpreterSetting : interpreterSettingManager + .getInterpreterSettings(noteId)) { + interpreterSettingManager.removeInterpretersForNote(interpreterSetting, "", noteId); + } + } + public void removeNote(String id, AuthenticationInfo subject) { Preconditions.checkNotNull(subject, "AuthenticationInfo should not be null"); diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterSettingTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterSettingTest.java index 0008751ba7..1aab757275 100644 --- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterSettingTest.java +++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterSettingTest.java @@ -204,4 +204,124 @@ public class InterpreterSettingTest { interpreterSetting.closeAndRemoveInterpreterGroup("note2", "user1"); assertEquals(0, interpreterSetting.getAllInterpreterGroups().size()); } + + @Test + public void perNoteScopedModeRemoveInterpreterGroupWhenNoteIsRemoved() { + InterpreterOption interpreterOption = new InterpreterOption(); + interpreterOption.setPerNote(InterpreterOption.SCOPED); + InterpreterSetting interpreterSetting = new InterpreterSetting("", "", "", new ArrayList(), new Properties(), new ArrayList(), interpreterOption, "", null); + + interpreterSetting.setInterpreterGroupFactory(new InterpreterGroupFactory() { + @Override + public InterpreterGroup createInterpreterGroup(String interpreterGroupId, + InterpreterOption option) { + return new InterpreterGroup(interpreterGroupId); + } + }); + + Interpreter mockInterpreter1 = mock(RemoteInterpreter.class); + List interpreterList1 = new ArrayList<>(); + interpreterList1.add(mockInterpreter1); + InterpreterGroup interpreterGroup = interpreterSetting.getInterpreterGroup("user1", "note1"); + interpreterGroup.put(interpreterSetting.getInterpreterSessionKey("user1", "note1"), interpreterList1); + + assertEquals(1, interpreterSetting.getAllInterpreterGroups().size()); + assertEquals(1, interpreterSetting.getInterpreterGroup("user1", "note1").size()); + + // This method will be called when remove note + interpreterSetting.closeAndRemoveInterpreterGroup("note1",""); + assertEquals(0, interpreterSetting.getAllInterpreterGroups().size()); + // Be careful that getInterpreterGroup makes interpreterGroup if it doesn't exist + assertEquals(0, interpreterSetting.getInterpreterGroup("user1","note1").size()); + } + + @Test + public void perNoteIsolatedModeRemoveInterpreterGroupWhenNoteIsRemoved() { + InterpreterOption interpreterOption = new InterpreterOption(); + interpreterOption.setPerNote(InterpreterOption.ISOLATED); + InterpreterSetting interpreterSetting = new InterpreterSetting("", "", "", new ArrayList(), new Properties(), new ArrayList(), interpreterOption, "", null); + + interpreterSetting.setInterpreterGroupFactory(new InterpreterGroupFactory() { + @Override + public InterpreterGroup createInterpreterGroup(String interpreterGroupId, + InterpreterOption option) { + return new InterpreterGroup(interpreterGroupId); + } + }); + + Interpreter mockInterpreter1 = mock(RemoteInterpreter.class); + List interpreterList1 = new ArrayList<>(); + interpreterList1.add(mockInterpreter1); + InterpreterGroup interpreterGroup = interpreterSetting.getInterpreterGroup("user1", "note1"); + interpreterGroup.put(interpreterSetting.getInterpreterSessionKey("user1", "note1"), interpreterList1); + + assertEquals(1, interpreterSetting.getAllInterpreterGroups().size()); + assertEquals(1, interpreterSetting.getInterpreterGroup("user1", "note1").size()); + + // This method will be called when remove note + interpreterSetting.closeAndRemoveInterpreterGroup("note1",""); + assertEquals(0, interpreterSetting.getAllInterpreterGroups().size()); + // Be careful that getInterpreterGroup makes interpreterGroup if it doesn't exist + assertEquals(0, interpreterSetting.getInterpreterGroup("user1","note1").size()); + } + + @Test + public void perUserScopedModeNeverRemoveInterpreterGroupWhenNoteIsRemoved() { + InterpreterOption interpreterOption = new InterpreterOption(); + interpreterOption.setPerUser(InterpreterOption.SCOPED); + InterpreterSetting interpreterSetting = new InterpreterSetting("", "", "", new ArrayList(), new Properties(), new ArrayList(), interpreterOption, "", null); + + interpreterSetting.setInterpreterGroupFactory(new InterpreterGroupFactory() { + @Override + public InterpreterGroup createInterpreterGroup(String interpreterGroupId, + InterpreterOption option) { + return new InterpreterGroup(interpreterGroupId); + } + }); + + Interpreter mockInterpreter1 = mock(RemoteInterpreter.class); + List interpreterList1 = new ArrayList<>(); + interpreterList1.add(mockInterpreter1); + InterpreterGroup interpreterGroup = interpreterSetting.getInterpreterGroup("user1", "note1"); + interpreterGroup.put(interpreterSetting.getInterpreterSessionKey("user1", "note1"), interpreterList1); + + assertEquals(1, interpreterSetting.getAllInterpreterGroups().size()); + assertEquals(1, interpreterSetting.getInterpreterGroup("user1", "note1").size()); + + // This method will be called when remove note + interpreterSetting.closeAndRemoveInterpreterGroup("note1",""); + assertEquals(1, interpreterSetting.getAllInterpreterGroups().size()); + // Be careful that getInterpreterGroup makes interpreterGroup if it doesn't exist + assertEquals(1, interpreterSetting.getInterpreterGroup("user1","note1").size()); + } + + @Test + public void perUserIsolatedModeNeverRemoveInterpreterGroupWhenNoteIsRemoved() { + InterpreterOption interpreterOption = new InterpreterOption(); + interpreterOption.setPerUser(InterpreterOption.ISOLATED); + InterpreterSetting interpreterSetting = new InterpreterSetting("", "", "", new ArrayList(), new Properties(), new ArrayList(), interpreterOption, "", null); + + interpreterSetting.setInterpreterGroupFactory(new InterpreterGroupFactory() { + @Override + public InterpreterGroup createInterpreterGroup(String interpreterGroupId, + InterpreterOption option) { + return new InterpreterGroup(interpreterGroupId); + } + }); + + Interpreter mockInterpreter1 = mock(RemoteInterpreter.class); + List interpreterList1 = new ArrayList<>(); + interpreterList1.add(mockInterpreter1); + InterpreterGroup interpreterGroup = interpreterSetting.getInterpreterGroup("user1", "note1"); + interpreterGroup.put(interpreterSetting.getInterpreterSessionKey("user1", "note1"), interpreterList1); + + assertEquals(1, interpreterSetting.getAllInterpreterGroups().size()); + assertEquals(1, interpreterSetting.getInterpreterGroup("user1", "note1").size()); + + // This method will be called when remove note + interpreterSetting.closeAndRemoveInterpreterGroup("note1",""); + assertEquals(1, interpreterSetting.getAllInterpreterGroups().size()); + // Be careful that getInterpreterGroup makes interpreterGroup if it doesn't exist + assertEquals(1, interpreterSetting.getInterpreterGroup("user1","note1").size()); + } }