From 5ec4027f29b00d99a2a1ad59e30c3a3fc90c049f Mon Sep 17 00:00:00 2001 From: Houssem Nasri Date: Mon, 23 Nov 2020 16:07:42 +0100 Subject: [PATCH 01/14] refactoring LibraryTab to be able to create an empty tab and feed it data when it's available --- src/main/java/org/jabref/gui/LibraryTab.java | 90 +++++++++++++++++--- 1 file changed, 80 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/jabref/gui/LibraryTab.java b/src/main/java/org/jabref/gui/LibraryTab.java index 2cb7174d95c..d6d220013eb 100644 --- a/src/main/java/org/jabref/gui/LibraryTab.java +++ b/src/main/java/org/jabref/gui/LibraryTab.java @@ -2,11 +2,7 @@ import java.io.File; import java.nio.file.Path; -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; -import java.util.Objects; -import java.util.Optional; +import java.util.*; import javafx.application.Platform; import javafx.beans.property.BooleanProperty; @@ -23,6 +19,7 @@ import org.jabref.gui.autocompleter.SuggestionProviders; import org.jabref.gui.collab.DatabaseChangeMonitor; import org.jabref.gui.collab.DatabaseChangePane; +import org.jabref.gui.dialogs.AutosaveUiManager; import org.jabref.gui.entryeditor.EntryEditor; import org.jabref.gui.externalfiletype.ExternalFileTypes; import org.jabref.gui.maintable.MainTable; @@ -34,6 +31,8 @@ import org.jabref.gui.undo.UndoableInsertEntries; import org.jabref.gui.undo.UndoableRemoveEntries; import org.jabref.gui.util.DefaultTaskExecutor; +import org.jabref.logic.autosaveandbackup.AutosaveManager; +import org.jabref.logic.autosaveandbackup.BackupManager; import org.jabref.logic.citationstyle.CitationStyleCache; import org.jabref.logic.l10n.Localization; import org.jabref.logic.pdf.FileAnnotationCache; @@ -52,6 +51,7 @@ import org.jabref.model.entry.event.EntryChangedEvent; import org.jabref.model.entry.field.Field; import org.jabref.model.entry.field.FieldFactory; +import org.jabref.preferences.JabRefPreferences; import org.jabref.preferences.PreferencesService; import com.google.common.eventbus.Subscribe; @@ -64,11 +64,11 @@ public class LibraryTab extends Tab { private static final Logger LOGGER = LoggerFactory.getLogger(LibraryTab.class); - private final BibDatabaseContext bibDatabaseContext; - private final MainTableDataModel tableModel; + private BibDatabaseContext bibDatabaseContext; + private MainTableDataModel tableModel; - private final CitationStyleCache citationStyleCache; - private final FileAnnotationCache annotationCache; + private CitationStyleCache citationStyleCache; + private FileAnnotationCache annotationCache; private final JabRefFrame frame; private final CountingUndoManager undoManager; @@ -76,7 +76,7 @@ public class LibraryTab extends Tab { private final SidePaneManager sidePaneManager; private final ExternalFileTypes externalFileTypes; - private final EntryEditor entryEditor; + private EntryEditor entryEditor; private final DialogService dialogService; private final PreferencesService preferencesService; @@ -140,6 +140,74 @@ public LibraryTab(JabRefFrame frame, }); } + public static LibraryTab createNewEmptyLibraryTab(JabRefFrame frame, Path file) { + BibDatabaseContext context = new BibDatabaseContext(); + context.setDatabasePath(file); + + return new LibraryTab(frame, frame.prefs(), context, ExternalFileTypes.getInstance()); + } + + public void feedData(BibDatabaseContext bibDatabaseContext) { + cleanUp(); + + this.bibDatabaseContext = Objects.requireNonNull(bibDatabaseContext); + + bibDatabaseContext.getDatabase().registerListener(this); + bibDatabaseContext.getMetaData().registerListener(this); + + + this.tableModel = new MainTableDataModel(getBibDatabaseContext(), preferencesService, Globals.stateManager); + citationStyleCache = new CitationStyleCache(bibDatabaseContext); + annotationCache = new FileAnnotationCache(bibDatabaseContext, preferencesService.getFilePreferences()); + + setupMainPanel(); + setupAutoCompletion(); + + this.getDatabase().registerListener(new SearchListener()); + this.getDatabase().registerListener(new EntriesRemovedListener()); + + // ensure that at each addition of a new entry, the entry is added to the groups interface + this.bibDatabaseContext.getDatabase().registerListener(new GroupTreeListener()); + // ensure that all entry changes mark the panel as changed + this.bibDatabaseContext.getDatabase().registerListener(this); + + this.getDatabase().registerListener(new UpdateTimestampListener(preferencesService)); + + this.entryEditor = new EntryEditor(this, externalFileTypes); + + Platform.runLater(() -> { + EasyBind.subscribe(changedProperty, this::updateTabTitle); + Globals.stateManager.getOpenDatabases().addListener((ListChangeListener) c -> + updateTabTitle(changedProperty.getValue())); + }); + + + if (isDatabaseReadyForAutoSave(bibDatabaseContext)) { + AutosaveManager autoSaver = AutosaveManager.start(bibDatabaseContext); + autoSaver.registerListener(new AutosaveUiManager(this)); + } + + BackupManager.start(this.bibDatabaseContext, Globals.entryTypesManager, Globals.prefs); + + trackOpenNewDatabase(this); + + } + + private boolean isDatabaseReadyForAutoSave(BibDatabaseContext context) { + return ((context.getLocation() == DatabaseLocation.SHARED) || + ((context.getLocation() == DatabaseLocation.LOCAL) && Globals.prefs.getBoolean(JabRefPreferences.LOCAL_AUTO_SAVE))) + && + context.getDatabasePath().isPresent(); + } + + private void trackOpenNewDatabase(LibraryTab libraryTab) { + Map properties = new HashMap<>(); + Map measurements = new HashMap<>(); + measurements.put("NumberOfEntries", (double) libraryTab.getBibDatabaseContext().getDatabase().getEntryCount()); + + Globals.getTelemetryClient().ifPresent(client -> client.trackEvent("OpenNewDatabase", properties, measurements)); + } + /** * Sets the title of the tab * modification-asterisk filename – path-fragment @@ -585,6 +653,8 @@ private void saveDividerLocation(Number position) { */ public void cleanUp() { changeMonitor.ifPresent(DatabaseChangeMonitor::unregister); + AutosaveManager.shutdown(bibDatabaseContext); + BackupManager.shutdown(bibDatabaseContext); } /** From e1af860e4c6f4d346955fe8851a0957dbc87f8b9 Mon Sep 17 00:00:00 2001 From: Houssem Nasri Date: Mon, 23 Nov 2020 16:27:42 +0100 Subject: [PATCH 02/14] applying the improvement when opening database using action --- .../importer/actions/OpenDatabaseAction.java | 59 +++++++++++++++---- 1 file changed, 46 insertions(+), 13 deletions(-) diff --git a/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java b/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java index 8b2f79ec02d..feb93fa1750 100644 --- a/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java +++ b/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java @@ -11,6 +11,9 @@ import java.util.Objects; import java.util.Optional; +import javafx.scene.Node; +import javafx.scene.control.ProgressIndicator; +import javafx.scene.layout.BorderPane; import org.jabref.gui.DialogService; import org.jabref.gui.Globals; import org.jabref.gui.JabRefFrame; @@ -30,6 +33,7 @@ import org.jabref.logic.shared.exception.InvalidDBMSConnectionPropertiesException; import org.jabref.logic.shared.exception.NotASharedDatabaseException; import org.jabref.logic.util.StandardFileType; +import org.jabref.model.database.BibDatabaseContext; import org.jabref.preferences.JabRefPreferences; import org.slf4j.Logger; @@ -59,8 +63,8 @@ public OpenDatabaseAction(JabRefFrame frame) { /** * Go through the list of post open actions, and perform those that need to be performed. * - * @param libraryTab The BasePanel where the database is shown. - * @param result The result of the BIB file parse operation. + * @param libraryTab The BasePanel where the database is shown. + * @param result The result of the BIB file parse operation. */ public static void performPostOpenActions(LibraryTab libraryTab, ParserResult result) { for (GUIPostOpenAction action : OpenDatabaseAction.POST_OPEN_ACTIONS) { @@ -159,17 +163,17 @@ public void openFiles(List filesToOpen, boolean raisePanel) { */ private void openTheFile(Path file, boolean raisePanel) { Objects.requireNonNull(file); - if (Files.exists(file)) { - - BackgroundTask.wrap(() -> loadDatabase(file)) - .onSuccess(result -> { - LibraryTab libraryTab = addNewDatabase(result, file, raisePanel); - OpenDatabaseAction.performPostOpenActions(libraryTab, result); - }) - .onFailure(ex -> dialogService.showErrorDialogAndWait(Localization.lang("Connection error"), - ex.getMessage() + "\n\n" + Localization.lang("A local copy will be opened."))) - .executeWith(Globals.TASK_EXECUTOR); - } + if (!Files.exists(file)) + return; + + BackgroundTask backgroundTask = BackgroundTask.wrap(() -> loadDatabase(file)); + LibraryTab libraryTab = LibraryTab.createNewEmptyLibraryTab(frame, file); + onDatabaseLoadingStarted(libraryTab, backgroundTask); + + backgroundTask.onSuccess(result -> onDatabaseLoadingSucceed(libraryTab, result)) + .onFailure(ex -> onDatabaseLoadingFailed(libraryTab, ex)) + .executeWith(Globals.TASK_EXECUTOR); + } private ParserResult loadDatabase(Path file) throws Exception { @@ -210,4 +214,33 @@ private LibraryTab addNewDatabase(ParserResult result, final Path file, boolean frame.addTab(libraryTab, raisePanel); return libraryTab; } + + /*The layout to display in the tab when it's loading*/ + public Node createLoadingLayout() { + ProgressIndicator progressIndicator = new ProgressIndicator(ProgressIndicator.INDETERMINATE_PROGRESS); + BorderPane pane = new BorderPane(); + pane.setCenter(progressIndicator); + + return pane; + } + + public void onDatabaseLoadingStarted(LibraryTab libraryTab, BackgroundTask backgroundTask) { + Node loadingLayout = createLoadingLayout(); + libraryTab.setContent(loadingLayout); + libraryTab.setOnCloseRequest(e -> backgroundTask.cancel()); + + frame.addTab(libraryTab, true); + } + + public void onDatabaseLoadingSucceed(LibraryTab libraryTab, ParserResult result) { + BibDatabaseContext context = result.getDatabaseContext(); + libraryTab.feedData(context); + + OpenDatabaseAction.performPostOpenActions(libraryTab, result); + } + + public void onDatabaseLoadingFailed(LibraryTab libraryTab, Exception ex) { + dialogService.showErrorDialogAndWait(Localization.lang("Connection error"), + ex.getMessage() + "\n\n" + Localization.lang("A local copy will be opened.")); + } } From 5f2d7ea06c9e37d961dd10a9bb9414f4773486fa Mon Sep 17 00:00:00 2001 From: Houssem Nasri Date: Mon, 23 Nov 2020 17:39:23 +0100 Subject: [PATCH 03/14] checkstyle --- src/main/java/org/jabref/gui/LibraryTab.java | 45 ++++++++++--------- .../importer/actions/OpenDatabaseAction.java | 6 ++- 2 files changed, 29 insertions(+), 22 deletions(-) diff --git a/src/main/java/org/jabref/gui/LibraryTab.java b/src/main/java/org/jabref/gui/LibraryTab.java index d6d220013eb..01d9d54aae5 100644 --- a/src/main/java/org/jabref/gui/LibraryTab.java +++ b/src/main/java/org/jabref/gui/LibraryTab.java @@ -2,7 +2,13 @@ import java.io.File; import java.nio.file.Path; -import java.util.*; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Optional; import javafx.application.Platform; import javafx.beans.property.BooleanProperty; @@ -93,7 +99,8 @@ public class LibraryTab extends Tab { private BibEntry showing; private SuggestionProviders suggestionProviders; - @SuppressWarnings({"FieldCanBeLocal"}) private Subscription dividerPositionSubscription; + @SuppressWarnings({"FieldCanBeLocal"}) + private Subscription dividerPositionSubscription; // the query the user searches when this BasePanel is active private Optional currentSearchQuery = Optional.empty(); private Optional changeMonitor = Optional.empty(); @@ -155,7 +162,6 @@ public void feedData(BibDatabaseContext bibDatabaseContext) { bibDatabaseContext.getDatabase().registerListener(this); bibDatabaseContext.getMetaData().registerListener(this); - this.tableModel = new MainTableDataModel(getBibDatabaseContext(), preferencesService, Globals.stateManager); citationStyleCache = new CitationStyleCache(bibDatabaseContext); annotationCache = new FileAnnotationCache(bibDatabaseContext, preferencesService.getFilePreferences()); @@ -181,7 +187,6 @@ public void feedData(BibDatabaseContext bibDatabaseContext) { updateTabTitle(changedProperty.getValue())); }); - if (isDatabaseReadyForAutoSave(bibDatabaseContext)) { AutosaveManager autoSaver = AutosaveManager.start(bibDatabaseContext); autoSaver.registerListener(new AutosaveUiManager(this)); @@ -210,13 +215,13 @@ private void trackOpenNewDatabase(LibraryTab libraryTab) { /** * Sets the title of the tab - * modification-asterisk filename – path-fragment - * + * modification-asterisk filename – path-fragment + *

* The modification-asterisk (*) is shown if the file was modified since last save * (path-fragment is only shown if filename is not (globally) unique) - * + *

* Example: - * *jabref-authors.bib – testbib + * *jabref-authors.bib – testbib */ public void updateTabTitle(boolean isChanged) { boolean isAutosaveEnabled = preferencesService.getShouldAutosave(); @@ -257,9 +262,9 @@ public void updateTabTitle(boolean isChanged) { // Unique path fragment List uniquePathParts = FileUtil.uniquePathSubstrings(collectAllDatabasePaths()); Optional uniquePathPart = uniquePathParts.stream() - .filter(part -> databasePath.toString().contains(part) - && !part.equals(fileName) && part.contains(File.separator)) - .findFirst(); + .filter(part -> databasePath.toString().contains(part) + && !part.equals(fileName) && part.contains(File.separator)) + .findFirst(); if (uniquePathPart.isPresent()) { String uniquePath = uniquePathPart.get(); // remove filename @@ -310,10 +315,10 @@ private static void addSharedDbInformation(StringBuilder text, BibDatabaseContex private List collectAllDatabasePaths() { List list = new ArrayList<>(); Globals.stateManager.getOpenDatabases().stream() - .map(BibDatabaseContext::getDatabasePath) - .forEachOrdered(pathOptional -> pathOptional.ifPresentOrElse( - path -> list.add(path.toAbsolutePath().toString()), - () -> list.add(""))); + .map(BibDatabaseContext::getDatabasePath) + .forEachOrdered(pathOptional -> pathOptional.ifPresentOrElse( + path -> list.add(path.toAbsolutePath().toString()), + () -> list.add(""))); return list; } @@ -454,9 +459,9 @@ private void createMainTable() { // Update entry editor and preview according to selected entries mainTable.addSelectionListener(event -> mainTable.getSelectedEntries() - .stream() - .findFirst() - .ifPresent(entryEditor::setEntry)); + .stream() + .findFirst() + .ifPresent(entryEditor::setEntry)); } public void setupMainPanel() { @@ -470,8 +475,8 @@ public void setupMainPanel() { // Saves the divider position as soon as it changes // We need to keep a reference to the subscription, otherwise the binding gets garbage collected dividerPositionSubscription = EasyBind.valueAt(splitPane.getDividers(), 0) - .mapObservable(SplitPane.Divider::positionProperty) - .subscribeToValues(this::saveDividerLocation); + .mapObservable(SplitPane.Divider::positionProperty) + .subscribeToValues(this::saveDividerLocation); // Add changePane in case a file is present - otherwise just add the splitPane to the panel Optional file = bibDatabaseContext.getDatabasePath(); diff --git a/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java b/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java index feb93fa1750..8eb0d3cc6b5 100644 --- a/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java +++ b/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java @@ -14,6 +14,7 @@ import javafx.scene.Node; import javafx.scene.control.ProgressIndicator; import javafx.scene.layout.BorderPane; + import org.jabref.gui.DialogService; import org.jabref.gui.Globals; import org.jabref.gui.JabRefFrame; @@ -163,8 +164,9 @@ public void openFiles(List filesToOpen, boolean raisePanel) { */ private void openTheFile(Path file, boolean raisePanel) { Objects.requireNonNull(file); - if (!Files.exists(file)) + if (!Files.exists(file)) { return; + } BackgroundTask backgroundTask = BackgroundTask.wrap(() -> loadDatabase(file)); LibraryTab libraryTab = LibraryTab.createNewEmptyLibraryTab(frame, file); @@ -215,7 +217,7 @@ private LibraryTab addNewDatabase(ParserResult result, final Path file, boolean return libraryTab; } - /*The layout to display in the tab when it's loading*/ + /* The layout to display in the tab when it's loading*/ public Node createLoadingLayout() { ProgressIndicator progressIndicator = new ProgressIndicator(ProgressIndicator.INDETERMINATE_PROGRESS); BorderPane pane = new BorderPane(); From a1ee47404496f82c2cfa194686e39ebf7f0bbd29 Mon Sep 17 00:00:00 2001 From: Houssem Nasri Date: Mon, 23 Nov 2020 20:28:46 +0100 Subject: [PATCH 04/14] add exception parameter to error dialog --- .../org/jabref/gui/importer/actions/OpenDatabaseAction.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java b/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java index 8eb0d3cc6b5..6ed8457f9c2 100644 --- a/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java +++ b/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java @@ -242,7 +242,9 @@ public void onDatabaseLoadingSucceed(LibraryTab libraryTab, ParserResult result) } public void onDatabaseLoadingFailed(LibraryTab libraryTab, Exception ex) { - dialogService.showErrorDialogAndWait(Localization.lang("Connection error"), - ex.getMessage() + "\n\n" + Localization.lang("A local copy will be opened.")); + String title = Localization.lang("Connection error"); + String content = String.format("%s\n\n%s", ex.getMessage(), Localization.lang("A local copy will be opened.")); + + dialogService.showErrorDialogAndWait(title, content, ex); } } From 1ca86161e986b2196902081a368fe9cb87cd7cd6 Mon Sep 17 00:00:00 2001 From: Houssem Nasri Date: Tue, 24 Nov 2020 11:30:59 +0100 Subject: [PATCH 05/14] Adding DataLoadingTask to LibraryTab --- src/main/java/org/jabref/gui/LibraryTab.java | 76 +++++++++---------- .../importer/actions/OpenDatabaseAction.java | 7 +- 2 files changed, 39 insertions(+), 44 deletions(-) diff --git a/src/main/java/org/jabref/gui/LibraryTab.java b/src/main/java/org/jabref/gui/LibraryTab.java index 01d9d54aae5..38f2edca469 100644 --- a/src/main/java/org/jabref/gui/LibraryTab.java +++ b/src/main/java/org/jabref/gui/LibraryTab.java @@ -36,6 +36,7 @@ import org.jabref.gui.undo.UndoableFieldChange; import org.jabref.gui.undo.UndoableInsertEntries; import org.jabref.gui.undo.UndoableRemoveEntries; +import org.jabref.gui.util.BackgroundTask; import org.jabref.gui.util.DefaultTaskExecutor; import org.jabref.logic.autosaveandbackup.AutosaveManager; import org.jabref.logic.autosaveandbackup.BackupManager; @@ -105,6 +106,8 @@ public class LibraryTab extends Tab { private Optional currentSearchQuery = Optional.empty(); private Optional changeMonitor = Optional.empty(); + private BackgroundTask dataLoadingTask; + public LibraryTab(JabRefFrame frame, PreferencesService preferencesService, BibDatabaseContext bibDatabaseContext, @@ -147,11 +150,18 @@ public LibraryTab(JabRefFrame frame, }); } - public static LibraryTab createNewEmptyLibraryTab(JabRefFrame frame, Path file) { + public void setDataLoadingTask(BackgroundTask dataLoadingTask) { + this.dataLoadingTask = dataLoadingTask; + } + + public static LibraryTab createNewEmptyLibraryTab(JabRefFrame frame, Path file, BackgroundTask dataLoadingTask) { BibDatabaseContext context = new BibDatabaseContext(); context.setDatabasePath(file); - return new LibraryTab(frame, frame.prefs(), context, ExternalFileTypes.getInstance()); + LibraryTab tab = new LibraryTab(frame, frame.prefs(), context, ExternalFileTypes.getInstance()); + tab.setDataLoadingTask(dataLoadingTask); + + return tab; } public void feedData(BibDatabaseContext bibDatabaseContext) { @@ -195,7 +205,6 @@ public void feedData(BibDatabaseContext bibDatabaseContext) { BackupManager.start(this.bibDatabaseContext, Globals.entryTypesManager, Globals.prefs); trackOpenNewDatabase(this); - } private boolean isDatabaseReadyForAutoSave(BibDatabaseContext context) { @@ -214,14 +223,11 @@ private void trackOpenNewDatabase(LibraryTab libraryTab) { } /** - * Sets the title of the tab - * modification-asterisk filename – path-fragment + * Sets the title of the tab modification-asterisk filename – path-fragment *

- * The modification-asterisk (*) is shown if the file was modified since last save - * (path-fragment is only shown if filename is not (globally) unique) + * The modification-asterisk (*) is shown if the file was modified since last save (path-fragment is only shown if filename is not (globally) unique) *

- * Example: - * *jabref-authors.bib – testbib + * Example: *jabref-authors.bib – testbib */ public void updateTabTitle(boolean isChanged) { boolean isAutosaveEnabled = preferencesService.getShouldAutosave(); @@ -262,9 +268,9 @@ public void updateTabTitle(boolean isChanged) { // Unique path fragment List uniquePathParts = FileUtil.uniquePathSubstrings(collectAllDatabasePaths()); Optional uniquePathPart = uniquePathParts.stream() - .filter(part -> databasePath.toString().contains(part) - && !part.equals(fileName) && part.contains(File.separator)) - .findFirst(); + .filter(part -> databasePath.toString().contains(part) + && !part.equals(fileName) && part.contains(File.separator)) + .findFirst(); if (uniquePathPart.isPresent()) { String uniquePath = uniquePathPart.get(); // remove filename @@ -315,10 +321,10 @@ private static void addSharedDbInformation(StringBuilder text, BibDatabaseContex private List collectAllDatabasePaths() { List list = new ArrayList<>(); Globals.stateManager.getOpenDatabases().stream() - .map(BibDatabaseContext::getDatabasePath) - .forEachOrdered(pathOptional -> pathOptional.ifPresentOrElse( - path -> list.add(path.toAbsolutePath().toString()), - () -> list.add(""))); + .map(BibDatabaseContext::getDatabasePath) + .forEachOrdered(pathOptional -> pathOptional.ifPresentOrElse( + path -> list.add(path.toAbsolutePath().toString()), + () -> list.add(""))); return list; } @@ -349,8 +355,7 @@ public JabRefFrame frame() { /** * Removes the selected entries from the database * - * @param cut If false the user will get asked if he really wants to delete the entries, and it will be localized as - * "deleted". If true the action will be localized as "cut" + * @param cut If false the user will get asked if he really wants to delete the entries, and it will be localized as "deleted". If true the action will be localized as "cut" */ public void delete(boolean cut) { delete(cut, mainTable.getSelectedEntries()); @@ -359,8 +364,7 @@ public void delete(boolean cut) { /** * Removes the selected entries from the database * - * @param cut If false the user will get asked if he really wants to delete the entries, and it will be localized as - * "deleted". If true the action will be localized as "cut" + * @param cut If false the user will get asked if he really wants to delete the entries, and it will be localized as "deleted". If true the action will be localized as "cut" */ private void delete(boolean cut, List entries) { if (entries.isEmpty()) { @@ -403,9 +407,7 @@ public void insertEntry(final BibEntry bibEntry) { } /** - * This method is called from JabRefFrame when the user wants to create a new entry or entries. - * It is necessary when the user would expect the added entry or one of the added entries - * to be selected in the entry editor + * This method is called from JabRefFrame when the user wants to create a new entry or entries. It is necessary when the user would expect the added entry or one of the added entries to be selected in the entry editor * * @param entries The new entries. */ @@ -459,9 +461,9 @@ private void createMainTable() { // Update entry editor and preview according to selected entries mainTable.addSelectionListener(event -> mainTable.getSelectedEntries() - .stream() - .findFirst() - .ifPresent(entryEditor::setEntry)); + .stream() + .findFirst() + .ifPresent(entryEditor::setEntry)); } public void setupMainPanel() { @@ -475,8 +477,8 @@ public void setupMainPanel() { // Saves the divider position as soon as it changes // We need to keep a reference to the subscription, otherwise the binding gets garbage collected dividerPositionSubscription = EasyBind.valueAt(splitPane.getDividers(), 0) - .mapObservable(SplitPane.Divider::positionProperty) - .subscribeToValues(this::saveDividerLocation); + .mapObservable(SplitPane.Divider::positionProperty) + .subscribeToValues(this::saveDividerLocation); // Add changePane in case a file is present - otherwise just add the splitPane to the panel Optional file = bibDatabaseContext.getDatabasePath(); @@ -518,8 +520,7 @@ public EntryEditor getEntryEditor() { } /** - * Sets the entry editor as the bottom component in the split pane. If an entry editor already was shown, makes sure - * that the divider doesn't move. Updates the mode to SHOWING_EDITOR. Then shows the given entry. + * Sets the entry editor as the bottom component in the split pane. If an entry editor already was shown, makes sure that the divider doesn't move. Updates the mode to SHOWING_EDITOR. Then shows the given entry. * * @param entry The entry to edit. */ @@ -558,8 +559,7 @@ public void closeBottomPane() { } /** - * This method selects the given entry, and scrolls it into view in the table. If an entryEditor is shown, it is - * given focus afterwards. + * This method selects the given entry, and scrolls it into view in the table. If an entryEditor is shown, it is given focus afterwards. */ public void clearAndSelect(final BibEntry bibEntry) { mainTable.clearAndSelect(bibEntry); @@ -574,8 +574,7 @@ public void selectNextEntry() { } /** - * This method is called from an EntryEditor when it should be closed. We relay to the selection listener, which - * takes care of the rest. + * This method is called from an EntryEditor when it should be closed. We relay to the selection listener, which takes care of the rest. */ public void entryEditorClosing() { closeBottomPane(); @@ -643,8 +642,7 @@ private boolean showDeleteConfirmationDialog(int numberOfEntries) { } /** - * Depending on whether a preview or an entry editor is showing, save the current divider location in the correct - * preference setting. + * Depending on whether a preview or an entry editor is showing, save the current divider location in the correct preference setting. */ private void saveDividerLocation(Number position) { if (mode == BasePanelMode.SHOWING_EDITOR) { @@ -663,8 +661,7 @@ public void cleanUp() { } /** - * Get an array containing the currently selected entries. The array is stable and not changed if the selection - * changes + * Get an array containing the currently selected entries. The array is stable and not changed if the selection changes * * @return A list containing the selected entries. Is never null. */ @@ -796,8 +793,7 @@ public void listen(EntriesRemovedEvent entriesRemovedEvent) { } /** - * Ensures that the results of the current search are updated when a new entry is inserted into the database Actual - * methods for performing search must run in javafx thread + * Ensures that the results of the current search are updated when a new entry is inserted into the database Actual methods for performing search must run in javafx thread */ private class SearchListener { diff --git a/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java b/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java index 6ed8457f9c2..b9b10f97da5 100644 --- a/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java +++ b/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java @@ -169,13 +169,12 @@ private void openTheFile(Path file, boolean raisePanel) { } BackgroundTask backgroundTask = BackgroundTask.wrap(() -> loadDatabase(file)); - LibraryTab libraryTab = LibraryTab.createNewEmptyLibraryTab(frame, file); + LibraryTab libraryTab = LibraryTab.createNewEmptyLibraryTab(frame, file, backgroundTask); onDatabaseLoadingStarted(libraryTab, backgroundTask); backgroundTask.onSuccess(result -> onDatabaseLoadingSucceed(libraryTab, result)) - .onFailure(ex -> onDatabaseLoadingFailed(libraryTab, ex)) - .executeWith(Globals.TASK_EXECUTOR); - + .onFailure(ex -> onDatabaseLoadingFailed(libraryTab, ex)) + .executeWith(Globals.TASK_EXECUTOR); } private ParserResult loadDatabase(Path file) throws Exception { From 8c0fd835d9245420f92508f53face49d506fafeb Mon Sep 17 00:00:00 2001 From: Houssem Nasri Date: Tue, 24 Nov 2020 11:39:10 +0100 Subject: [PATCH 06/14] Add getter for dataLoadingTask --- src/main/java/org/jabref/gui/LibraryTab.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/main/java/org/jabref/gui/LibraryTab.java b/src/main/java/org/jabref/gui/LibraryTab.java index 38f2edca469..eddf9cf91f3 100644 --- a/src/main/java/org/jabref/gui/LibraryTab.java +++ b/src/main/java/org/jabref/gui/LibraryTab.java @@ -154,6 +154,13 @@ public void setDataLoadingTask(BackgroundTask dataLoadingTask) { this.dataLoadingTask = dataLoadingTask; } + public BackgroundTask getDataLoadingTask() { + if (dataLoadingTask == null) { + return null; + } + return dataLoadingTask; + } + public static LibraryTab createNewEmptyLibraryTab(JabRefFrame frame, Path file, BackgroundTask dataLoadingTask) { BibDatabaseContext context = new BibDatabaseContext(); context.setDatabasePath(file); From f43c2ff9d3dd99384f8dff9b545853fafbc2333a Mon Sep 17 00:00:00 2001 From: Houssem Nasri Date: Tue, 24 Nov 2020 11:48:25 +0100 Subject: [PATCH 07/14] Fixing memory leak by canceling dataLoadingTask when tab is closed --- src/main/java/org/jabref/gui/JabRefFrame.java | 16 ++++++---------- src/main/java/org/jabref/gui/LibraryTab.java | 4 ++-- .../gui/importer/actions/OpenDatabaseAction.java | 5 ++--- 3 files changed, 10 insertions(+), 15 deletions(-) diff --git a/src/main/java/org/jabref/gui/JabRefFrame.java b/src/main/java/org/jabref/gui/JabRefFrame.java index 963df97bb57..959a22e6071 100644 --- a/src/main/java/org/jabref/gui/JabRefFrame.java +++ b/src/main/java/org/jabref/gui/JabRefFrame.java @@ -338,8 +338,7 @@ public JabRefPreferences prefs() { *

* FIXME: Currently some threads remain and therefore hinder JabRef to be closed properly * - * @param filenames the filenames of all currently opened files - used for storing them if prefs openLastEdited is - * set to true + * @param filenames the filenames of all currently opened files - used for storing them if prefs openLastEdited is set to true */ private void tearDownJabRef(List filenames) { // prefs.putBoolean(JabRefPreferences.WINDOW_MAXIMISED, getExtendedState() == Frame.MAXIMIZED_BOTH); @@ -362,9 +361,7 @@ private void tearDownJabRef(List filenames) { } /** - * General info dialog. The MacAdapter calls this method when "Quit" is selected from the application menu, Cmd-Q - * is pressed, or "Quit" is selected from the Dock. The function returns a boolean indicating if quitting is ok or - * not. + * General info dialog. The MacAdapter calls this method when "Quit" is selected from the application menu, Cmd-Q is pressed, or "Quit" is selected from the Dock. The function returns a boolean indicating if quitting is ok or not. *

* Non-OSX JabRef calls this when choosing "Quit" from the menu *

@@ -989,10 +986,8 @@ public void addParserResult(ParserResult parserResult, boolean focusPanel) { } /** - * This method causes all open LibraryTabs to set up their tables anew. When called from PreferencesDialogViewModel, - * this updates to the new settings. - * We need to notify all tabs about the changes to avoid problems when changing the column set. - * */ + * This method causes all open LibraryTabs to set up their tables anew. When called from PreferencesDialogViewModel, this updates to the new settings. We need to notify all tabs about the changes to avoid problems when changing the column set. + */ public void setupAllTables() { tabbedPane.getTabs().forEach(tab -> { LibraryTab libraryTab = (LibraryTab) tab; @@ -1013,7 +1008,7 @@ private ContextMenu createTabContextMenu(KeyBindingRepository keyBindingReposito new SeparatorMenuItem(), factory.createMenuItem(StandardActions.OPEN_DATABASE_FOLDER, new OpenDatabaseFolder()), factory.createMenuItem(StandardActions.OPEN_CONSOLE, new OpenConsoleAction(stateManager)) - ); + ); return contextMenu; } @@ -1023,6 +1018,7 @@ public void addTab(LibraryTab libraryTab, boolean raisePanel) { libraryTab.setOnCloseRequest(event -> { closeTab(libraryTab); + libraryTab.getDataLoadingTask().cancel(); event.consume(); }); diff --git a/src/main/java/org/jabref/gui/LibraryTab.java b/src/main/java/org/jabref/gui/LibraryTab.java index eddf9cf91f3..b1d37daa78b 100644 --- a/src/main/java/org/jabref/gui/LibraryTab.java +++ b/src/main/java/org/jabref/gui/LibraryTab.java @@ -105,8 +105,8 @@ public class LibraryTab extends Tab { // the query the user searches when this BasePanel is active private Optional currentSearchQuery = Optional.empty(); private Optional changeMonitor = Optional.empty(); - - private BackgroundTask dataLoadingTask; + // initializing it so we prevent NullPointerException + private BackgroundTask dataLoadingTask = BackgroundTask.wrap(()-> { }); public LibraryTab(JabRefFrame frame, PreferencesService preferencesService, diff --git a/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java b/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java index b9b10f97da5..0b659f8629e 100644 --- a/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java +++ b/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java @@ -170,7 +170,7 @@ private void openTheFile(Path file, boolean raisePanel) { BackgroundTask backgroundTask = BackgroundTask.wrap(() -> loadDatabase(file)); LibraryTab libraryTab = LibraryTab.createNewEmptyLibraryTab(frame, file, backgroundTask); - onDatabaseLoadingStarted(libraryTab, backgroundTask); + onDatabaseLoadingStarted(libraryTab); backgroundTask.onSuccess(result -> onDatabaseLoadingSucceed(libraryTab, result)) .onFailure(ex -> onDatabaseLoadingFailed(libraryTab, ex)) @@ -225,10 +225,9 @@ public Node createLoadingLayout() { return pane; } - public void onDatabaseLoadingStarted(LibraryTab libraryTab, BackgroundTask backgroundTask) { + public void onDatabaseLoadingStarted(LibraryTab libraryTab) { Node loadingLayout = createLoadingLayout(); libraryTab.setContent(loadingLayout); - libraryTab.setOnCloseRequest(e -> backgroundTask.cancel()); frame.addTab(libraryTab, true); } From ef088af6ca35eb4ac3707ffebecffd45fbca86ea Mon Sep 17 00:00:00 2001 From: Houssem Nasri Date: Tue, 24 Nov 2020 13:19:58 +0100 Subject: [PATCH 08/14] Relocate LibraryTab method trackOpenNewDatabase() to OpenDatabaseAction --- src/main/java/org/jabref/gui/LibraryTab.java | 9 --------- .../gui/importer/actions/OpenDatabaseAction.java | 11 +++++++++++ 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/jabref/gui/LibraryTab.java b/src/main/java/org/jabref/gui/LibraryTab.java index b1d37daa78b..e388dc086d9 100644 --- a/src/main/java/org/jabref/gui/LibraryTab.java +++ b/src/main/java/org/jabref/gui/LibraryTab.java @@ -211,7 +211,6 @@ public void feedData(BibDatabaseContext bibDatabaseContext) { BackupManager.start(this.bibDatabaseContext, Globals.entryTypesManager, Globals.prefs); - trackOpenNewDatabase(this); } private boolean isDatabaseReadyForAutoSave(BibDatabaseContext context) { @@ -221,14 +220,6 @@ private boolean isDatabaseReadyForAutoSave(BibDatabaseContext context) { context.getDatabasePath().isPresent(); } - private void trackOpenNewDatabase(LibraryTab libraryTab) { - Map properties = new HashMap<>(); - Map measurements = new HashMap<>(); - measurements.put("NumberOfEntries", (double) libraryTab.getBibDatabaseContext().getDatabase().getEntryCount()); - - Globals.getTelemetryClient().ifPresent(client -> client.trackEvent("OpenNewDatabase", properties, measurements)); - } - /** * Sets the title of the tab modification-asterisk filename – path-fragment *

diff --git a/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java b/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java index 0b659f8629e..5daa2851ab5 100644 --- a/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java +++ b/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java @@ -6,8 +6,10 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.HashMap; import java.util.Iterator; import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.Optional; @@ -216,6 +218,14 @@ private LibraryTab addNewDatabase(ParserResult result, final Path file, boolean return libraryTab; } + private void trackOpenNewDatabase(LibraryTab libraryTab) { + Map properties = new HashMap<>(); + Map measurements = new HashMap<>(); + measurements.put("NumberOfEntries", (double) libraryTab.getBibDatabaseContext().getDatabase().getEntryCount()); + + Globals.getTelemetryClient().ifPresent(client -> client.trackEvent("OpenNewDatabase", properties, measurements)); + } + /* The layout to display in the tab when it's loading*/ public Node createLoadingLayout() { ProgressIndicator progressIndicator = new ProgressIndicator(ProgressIndicator.INDETERMINATE_PROGRESS); @@ -237,6 +247,7 @@ public void onDatabaseLoadingSucceed(LibraryTab libraryTab, ParserResult result) libraryTab.feedData(context); OpenDatabaseAction.performPostOpenActions(libraryTab, result); + trackOpenNewDatabase(libraryTab); } public void onDatabaseLoadingFailed(LibraryTab libraryTab, Exception ex) { From 77061c80a8e9c7e6dce2f028e425e899987e41b6 Mon Sep 17 00:00:00 2001 From: Houssem Nasri Date: Tue, 24 Nov 2020 14:36:31 +0100 Subject: [PATCH 09/14] Refactor LibraryTab to handle dataLoadingTask callbacks --- src/main/java/org/jabref/gui/LibraryTab.java | 67 ++++++++++++++++--- .../importer/actions/OpenDatabaseAction.java | 44 +----------- 2 files changed, 59 insertions(+), 52 deletions(-) diff --git a/src/main/java/org/jabref/gui/LibraryTab.java b/src/main/java/org/jabref/gui/LibraryTab.java index e388dc086d9..baef0327cd7 100644 --- a/src/main/java/org/jabref/gui/LibraryTab.java +++ b/src/main/java/org/jabref/gui/LibraryTab.java @@ -4,9 +4,7 @@ import java.nio.file.Path; import java.util.ArrayList; import java.util.Collections; -import java.util.HashMap; import java.util.List; -import java.util.Map; import java.util.Objects; import java.util.Optional; @@ -16,9 +14,11 @@ import javafx.collections.ListChangeListener; import javafx.geometry.Orientation; import javafx.scene.Node; +import javafx.scene.control.ProgressIndicator; import javafx.scene.control.SplitPane; import javafx.scene.control.Tab; import javafx.scene.control.Tooltip; +import javafx.scene.layout.BorderPane; import org.jabref.gui.autocompleter.AutoCompletePreferences; import org.jabref.gui.autocompleter.PersonNameSuggestionProvider; @@ -28,6 +28,7 @@ import org.jabref.gui.dialogs.AutosaveUiManager; import org.jabref.gui.entryeditor.EntryEditor; import org.jabref.gui.externalfiletype.ExternalFileTypes; +import org.jabref.gui.importer.actions.OpenDatabaseAction; import org.jabref.gui.maintable.MainTable; import org.jabref.gui.maintable.MainTableDataModel; import org.jabref.gui.specialfields.SpecialFieldDatabaseChangeListener; @@ -41,6 +42,7 @@ import org.jabref.logic.autosaveandbackup.AutosaveManager; import org.jabref.logic.autosaveandbackup.BackupManager; import org.jabref.logic.citationstyle.CitationStyleCache; +import org.jabref.logic.importer.ParserResult; import org.jabref.logic.l10n.Localization; import org.jabref.logic.pdf.FileAnnotationCache; import org.jabref.logic.search.SearchQuery; @@ -106,7 +108,7 @@ public class LibraryTab extends Tab { private Optional currentSearchQuery = Optional.empty(); private Optional changeMonitor = Optional.empty(); // initializing it so we prevent NullPointerException - private BackgroundTask dataLoadingTask = BackgroundTask.wrap(()-> { }); + private BackgroundTask dataLoadingTask = BackgroundTask.wrap(() -> null); public LibraryTab(JabRefFrame frame, PreferencesService preferencesService, @@ -150,7 +152,7 @@ public LibraryTab(JabRefFrame frame, }); } - public void setDataLoadingTask(BackgroundTask dataLoadingTask) { + public void setDataLoadingTask(BackgroundTask dataLoadingTask) { this.dataLoadingTask = dataLoadingTask; } @@ -161,14 +163,34 @@ public BackgroundTask getDataLoadingTask() { return dataLoadingTask; } - public static LibraryTab createNewEmptyLibraryTab(JabRefFrame frame, Path file, BackgroundTask dataLoadingTask) { - BibDatabaseContext context = new BibDatabaseContext(); - context.setDatabasePath(file); + /* The layout to display in the tab when it's loading*/ + public Node createLoadingLayout() { + ProgressIndicator progressIndicator = new ProgressIndicator(ProgressIndicator.INDETERMINATE_PROGRESS); + BorderPane pane = new BorderPane(); + pane.setCenter(progressIndicator); - LibraryTab tab = new LibraryTab(frame, frame.prefs(), context, ExternalFileTypes.getInstance()); - tab.setDataLoadingTask(dataLoadingTask); + return pane; + } + + public void onDatabaseLoadingStarted() { + Node loadingLayout = createLoadingLayout(); + getMainTable().placeholderProperty().setValue(loadingLayout); + + frame.addTab(this, true); + } + + public void onDatabaseLoadingSucceed(ParserResult result) { + BibDatabaseContext context = result.getDatabaseContext(); + feedData(context); - return tab; + OpenDatabaseAction.performPostOpenActions(this, result); + } + + public void onDatabaseLoadingFailed(Exception ex) { + String title = Localization.lang("Connection error"); + String content = String.format("%s\n\n%s", ex.getMessage(), Localization.lang("A local copy will be opened.")); + + dialogService.showErrorDialogAndWait(title, content, ex); } public void feedData(BibDatabaseContext bibDatabaseContext) { @@ -210,7 +232,6 @@ public void feedData(BibDatabaseContext bibDatabaseContext) { } BackupManager.start(this.bibDatabaseContext, Globals.entryTypesManager, Globals.prefs); - } private boolean isDatabaseReadyForAutoSave(BibDatabaseContext context) { @@ -811,4 +832,28 @@ public void listen(EntriesRemovedEvent removedEntriesEvent) { DefaultTaskExecutor.runInJavaFXThread(() -> frame.getGlobalSearchBar().performSearch()); } } + + public static class Factory { + public LibraryTab createModernLibraryTab(JabRefFrame frame, Path file, BackgroundTask dataLoadingTask) { + BibDatabaseContext context = new BibDatabaseContext(); + context.setDatabasePath(file); + + LibraryTab newTab = new LibraryTab(frame, frame.prefs(), context, ExternalFileTypes.getInstance()); + newTab.setDataLoadingTask(dataLoadingTask); + + dataLoadingTask.onRunning(newTab::onDatabaseLoadingStarted) + .onSuccess(newTab::onDatabaseLoadingSucceed) + .onFailure(newTab::onDatabaseLoadingFailed) + .executeWith(Globals.TASK_EXECUTOR); + + return newTab; + } + + public LibraryTab createClassicLibraryTab(JabRefFrame frame, + PreferencesService preferencesService, + BibDatabaseContext bibDatabaseContext, + ExternalFileTypes externalFileTypes) { + return new LibraryTab(frame, preferencesService, bibDatabaseContext, externalFileTypes); + } + } } diff --git a/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java b/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java index 5daa2851ab5..6b6be9724e0 100644 --- a/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java +++ b/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java @@ -13,10 +13,6 @@ import java.util.Objects; import java.util.Optional; -import javafx.scene.Node; -import javafx.scene.control.ProgressIndicator; -import javafx.scene.layout.BorderPane; - import org.jabref.gui.DialogService; import org.jabref.gui.Globals; import org.jabref.gui.JabRefFrame; @@ -36,7 +32,6 @@ import org.jabref.logic.shared.exception.InvalidDBMSConnectionPropertiesException; import org.jabref.logic.shared.exception.NotASharedDatabaseException; import org.jabref.logic.util.StandardFileType; -import org.jabref.model.database.BibDatabaseContext; import org.jabref.preferences.JabRefPreferences; import org.slf4j.Logger; @@ -171,12 +166,10 @@ private void openTheFile(Path file, boolean raisePanel) { } BackgroundTask backgroundTask = BackgroundTask.wrap(() -> loadDatabase(file)); - LibraryTab libraryTab = LibraryTab.createNewEmptyLibraryTab(frame, file, backgroundTask); - onDatabaseLoadingStarted(libraryTab); + LibraryTab.Factory libraryTabCreator = new LibraryTab.Factory(); + LibraryTab newTab = libraryTabCreator.createModernLibraryTab(frame, file, backgroundTask); - backgroundTask.onSuccess(result -> onDatabaseLoadingSucceed(libraryTab, result)) - .onFailure(ex -> onDatabaseLoadingFailed(libraryTab, ex)) - .executeWith(Globals.TASK_EXECUTOR); + backgroundTask.onFinished(() -> trackOpenNewDatabase(newTab)); } private ParserResult loadDatabase(Path file) throws Exception { @@ -225,35 +218,4 @@ private void trackOpenNewDatabase(LibraryTab libraryTab) { Globals.getTelemetryClient().ifPresent(client -> client.trackEvent("OpenNewDatabase", properties, measurements)); } - - /* The layout to display in the tab when it's loading*/ - public Node createLoadingLayout() { - ProgressIndicator progressIndicator = new ProgressIndicator(ProgressIndicator.INDETERMINATE_PROGRESS); - BorderPane pane = new BorderPane(); - pane.setCenter(progressIndicator); - - return pane; - } - - public void onDatabaseLoadingStarted(LibraryTab libraryTab) { - Node loadingLayout = createLoadingLayout(); - libraryTab.setContent(loadingLayout); - - frame.addTab(libraryTab, true); - } - - public void onDatabaseLoadingSucceed(LibraryTab libraryTab, ParserResult result) { - BibDatabaseContext context = result.getDatabaseContext(); - libraryTab.feedData(context); - - OpenDatabaseAction.performPostOpenActions(libraryTab, result); - trackOpenNewDatabase(libraryTab); - } - - public void onDatabaseLoadingFailed(LibraryTab libraryTab, Exception ex) { - String title = Localization.lang("Connection error"); - String content = String.format("%s\n\n%s", ex.getMessage(), Localization.lang("A local copy will be opened.")); - - dialogService.showErrorDialogAndWait(title, content, ex); - } } From b7ca2d379021300818bb1ee5a50532f3f2d36203 Mon Sep 17 00:00:00 2001 From: Houssem Nasri Date: Tue, 24 Nov 2020 19:00:53 +0100 Subject: [PATCH 10/14] Put performPostOpenActions() before feedData() for performance reasons --- src/main/java/org/jabref/gui/LibraryTab.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/jabref/gui/LibraryTab.java b/src/main/java/org/jabref/gui/LibraryTab.java index baef0327cd7..0d01659b61c 100644 --- a/src/main/java/org/jabref/gui/LibraryTab.java +++ b/src/main/java/org/jabref/gui/LibraryTab.java @@ -181,9 +181,9 @@ public void onDatabaseLoadingStarted() { public void onDatabaseLoadingSucceed(ParserResult result) { BibDatabaseContext context = result.getDatabaseContext(); - feedData(context); - OpenDatabaseAction.performPostOpenActions(this, result); + + feedData(context); } public void onDatabaseLoadingFailed(Exception ex) { From 2e71e7a3c5297cbe34844e21d7b016b958c5702b Mon Sep 17 00:00:00 2001 From: Houssem Nasri Date: Tue, 24 Nov 2020 23:46:30 +0100 Subject: [PATCH 11/14] Fix groups pane not updating bug --- src/main/java/org/jabref/gui/LibraryTab.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/main/java/org/jabref/gui/LibraryTab.java b/src/main/java/org/jabref/gui/LibraryTab.java index 0d01659b61c..5da3586c57e 100644 --- a/src/main/java/org/jabref/gui/LibraryTab.java +++ b/src/main/java/org/jabref/gui/LibraryTab.java @@ -184,6 +184,12 @@ public void onDatabaseLoadingSucceed(ParserResult result) { OpenDatabaseAction.performPostOpenActions(this, result); feedData(context); + // a temporary workaround to update groups pane + Globals.stateManager.activeDatabaseProperty().bind( + EasyBind.map(frame.getTabbedPane().getSelectionModel().selectedItemProperty(), + selectedTab -> Optional.ofNullable(selectedTab) + .map(tab -> (LibraryTab) tab) + .map(LibraryTab::getBibDatabaseContext))); } public void onDatabaseLoadingFailed(Exception ex) { From 5783b3078297d39019f7149496bb1b3dfe5d4a54 Mon Sep 17 00:00:00 2001 From: Houssem Nasri Date: Fri, 27 Nov 2020 23:07:34 +0100 Subject: [PATCH 12/14] Cleanup code --- src/main/java/org/jabref/gui/LibraryTab.java | 7 ++----- .../gui/importer/actions/OpenDatabaseAction.java | 14 ++------------ 2 files changed, 4 insertions(+), 17 deletions(-) diff --git a/src/main/java/org/jabref/gui/LibraryTab.java b/src/main/java/org/jabref/gui/LibraryTab.java index 5da3586c57e..60d1fe905b5 100644 --- a/src/main/java/org/jabref/gui/LibraryTab.java +++ b/src/main/java/org/jabref/gui/LibraryTab.java @@ -157,14 +157,11 @@ public void setDataLoadingTask(BackgroundTask dataLoadingTask) { } public BackgroundTask getDataLoadingTask() { - if (dataLoadingTask == null) { - return null; - } return dataLoadingTask; } /* The layout to display in the tab when it's loading*/ - public Node createLoadingLayout() { + public Node createLoadingAnimationLayout() { ProgressIndicator progressIndicator = new ProgressIndicator(ProgressIndicator.INDETERMINATE_PROGRESS); BorderPane pane = new BorderPane(); pane.setCenter(progressIndicator); @@ -173,7 +170,7 @@ public Node createLoadingLayout() { } public void onDatabaseLoadingStarted() { - Node loadingLayout = createLoadingLayout(); + Node loadingLayout = createLoadingAnimationLayout(); getMainTable().placeholderProperty().setValue(loadingLayout); frame.addTab(this, true); diff --git a/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java b/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java index 6b6be9724e0..a2d488fe70a 100644 --- a/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java +++ b/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java @@ -166,8 +166,8 @@ private void openTheFile(Path file, boolean raisePanel) { } BackgroundTask backgroundTask = BackgroundTask.wrap(() -> loadDatabase(file)); - LibraryTab.Factory libraryTabCreator = new LibraryTab.Factory(); - LibraryTab newTab = libraryTabCreator.createModernLibraryTab(frame, file, backgroundTask); + LibraryTab.Factory libraryTabFactory = new LibraryTab.Factory(); + LibraryTab newTab = libraryTabFactory.createModernLibraryTab(frame, file, backgroundTask); backgroundTask.onFinished(() -> trackOpenNewDatabase(newTab)); } @@ -201,16 +201,6 @@ private ParserResult loadDatabase(Path file) throws Exception { return result; } - private LibraryTab addNewDatabase(ParserResult result, final Path file, boolean raisePanel) { - if (result.hasWarnings()) { - ParserResultWarningDialog.showParserResultWarningDialog(result, frame); - } - - LibraryTab libraryTab = new LibraryTab(frame, Globals.prefs, result.getDatabaseContext(), ExternalFileTypes.getInstance()); - frame.addTab(libraryTab, raisePanel); - return libraryTab; - } - private void trackOpenNewDatabase(LibraryTab libraryTab) { Map properties = new HashMap<>(); Map measurements = new HashMap<>(); From 6c3149ce7ee0996c77aa7fbc7be1334d8dbf323a Mon Sep 17 00:00:00 2001 From: Carl Christian Snethlage Date: Sat, 5 Dec 2020 20:24:36 +0100 Subject: [PATCH 13/14] Extracted JabRefPreferences, fixed imports --- src/main/java/org/jabref/gui/JabRefFrame.java | 7 +++++-- src/main/java/org/jabref/gui/LibraryTab.java | 3 +-- .../jabref/gui/importer/actions/OpenDatabaseAction.java | 2 -- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/jabref/gui/JabRefFrame.java b/src/main/java/org/jabref/gui/JabRefFrame.java index 9e3e2e7f6c5..9c54e679995 100644 --- a/src/main/java/org/jabref/gui/JabRefFrame.java +++ b/src/main/java/org/jabref/gui/JabRefFrame.java @@ -338,7 +338,8 @@ public JabRefPreferences prefs() { *

* FIXME: Currently some threads remain and therefore hinder JabRef to be closed properly * - * @param filenames the filenames of all currently opened files - used for storing them if prefs openLastEdited is set to true + * @param filenames the filenames of all currently opened files - used for storing them if prefs openLastEdited is + * set to true */ private void tearDownJabRef(List filenames) { // prefs.putBoolean(JabRefPreferences.WINDOW_MAXIMISED, getExtendedState() == Frame.MAXIMIZED_BOTH); @@ -361,7 +362,9 @@ private void tearDownJabRef(List filenames) { } /** - * General info dialog. The MacAdapter calls this method when "Quit" is selected from the application menu, Cmd-Q is pressed, or "Quit" is selected from the Dock. The function returns a boolean indicating if quitting is ok or not. + * General info dialog. The MacAdapter calls this method when "Quit" is selected from the application menu, Cmd-Q + * is pressed, or "Quit" is selected from the Dock. The function returns a boolean indicating if quitting is ok or + * not. *

* Non-OSX JabRef calls this when choosing "Quit" from the menu *

diff --git a/src/main/java/org/jabref/gui/LibraryTab.java b/src/main/java/org/jabref/gui/LibraryTab.java index 60d1fe905b5..fc0aeddfb3c 100644 --- a/src/main/java/org/jabref/gui/LibraryTab.java +++ b/src/main/java/org/jabref/gui/LibraryTab.java @@ -60,7 +60,6 @@ import org.jabref.model.entry.event.EntryChangedEvent; import org.jabref.model.entry.field.Field; import org.jabref.model.entry.field.FieldFactory; -import org.jabref.preferences.JabRefPreferences; import org.jabref.preferences.PreferencesService; import com.google.common.eventbus.Subscribe; @@ -239,7 +238,7 @@ public void feedData(BibDatabaseContext bibDatabaseContext) { private boolean isDatabaseReadyForAutoSave(BibDatabaseContext context) { return ((context.getLocation() == DatabaseLocation.SHARED) || - ((context.getLocation() == DatabaseLocation.LOCAL) && Globals.prefs.getBoolean(JabRefPreferences.LOCAL_AUTO_SAVE))) + ((context.getLocation() == DatabaseLocation.LOCAL) && preferencesService.getShouldAutosave())) && context.getDatabasePath().isPresent(); } diff --git a/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java b/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java index a2d488fe70a..46b19adb956 100644 --- a/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java +++ b/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java @@ -19,8 +19,6 @@ import org.jabref.gui.LibraryTab; import org.jabref.gui.actions.SimpleCommand; import org.jabref.gui.dialogs.BackupUIManager; -import org.jabref.gui.externalfiletype.ExternalFileTypes; -import org.jabref.gui.importer.ParserResultWarningDialog; import org.jabref.gui.shared.SharedDatabaseUIManager; import org.jabref.gui.util.BackgroundTask; import org.jabref.gui.util.FileDialogConfiguration; From 8b32c0953afc19d22abed054e4ac7d03d7d63b7d Mon Sep 17 00:00:00 2001 From: Carl Christian Snethlage Date: Sat, 5 Dec 2020 20:33:01 +0100 Subject: [PATCH 14/14] Extracted JabRefPreferences, fixed imports, removed dead code, shortened method name --- src/main/java/org/jabref/gui/LibraryTab.java | 39 ++++++++++--------- .../importer/actions/OpenDatabaseAction.java | 2 +- 2 files changed, 22 insertions(+), 19 deletions(-) diff --git a/src/main/java/org/jabref/gui/LibraryTab.java b/src/main/java/org/jabref/gui/LibraryTab.java index fc0aeddfb3c..b8f1f8aabf5 100644 --- a/src/main/java/org/jabref/gui/LibraryTab.java +++ b/src/main/java/org/jabref/gui/LibraryTab.java @@ -246,7 +246,8 @@ private boolean isDatabaseReadyForAutoSave(BibDatabaseContext context) { /** * Sets the title of the tab modification-asterisk filename – path-fragment *

- * The modification-asterisk (*) is shown if the file was modified since last save (path-fragment is only shown if filename is not (globally) unique) + * The modification-asterisk (*) is shown if the file was modified since last save (path-fragment is only shown if + * filename is not (globally) unique) *

* Example: *jabref-authors.bib – testbib */ @@ -376,7 +377,8 @@ public JabRefFrame frame() { /** * Removes the selected entries from the database * - * @param cut If false the user will get asked if he really wants to delete the entries, and it will be localized as "deleted". If true the action will be localized as "cut" + * @param cut If false the user will get asked if he really wants to delete the entries, and it will be localized as + * "deleted". If true the action will be localized as "cut" */ public void delete(boolean cut) { delete(cut, mainTable.getSelectedEntries()); @@ -385,7 +387,8 @@ public void delete(boolean cut) { /** * Removes the selected entries from the database * - * @param cut If false the user will get asked if he really wants to delete the entries, and it will be localized as "deleted". If true the action will be localized as "cut" + * @param cut If false the user will get asked if he really wants to delete the entries, and it will be localized as + * "deleted". If true the action will be localized as "cut" */ private void delete(boolean cut, List entries) { if (entries.isEmpty()) { @@ -428,7 +431,8 @@ public void insertEntry(final BibEntry bibEntry) { } /** - * This method is called from JabRefFrame when the user wants to create a new entry or entries. It is necessary when the user would expect the added entry or one of the added entries to be selected in the entry editor + * This method is called from JabRefFrame when the user wants to create a new entry or entries. It is necessary when + * the user would expect the added entry or one of the added entries to be selected in the entry editor * * @param entries The new entries. */ @@ -541,7 +545,8 @@ public EntryEditor getEntryEditor() { } /** - * Sets the entry editor as the bottom component in the split pane. If an entry editor already was shown, makes sure that the divider doesn't move. Updates the mode to SHOWING_EDITOR. Then shows the given entry. + * Sets the entry editor as the bottom component in the split pane. If an entry editor already was shown, makes sure + * that the divider doesn't move. Updates the mode to SHOWING_EDITOR. Then shows the given entry. * * @param entry The entry to edit. */ @@ -580,7 +585,8 @@ public void closeBottomPane() { } /** - * This method selects the given entry, and scrolls it into view in the table. If an entryEditor is shown, it is given focus afterwards. + * This method selects the given entry, and scrolls it into view in the table. If an entryEditor is shown, it is + * given focus afterwards. */ public void clearAndSelect(final BibEntry bibEntry) { mainTable.clearAndSelect(bibEntry); @@ -595,7 +601,8 @@ public void selectNextEntry() { } /** - * This method is called from an EntryEditor when it should be closed. We relay to the selection listener, which takes care of the rest. + * This method is called from an EntryEditor when it should be closed. We relay to the selection listener, which + * takes care of the rest. */ public void entryEditorClosing() { closeBottomPane(); @@ -663,7 +670,8 @@ private boolean showDeleteConfirmationDialog(int numberOfEntries) { } /** - * Depending on whether a preview or an entry editor is showing, save the current divider location in the correct preference setting. + * Depending on whether a preview or an entry editor is showing, save the current divider location in the correct + * preference setting. */ private void saveDividerLocation(Number position) { if (mode == BasePanelMode.SHOWING_EDITOR) { @@ -682,7 +690,8 @@ public void cleanUp() { } /** - * Get an array containing the currently selected entries. The array is stable and not changed if the selection changes + * Get an array containing the currently selected entries. The array is stable and not changed if the selection + * changes * * @return A list containing the selected entries. Is never null. */ @@ -814,7 +823,8 @@ public void listen(EntriesRemovedEvent entriesRemovedEvent) { } /** - * Ensures that the results of the current search are updated when a new entry is inserted into the database Actual methods for performing search must run in javafx thread + * Ensures that the results of the current search are updated when a new entry is inserted into the database Actual + * methods for performing search must run in javafx thread */ private class SearchListener { @@ -836,7 +846,7 @@ public void listen(EntriesRemovedEvent removedEntriesEvent) { } public static class Factory { - public LibraryTab createModernLibraryTab(JabRefFrame frame, Path file, BackgroundTask dataLoadingTask) { + public LibraryTab createLibraryTab(JabRefFrame frame, Path file, BackgroundTask dataLoadingTask) { BibDatabaseContext context = new BibDatabaseContext(); context.setDatabasePath(file); @@ -850,12 +860,5 @@ public LibraryTab createModernLibraryTab(JabRefFrame frame, Path file, Backgroun return newTab; } - - public LibraryTab createClassicLibraryTab(JabRefFrame frame, - PreferencesService preferencesService, - BibDatabaseContext bibDatabaseContext, - ExternalFileTypes externalFileTypes) { - return new LibraryTab(frame, preferencesService, bibDatabaseContext, externalFileTypes); - } } } diff --git a/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java b/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java index 46b19adb956..133b436ef49 100644 --- a/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java +++ b/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java @@ -165,7 +165,7 @@ private void openTheFile(Path file, boolean raisePanel) { BackgroundTask backgroundTask = BackgroundTask.wrap(() -> loadDatabase(file)); LibraryTab.Factory libraryTabFactory = new LibraryTab.Factory(); - LibraryTab newTab = libraryTabFactory.createModernLibraryTab(frame, file, backgroundTask); + LibraryTab newTab = libraryTabFactory.createLibraryTab(frame, file, backgroundTask); backgroundTask.onFinished(() -> trackOpenNewDatabase(newTab)); }