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 6 commits
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
@@ -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 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)));
}
}
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
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 @@ -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
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