This is an automated email from the ASF dual-hosted git repository.
ParkGyeongTae pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/zeppelin.git
The following commit(s) were added to refs/heads/master by this push:
new 0c4b30f863 [ZEPPELIN-6345] Fire NoteRemove event for each note when
deleting a folder
0c4b30f863 is described below
commit 0c4b30f8633f9d2809084d06c168703599aa21ae
Author: HwangRock <[email protected]>
AuthorDate: Mon Jul 13 23:28:27 2026 +0900
[ZEPPELIN-6345] Fire NoteRemove event for each note when deleting a folder
### What is this PR for?
Deleting a folder left its notes searchable after removal.
`removeFolder(String, AuthenticationInfo)` in `Notebook` called
`noteManager.removeFolder` first, which detached the notes from the tree, so
the following `removeNote(noteId)` loaded a `null` note and skipped
`fireNoteRemoveEvent`. `SearchService.deleteNoteIndex` was therefore never
invoked and the Lucene search index kept stale documents for the deleted notes,
so they still showed up in search results after emptying the trash [...]
This PR loads the notes non-destructively via a new
`NoteManager.getNoteInfoRecursively`, runs the same per-note cleanup as
`removeNote(Note, AuthenticationInfo)` (`setRemoved`, `removeNoteAuth`,
`fireNoteRemoveEvent`) for each note, and only then deletes the folder. This
also removes the pre-existing `NotebookRepo.remove is called twice` TODO, since
the repo remove now happens once.
### What type of PR is it?
Bug Fix
### What is the Jira issue?
* https://issues.apache.org/jira/browse/ZEPPELIN-6345
### Screenshots (if appropriate)
https://github.com/user-attachments/assets/3bce191f-6f12-443a-bd75-2a0ada28712c
### How should this be tested?
* Added `NotebookTest#testRemoveFolderFiresNoteRemoveEventForEachNote`:
creates two notes under `/folder1`, registers a `NoteEventListener` counting
`onNoteRemove`, calls `removeFolder("/folder1", ...)`, and asserts the event
fired once per note. Fails before the change (count `0`), passes after.
* Manual: create notes, move them to trash, empty the trash, then search
for a deleted note. Before the fix it still appears in results; after the fix
it is gone.
### Questions:
* Does the license files need to update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No
Closes #5292 from HwangRock/ZEPPELIN-6345-pr.
Signed-off-by: ParkGyeongTae <[email protected]>
---
.../org/apache/zeppelin/notebook/NoteManager.java | 11 ++++++
.../org/apache/zeppelin/notebook/Notebook.java | 17 +++++++--
.../org/apache/zeppelin/notebook/NotebookTest.java | 42 ++++++++++++++++++++++
3 files changed, 67 insertions(+), 3 deletions(-)
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 e0469a825a..31bb94de32 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 class NoteManager {
}
}
+ /**
+ * Returns the NoteInfo of all notes under the given folder, without
removing them.
+ *
+ * @param folderPath
+ * @return
+ * @throws IOException
+ */
+ public List<NoteInfo> 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 713cc79322..83f0032822 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 class Notebook {
public void removeFolder(String folderPath, AuthenticationInfo subject)
throws IOException {
LOGGER.info("Remove folder {}", folderPath);
- // TODO(zjffdu) NotebookRepo.remove is called twice here
- List<NoteInfo> 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<NoteInfo> 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 c0bdcc116d..39e6ec70e3 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 @@ class NotebookTest extends AbstractInterpreterTest
implements ParagraphJobListen
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);
