From 05a0ce9755d044557d97c48103d02bfb6b43c1a0 Mon Sep 17 00:00:00 2001 From: Benedikt Tutzer Date: Thu, 19 Aug 2021 20:31:20 +0200 Subject: [PATCH] Improved progress indication for fulltext-index operations (#7981) --- .../RebuildFulltextSearchIndexAction.java | 3 +- .../search/indexing/IndexingTaskManager.java | 127 ++++++++---------- .../logic/pdf/search/indexing/PdfIndexer.java | 12 +- src/main/resources/l10n/JabRef_en.properties | 2 + .../pdf/search/indexing/PdfIndexerTest.java | 22 ++- .../pdf/search/retrieval/PdfSearcherTest.java | 4 +- 6 files changed, 77 insertions(+), 93 deletions(-) diff --git a/src/main/java/org/jabref/gui/search/RebuildFulltextSearchIndexAction.java b/src/main/java/org/jabref/gui/search/RebuildFulltextSearchIndexAction.java index f103ed07a8d..2de75512b39 100644 --- a/src/main/java/org/jabref/gui/search/RebuildFulltextSearchIndexAction.java +++ b/src/main/java/org/jabref/gui/search/RebuildFulltextSearchIndexAction.java @@ -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); diff --git a/src/main/java/org/jabref/logic/pdf/search/indexing/IndexingTaskManager.java b/src/main/java/org/jabref/logic/pdf/search/indexing/IndexingTaskManager.java index 78c0f1d4328..701cd221644 100644 --- a/src/main/java/org/jabref/logic/pdf/search/indexing/IndexingTaskManager.java +++ b/src/main/java/org/jabref/logic/pdf/search/indexing/IndexingTaskManager.java @@ -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; @@ -17,110 +17,91 @@ */ public class IndexingTaskManager extends BackgroundTask { - Queue> taskQueue = new LinkedList<>(); - TaskExecutor taskExecutor; + private final Queue 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 task) { - task.onFinished(() -> { - this.progressProperty().unbind(); - this.updateProgress(1, 1); - taskQueue.poll(); // This is the task that just finished - if (!taskQueue.isEmpty()) { - BackgroundTask 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() { - @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() { - @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() { - @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 linkedFiles, BibDatabaseContext databaseContext) { - enqueueTask(new BackgroundTask() { - @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 linkedFiles) { - enqueueTask(new BackgroundTask() { - @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() { - @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))); } } diff --git a/src/main/java/org/jabref/logic/pdf/search/indexing/PdfIndexer.java b/src/main/java/org/jabref/logic/pdf/search/indexing/PdfIndexer.java index 2c0d2cf605e..4e430bb9a43 100644 --- a/src/main/java/org/jabref/logic/pdf/search/indexing/PdfIndexer.java +++ b/src/main/java/org/jabref/logic/pdf/search/indexing/PdfIndexer.java @@ -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; @@ -59,13 +56,8 @@ 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 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)); @@ -73,8 +65,6 @@ public void createIndex(BibDatabase database, BibDatabaseContext context) { } 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) { diff --git a/src/main/resources/l10n/JabRef_en.properties b/src/main/resources/l10n/JabRef_en.properties index eccf97608ce..f6b6af3730b 100644 --- a/src/main/resources/l10n/JabRef_en.properties +++ b/src/main/resources/l10n/JabRef_en.properties @@ -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 diff --git a/src/test/java/org/jabref/logic/pdf/search/indexing/PdfIndexerTest.java b/src/test/java/org/jabref/logic/pdf/search/indexing/PdfIndexerTest.java index 82f220df784..4d948f7f37f 100644 --- a/src/test/java/org/jabref/logic/pdf/search/indexing/PdfIndexerTest.java +++ b/src/test/java/org/jabref/logic/pdf/search/indexing/PdfIndexerTest.java @@ -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); } @@ -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()))) { @@ -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()))) { @@ -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()))) { @@ -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()))) { @@ -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()))) { @@ -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()); @@ -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()))) { diff --git a/src/test/java/org/jabref/logic/pdf/search/retrieval/PdfSearcherTest.java b/src/test/java/org/jabref/logic/pdf/search/retrieval/PdfSearcherTest.java index 4be7e5fc2cd..0279cc25dff 100644 --- a/src/test/java/org/jabref/logic/pdf/search/retrieval/PdfSearcherTest.java +++ b/src/test/java/org/jabref/logic/pdf/search/retrieval/PdfSearcherTest.java @@ -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); @@ -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