From 9bc2a9566f7df3f99f9f64d2c98880975d11b398 Mon Sep 17 00:00:00 2001 From: dev-donghwan Date: Sun, 16 Aug 2026 03:06:46 +0900 Subject: [PATCH] [ZEPPELIN-6414] Apply the authorization filter before the search cutoff The REST layer asked the search service for results and then removed the ones the caller may not read. Both search services cut their results down to twenty before returning them, so the removal ran on an already shortened list and a caller with access to few notes was served fewer results than it is allowed to see, down to none at all. In EmbeddingSearch the notes the caller cannot read also fed the table boost, so they moved the ranking of the results that were kept. The read check now travels with the query as a predicate over the note id, and every implementation applies it before it cuts anything: EmbeddingSearch drops the entries while it scores them, so neither the table weights nor the cutoff see them, and LuceneSearch walks the hits in score order until it has collected a full page of readable ones. LuceneSearch reads only the id field to decide on a hit and loads the rest of the document for the hits that are kept. query(String) is gone, so there is no longer a way to search without saying who is asking. --- .../apache/zeppelin/rest/NotebookRestApi.java | 17 +-- .../zeppelin/search/EmbeddingSearch.java | 25 ++- .../apache/zeppelin/search/LuceneSearch.java | 142 +++++++++++------- .../zeppelin/search/NoSearchService.java | 3 +- .../apache/zeppelin/search/SearchService.java | 6 +- .../zeppelin/search/EmbeddingSearchTest.java | 71 +++++++-- .../zeppelin/search/LuceneSearchTest.java | 74 ++++++++- 7 files changed, 244 insertions(+), 94 deletions(-) diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/rest/NotebookRestApi.java b/zeppelin-server/src/main/java/org/apache/zeppelin/rest/NotebookRestApi.java index 192cd5056c5..85f5b07cc36 100644 --- a/zeppelin-server/src/main/java/org/apache/zeppelin/rest/NotebookRestApi.java +++ b/zeppelin-server/src/main/java/org/apache/zeppelin/rest/NotebookRestApi.java @@ -1215,18 +1215,11 @@ public Response search(@QueryParam("q") String queryTerm) { HashSet userAndRoles = new HashSet<>(); userAndRoles.add(principal); userAndRoles.addAll(roles); - List> notesFound = noteSearchService.query(queryTerm); - for (int i = 0; i < notesFound.size(); i++) { - String[] ids = notesFound.get(i).get("id").split("/", 2); - String noteId = ids[0]; - if (!authorizationService.isOwner(noteId, userAndRoles) && - !authorizationService.isReader(noteId, userAndRoles) && - !authorizationService.isWriter(noteId, userAndRoles) && - !authorizationService.isRunner(noteId, userAndRoles)) { - notesFound.remove(i); - i--; - } - } + // isReader() already covers owners, writers and runners. Handing the check to the + // search service keeps it in front of the cutoff, so a caller with access to few notes + // is still served a full result set of the notes it may read. + List> notesFound = noteSearchService.query(queryTerm, + noteId -> authorizationService.isReader(noteId, userAndRoles)); LOGGER.info("{} notes found", notesFound.size()); return new JsonResponse<>(Status.OK, notesFound).build(); } diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/search/EmbeddingSearch.java b/zeppelin-server/src/main/java/org/apache/zeppelin/search/EmbeddingSearch.java index 2d60d3fc286..68e35fdc88b 100644 --- a/zeppelin-server/src/main/java/org/apache/zeppelin/search/EmbeddingSearch.java +++ b/zeppelin-server/src/main/java/org/apache/zeppelin/search/EmbeddingSearch.java @@ -42,6 +42,7 @@ import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.function.Predicate; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.Executors; @@ -509,15 +510,11 @@ private String buildParagraphText(String noteName, Paragraph p) { // ---- SearchService implementation ---- @Override - // TODO(ZEPPELIN-6414): Accept user/roles (or a readability Predicate) and apply the auth - // filter before Phase-1 table collection and before the top-K cutoff. Currently the REST - // layer filters after truncation, which can hide results the caller is authorized for and - // lets inaccessible notes contaminate the table-boost ranking. Requires a SearchService - // interface change that also affects LuceneSearch. - public List> query(String queryStr) { + public List> query(String queryStr, Predicate readable) { if (StringUtils.isBlank(queryStr) || index.isEmpty()) { return Collections.emptyList(); } + Map readableNotes = new HashMap<>(); float[] queryEmbedding = embed(queryStr); String queryLower = queryStr.toLowerCase(Locale.ROOT); @@ -527,6 +524,12 @@ public List> query(String queryStr) { indexLock.readLock().lock(); try { for (Map.Entry entry : index.entrySet()) { + // Dropping the entries here keeps them out of the table weights below and out of + // the cutoff, so the caller is served its own top results and not what is left of + // everyone's top results. + if (!readableNotes.computeIfAbsent(noteIdOf(entry.getKey()), readable::test)) { + continue; + } float sim = cosineSimilarity(queryEmbedding, entry.getValue().embedding); IndexEntry ie = entry.getValue(); if (ie.text != null && ie.text.toLowerCase(Locale.ROOT).contains(queryLower)) { @@ -612,6 +615,16 @@ public List> query(String queryStr) { return results; } + /** + * The key of an indexed entry is either a noteId or a noteId followed by the paragraph. + * + * @see #formatId(String, Paragraph) + */ + private static String noteIdOf(String docId) { + int separator = docId.indexOf('/'); + return separator < 0 ? docId : docId.substring(0, separator); + } + @Override public void addNoteIndex(String noteId) { try { diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/search/LuceneSearch.java b/zeppelin-server/src/main/java/org/apache/zeppelin/search/LuceneSearch.java index 904069fb332..d11dd50acdb 100644 --- a/zeppelin-server/src/main/java/org/apache/zeppelin/search/LuceneSearch.java +++ b/zeppelin-server/src/main/java/org/apache/zeppelin/search/LuceneSearch.java @@ -21,10 +21,13 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.util.ArrayList; +import java.util.HashMap; import java.util.Collections; import java.util.Date; import java.util.List; import java.util.Map; +import java.util.Set; +import java.util.function.Predicate; import javax.annotation.PreDestroy; import jakarta.inject.Inject; @@ -76,6 +79,12 @@ public class LuceneSearch extends SearchService { private static final String SEARCH_FIELD_TITLE = "header"; private static final String PARAGRAPH = "paragraph"; private static final String ID_FIELD = "id"; + /** Number of results a query returns at most. */ + private static final int MAX_RESULTS = 20; + /** Number of hits pulled from the index per round while looking for readable ones. */ + private static final int HIT_BATCH_SIZE = 20; + /** Only the id is needed to tell whether the caller may read a hit. */ + private static final Set ID_FIELD_ONLY = Collections.singleton(ID_FIELD); private final Directory indexDirectory; private final IndexWriter indexWriter; @@ -113,7 +122,7 @@ public LuceneSearch(ZeppelinConfiguration zConf, Notebook notebook) throws IOExc * @see org.apache.zeppelin.search.Search#query(java.lang.String) */ @Override - public List> query(String queryStr) { + public List> query(String queryStr, Predicate readable) { if (null == indexDirectory) { throw new IllegalStateException( "Something went wrong on instance creation time, index dir is null"); @@ -134,7 +143,7 @@ public List> query(String queryStr) { SimpleHTMLFormatter htmlFormatter = new SimpleHTMLFormatter(); Highlighter highlighter = new Highlighter(htmlFormatter, new QueryScorer(query)); - result = doSearch(indexSearcher, query, analyzer, highlighter); + result = doSearch(indexSearcher, query, analyzer, highlighter, readable); } catch (IOException e) { LOGGER.error("Failed to open index dir {}, make sure indexing finished OK", indexDirectory, e); } catch (ParseException e) { @@ -144,64 +153,40 @@ public List> query(String queryStr) { } private List> doSearch( - IndexSearcher searcher, Query query, Analyzer analyzer, Highlighter highlighter) { + IndexSearcher searcher, Query query, Analyzer analyzer, Highlighter highlighter, + Predicate readable) { List> matchingParagraphs = new ArrayList<>(); - ScoreDoc[] hits; + Map readableNotes = new HashMap<>(); try { - hits = searcher.search(query, 20).scoreDocs; - for (int i = 0; i < hits.length; i++) { - LOGGER.debug("doc={} score={}", hits[i].doc, hits[i].score); - - int id = hits[i].doc; - Document doc = searcher.doc(id); - String path = doc.get(ID_FIELD); - if (path != null) { - LOGGER.debug( "{}. {}", (i + 1), path); - String title = doc.get("title"); - if (title != null) { - LOGGER.debug(" Title: {}", doc.get("title")); + // Walk the hits in score order and keep the ones the caller may read until the result + // set is full or the hits run out. Reading is checked here and not on the result set, + // because a cut that runs first would hide results the caller is allowed to see. + ScoreDoc lastHit = null; + while (matchingParagraphs.size() < MAX_RESULTS) { + ScoreDoc[] hits = (lastHit == null + ? searcher.search(query, HIT_BATCH_SIZE) + : searcher.searchAfter(lastHit, query, HIT_BATCH_SIZE)).scoreDocs; + if (hits.length == 0) { + break; + } + lastHit = hits[hits.length - 1]; + for (ScoreDoc hit : hits) { + if (matchingParagraphs.size() >= MAX_RESULTS) { + break; } - - String text = doc.get(SEARCH_FIELD_TEXT); - String header = doc.get(SEARCH_FIELD_TITLE); - String fragment = ""; - - if (text != null) { - TokenStream tokenStream = - TokenSources.getTokenStream( - searcher.getIndexReader(), id, SEARCH_FIELD_TEXT, analyzer); - TextFragment[] frags = highlighter.getBestTextFragments(tokenStream, text, true, 3); - LOGGER.debug(" {} fragments found for query '{}'", frags.length, query); - for (TextFragment frag : frags) { - if ((frag != null) && (frag.getScore() > 0)) { - LOGGER.debug(" Fragment: {}", frag); - } - } - fragment = (frags != null && frags.length > 0) ? frags[0].toString() : ""; + LOGGER.debug("doc={} score={}", hit.doc, hit.score); + // Read the id alone to decide on a hit. The rest of the document is only worth + // loading for the hits that end up in the result set. + String path = searcher.doc(hit.doc, ID_FIELD_ONLY).get(ID_FIELD); + if (path == null) { + LOGGER.info("No {} for this document", ID_FIELD); + continue; } - - if (header != null) { - TokenStream tokenTitle = - TokenSources.getTokenStream( - searcher.getIndexReader(), id, SEARCH_FIELD_TITLE, analyzer); - TextFragment[] frgTitle = highlighter.getBestTextFragments(tokenTitle, header, true, 3); - header = (frgTitle != null && frgTitle.length > 0) ? frgTitle[0].toString() : ""; - } else { - header = ""; + if (!readableNotes.computeIfAbsent(noteIdOf(path), readable::test)) { + continue; } matchingParagraphs.add( - ImmutableMap.builder() - .put("id", path) - .put("name", title) - .put("snippet", fragment) - .put("text", text) - .put("header", header) - .put("title", header) - .put("tables", "") - .put("output", "") - .build()); - } else { - LOGGER.info("{}. No {} for this document", i + 1, ID_FIELD); + toMatch(searcher, query, analyzer, highlighter, hit.doc, searcher.doc(hit.doc))); } } } catch (IOException | InvalidTokenOffsetsException e) { @@ -210,6 +195,55 @@ private List> doSearch( return matchingParagraphs; } + private Map toMatch(IndexSearcher searcher, Query query, Analyzer analyzer, + Highlighter highlighter, int docId, Document doc) + throws IOException, InvalidTokenOffsetsException { + String path = doc.get(ID_FIELD); + String title = doc.get("title"); + String text = doc.get(SEARCH_FIELD_TEXT); + String header = doc.get(SEARCH_FIELD_TITLE); + String fragment = ""; + + if (text != null) { + TokenStream tokenStream = + TokenSources.getTokenStream( + searcher.getIndexReader(), docId, SEARCH_FIELD_TEXT, analyzer); + TextFragment[] frags = highlighter.getBestTextFragments(tokenStream, text, true, 3); + LOGGER.debug(" {} fragments found for query '{}'", frags.length, query); + fragment = (frags != null && frags.length > 0) ? frags[0].toString() : ""; + } + + if (header != null) { + TokenStream tokenTitle = + TokenSources.getTokenStream( + searcher.getIndexReader(), docId, SEARCH_FIELD_TITLE, analyzer); + TextFragment[] frgTitle = highlighter.getBestTextFragments(tokenTitle, header, true, 3); + header = (frgTitle != null && frgTitle.length > 0) ? frgTitle[0].toString() : ""; + } else { + header = ""; + } + return ImmutableMap.builder() + .put("id", path) + .put("name", title) + .put("snippet", fragment) + .put("text", text) + .put("header", header) + .put("title", header) + .put("tables", "") + .put("output", "") + .build(); + } + + /** + * The id of an indexed document is either a noteId or a noteId followed by the paragraph. + * + * @see #formatId(String, Paragraph) + */ + private static String noteIdOf(String documentId) { + int separator = documentId.indexOf('/'); + return separator < 0 ? documentId : documentId.substring(0, separator); + } + /* (non-Javadoc) * @see org.apache.zeppelin.search.Search#updateIndexDoc(org.apache.zeppelin.notebook.Note) */ diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/search/NoSearchService.java b/zeppelin-server/src/main/java/org/apache/zeppelin/search/NoSearchService.java index 1d0d33d3040..10a0493236c 100644 --- a/zeppelin-server/src/main/java/org/apache/zeppelin/search/NoSearchService.java +++ b/zeppelin-server/src/main/java/org/apache/zeppelin/search/NoSearchService.java @@ -21,6 +21,7 @@ import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.function.Predicate; public class NoSearchService extends SearchService { @@ -30,7 +31,7 @@ public NoSearchService() { } @Override - public List> query(String queryStr) { + public List> query(String queryStr, Predicate readable) { return Collections.emptyList(); } diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/search/SearchService.java b/zeppelin-server/src/main/java/org/apache/zeppelin/search/SearchService.java index d36e4b1693b..62363867fe3 100644 --- a/zeppelin-server/src/main/java/org/apache/zeppelin/search/SearchService.java +++ b/zeppelin-server/src/main/java/org/apache/zeppelin/search/SearchService.java @@ -19,6 +19,7 @@ import java.io.IOException; import java.util.List; import java.util.Map; +import java.util.function.Predicate; import org.apache.zeppelin.notebook.NoteEventAsyncListener; import javax.annotation.PreDestroy; @@ -39,9 +40,12 @@ protected SearchService(String name) { * Full-text search in all the notes * * @param queryStr a query + * @param readable tells for a noteId whether the caller may read it. Entries the caller + * cannot read are dropped before the result set is cut down, so that the + * caller is not served fewer results than it is allowed to see. * @return A list of matching paragraphs (id, text, snippet w/ highlight) */ - public abstract List> query(String queryStr); + public abstract List> query(String queryStr, Predicate readable); /** * Updates note index for the given note, only update index of note meta info, diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/search/EmbeddingSearchTest.java b/zeppelin-server/src/test/java/org/apache/zeppelin/search/EmbeddingSearchTest.java index 902925eb7c3..e311cbd32b7 100644 --- a/zeppelin-server/src/test/java/org/apache/zeppelin/search/EmbeddingSearchTest.java +++ b/zeppelin-server/src/test/java/org/apache/zeppelin/search/EmbeddingSearchTest.java @@ -26,6 +26,9 @@ import java.io.File; import java.io.IOException; import java.nio.file.Files; +import java.util.HashSet; +import java.util.Set; +import java.util.function.Predicate; import java.util.List; import java.util.Map; @@ -127,7 +130,7 @@ void canIndexAndQuery() throws IOException, InterruptedException { drainSearchEvents(); // when — semantic search for a meaningful phrase - List> results = searchService.query("testing something"); + List> results = searchService.query("testing something", id -> true); // then assertFalse(results.isEmpty()); @@ -144,7 +147,7 @@ void canIndexAndQueryByNotebookName() throws IOException, InterruptedException { drainSearchEvents(); // when - List> results = searchService.query("Notebook1"); + List> results = searchService.query("Notebook1", id -> true); // then assertFalse(results.isEmpty()); @@ -159,7 +162,7 @@ void canIndexAndQueryByParagraphTitle() throws IOException, InterruptedException drainSearchEvents(); // when - List> results = searchService.query("testingTitleSearch"); + List> results = searchService.query("testingTitleSearch", id -> true); // then assertFalse(results.isEmpty()); @@ -178,7 +181,7 @@ void semanticSearchFindsRelatedConcepts() throws IOException, InterruptedExcepti drainSearchEvents(); // when — natural language query, no exact keyword match - List> results = searchService.query("yesterday's spending"); + List> results = searchService.query("yesterday's spending", id -> true); // then — should rank the spend query higher than the user count query assertFalse(results.isEmpty()); @@ -193,7 +196,7 @@ void indexKeyContract() throws IOException, InterruptedException { drainSearchEvents(); // when - List> results = searchService.query("test"); + List> results = searchService.query("test", id -> true); assertFalse(results.isEmpty()); // then — find the paragraph result (not the note-name result) @@ -214,7 +217,7 @@ void indexKeyContract() throws IOException, InterruptedException { void canNotSearchBeforeIndexing() { // given NO indexing was done // when - List> result = searchService.query("anything"); + List> result = searchService.query("anything", id -> true); // then assertTrue(result.isEmpty()); } @@ -235,7 +238,8 @@ void canIndexAndReIndex() throws IOException, InterruptedException { }); // then — updated content should now be findable - List> results = searchService.query("reindexing updated content"); + List> results = + searchService.query("reindexing updated content", id -> true); assertFalse(results.isEmpty()); } @@ -252,16 +256,16 @@ void canDeleteFromIndex() throws IOException, InterruptedException { String note2Id = newNoteWithParagraphs("Notebook2", "not test", "not test at all"); drainSearchEvents(); - assertFalse(searchService.query("Notebook2").isEmpty()); + assertFalse(searchService.query("Notebook2", id -> true).isEmpty()); // when searchService.deleteNoteIndex(note2Id); // then — no results should reference the deleted note's ID - boolean foundNote2After = searchService.query("not test at all").stream() + boolean foundNote2After = searchService.query("not test at all", id -> true).stream() .anyMatch(r -> r.get("id").startsWith(note2Id)); assertFalse(foundNote2After, "Note2 should be removed from index after deletion"); - assertFalse(searchService.query("Notebook1").isEmpty()); + assertFalse(searchService.query("Notebook1", id -> true).isEmpty()); } @Test @@ -282,7 +286,7 @@ void indexParagraphUpdatedOnNoteSave() throws IOException, InterruptedException drainSearchEvents(); // then — "Notebook1" note name should still be findable - assertFalse(searchService.query("Notebook1").isEmpty()); + assertFalse(searchService.query("Notebook1", id -> true).isEmpty()); } @Test @@ -302,7 +306,7 @@ void newParagraphIsLiveIndexed() throws IOException, InterruptedException { drainSearchEvents(); // then — the new paragraph should be findable by semantic query - List> results = searchService.query("lifetime value"); + List> results = searchService.query("lifetime value", id -> true); assertFalse(results.isEmpty(), "Newly added paragraph should be searchable"); boolean found = results.stream() .anyMatch(r -> r.get("text").contains("lifetime_value")); @@ -311,6 +315,49 @@ void newParagraphIsLiveIndexed() throws IOException, InterruptedException { // ---- Helper methods (same as LuceneSearchTest) ---- + @Test + void keepsReadableResultsThatTheCutWouldHide() throws IOException, InterruptedException { + // given: more notes than one result set holds. The notes the caller may not read match + // the query exactly, the ones it may read carry the same words but say more, so they + // score lower and fall outside the cut. + String queryStr = "quarterly revenue report"; + Set readableNoteIds = new HashSet<>(); + for (int i = 0; i < 25; i++) { + newNoteWithParagraph("Hidden" + i, queryStr); + } + for (int i = 0; i < 3; i++) { + readableNoteIds.add(newNoteWithParagraph("Mine" + i, queryStr + + " which also walks through unrelated kitchen recipes, holiday photographs and" + + " a long list of gardening tips that have nothing to do with the numbers")); + } + drainSearchEvents(); + Predicate readable = readableNoteIds::contains; + + // the fixture has to be one where cutting first actually loses results, otherwise this + // test would pass on any implementation + List> unfiltered = searchService.query(queryStr, id -> true); + long readableWithinCut = unfiltered.stream() + .filter(result -> readable.test(noteIdOf(result.get("id")))) + .count(); + assertTrue(readableWithinCut < readableNoteIds.size(), + "the readable notes have to fall outside the cut for this test to mean anything"); + + // when + List> results = searchService.query(queryStr, readable); + + // then: every readable note comes back, and nothing else does + assertEquals(readableNoteIds.size(), results.size()); + for (Map result : results) { + assertTrue(readable.test(noteIdOf(result.get("id"))), + "a result the caller may not read: " + result.get("id")); + } + } + + private static String noteIdOf(String documentId) { + int separator = documentId.indexOf('/'); + return separator < 0 ? documentId : documentId.substring(0, separator); + } + private String newNoteWithParagraph(String noteName, String parText) throws IOException { String noteId = newNote(noteName); notebook.processNote(noteId, note -> { diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/search/LuceneSearchTest.java b/zeppelin-server/src/test/java/org/apache/zeppelin/search/LuceneSearchTest.java index a27c2df3585..9a0d386085f 100644 --- a/zeppelin-server/src/test/java/org/apache/zeppelin/search/LuceneSearchTest.java +++ b/zeppelin-server/src/test/java/org/apache/zeppelin/search/LuceneSearchTest.java @@ -28,6 +28,9 @@ import java.io.File; import java.io.IOException; import java.nio.file.Files; +import java.util.HashSet; +import java.util.Set; +import java.util.function.Predicate; import java.util.List; import java.util.Map; @@ -98,7 +101,7 @@ void canIndexAndQuery() throws IOException, InterruptedException { drainSearchEvents(); // when - List> results = noteSearchService.query("all"); + List> results = noteSearchService.query("all", id -> true); // then assertFalse(results.isEmpty()); @@ -118,7 +121,7 @@ void canIndexAndQueryByNotebookName() throws IOException, InterruptedException { drainSearchEvents(); // when - List> results = noteSearchService.query("Notebook1"); + List> results = noteSearchService.query("Notebook1", id -> true); // then assertFalse(results.isEmpty()); @@ -134,7 +137,7 @@ void canIndexAndQueryByParagraphTitle() throws IOException, InterruptedException drainSearchEvents(); // when - List> results = noteSearchService.query("testingTitleSearch"); + List> results = noteSearchService.query("testingTitleSearch", id -> true); // then assertFalse(results.isEmpty()); @@ -168,7 +171,7 @@ void indexKeyContract() throws IOException, InterruptedException { void canNotSearchBeforeIndexing() { // given NO noteSearchService.index() was called // when - List> result = noteSearchService.query("anything"); + List> result = noteSearchService.query("anything", id -> true); // then assertTrue(result.isEmpty()); // assert logs were printed @@ -193,10 +196,10 @@ void canIndexAndReIndex() throws IOException, InterruptedException { }); // then - List> results = noteSearchService.query("all"); + List> results = noteSearchService.query("all", id -> true); assertTrue(results.isEmpty()); - results = noteSearchService.query("indeed"); + results = noteSearchService.query("indeed", id -> true); assertFalse(results.isEmpty()); } @@ -221,7 +224,7 @@ void canDeleteFromIndex() throws IOException, InterruptedException { noteSearchService.deleteNoteIndex(note2Id); // then - assertTrue(noteSearchService.query("all").isEmpty()); + assertTrue(noteSearchService.query("all", id -> true).isEmpty()); assertTrue(resultForQuery("Notebook2").isEmpty()); List> results = resultForQuery("test"); @@ -287,7 +290,7 @@ void indexNoteNameUpdatedOnNoteSave() throws IOException, InterruptedException { } private List> resultForQuery(String q) { - return noteSearchService.query(q); + return noteSearchService.query(q, id -> true); } /** @@ -297,6 +300,61 @@ private List> resultForQuery(String q) { * @param parText text of the paragraph * @return Note */ + @Test + void keepsReadableResultsThatTheCutWouldHide() throws IOException, InterruptedException { + // given: more notes than one result set holds, and only a few of them readable + Set readableNoteIds = new HashSet<>(); + for (int i = 0; i < 25; i++) { + newNoteWithParagraph("Hidden" + i, "shared search term"); + } + for (int i = 0; i < 3; i++) { + readableNoteIds.add(newNoteWithParagraph("Mine" + i, "shared search term")); + } + drainSearchEvents(); + Predicate readable = readableNoteIds::contains; + + // the fixture has to be one where cutting first actually loses results, otherwise this + // test would pass on any implementation + List> unfiltered = noteSearchService.query("shared search term", + id -> true); + long readableWithinCut = unfiltered.stream() + .filter(result -> readable.test(noteIdOf(result.get("id")))) + .count(); + assertTrue(readableWithinCut < readableNoteIds.size(), + "the readable notes have to fall outside the cut for this test to mean anything"); + + // when + List> results = noteSearchService.query("shared search term", readable); + + // then: every readable note comes back, and nothing else does + assertEquals(readableNoteIds.size(), results.size()); + for (Map result : results) { + assertTrue(readable.test(noteIdOf(result.get("id"))), + "a result the caller may not read: " + result.get("id")); + } + } + + @Test + void returnsNothingWhenTheCallerMayReadNothing() throws IOException, InterruptedException { + // given + for (int i = 0; i < 25; i++) { + newNoteWithParagraph("Hidden" + i, "shared search term"); + } + drainSearchEvents(); + + // when: the walk has to end on its own once the hits run out + List> results = noteSearchService.query("shared search term", + id -> false); + + // then + assertTrue(results.isEmpty(), () -> "unreadable results were returned: " + results); + } + + private static String noteIdOf(String documentId) { + int separator = documentId.indexOf('/'); + return separator < 0 ? documentId : documentId.substring(0, separator); + } + private String newNoteWithParagraph(String noteName, String parText) throws IOException { String note1Id = newNote(noteName); notebook.processNote(note1Id,