Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved progress indication for fulltext-index operations #7981

Merged
merged 7 commits into from
Aug 19, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,8 @@ private void rebuildIndex() {
return;
}
try {
currentLibraryTab.get().getIndexingTaskManager().createIndex(PdfIndexer.of(databaseContext, filePreferences), databaseContext.getDatabase(), databaseContext);
currentLibraryTab.get().getIndexingTaskManager().createIndex(PdfIndexer.of(databaseContext, filePreferences));
currentLibraryTab.get().getIndexingTaskManager().addToIndex(PdfIndexer.of(databaseContext, filePreferences), databaseContext);
} catch (IOException e) {
dialogService.notify(Localization.lang("Failed to access fulltext search index"));
LOGGER.error("Failed to access fulltext search index", e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import java.util.Queue;

import org.jabref.gui.util.BackgroundTask;
import org.jabref.gui.util.DefaultTaskExecutor;
import org.jabref.gui.util.TaskExecutor;
import org.jabref.logic.l10n.Localization;
import org.jabref.model.database.BibDatabase;
Expand Down Expand Up @@ -39,27 +40,30 @@ protected Void call() throws Exception {
private void enqueueTask(BackgroundTask<Void> task) {
task.onFinished(() -> {
this.progressProperty().unbind();
this.messageProperty().unbind();
this.updateProgress(1, 1);
taskQueue.poll(); // This is the task that just finished
if (!taskQueue.isEmpty()) {
BackgroundTask<Void> nextTask = taskQueue.poll();
nextTask.executeWith(taskExecutor);
this.progressProperty().bind(nextTask.progressProperty());
this.messageProperty().bind(nextTask.messageProperty());
btut marked this conversation as resolved.
Show resolved Hide resolved
}
});
taskQueue.add(task);
if (taskQueue.size() == 1) {
task.executeWith(taskExecutor);
this.progressProperty().bind(task.progressProperty());
this.messageProperty().bind(task.messageProperty());
}
}

public void createIndex(PdfIndexer indexer, BibDatabase database, BibDatabaseContext context) {
public void createIndex(PdfIndexer indexer) {
enqueueTask(new BackgroundTask<Void>() {
@Override
protected Void call() throws Exception {
this.updateProgress(-1, 1);
indexer.createIndex(database, context);
indexer.createIndex();
return null;
}
});
Expand All @@ -70,7 +74,21 @@ public void addToIndex(PdfIndexer indexer, BibDatabaseContext databaseContext) {
@Override
protected Void call() throws Exception {
this.updateProgress(-1, 1);
indexer.addToIndex(databaseContext);
final int numFiles = databaseContext.getEntries().stream().map(BibEntry::getFiles).map(List::size).mapToInt(Integer::intValue).sum();

int progressCounter = 0;
this.updateProgress(0, numFiles);
this.updateMessage(Localization.lang("%0 of %1 files added to the index", 0, numFiles));

for (BibEntry entry : databaseContext.getEntries()) {
for (LinkedFile file : entry.getFiles()) {
indexer.addToIndex(entry, file, databaseContext);
progressCounter++;
final int lambdaProgressCounter = progressCounter;
this.updateProgress(lambdaProgressCounter, numFiles);
this.updateMessage(Localization.lang("%0 of %1 files added to the index", lambdaProgressCounter, numFiles));
}
}
return null;
}
});
Expand All @@ -81,7 +99,19 @@ public void addToIndex(PdfIndexer indexer, BibEntry entry, BibDatabaseContext da
@Override
protected Void call() throws Exception {
this.updateProgress(-1, 1);
indexer.addToIndex(entry, databaseContext);
final int numFiles = entry.getFiles().size();
btut marked this conversation as resolved.
Show resolved Hide resolved

int progressCounter = 0;
this.updateProgress(0, numFiles);
this.updateMessage(Localization.lang("%0 of %1 files added to the index", 0, numFiles));

for (LinkedFile file : entry.getFiles()) {
indexer.addToIndex(entry, file, databaseContext);
progressCounter++;
final int lambdaProgressCounter = progressCounter;
this.updateProgress(lambdaProgressCounter, numFiles);
this.updateMessage(Localization.lang("%0 of %1 files added to the index", lambdaProgressCounter, numFiles));
}
return null;
}
});
Expand All @@ -91,8 +121,11 @@ public void addToIndex(PdfIndexer indexer, BibEntry entry, List<LinkedFile> link
enqueueTask(new BackgroundTask<Void>() {
@Override
protected Void call() throws Exception {
this.updateProgress(-1, 1);
this.updateProgress(0, 1);
this.updateMessage(Localization.lang("%0 of %1 files added to the index", 0, 1));
indexer.addToIndex(entry, linkedFiles, databaseContext);
this.updateProgress(1, 1);
this.updateMessage(Localization.lang("%0 of %1 files added to the index", 1, 1));
return null;
}
});
Expand Down Expand Up @@ -121,6 +154,6 @@ protected Void call() throws Exception {
}

public void updateDatabaseName(String name) {
this.updateMessage(name);
this.titleProperty().set(Localization.lang("Indexing for %0", name));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -58,22 +58,15 @@ public static PdfIndexer of(BibDatabaseContext databaseContext, FilePreferences
/**
* Adds all PDF files linked to an entry in the database to new Lucene search index. Any previous state of the
* Lucene search index will be deleted!
*
* @param database a bibtex database to link the pdf files to
*/
public void createIndex(BibDatabase database, BibDatabaseContext context) {
this.databaseContext = context;
final ObservableList<BibEntry> entries = database.getEntries();

public void createIndex() {
// Create new index by creating IndexWriter but not writing anything.
try {
IndexWriter indexWriter = new IndexWriter(directoryToIndex, new IndexWriterConfig(new EnglishStemAnalyzer()).setOpenMode(IndexWriterConfig.OpenMode.CREATE));
indexWriter.close();
} catch (IOException e) {
LOGGER.warn("Could not create new Index!", e);
}
// Re-use existing facilities for writing the actual entries
entries.stream().filter(entry -> !entry.getFiles().isEmpty()).forEach(this::writeToIndex);
}

public void addToIndex(BibDatabaseContext databaseContext) {
Expand Down
2 changes: 2 additions & 0 deletions src/main/resources/l10n/JabRef_en.properties
Original file line number Diff line number Diff line change
Expand Up @@ -443,6 +443,8 @@ Independent\ group\:\ When\ selected,\ view\ only\ this\ group's\ entries=Indepe
I\ Agree=I Agree

Indexing\ pdf\ files=Indexing pdf files
Indexing\ for\ %0=Indexing for %0
%0\ of\ %1\ linked\ files\ added\ to\ the\ index=%0 of %1 files added to the index

Invalid\ citation\ key=Invalid citation key

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ public void setUp(@TempDir Path indexDir) throws IOException {
when(context.getFileDirectories(Mockito.any())).thenReturn(Collections.singletonList(Path.of("src/test/resources/pdfs")));
when(context.getFulltextIndexPath()).thenReturn(indexDir);
when(context.getDatabase()).thenReturn(database);
when(context.getEntries()).thenReturn(database.getEntries());
this.indexer = PdfIndexer.of(context, filePreferences);
}

Expand All @@ -52,7 +53,8 @@ public void exampleThesisIndex() throws IOException {
database.insertEntry(entry);

// when
indexer.createIndex(database, context);
indexer.createIndex();
indexer.addToIndex(context);

// then
try (IndexReader reader = DirectoryReader.open(new NIOFSDirectory(context.getFulltextIndexPath()))) {
Expand All @@ -69,7 +71,8 @@ public void exampleThesisIndexWithKey() throws IOException {
database.insertEntry(entry);

// when
indexer.createIndex(database, context);
indexer.createIndex();
indexer.addToIndex(context);

// then
try (IndexReader reader = DirectoryReader.open(new NIOFSDirectory(context.getFulltextIndexPath()))) {
Expand All @@ -86,7 +89,8 @@ public void metaDataIndex() throws IOException {
database.insertEntry(entry);

// when
indexer.createIndex(database, context);
indexer.createIndex();
indexer.addToIndex(context);

// then
try (IndexReader reader = DirectoryReader.open(new NIOFSDirectory(context.getFulltextIndexPath()))) {
Expand All @@ -102,7 +106,8 @@ public void testFlushIndex() throws IOException {
entry.setFiles(Collections.singletonList(new LinkedFile("Example Thesis", "thesis-example.pdf", StandardFileType.PDF.getName())));
database.insertEntry(entry);

indexer.createIndex(database, context);
indexer.createIndex();
indexer.addToIndex(context);
// index actually exists
try (IndexReader reader = DirectoryReader.open(new NIOFSDirectory(context.getFulltextIndexPath()))) {
assertEquals(1, reader.numDocs());
Expand All @@ -124,7 +129,8 @@ public void exampleThesisIndexAppendMetaData() throws IOException {
exampleThesis.setCitationKey("ExampleThesis2017");
exampleThesis.setFiles(Collections.singletonList(new LinkedFile("Example Thesis", "thesis-example.pdf", StandardFileType.PDF.getName())));
database.insertEntry(exampleThesis);
indexer.createIndex(database, context);
indexer.createIndex();
indexer.addToIndex(context);

// index with first entry
try (IndexReader reader = DirectoryReader.open(new NIOFSDirectory(context.getFulltextIndexPath()))) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ public void setUp(@TempDir Path indexDir) throws IOException {
when(context.getFileDirectories(Mockito.any())).thenReturn(Collections.singletonList(Path.of("src/test/resources/pdfs")));
when(context.getFulltextIndexPath()).thenReturn(indexDir);
when(context.getDatabase()).thenReturn(database);
when(context.getEntries()).thenReturn(database.getEntries());
BibEntry examplePdf = new BibEntry(StandardEntryType.Article);
examplePdf.setFiles(Collections.singletonList(new LinkedFile("Example Entry", "example.pdf", StandardFileType.PDF.getName())));
database.insertEntry(examplePdf);
Expand All @@ -55,7 +56,8 @@ public void setUp(@TempDir Path indexDir) throws IOException {
PdfIndexer indexer = PdfIndexer.of(context, filePreferences);
search = PdfSearcher.of(context);

indexer.createIndex(database, context);
indexer.createIndex();
indexer.addToIndex(context);
}

@Test
Expand Down