diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/notebook/NoteManager.java b/zeppelin-server/src/main/java/org/apache/zeppelin/notebook/NoteManager.java index e0469a825a9..31bb94de327 100644 --- a/zeppelin-server/src/main/java/org/apache/zeppelin/notebook/NoteManager.java +++ b/zeppelin-server/src/main/java/org/apache/zeppelin/notebook/NoteManager.java @@ -288,6 +288,17 @@ public void moveFolder(String folderPath, } } + /** + * Returns the NoteInfo of all notes under the given folder, without removing them. + * + * @param folderPath + * @return + * @throws IOException + */ + public List getNoteInfoRecursively(String folderPath) throws IOException { + return getFolder(folderPath).getNoteInfoRecursively(); + } + /** * Remove the folder from the tree and returns the affected NoteInfo under this folder. * diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/notebook/Notebook.java b/zeppelin-server/src/main/java/org/apache/zeppelin/notebook/Notebook.java index 713cc793223..83f0032822f 100644 --- a/zeppelin-server/src/main/java/org/apache/zeppelin/notebook/Notebook.java +++ b/zeppelin-server/src/main/java/org/apache/zeppelin/notebook/Notebook.java @@ -552,11 +552,22 @@ public void moveFolder(String folderPath, String newFolderPath, AuthenticationIn public void removeFolder(String folderPath, AuthenticationInfo subject) throws IOException { LOGGER.info("Remove folder {}", folderPath); - // TODO(zjffdu) NotebookRepo.remove is called twice here - List noteInfos = noteManager.removeFolder(folderPath, subject); + // Notes must be loaded and their remove listeners fired before the folder (and its + // underlying repo storage) is deleted, otherwise the note content is no longer + // available to run the same per-note cleanup as removeNote(String, AuthenticationInfo). + List noteInfos = noteManager.getNoteInfoRecursively(folderPath); for (NoteInfo noteInfo : noteInfos) { - removeNote(noteInfo.getId(), subject); + processNote(noteInfo.getId(), + note -> { + if (note != null) { + note.setRemoved(true); + authorizationService.removeNoteAuth(note.getId()); + fireNoteRemoveEvent(note, subject); + } + return null; + }); } + noteManager.removeFolder(folderPath, subject); } public void emptyTrash(AuthenticationInfo subject) throws IOException { diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java b/zeppelin-server/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java index c0bdcc116de..39e6ec70e38 100644 --- a/zeppelin-server/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java +++ b/zeppelin-server/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java @@ -1701,6 +1701,48 @@ public void onParagraphStatusChange(Paragraph p, Status status) { assertEquals(1, onParagraphRemove.get()); } + @Test + void testRemoveFolderFiresNoteRemoveEventForEachNote() throws IOException { + final AtomicInteger onNoteRemove = new AtomicInteger(0); + notebook.addNotebookEventListener(new NoteEventListener() { + @Override + public void onNoteRemove(Note note, AuthenticationInfo subject) { + onNoteRemove.incrementAndGet(); + } + + @Override + public void onNoteCreate(Note note, AuthenticationInfo subject) { + } + + @Override + public void onNoteUpdate(Note note, AuthenticationInfo subject) { + } + + @Override + public void onParagraphRemove(Paragraph p) { + } + + @Override + public void onParagraphCreate(Paragraph p) { + } + + @Override + public void onParagraphUpdate(Paragraph p) { + } + + @Override + public void onParagraphStatusChange(Paragraph p, Status status) { + } + }); + + notebook.createNote("/folder1/note1", anonymous); + notebook.createNote("/folder1/note2", anonymous); + + notebook.removeFolder("/folder1", anonymous); + + assertEquals(2, onNoteRemove.get()); + } + @Test void testGetAllNotes() throws Exception { String note1Id = notebook.createNote("note1", anonymous);