Skip to content

Commit

Permalink
Improved progress indication for fulltext-index operations (#7981)
Browse files Browse the repository at this point in the history
  • Loading branch information
@btut
btut authored Aug 19, 2021
1 parent 5f3abb5 commit 05a0ce9
Show file tree
Hide file tree
Showing 6 changed files with 77 additions and 93 deletions.
3 changes: 2 additions & 1 deletion src/main/java/org/jabref/gui/search/RebuildFulltextSearchIndexAction.java
View file
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
127 changes: 54 additions & 73 deletions src/main/java/org/jabref/logic/pdf/search/indexing/IndexingTaskManager.java
View file
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
package org.jabref.logic.pdf.search.indexing;

import java.util.LinkedList;
import java.util.List;
import java.util.Queue;
import java.util.concurrent.ConcurrentLinkedQueue;

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;
import org.jabref.model.database.BibDatabaseContext;
import org.jabref.model.entry.BibEntry;
import org.jabref.model.entry.LinkedFile;
Expand All @@ -17,110 +17,91 @@
*/
public class IndexingTaskManager extends BackgroundTask<Void> {

Queue<BackgroundTask<Void>> taskQueue = new LinkedList<>();
TaskExecutor taskExecutor;
private final Queue<Runnable> taskQueue = new ConcurrentLinkedQueue<>();
private TaskExecutor taskExecutor;
private int numOfIndexedFiles = 0;

private final Object lock = new Object();
private boolean isRunning = false;

public IndexingTaskManager(TaskExecutor taskExecutor) {
this.taskExecutor = taskExecutor;
showToUser(true);
// the task itself is a nop, but it's progress property will be updated by the child-tasks it creates that actually interact with the index
this.updateProgress(1, 1);
this.titleProperty().set(Localization.lang("Indexing pdf files"));
this.executeWith(taskExecutor);
DefaultTaskExecutor.runInJavaFXThread(() -> {
this.updateProgress(1, 1);
this.titleProperty().set(Localization.lang("Indexing pdf files"));
});
}

@Override
protected Void call() throws Exception {
// update index to make sure it is up to date
this.updateProgress(1, 1);
synchronized (lock) {
isRunning = true;
}
updateProgress();
while (!taskQueue.isEmpty()) {
taskQueue.poll().run();
numOfIndexedFiles++;
updateProgress();
}
synchronized (lock) {
isRunning = false;
}
return null;
}

private void enqueueTask(BackgroundTask<Void> task) {
task.onFinished(() -> {
this.progressProperty().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());
}
private void updateProgress() {
DefaultTaskExecutor.runInJavaFXThread(() -> {
updateMessage(Localization.lang("%0 of %1 linked files added to the index", numOfIndexedFiles, numOfIndexedFiles + taskQueue.size()));
updateProgress(numOfIndexedFiles, numOfIndexedFiles + taskQueue.size());
});
taskQueue.add(task);
if (taskQueue.size() == 1) {
task.executeWith(taskExecutor);
this.progressProperty().bind(task.progressProperty());
}
}

public void createIndex(PdfIndexer indexer, BibDatabase database, BibDatabaseContext context) {
enqueueTask(new BackgroundTask<Void>() {
@Override
protected Void call() throws Exception {
this.updateProgress(-1, 1);
indexer.createIndex(database, context);
return null;
private void enqueueTask(Runnable indexingTask) {
taskQueue.add(indexingTask);
// What if already running?
synchronized (lock) {
if (!isRunning) {
isRunning = true;
this.executeWith(taskExecutor);
showToUser(false);
}
});
}
}

public void createIndex(PdfIndexer indexer) {
enqueueTask(() -> indexer.createIndex());
}

public void addToIndex(PdfIndexer indexer, BibDatabaseContext databaseContext) {
enqueueTask(new BackgroundTask<Void>() {
@Override
protected Void call() throws Exception {
this.updateProgress(-1, 1);
indexer.addToIndex(databaseContext);
return null;
for (BibEntry entry : databaseContext.getEntries()) {
for (LinkedFile file : entry.getFiles()) {
enqueueTask(() -> indexer.addToIndex(entry, file, databaseContext));
}
});
}
}

public void addToIndex(PdfIndexer indexer, BibEntry entry, BibDatabaseContext databaseContext) {
enqueueTask(new BackgroundTask<Void>() {
@Override
protected Void call() throws Exception {
this.updateProgress(-1, 1);
indexer.addToIndex(entry, databaseContext);
return null;
}
});
addToIndex(indexer, entry, entry.getFiles(), databaseContext);
}

public void addToIndex(PdfIndexer indexer, BibEntry entry, List<LinkedFile> linkedFiles, BibDatabaseContext databaseContext) {
enqueueTask(new BackgroundTask<Void>() {
@Override
protected Void call() throws Exception {
this.updateProgress(-1, 1);
indexer.addToIndex(entry, linkedFiles, databaseContext);
return null;
}
});
for (LinkedFile file : linkedFiles) {
enqueueTask(() -> indexer.addToIndex(entry, file, databaseContext));
}
}

public void removeFromIndex(PdfIndexer indexer, BibEntry entry, List<LinkedFile> linkedFiles) {
enqueueTask(new BackgroundTask<Void>() {
@Override
protected Void call() throws Exception {
this.updateProgress(-1, 1);
indexer.removeFromIndex(entry, linkedFiles);
return null;
}
});
for (LinkedFile file : linkedFiles) {
enqueueTask(() -> indexer.removeFromIndex(entry, file));
}
}

public void removeFromIndex(PdfIndexer indexer, BibEntry entry) {
enqueueTask(new BackgroundTask<Void>() {
@Override
protected Void call() throws Exception {
this.updateProgress(-1, 1);
indexer.removeFromIndex(entry);
return null;
}
});
removeFromIndex(indexer, entry, entry.getFiles());
}

public void updateDatabaseName(String name) {
this.updateMessage(name);
DefaultTaskExecutor.runInJavaFXThread(() -> this.titleProperty().set(Localization.lang("Indexing for %0", name)));
}
}
12 changes: 1 addition & 11 deletions src/main/java/org/jabref/logic/pdf/search/indexing/PdfIndexer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,8 @@
import java.util.Optional;
import java.util.concurrent.TimeUnit;

import javafx.collections.ObservableList;

import org.jabref.gui.LibraryTab;
import org.jabref.logic.util.StandardFileType;
import org.jabref.model.database.BibDatabase;
import org.jabref.model.database.BibDatabaseContext;
import org.jabref.model.entry.BibEntry;
import org.jabref.model.entry.LinkedFile;
Expand Down Expand Up @@ -59,22 +56,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
View file
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 linked files added to the index

Invalid\ citation\ key=Invalid citation key

Expand Down
22 changes: 15 additions & 7 deletions src/test/java/org/jabref/logic/pdf/search/indexing/PdfIndexerTest.java
View file
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 @@ -68,7 +70,8 @@ public void dontIndexNonPdf() 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 @@ -84,7 +87,8 @@ public void dontIndexOnlineLinks() 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 @@ -101,7 +105,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 @@ -118,7 +123,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 @@ -134,7 +140,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 @@ -156,7 +163,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
4 changes: 3 additions & 1 deletion src/test/java/org/jabref/logic/pdf/search/retrieval/PdfSearcherTest.java
View file
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

0 comments on commit 05a0ce9

Please sign in to comment.