Refine BackupManager (#9054)

* Refine BackupManager Co-authored-by: Christoph <siedlerkiller@gmail.com> Co-authored-by: Carl Christian Snethlage <50491877+calixtus@users.noreply.github.com> * Fix BackupManager Co-authored-by: Carl Christian Snethlage <50491877+calixtus@users.noreply.github.com> * Exchange "bak" and "sav" Co-authored-by: Christoph <siedlerkiller@gmail.com> * Refine backup message to show backup path Co-authored-by: Christoph <siedlerkiller@gmail.com> * Introduce BackupFileUtil * Use AtomicFileWriter * Update src/main/java/org/jabref/logic/util/io/BackupFileUtil.java Co-authored-by: Christoph <siedlerkiller@gmail.com> * More comments * Align JabRefGUI#openLastEditedDatabases() with OpenDatabaseAction#loadDatabase(Path) * Remove unused parameter "raisePanel" * WIP: Fix opening of databases i) on startup, ii) using File > Open - i: Reuse OpenDataBaseAction (instead of code duplication) - ii: Remove double initialization of BackupManager * Experiment with needsBackup * Fix variable name * Remove obsolete method * Remove duplicate code * Switch from DelayTaskThrottler to Java's ScheduledThreadPoolExecutor * Group together code cloned methods * Enable "needsBackup" strategy * Ensure that backup is a recent one * Fix checkstyle * Removed JabRefGUI import * Introdce OS.APP_DIR_APP_* Co-authored-by: Christoph <siedlerkiller@gmail.com> Co-authored-by: Carl Christian Snethlage <50491877+calixtus@users.noreply.github.com> Co-authored-by: ThiloteE <73715071+ThiloteE@users.noreply.github.com> Co-authored-by: Houssem Nasri <housi.housi2015@gmail.com> Co-authored-by: Benedikt Tutzer <btut@users.noreply.github.com> * Fix checkstyle * Try to fix tests * Remove check for concrete hash code * Fix checkstyle * Refine comment on expected behavior Co-authored-by: Christoph <siedlerkiller@gmail.com> Co-authored-by: Carl Christian Snethlage <50491877+calixtus@users.noreply.github.com> Co-authored-by: ThiloteE <73715071+ThiloteE@users.noreply.github.com> Co-authored-by: Houssem Nasri <housi.housi2015@gmail.com> Co-authored-by: Benedikt Tutzer <btut@users.noreply.github.com>
JabRef · Aug 15, 2022 · f7648e8 · f7648e8
1 parent 132921c
commit f7648e8
Show file tree

Hide file tree

Showing 33 changed files with 478 additions and 179 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,13 +11,17 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
 
 ### Added
 
+- In case a backup is found, the filename of the backup is shown.
+- On startup, JabRef notifies the user if there were parsing errors during opening.
 - We integrated a new three-way merge UI for merging entries in the Entries Merger Dialog, the Duplicate Resolver Dialog, the Entry Importer Dialog, and the External Changes Resolver Dialog. [#8945](https://github.com/JabRef/jabref/pull/8945)
 - We added the ability to merge groups, keywords, comments and files when merging entries. [#9022](https://github.com/JabRef/jabref/pull/9022)
 
 ### Changed
 
 - We improved the Citavi Importer to also import so called Knowledge-items into the field `comment` of the corresponding entry [#9025](https://github.com/JabRef/jabref/issues/9025)
 - We removed wrapping of string constants when writing to a `.bib` file.
+- We call backup files `.bak` and temporary writing files now `.sav`.
+- JabRef keeps 10 older versions of a `.bib` file in the [user data dir](https://github.com/harawata/appdirs#supported-directories) (instead of a single `.sav` (now: `.bak`) file in the directory of the `.bib` file)
 - We changed the button label from "Return to JabRef" to "Return to library" to better indicate the purpose of the action.
 
 ### Fixed

diff --git a/src/main/java/org/jabref/gui/JabRefFrame.java b/src/main/java/org/jabref/gui/JabRefFrame.java
@@ -61,7 +61,6 @@
 import org.jabref.gui.copyfiles.CopyFilesAction;
 import org.jabref.gui.customentrytypes.CustomizeEntryAction;
 import org.jabref.gui.desktop.JabRefDesktop;
-import org.jabref.gui.dialogs.AutosaveUiManager;
 import org.jabref.gui.documentviewer.ShowDocumentViewerAction;
 import org.jabref.gui.duplicationFinder.DuplicateSearch;
 import org.jabref.gui.edit.CopyMoreAction;
@@ -257,7 +256,7 @@ private void initDragAndDrop() {
                             tabbedPane.getTabs().remove(dndIndicator);
                             List<Path> bibFiles = DragAndDropHelper.getBibFiles(tabDragEvent.getDragboard());
                             OpenDatabaseAction openDatabaseAction = this.getOpenDatabaseAction();
-                            openDatabaseAction.openFiles(bibFiles, true);
+                            openDatabaseAction.openFiles(bibFiles);
                             tabDragEvent.setDropCompleted(true);
                             tabDragEvent.consume();
                         } else {
@@ -280,7 +279,7 @@ private void initDragAndDrop() {
                 tabbedPane.getTabs().remove(dndIndicator);
                 List<Path> bibFiles = DragAndDropHelper.getBibFiles(event.getDragboard());
                 OpenDatabaseAction openDatabaseAction = this.getOpenDatabaseAction();
-                openDatabaseAction.openFiles(bibFiles, true);
+                openDatabaseAction.openFiles(bibFiles);
                 event.setDropCompleted(true);
                 event.consume();
             });
@@ -380,8 +379,7 @@ private void showTrackingNotification() {
      */
     public void openAction(String filePath) {
         Path file = Path.of(filePath);
-        // all the logic is done in openIt. Even raising an existing panel
-        getOpenDatabaseAction().openFile(file, true);
+        getOpenDatabaseAction().openFile(file);
     }
 
     /**
@@ -1048,6 +1046,11 @@ hide it and clip it to a square of (width x width) each time width is updated.
         return new Group(indicator);
     }
 
+    /**
+     * Might be called when a user asks JabRef at the command line
+     * i) to import a file or
+     * ii) to open a .bib file
+     */
     public void addParserResult(ParserResult parserResult, boolean focusPanel) {
         if (parserResult.toOpenTab()) {
             // Add the entries to the open tab.
@@ -1059,7 +1062,7 @@ public void addParserResult(ParserResult parserResult, boolean focusPanel) {
                 addImportedEntries(libraryTab, parserResult);
             }
         } else {
-            // only add tab if DB is not already open
+            // only add tab if library is not already open
             Optional<LibraryTab> libraryTab = getLibraryTabs().stream()
                                                               .filter(p -> p.getBibDatabaseContext()
                                                                             .getDatabasePath()
@@ -1119,17 +1122,6 @@ public void addTab(LibraryTab libraryTab, boolean raisePanel) {
         }
 
         libraryTab.getUndoManager().registerListener(new UndoRedoEventManager());
-
-        BibDatabaseContext context = libraryTab.getBibDatabaseContext();
-
-        if (readyForAutosave(context)) {
-            AutosaveManager autosaver = AutosaveManager.start(context);
-            autosaver.registerListener(new AutosaveUiManager(libraryTab));
-        }
-
-        BackupManager.start(context, Globals.entryTypesManager, prefs);
-
-        trackOpenNewDatabase(libraryTab);
     }
 
     private void trackOpenNewDatabase(LibraryTab libraryTab) {
@@ -1147,13 +1139,6 @@ public LibraryTab addTab(BibDatabaseContext databaseContext, boolean raisePanel)
         return libraryTab;
     }
 
-    private boolean readyForAutosave(BibDatabaseContext context) {
-        return ((context.getLocation() == DatabaseLocation.SHARED)
-                || ((context.getLocation() == DatabaseLocation.LOCAL)
-                && prefs.getImportExportPreferences().shouldAutoSave()))
-                && context.getDatabasePath().isPresent();
-    }
-
     /**
      * Opens the import inspection dialog to let the user decide which of the given entries to import.
      *

diff --git a/src/main/java/org/jabref/gui/JabRefGUI.java b/src/main/java/org/jabref/gui/JabRefGUI.java
@@ -1,7 +1,5 @@
 package org.jabref.gui;
 
-import java.io.IOException;
-import java.nio.file.Files;
 import java.nio.file.Path;
 import java.sql.SQLException;
 import java.util.ArrayList;
@@ -14,15 +12,12 @@
 import javafx.stage.Screen;
 import javafx.stage.Stage;
 
-import org.jabref.gui.dialogs.BackupUIManager;
 import org.jabref.gui.help.VersionWorker;
 import org.jabref.gui.icon.IconTheme;
 import org.jabref.gui.importer.ParserResultWarningDialog;
 import org.jabref.gui.importer.actions.OpenDatabaseAction;
 import org.jabref.gui.keyboard.TextInputKeyBindings;
 import org.jabref.gui.shared.SharedDatabaseUIManager;
-import org.jabref.logic.autosaveandbackup.BackupManager;
-import org.jabref.logic.importer.OpenDatabase;
 import org.jabref.logic.importer.ParserResult;
 import org.jabref.logic.l10n.Localization;
 import org.jabref.logic.shared.DatabaseNotSupportedException;
@@ -77,8 +72,8 @@ private void openWindow(Stage mainStage) {
         if (guiPreferences.isWindowMaximised()) {
             mainStage.setMaximized(true);
         } else if ((Screen.getScreens().size() == 1) && isWindowPositionOutOfBounds()) {
-            // corrects the Window, if its outside of the mainscreen
-            LOGGER.debug("The Jabref Window is outside the Main Monitor\n");
+            // corrects the Window, if it is outside the mainscreen
+            LOGGER.debug("The Jabref window is outside the main screen\n");
             mainStage.setX(0);
             mainStage.setY(0);
             mainStage.setWidth(1024);
@@ -135,6 +130,8 @@ private void openDatabases() {
             openLastEditedDatabases();
         }
 
+        // From here on, the libraries provided by command line arguments are treated
+
         // Remove invalid databases
         List<ParserResult> invalidDatabases = bibDatabases.stream()
                                                           .filter(ParserResult::isInvalid)
@@ -266,34 +263,8 @@ private void openLastEditedDatabases() {
             return;
         }
 
-        for (String fileName : lastFiles) {
-            Path dbFile = Path.of(fileName);
-
-            // Already parsed via command line parameter, e.g., "jabref.jar somefile.bib"
-            if (isLoaded(dbFile) || !Files.exists(dbFile)) {
-                continue;
-            }
-
-            if (BackupManager.backupFileDiffers(dbFile)) {
-                BackupUIManager.showRestoreBackupDialog(mainFrame.getDialogService(), dbFile);
-            }
-
-            ParserResult parsedDatabase;
-            try {
-                parsedDatabase = OpenDatabase.loadDatabase(
-                        dbFile,
-                        preferencesService.getImportFormatPreferences(),
-                        Globals.getFileUpdateMonitor());
-            } catch (IOException ex) {
-                LOGGER.error("Error opening file '{}'", dbFile, ex);
-                parsedDatabase = ParserResult.fromError(ex);
-            }
-            bibDatabases.add(parsedDatabase);
-        }
-    }
-
-    private boolean isLoaded(Path fileToOpen) {
-        return bibDatabases.stream().anyMatch(pr -> pr.getPath().isPresent() && pr.getPath().get().equals(fileToOpen));
+        List<Path> filesToOpen = lastFiles.stream().map(file -> Path.of(file)).collect(Collectors.toList());
+        getMainFrame().getOpenDatabaseAction().openFiles(filesToOpen);
     }
 
     public static JabRefFrame getMainFrame() {

diff --git a/src/main/java/org/jabref/gui/LibraryTab.java b/src/main/java/org/jabref/gui/LibraryTab.java
@@ -212,6 +212,7 @@ public void onDatabaseLoadingSucceed(ParserResult result) {
         OpenDatabaseAction.performPostOpenActions(this, result);
 
         feedData(context);
+
         // a temporary workaround to update groups pane
         stateManager.activeDatabaseProperty().bind(
                 EasyBind.map(frame.getTabbedPane().getSelectionModel().selectedItemProperty(),
@@ -261,12 +262,17 @@ public void feedData(BibDatabaseContext bibDatabaseContext) {
                     updateTabTitle(changedProperty.getValue()));
         });
 
+        installAutosaveManagerAndBackupManager();
+    }
+
+    public void installAutosaveManagerAndBackupManager() {
         if (isDatabaseReadyForAutoSave(bibDatabaseContext)) {
-            AutosaveManager autoSaver = AutosaveManager.start(bibDatabaseContext);
-            autoSaver.registerListener(new AutosaveUiManager(this));
+            AutosaveManager autosaveManager = AutosaveManager.start(bibDatabaseContext);
+            autosaveManager.registerListener(new AutosaveUiManager(this));
+        }
+        if (isDatabaseReadyForBackup(bibDatabaseContext)) {
+            BackupManager.start(bibDatabaseContext, Globals.entryTypesManager, preferencesService);
         }
-
-        BackupManager.start(this.bibDatabaseContext, Globals.entryTypesManager, preferencesService);
     }
 
     private boolean isDatabaseReadyForAutoSave(BibDatabaseContext context) {
@@ -276,6 +282,10 @@ private boolean isDatabaseReadyForAutoSave(BibDatabaseContext context) {
                 && context.getDatabasePath().isPresent();
     }
 
+    private boolean isDatabaseReadyForBackup(BibDatabaseContext context) {
+        return (context.getLocation() == DatabaseLocation.LOCAL) && context.getDatabasePath().isPresent();
+    }
+
     /**
      * Sets the title of the tab modification-asterisk filename – path-fragment
      * <p>

diff --git a/src/main/java/org/jabref/gui/dialogs/BackupUIManager.java b/src/main/java/org/jabref/gui/dialogs/BackupUIManager.java
@@ -6,6 +6,8 @@
 import org.jabref.gui.util.DefaultTaskExecutor;
 import org.jabref.logic.autosaveandbackup.BackupManager;
 import org.jabref.logic.l10n.Localization;
+import org.jabref.logic.util.BackupFileType;
+import org.jabref.logic.util.io.BackupFileUtil;
 
 /**
  * Stores all user dialogs related to {@link BackupManager}.
@@ -17,7 +19,10 @@ private BackupUIManager() {
 
     public static void showRestoreBackupDialog(DialogService dialogService, Path originalPath) {
         String content = new StringBuilder()
-                .append(Localization.lang("A backup file for '%0' was found.", originalPath.getFileName().toString()))
+                .append(Localization.lang("A backup file for '%0' was found at '%1'.",
+                        originalPath.getFileName().toString(),
+                        // We need to determine the path "manually" as the path does not get passed through when a diff is detected.
+                        BackupFileUtil.getPathOfLatestExisingBackupFile(originalPath, BackupFileType.BACKUP).map(Path::toString).orElse(Localization.lang("File not found"))))
                 .append("\n")
                 .append(Localization.lang("This could indicate that JabRef did not shut down cleanly last time the file was used."))
                 .append("\n\n")

diff --git a/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java b/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java
@@ -19,7 +19,6 @@
 import org.jabref.gui.DialogService;
 import org.jabref.gui.JabRefFrame;
 import org.jabref.gui.LibraryTab;
-import org.jabref.gui.dialogs.AutosaveUiManager;
 import org.jabref.gui.util.BackgroundTask;
 import org.jabref.gui.util.FileDialogConfiguration;
 import org.jabref.logic.autosaveandbackup.AutosaveManager;
@@ -104,7 +103,7 @@ public void saveSelectedAsPlain() {
     }
 
     /**
-     * @param file the new file name to save the data base to. This is stored in the database context of the panel upon
+     * @param file the new file name to save the database to. This is stored in the database context of the panel upon
      *             successful save.
      * @return true on successful save
      */
@@ -131,19 +130,13 @@ boolean saveAs(Path file, SaveDatabaseMode mode) {
 
         if (saveResult) {
             // we managed to successfully save the file
-            // thus, we can store the store the path into the context
+            // thus, we can store the path into the context
             context.setDatabasePath(file);
             libraryTab.updateTabTitle(false);
 
             // Reset (here: uninstall and install again) AutosaveManager and BackupManager for the new file name
             libraryTab.resetChangeMonitor();
-            if (readyForAutosave(context)) {
-                AutosaveManager autosaver = AutosaveManager.start(context);
-                autosaver.registerListener(new AutosaveUiManager(libraryTab));
-            }
-            if (readyForBackup(context)) {
-                BackupManager.start(context, entryTypesManager, preferences);
-            }
+            libraryTab.installAutosaveManagerAndBackupManager();
 
             frame.getFileHistory().newFile(file);
         }
@@ -284,15 +277,4 @@ private void saveWithDifferentEncoding(Path file, boolean selectedOnly, Charset
             }
         }
     }
-
-    private boolean readyForAutosave(BibDatabaseContext context) {
-        return ((context.getLocation() == DatabaseLocation.SHARED)
-                || ((context.getLocation() == DatabaseLocation.LOCAL)
-                && preferences.getImportExportPreferences().shouldAutoSave()))
-                && context.getDatabasePath().isPresent();
-    }
-
-    private boolean readyForBackup(BibDatabaseContext context) {
-        return (context.getLocation() == DatabaseLocation.LOCAL) && context.getDatabasePath().isPresent();
-    }
 }
diff --git a/src/main/java/org/jabref/gui/externalfiles/FileExtensionViewModel.java b/src/main/java/org/jabref/gui/externalfiles/FileExtensionViewModel.java
@@ -21,7 +21,7 @@ public class FileExtensionViewModel {
 
     FileExtensionViewModel(FileType fileType, FilePreferences filePreferences) {
         this.description = Localization.lang("%0 file", fileType.getName());
-        this.extensions = fileType.getExtensionsWithDot();
+        this.extensions = fileType.getExtensionsWithAsteriskAndDot();
         this.filePreferences = filePreferences;
     }