Skip to content

Commit e915a69

Browse files
bzzbzz
committed
ZEPPELIN-501: update notebooks on save + tests
1 parent 3f20904 commit e915a69

File tree

4 files changed

+135
-48
lines changed

4 files changed

+135
-48
lines changed

‎zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Note.java‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
import org.apache.zeppelin.scheduler.Job;
3939
import org.apache.zeppelin.scheduler.Job.Status;
4040
import org.apache.zeppelin.scheduler.JobListener;
41+
import org.apache.zeppelin.search.SearchService;
4142
import org.slf4j.Logger;
4243
import org.slf4j.LoggerFactory;
4344

@@ -60,6 +61,7 @@ public class Note implements Serializable, JobListener {
6061
private transient NoteInterpreterLoader replLoader;
6162
private transient JobListenerFactory jobListenerFactory;
6263
private transient NotebookRepo repo;
64+
private transient SearchService index;
6365

6466
/**
6567
* note configurations.
@@ -78,10 +80,12 @@ public class Note implements Serializable, JobListener {
7880

7981
public Note() {}
8082

81-
public Note(NotebookRepo repo, NoteInterpreterLoader replLoader, JobListenerFactory jlFactory) {
83+
public Note(NotebookRepo repo, NoteInterpreterLoader replLoader,
84+
JobListenerFactory jlFactory, SearchService noteIndex) {
8285
this.repo = repo;
8386
this.replLoader = replLoader;
8487
this.jobListenerFactory = jlFactory;
88+
this.index = noteIndex;
8589
generateId();
8690
}
8791

@@ -293,7 +297,7 @@ public Paragraph getLastParagraph() {
293297
return paragraphs.get(paragraphs.size() - 1);
294298
}
295299
}
296-
300+
297301
public List<Map<String, String>> generateParagraphsInfo (){
298302
List<Map<String, String>> paragraphsInfo = new LinkedList<>();
299303
synchronized (paragraphs) {
@@ -307,7 +311,7 @@ public List<Map<String, String>> generateParagraphsInfo (){
307311
}
308312
}
309313
return paragraphsInfo;
310-
}
314+
}
311315

312316
/**
313317
* Run all paragraphs sequentially.
@@ -373,6 +377,7 @@ private void snapshotAngularObjectRegistry() {
373377

374378
public void persist() throws IOException {
375379
snapshotAngularObjectRegistry();
380+
index.updateIndexDoc(this);
376381
repo.save(this);
377382
}
378383

‎zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Notebook.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ public Note createNote() throws IOException {
136136
*/
137137
public Note createNote(List<String> interpreterIds) throws IOException {
138138
NoteInterpreterLoader intpLoader = new NoteInterpreterLoader(replFactory);
139-
Note note = new Note(notebookRepo, intpLoader, jobListenerFactory);
139+
Note note = new Note(notebookRepo, intpLoader, jobListenerFactory, notebookIndex);
140140
intpLoader.setNoteId(note.id());
141141
synchronized (notes) {
142142
notes.put(note.id(), note);

‎zeppelin-zengine/src/main/java/org/apache/zeppelin/search/SearchService.java‎

Lines changed: 63 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -160,19 +160,42 @@ private List<Map<String, String>> doSearch(IndexSearcher searcher, Query query,
160160
}
161161
} catch (IOException | InvalidTokenOffsetsException e) {
162162
LOG.error("Exception on searching for {}", query, e);
163-
;
164163
}
165164
return matchingParagraphs;
166165
}
167166

167+
/**
168+
* Updates all documents in index for the given note:
169+
* - name
170+
* - all paragraphs
171+
*
172+
* @param note a Note to update index for
173+
* @throws IOException
174+
*/
168175
public void updateIndexDoc(Note note) throws IOException {
176+
updateIndexNoteName(note);
177+
for (Paragraph p: note.getParagraphs()) {
178+
updateIndexParagraph(note, p);
179+
}
180+
}
181+
182+
private void updateIndexNoteName(Note note) throws IOException {
169183
updateDoc(note.getId(), note.getName(), null);
170184
}
171185

172-
void updateIndexDoc(Note note, Paragraph p) throws IOException {
186+
private void updateIndexParagraph(Note note, Paragraph p) throws IOException {
173187
updateDoc(note.getId(), note.getName(), p);
174188
}
175189

190+
/**
191+
* Updates index for the given note: either note.name or a paragraph
192+
* If paragraph is <code>null</code> - updates only for the note.name
193+
*
194+
* @param noteId
195+
* @param noteName
196+
* @param p
197+
* @throws IOException
198+
*/
176199
private void updateDoc(String noteId, String noteName, Paragraph p) throws IOException {
177200
String id = formatId(noteId, p);
178201
Document doc = newDocument(id, noteName, p);
@@ -184,6 +207,44 @@ private void updateDoc(String noteId, String noteName, Paragraph p) throws IOExc
184207
}
185208
}
186209

210+
/**
211+
* If paragraph is not null, id is <noteId>/paragraphs/<paragraphId>,
212+
* otherwise it's just <noteId>.
213+
*/
214+
static String formatId(String noteId, Paragraph p) {
215+
String id = noteId;
216+
if (null != p) {
217+
id = Joiner.on('/').join(id, "paragraphs", p.getId());
218+
}
219+
return id;
220+
}
221+
222+
/**
223+
* If paragraph is not null, indexes code in the paragraph,
224+
* otherwise indexes the notebook name.
225+
*
226+
* @param id id of the document, different for Note name and paragraph
227+
* @param noteName name of the note
228+
* @param p paragraph
229+
* @return
230+
*/
231+
private Document newDocument(String id, String noteName, Paragraph p) {
232+
Document doc = new Document();
233+
234+
Field pathField = new StringField(ID_FIELD, id, Field.Store.YES);
235+
doc.add(pathField);
236+
doc.add(new StringField("title", noteName, Field.Store.YES));
237+
238+
if (null != p) {
239+
doc.add(new TextField(SEARCH_FIELD, p.getText(), Field.Store.YES));
240+
Date date = p.getDateStarted() != null ? p.getDateStarted() : p.getDateCreated();
241+
doc.add(new LongField("modified", date.getTime(), Field.Store.NO));
242+
} else {
243+
doc.add(new TextField(SEARCH_FIELD, noteName, Field.Store.YES));
244+
}
245+
return doc;
246+
}
247+
187248
/**
188249
* Indexes full collection of notes: all the paragraphs + Note names
189250
*
@@ -277,43 +338,4 @@ private void indexDoc(IndexWriter w, String noteId, String noteName, Paragraph p
277338
w.addDocument(doc);
278339
}
279340

280-
/**
281-
* If paragraph is not null, indexes code in the paragraph,
282-
* otherwise indexes the notebook name.
283-
*
284-
* @param id id of the document, different for Note name and paragraph
285-
* @param noteName name of the note
286-
* @param p paragraph
287-
* @return
288-
*/
289-
private Document newDocument(String id, String noteName, Paragraph p) {
290-
Document doc = new Document();
291-
292-
Field pathField = new StringField(ID_FIELD, id, Field.Store.YES);
293-
doc.add(pathField);
294-
doc.add(new StringField("title", noteName, Field.Store.YES));
295-
296-
if (null != p) {
297-
doc.add(new TextField(SEARCH_FIELD, p.getText(), Field.Store.YES));
298-
Date date = p.getDateStarted() != null ? p.getDateStarted() : p.getDateCreated();
299-
doc.add(new LongField("modified", date.getTime(), Field.Store.NO));
300-
} else {
301-
doc.add(new TextField(SEARCH_FIELD, noteName, Field.Store.YES));
302-
}
303-
return doc;
304-
}
305-
306-
/**
307-
* If paragraph is not null, id is <noteId>/paragraphs/<paragraphId>,
308-
* otherwise it's just <noteId>.
309-
*/
310-
static String formatId(String noteId, Paragraph p) {
311-
String id = noteId;
312-
if (null != p) {
313-
id = Joiner.on('/').join(id, "paragraphs", p.getId());
314-
}
315-
return id;
316-
}
317-
318-
319341
}

‎zeppelin-zengine/src/test/java/org/apache/zeppelin/search/SearchServiceTest.java‎

Lines changed: 63 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,22 +17,40 @@
1717
package org.apache.zeppelin.search;
1818

1919
import static com.google.common.truth.Truth.assertThat;
20+
import static org.mockito.Mockito.*;
2021
import static org.apache.zeppelin.search.SearchService.formatId;
2122

2223
import java.io.IOException;
2324
import java.util.Arrays;
2425
import java.util.List;
2526
import java.util.Map;
2627

28+
import org.apache.zeppelin.interpreter.InterpreterSetting;
2729
import org.apache.zeppelin.notebook.Note;
30+
import org.apache.zeppelin.notebook.NoteInterpreterLoader;
2831
import org.apache.zeppelin.notebook.Paragraph;
32+
import org.apache.zeppelin.notebook.repo.NotebookRepo;
2933
import org.junit.After;
3034
import org.junit.Before;
35+
import org.junit.BeforeClass;
3136
import org.junit.Test;
3237

38+
import com.google.common.collect.ImmutableList;
39+
3340
public class SearchServiceTest {
3441

35-
SearchService notebookIndex;
42+
private static NoteInterpreterLoader replLoaderMock;
43+
private static NotebookRepo notebookRepoMock;
44+
private SearchService notebookIndex;
45+
46+
@BeforeClass
47+
public static void beforeStartUp() {
48+
notebookRepoMock = mock(NotebookRepo.class);
49+
replLoaderMock = mock(NoteInterpreterLoader.class);
50+
51+
when(replLoaderMock.getInterpreterSettings())
52+
.thenReturn(ImmutableList.<InterpreterSetting>of());
53+
}
3654

3755
@Before
3856
public void startUp() {
@@ -106,7 +124,7 @@ public void canNotSearchBeforeIndexing() {
106124
//when
107125
Paragraph p2 = note2.getLastParagraph();
108126
p2.setText("test indeed");
109-
notebookIndex.updateIndexDoc(note2, p2);
127+
notebookIndex.updateIndexDoc(note2);
110128

111129
//then
112130
List<Map<String, String>> results = notebookIndex.query("all");
@@ -135,6 +153,48 @@ public void canNotSearchBeforeIndexing() {
135153
assertThat(results.size()).isEqualTo(1);
136154
}
137155

156+
@Test public void indexParagraphUpdatedOnNoteSave() throws IOException {
157+
//given: total 2 notebooks, 3 paragraphs
158+
Note note1 = newNoteWithParapgraph("Notebook1", "test");
159+
Note note2 = newNoteWithParapgraphs("Notebook2", "not test", "not test at all");
160+
notebookIndex.addIndexDocs(Arrays.asList(note1, note2));
161+
assertThat(resultForQuery("test").size()).isEqualTo(3);
162+
163+
//when
164+
Paragraph p1 = note1.getLastParagraph();
165+
p1.setText("no no no");
166+
note1.persist();
167+
168+
//then
169+
assertThat(resultForQuery("Notebook1").size()).isEqualTo(1);
170+
171+
List<Map<String, String>> results = resultForQuery("test");
172+
assertThat(results).isNotEmpty();
173+
assertThat(results.size()).isEqualTo(2);
174+
175+
//does not include Notebook1's paragraph any more
176+
for (Map<String, String> result: results) {
177+
assertThat(result.get("id").startsWith(note1.getId())).isFalse();;
178+
}
179+
}
180+
181+
@Test public void indexNoteNameUpdatedOnNoteSave() throws IOException {
182+
//given: total 2 notebooks, 3 paragraphs
183+
Note note1 = newNoteWithParapgraph("Notebook1", "test");
184+
Note note2 = newNoteWithParapgraphs("Notebook2", "not test", "not test at all");
185+
notebookIndex.addIndexDocs(Arrays.asList(note1, note2));
186+
assertThat(resultForQuery("test").size()).isEqualTo(3);
187+
188+
//when
189+
note1.setName("NotebookN");
190+
note1.persist();
191+
192+
//then
193+
assertThat(resultForQuery("Notebook1")).isEmpty();
194+
assertThat(resultForQuery("NotebookN")).isNotEmpty();
195+
assertThat(resultForQuery("NotebookN").size()).isEqualTo(1);
196+
}
197+
138198
private List<Map<String, String>> resultForQuery(String q) {
139199
return notebookIndex.query(q);
140200
}
@@ -172,7 +232,7 @@ private Paragraph addParagraphWithText(Note note, String text) {
172232
}
173233

174234
private Note newNote(String name) {
175-
Note note = new Note(null, null, null);
235+
Note note = new Note(notebookRepoMock, replLoaderMock, null, notebookIndex);
176236
note.setName(name);
177237
return note;
178238
}

0 commit comments

Comments
 (0)