diff --git a/CHANGELOG.md b/CHANGELOG.md index b9bafd112e6..560c53118de 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ 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) @@ -18,6 +20,8 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve - 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 index 4d07c12aca2..81b027b5b8c 100644 --- 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 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 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 = 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 index 3df6414a972..2acb97939c1 100644 --- 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 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 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 index ad40925cacc..246b9d554ab 100644 --- 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 *

diff --git a/src/main/java/org/jabref/gui/dialogs/BackupUIManager.java b/src/main/java/org/jabref/gui/dialogs/BackupUIManager.java index 8a344ee8bfb..c6f7cab9c5d 100644 --- 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 index 293e03d45be..e22f5790ceb 100644 --- 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 index 6344babd905..edab9aa8644 100644 --- 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; } 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 052dba2ccac..45e95158e29 100644 --- a/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java +++ b/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java @@ -5,8 +5,6 @@ import java.nio.file.Path; import java.sql.SQLException; import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collections; import java.util.HashMap; import java.util.Iterator; import java.util.List; @@ -21,6 +19,7 @@ import org.jabref.gui.StateManager; import org.jabref.gui.actions.SimpleCommand; import org.jabref.gui.dialogs.BackupUIManager; +import org.jabref.gui.menus.FileHistoryMenu; import org.jabref.gui.shared.SharedDatabaseUIManager; import org.jabref.gui.theme.ThemeManager; import org.jabref.gui.util.BackgroundTask; @@ -46,7 +45,7 @@ public class OpenDatabaseAction extends SimpleCommand { // List of actions that may need to be called after opening the file. Such as // upgrade actions etc. that may depend on the JabRef version that wrote the file: - private static final List POST_OPEN_ACTIONS = Arrays.asList( + private static final List POST_OPEN_ACTIONS = List.of( // Migrations: // Warning for migrating the Review into the Comment field new MergeReviewIntoCommentAction(), @@ -95,7 +94,7 @@ public void execute() { .build(); List filesToOpen = dialogService.showFileOpenDialogAndGetMultipleFiles(fileDialogConfiguration); - openFiles(filesToOpen, true); + openFiles(filesToOpen); } /** @@ -111,20 +110,22 @@ private Path getInitialDirectory() { } /** - * Opens the given file. If null or 404, nothing happens + * Opens the given file. If null or 404, nothing happens. + * In case the file is already opened, that panel is raised. * * @param file the file, may be null or not existing */ - public void openFile(Path file, boolean raisePanel) { - openFiles(new ArrayList<>(List.of(file)), raisePanel); + public void openFile(Path file) { + openFiles(new ArrayList<>(List.of(file))); } /** - * Opens the given files. If one of it is null or 404, nothing happens + * Opens the given files. If one of it is null or 404, nothing happens. + * In case the file is already opened, that panel is raised. * * @param filesToOpen the filesToOpen, may be null or not existing */ - public void openFiles(List filesToOpen, boolean raisePanel) { + public void openFiles(List filesToOpen) { LibraryTab toRaise = null; int initialCount = filesToOpen.size(); int removed = 0; @@ -152,16 +153,12 @@ public void openFiles(List filesToOpen, boolean raisePanel) { // Run the actual open in a thread to prevent the program // locking until the file is loaded. if (!filesToOpen.isEmpty()) { - final List theFiles = Collections.unmodifiableList(filesToOpen); - - for (Path theFile : theFiles) { + FileHistoryMenu fileHistory = frame.getFileHistory(); + filesToOpen.forEach(theFile -> { // This method will execute the concrete file opening and loading in a background thread - openTheFile(theFile, raisePanel); - } - - for (Path theFile : theFiles) { - frame.getFileHistory().newFile(theFile); - } + openTheFile(theFile); + fileHistory.newFile(theFile); + }); } else if (toRaise != null) { // If no files are remaining to open, this could mean that a file was // already open. If so, we may have to raise the correct tab: @@ -170,9 +167,11 @@ public void openFiles(List filesToOpen, boolean raisePanel) { } /** - * @param file the file, may be null or not existing + * This is the real file opening. Should be called via {@link #openFile(Path)} + * + * @param file the file, may be NOT null, but may not be existing */ - private void openTheFile(Path file, boolean raisePanel) { + private void openTheFile(Path file) { Objects.requireNonNull(file); if (!Files.exists(file)) { return; @@ -185,6 +184,10 @@ private void openTheFile(Path file, boolean raisePanel) { backgroundTask.onFinished(() -> trackOpenNewDatabase(newTab)); } + /** + * This method is similar to {@link org.jabref.gui.JabRefGUI#openLastEditedDatabases()}. + * This method also has the capability to open remote shared databases + */ private ParserResult loadDatabase(Path file) throws Exception { Path fileToLoad = file.toAbsolutePath(); diff --git a/src/main/java/org/jabref/gui/menus/FileHistoryMenu.java b/src/main/java/org/jabref/gui/menus/FileHistoryMenu.java index 7a748f6d838..43a80641bf1 100644 --- a/src/main/java/org/jabref/gui/menus/FileHistoryMenu.java +++ b/src/main/java/org/jabref/gui/menus/FileHistoryMenu.java @@ -92,6 +92,6 @@ public void openFile(Path file) { setItems(); return; } - openDatabaseAction.openFile(file, true); + openDatabaseAction.openFile(file); } } diff --git a/src/main/java/org/jabref/gui/slr/ExistingStudySearchAction.java b/src/main/java/org/jabref/gui/slr/ExistingStudySearchAction.java index c45b7f2fc1c..284e8c9a487 100644 --- a/src/main/java/org/jabref/gui/slr/ExistingStudySearchAction.java +++ b/src/main/java/org/jabref/gui/slr/ExistingStudySearchAction.java @@ -130,7 +130,7 @@ public void crawl() { dialogService.showErrorDialogAndWait(Localization.lang("Error during persistence of crawling results."), e); }) .onSuccess(unused -> { - new OpenDatabaseAction(frame, preferencesService, dialogService, stateManager, themeManager).openFile(Path.of(studyDirectory.toString(), "studyResult.bib"), true); + new OpenDatabaseAction(frame, preferencesService, dialogService, stateManager, themeManager).openFile(Path.of(studyDirectory.toString(), "studyResult.bib")); // If finished reset command object for next use studyDirectory = null; }) diff --git a/src/main/java/org/jabref/gui/util/FileFilterConverter.java b/src/main/java/org/jabref/gui/util/FileFilterConverter.java index 623fcb54cb2..3f51bf534ee 100644 --- a/src/main/java/org/jabref/gui/util/FileFilterConverter.java +++ b/src/main/java/org/jabref/gui/util/FileFilterConverter.java @@ -28,11 +28,11 @@ private FileFilterConverter() { public static FileChooser.ExtensionFilter toExtensionFilter(FileType fileType) { String description = Localization.lang("%0 file", fileType.toString()); - return new FileChooser.ExtensionFilter(description, fileType.getExtensionsWithDot()); + return new FileChooser.ExtensionFilter(description, fileType.getExtensionsWithAsteriskAndDot()); } public static FileChooser.ExtensionFilter toExtensionFilter(String description, FileType fileType) { - return new FileChooser.ExtensionFilter(description, fileType.getExtensionsWithDot()); + return new FileChooser.ExtensionFilter(description, fileType.getExtensionsWithAsteriskAndDot()); } public static Optional getImporter(FileChooser.ExtensionFilter extensionFilter, Collection importers) { @@ -46,7 +46,7 @@ public static Optional getExporter(FileChooser.ExtensionFilter extensi public static FileChooser.ExtensionFilter forAllImporters(SortedSet importers) { List fileTypes = importers.stream().map(Importer::getFileType).collect(Collectors.toList()); List flatExtensions = fileTypes.stream() - .flatMap(type -> type.getExtensionsWithDot().stream()) + .flatMap(type -> type.getExtensionsWithAsteriskAndDot().stream()) .collect(Collectors.toList()); return new FileChooser.ExtensionFilter(Localization.lang("Available import formats"), flatExtensions); diff --git a/src/main/java/org/jabref/logic/autosaveandbackup/AutosaveManager.java b/src/main/java/org/jabref/logic/autosaveandbackup/AutosaveManager.java index 90824d45e06..3e53d41357d 100644 --- a/src/main/java/org/jabref/logic/autosaveandbackup/AutosaveManager.java +++ b/src/main/java/org/jabref/logic/autosaveandbackup/AutosaveManager.java @@ -61,9 +61,9 @@ private void shutdown() { * @param bibDatabaseContext Associated {@link BibDatabaseContext} */ public static AutosaveManager start(BibDatabaseContext bibDatabaseContext) { - AutosaveManager autosaver = new AutosaveManager(bibDatabaseContext); - runningInstances.add(autosaver); - return autosaver; + AutosaveManager autosaveManager = new AutosaveManager(bibDatabaseContext); + runningInstances.add(autosaveManager); + return autosaveManager; } /** diff --git a/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java b/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java index 65656f71f9a..750f201ee83 100644 --- a/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java +++ b/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java @@ -1,25 +1,31 @@ package org.jabref.logic.autosaveandbackup; import java.io.IOException; +import java.io.Writer; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardCopyOption; import java.util.HashSet; +import java.util.List; import java.util.Optional; +import java.util.Queue; import java.util.Set; import java.util.concurrent.BlockingQueue; import java.util.concurrent.ExecutorService; +import java.util.concurrent.LinkedBlockingQueue; +import java.util.concurrent.ScheduledThreadPoolExecutor; +import java.util.concurrent.TimeUnit; import org.jabref.logic.bibtex.InvalidFieldValueException; import org.jabref.logic.exporter.AtomicFileWriter; import org.jabref.logic.exporter.BibWriter; import org.jabref.logic.exporter.BibtexDatabaseWriter; import org.jabref.logic.exporter.SavePreferences; +import org.jabref.logic.util.BackupFileType; import org.jabref.logic.util.CoarseChangeFilter; -import org.jabref.logic.util.DelayTaskThrottler; -import org.jabref.logic.util.io.FileUtil; +import org.jabref.logic.util.io.BackupFileUtil; import org.jabref.model.database.BibDatabaseContext; import org.jabref.model.database.event.BibDatabaseContextChangedEvent; import org.jabref.model.entry.BibEntryTypesManager; @@ -40,35 +46,55 @@ public class BackupManager { private static final Logger LOGGER = LoggerFactory.getLogger(BackupManager.class); - // This differs from org.jabref.logic.exporter.AtomicFileOutputStream.BACKUP_EXTENSION, which is used for copying the .bib away before overwriting on save. - private static final String AUTOSAVE_FILE_EXTENSION = ".sav"; + private static final int MAXIMUM_BACKUP_FILE_COUNT = 10; + + private static final int DELAY_BETWEEN_BACKUP_ATTEMPTS_IN_SECONDS = 20; private static Set runningInstances = new HashSet<>(); private final BibDatabaseContext bibDatabaseContext; private final PreferencesService preferences; - private final DelayTaskThrottler throttler; + private final ScheduledThreadPoolExecutor executor; private final CoarseChangeFilter changeFilter; private final BibEntryTypesManager entryTypesManager; + + // Contains a list of all backup paths + // During a write, the less recent backup file is deleted + private final Queue backupFilesQueue = new LinkedBlockingQueue<>(); + + private boolean needsBackup = true; + private BackupManager(BibDatabaseContext bibDatabaseContext, BibEntryTypesManager entryTypesManager, PreferencesService preferences) { this.bibDatabaseContext = bibDatabaseContext; this.entryTypesManager = entryTypesManager; this.preferences = preferences; - this.throttler = new DelayTaskThrottler(15_000); + this.executor = new ScheduledThreadPoolExecutor(2); changeFilter = new CoarseChangeFilter(bibDatabaseContext); changeFilter.registerListener(this); } - static Path getBackupPath(Path originalPath) { - return FileUtil.addExtension(originalPath, AUTOSAVE_FILE_EXTENSION); + /** + * Determines the most recent backup file name + */ + static Path getBackupPathForNewBackup(Path originalPath) { + return BackupFileUtil.getPathForNewBackupFileAndCreateDirectory(originalPath, BackupFileType.BACKUP); + } + + /** + * Determines the most recent existing backup file name + */ + static Optional getLatestBackupPath(Path originalPath) { + return BackupFileUtil.getPathOfLatestExisingBackupFile(originalPath, BackupFileType.BACKUP); } /** * Starts the BackupManager which is associated with the given {@link BibDatabaseContext}. As long as no database * file is present in {@link BibDatabaseContext}, the {@link BackupManager} will do nothing. * + * This method is not thread-safe. The caller has to ensure that this method is not called in parallel. + * * @param bibDatabaseContext Associated {@link BibDatabaseContext} */ public static BackupManager start(BibDatabaseContext bibDatabaseContext, BibEntryTypesManager entryTypesManager, PreferencesService preferences) { @@ -99,13 +125,13 @@ public static void shutdown(BibDatabaseContext bibDatabaseContext) { * the user checks the output. */ public static boolean backupFileDiffers(Path originalPath) { - Path backupPath = getBackupPath(originalPath); - if (!Files.exists(backupPath) || Files.isDirectory(backupPath)) { + Optional backupPath = getLatestBackupPath(originalPath); + if (backupPath.isEmpty()) { return false; } try { - return Files.mismatch(originalPath, backupPath) != -1L; + return Files.mismatch(originalPath, backupPath.get()) != -1L; } catch (IOException e) { LOGGER.debug("Could not compare original file and backup file.", e); // User has to investigate in this case @@ -119,28 +145,62 @@ public static boolean backupFileDiffers(Path originalPath) { * @param originalPath Path to the file which should be equalized to the backup file. */ public static void restoreBackup(Path originalPath) { - Path backupPath = getBackupPath(originalPath); + Optional backupPath = getLatestBackupPath(originalPath); + if (backupPath.isEmpty()) { + LOGGER.error("There is no backup file"); + return; + } try { - Files.copy(backupPath, originalPath, StandardCopyOption.REPLACE_EXISTING); + Files.copy(backupPath.get(), originalPath, StandardCopyOption.REPLACE_EXISTING); } catch (IOException e) { LOGGER.error("Error while restoring the backup file.", e); } } - private Optional determineBackupPath() { - return bibDatabaseContext.getDatabasePath().map(BackupManager::getBackupPath); + private Optional determineBackupPathForNewBackup() { + return bibDatabaseContext.getDatabasePath().map(BackupManager::getBackupPathForNewBackup); } + /** + * This method is called as soon as the scheduler says: "Do the backup" + * + * SIDE EFFECT: Deletes oldest backup file + * + * @param backupPath the path where the library should be backed up to + */ private void performBackup(Path backupPath) { + if (!needsBackup) { + return; + } + + // We opted for "while" to delete backups in case there are more than 10 + while (backupFilesQueue.size() >= MAXIMUM_BACKUP_FILE_COUNT) { + Path lessRecentBackupFile = backupFilesQueue.poll(); + try { + Files.delete(lessRecentBackupFile); + } catch (IOException e) { + LOGGER.error("Could not delete backup file {}", lessRecentBackupFile, e); + } + } + // code similar to org.jabref.gui.exporter.SaveDatabaseAction.saveDatabase GeneralPreferences generalPreferences = preferences.getGeneralPreferences(); SavePreferences savePreferences = preferences.getSavePreferences() .withMakeBackup(false); Charset encoding = bibDatabaseContext.getMetaData().getEncoding().orElse(StandardCharsets.UTF_8); - try (AtomicFileWriter fileWriter = new AtomicFileWriter(backupPath, encoding)) { - BibWriter bibWriter = new BibWriter(fileWriter, bibDatabaseContext.getDatabase().getNewLineSeparator()); + // We want to have successful backups only + // Thus, we do not use a plain "FileWriter", but the "AtomicFileWriter" + // Example: What happens if one hard powers off the machine (or kills the jabref process) during the write of the backup? + // This MUST NOT create a broken backup file that then jabref wants to "restore" from? + try (Writer writer = new AtomicFileWriter(backupPath, encoding, false)) { + BibWriter bibWriter = new BibWriter(writer, bibDatabaseContext.getDatabase().getNewLineSeparator()); new BibtexDatabaseWriter(bibWriter, generalPreferences, savePreferences, entryTypesManager) .saveDatabase(bibDatabaseContext); + backupFilesQueue.add(backupPath); + + // We wrote the file successfully + // Thus, we currently do not need any new backup + this.needsBackup = false; } catch (IOException e) { logIfCritical(backupPath, e); } @@ -162,32 +222,51 @@ private void logIfCritical(Path backupPath, IOException e) { @Subscribe public synchronized void listen(@SuppressWarnings("unused") BibDatabaseContextChangedEvent event) { if (!event.isFilteredOut()) { - startBackupTask(); + this.needsBackup = true; } } private void startBackupTask() { - throttler.schedule(() -> determineBackupPath().ifPresent(this::performBackup)); + fillQueue(); + + executor.scheduleAtFixedRate( + // We need to determine the backup path on each action, because we use the timestamp in the filename + () -> determineBackupPathForNewBackup().ifPresent(this::performBackup), + DELAY_BETWEEN_BACKUP_ATTEMPTS_IN_SECONDS, + DELAY_BETWEEN_BACKUP_ATTEMPTS_IN_SECONDS, + TimeUnit.SECONDS); + } + + private void fillQueue() { + Path backupDir = BackupFileUtil.getAppDataBackupDir(); + if (!Files.exists(backupDir)) { + return; + } + bibDatabaseContext.getDatabasePath().ifPresent(databasePath -> { + // code similar to {@link org.jabref.logic.util.io.BackupFileUtil.getPathOfLatestExisingBackupFile} + final String prefix = BackupFileUtil.getUniqueFilePrefix(databasePath) + "--" + databasePath.getFileName(); + try { + List allSavFiles = Files.list(backupDir) + // just list the .sav belonging to the given targetFile + .filter(p -> p.getFileName().toString().startsWith(prefix)) + .sorted().toList(); + backupFilesQueue.addAll(allSavFiles); + } catch (IOException e) { + LOGGER.error("Could not determine most recent file", e); + } + }); } /** - * Unregisters the BackupManager from the eventBus of {@link BibDatabaseContext} and deletes the backup file. This - * method should only be used when closing a database/JabRef legally. + * Unregisters the BackupManager from the eventBus of {@link BibDatabaseContext}. + * This method should only be used when closing a database/JabRef in a normal way. */ private void shutdown() { changeFilter.unregisterListener(this); changeFilter.shutdown(); - throttler.shutdown(); - determineBackupPath().ifPresent(this::deleteBackupFile); - } + executor.shutdown(); - private void deleteBackupFile(Path backupPath) { - try { - if (Files.exists(backupPath) && !Files.isDirectory(backupPath)) { - Files.delete(backupPath); - } - } catch (IOException e) { - LOGGER.error("Error while deleting the backup file.", e); - } + // Ensure that backup is a recent one + determineBackupPathForNewBackup().ifPresent(this::performBackup); } } diff --git a/src/main/java/org/jabref/logic/exporter/AtomicFileOutputStream.java b/src/main/java/org/jabref/logic/exporter/AtomicFileOutputStream.java index c1589735c23..09b0e25f460 100644 --- a/src/main/java/org/jabref/logic/exporter/AtomicFileOutputStream.java +++ b/src/main/java/org/jabref/logic/exporter/AtomicFileOutputStream.java @@ -12,6 +12,7 @@ import java.util.EnumSet; import java.util.Set; +import org.jabref.logic.util.BackupFileType; import org.jabref.logic.util.io.FileUtil; import org.slf4j.Logger; @@ -47,7 +48,7 @@ public class AtomicFileOutputStream extends FilterOutputStream { private static final Logger LOGGER = LoggerFactory.getLogger(AtomicFileOutputStream.class); private static final String TEMPORARY_EXTENSION = ".tmp"; - private static final String BACKUP_EXTENSION = ".bak"; + private static final String SAVE_EXTENSION = BackupFileType.SAVE.getExtensions().get(0); /** * The file we want to create/replace. @@ -76,7 +77,7 @@ public AtomicFileOutputStream(Path path, boolean keepBackup) throws IOException this.targetFile = path; this.temporaryFile = getPathOfTemporaryFile(path); - this.backupFile = getPathOfBackupFile(path); + this.backupFile = getPathOfSaveBackupFile(path); this.keepBackup = keepBackup; try { @@ -104,8 +105,8 @@ private static Path getPathOfTemporaryFile(Path targetFile) { return FileUtil.addExtension(targetFile, TEMPORARY_EXTENSION); } - private static Path getPathOfBackupFile(Path targetFile) { - return FileUtil.addExtension(targetFile, BACKUP_EXTENSION); + private static Path getPathOfSaveBackupFile(Path targetFile) { + return FileUtil.addExtension(targetFile, SAVE_EXTENSION); } /** diff --git a/src/main/java/org/jabref/logic/util/BackupFileType.java b/src/main/java/org/jabref/logic/util/BackupFileType.java new file mode 100644 index 00000000000..b24cc3faeb8 --- /dev/null +++ b/src/main/java/org/jabref/logic/util/BackupFileType.java @@ -0,0 +1,32 @@ +package org.jabref.logic.util; + +import java.util.Collections; +import java.util.List; + +public enum BackupFileType implements FileType { + + // Used at BackupManager + BACKUP("Backup", "bak"), + + // Used when writing the .bib file. See {@link org.jabref.logic.exporter.AtomicFileWriter} + // Used for copying the .bib away before overwriting on save. + SAVE("AutoSaveFile", "sav"); + + private final List extensions; + private final String name; + + BackupFileType(String name, String extension) { + this.extensions = Collections.singletonList(extension); + this.name = name; + } + + @Override + public List getExtensions() { + return extensions; + } + + @Override + public String getName() { + return this.name; + } +} diff --git a/src/main/java/org/jabref/logic/util/DelayTaskThrottler.java b/src/main/java/org/jabref/logic/util/DelayTaskThrottler.java index 17fe4770937..4a5e7422edb 100644 --- a/src/main/java/org/jabref/logic/util/DelayTaskThrottler.java +++ b/src/main/java/org/jabref/logic/util/DelayTaskThrottler.java @@ -51,17 +51,6 @@ public ScheduledFuture schedule(Runnable command) { return scheduledTask; } - // Execute scheduled Runnable early - public void execute(Runnable command) { - delay = 0; - schedule(command); - } - - // Cancel scheduled Runnable gracefully - public void cancel() { - scheduledTask.cancel(false); - } - public ScheduledFuture scheduleTask(Callable command) { if (scheduledTask != null) { cancel(); @@ -74,6 +63,17 @@ public ScheduledFuture scheduleTask(Callable command) { return scheduledTask; } + // Execute scheduled Runnable early + public void execute(Runnable command) { + delay = 0; + schedule(command); + } + + // Cancel scheduled Runnable gracefully + public void cancel() { + scheduledTask.cancel(false); + } + /** * Shuts everything down. Upon termination, this method returns. */ diff --git a/src/main/java/org/jabref/logic/util/FileType.java b/src/main/java/org/jabref/logic/util/FileType.java index 7e4ba2199ec..c054d54d7bc 100644 --- a/src/main/java/org/jabref/logic/util/FileType.java +++ b/src/main/java/org/jabref/logic/util/FileType.java @@ -8,7 +8,7 @@ */ public interface FileType { - default List getExtensionsWithDot() { + default List getExtensionsWithAsteriskAndDot() { return getExtensions().stream() .map(extension -> "*." + extension) .collect(Collectors.toList()); diff --git a/src/main/java/org/jabref/logic/util/OS.java b/src/main/java/org/jabref/logic/util/OS.java index d56a8a41363..fabbadbd2c9 100644 --- a/src/main/java/org/jabref/logic/util/OS.java +++ b/src/main/java/org/jabref/logic/util/OS.java @@ -8,6 +8,9 @@ public class OS { public static final String NEWLINE = System.lineSeparator(); + public static final String APP_DIR_APP_NAME = "JabRef"; + public static final String APP_DIR_APP_AUTHOR = "org.jabref"; + // File separator obtained from system private static final String FILE_SEPARATOR = System.getProperty("file.separator"); diff --git a/src/main/java/org/jabref/logic/util/io/BackupFileUtil.java b/src/main/java/org/jabref/logic/util/io/BackupFileUtil.java new file mode 100644 index 00000000000..34fd3810990 --- /dev/null +++ b/src/main/java/org/jabref/logic/util/io/BackupFileUtil.java @@ -0,0 +1,123 @@ +package org.jabref.logic.util.io; + +import java.io.IOException; +import java.nio.ByteBuffer; +import java.nio.file.Files; +import java.nio.file.Path; +import java.time.ZonedDateTime; +import java.time.format.DateTimeFormatter; +import java.util.HexFormat; +import java.util.Optional; + +import org.jabref.logic.util.BackupFileType; +import org.jabref.logic.util.BuildInfo; +import org.jabref.logic.util.OS; + +import net.harawata.appdirs.AppDirsFactory; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class BackupFileUtil { + + private static final Logger LOGGER = LoggerFactory.getLogger(BackupFileUtil.class); + + private BackupFileUtil() { + } + + public static Path getAppDataBackupDir() { + Path directory = Path.of(AppDirsFactory.getInstance().getUserDataDir( + OS.APP_DIR_APP_NAME, + new BuildInfo().version.toString(), + OS.APP_DIR_APP_AUTHOR)) + .resolve("backups"); + return directory; + } + + /** + * Determines the path of the backup file (using the given extension) + * + *

+ * As default, a directory inside the user temporary dir is used.
+ * In case a AUTOSAVE backup is requested, a timestamp is added + *

+ *

+ * SIDE EFFECT: Creates the directory. + * In case that fails, the return path of the .bak file is set to be next to the .bib file + *

+ *

+ * Note that this backup is different from the .sav file generated by {@link org.jabref.logic.autosaveandbackup.BackupManager} + * (and configured in the preferences as "make backups") + *

+ */ + public static Path getPathForNewBackupFileAndCreateDirectory(Path targetFile, BackupFileType fileType) { + String extension = "." + fileType.getExtensions().get(0); + String timeSuffix = ZonedDateTime.now().format(DateTimeFormatter.ofPattern("yyyy-MM-dd--HH.mm.ss")); + + // We choose the data directory, because a ".bak" file should survive cache cleanups + Path directory = getAppDataBackupDir(); + try { + Files.createDirectories(directory); + } catch (IOException e) { + Path result = FileUtil.addExtension(targetFile, extension); + LOGGER.warn("Could not create bib writing directory {}, using {} as file", directory, result, e); + return result; + } + String baseFileName = getUniqueFilePrefix(targetFile) + "--" + targetFile.getFileName() + "--" + timeSuffix; + Path fileName = FileUtil.addExtension(Path.of(baseFileName), extension); + return directory.resolve(fileName); + } + + /** + * Finds the latest backup (.sav). If it does not exist, an empty optional is returned + */ + public static Optional getPathOfLatestExisingBackupFile(Path targetFile, BackupFileType fileType) { + // The code is similar to "getPathForNewBackupFileAndCreateDirectory" + + String extension = "." + fileType.getExtensions().get(0); + + Path directory = getAppDataBackupDir(); + if (Files.notExists(directory)) { + // In case there is no app directory, we search in the directory of the bib file + Path result = FileUtil.addExtension(targetFile, extension); + if (Files.exists(result)) { + return Optional.of(result); + } else { + return Optional.empty(); + } + } + + // Search the directory for the latest file + final String prefix = getUniqueFilePrefix(targetFile) + "--" + targetFile.getFileName(); + Optional mostRecentFile; + try { + mostRecentFile = Files.list(directory) + // just list the .sav belonging to the given targetFile + .filter(p -> p.getFileName().toString().startsWith(prefix)) + .sorted() + .reduce((first, second) -> second); + } catch (IOException e) { + LOGGER.error("Could not determine most recent file", e); + return Optional.empty(); + } + return mostRecentFile; + } + + /** + *

+ * Determines a unique file prefix. + *

+ *

+ * When creating a backup file, the backup file should belong to the original file. + * Just adding ".bak" suffix to the filename, does not work in all cases: + * It may be possible that the user has opened "paper.bib" twice. + * Thus, we need to create a unique prefix to distinguish these files. + *

+ */ + public static String getUniqueFilePrefix(Path targetFile) { + // Idea: use the hash code and convert it to hex + // Thereby, use positive values only and use length 4 + int positiveCode = Math.abs(targetFile.hashCode()); + byte[] array = ByteBuffer.allocate(4).putInt(positiveCode).array(); + return HexFormat.of().formatHex(array); + } +} diff --git a/src/main/java/org/jabref/logic/util/io/FileUtil.java b/src/main/java/org/jabref/logic/util/io/FileUtil.java index f60ff2b110c..379d2318c80 100644 --- a/src/main/java/org/jabref/logic/util/io/FileUtil.java +++ b/src/main/java/org/jabref/logic/util/io/FileUtil.java @@ -26,15 +26,25 @@ import org.jabref.model.database.BibDatabase; import org.jabref.model.database.BibDatabaseContext; import org.jabref.model.entry.BibEntry; +import org.jabref.model.util.FileHelper; import org.jabref.model.util.OptionalUtil; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +/** + * The idea of this class is to add general functionality that could possibly even in the + * Java NIO package, + * such as getting/adding file extension etc. + * + * This class is the "successor" of {@link FileHelper}. In case you miss something here, + * please look at {@link FileHelper} and migrate the functionality to here. + */ public class FileUtil { public static final boolean IS_POSIX_COMPLIANT = FileSystems.getDefault().supportedFileAttributeViews().contains("posix"); public static final int MAXIMUM_FILE_NAME_LENGTH = 255; + private static final Logger LOGGER = LoggerFactory.getLogger(FileUtil.class); private FileUtil() { @@ -99,6 +109,8 @@ public static String getValidFileName(String fileName) { * Adds an extension to the given file name. The original extension is not replaced. That means, "demo.bib", ".sav" * gets "demo.bib.sav" and not "demo.sav" * + * Warning! If "ext" is passed, this is literally added. Thus addExtension("tmp.txt", "ext") leads to "tmp.txtext" + * * @param path the path to add the extension to * @param extension the extension to add * @return the with the modified file name diff --git a/src/main/java/org/jabref/model/database/BibDatabase.java b/src/main/java/org/jabref/model/database/BibDatabase.java index 72a559ff663..820b8dcc107 100644 --- a/src/main/java/org/jabref/model/database/BibDatabase.java +++ b/src/main/java/org/jabref/model/database/BibDatabase.java @@ -552,7 +552,7 @@ public void setEpilog(String epilog) { } /** - * Registers an listener object (subscriber) to the internal event bus. + * Registers a listener object (subscriber) to the internal event bus. * The following events are posted: * * - {@link EntriesAddedEvent} diff --git a/src/main/java/org/jabref/model/database/BibDatabaseContext.java b/src/main/java/org/jabref/model/database/BibDatabaseContext.java index e5824aecc61..662a1ecde36 100644 --- a/src/main/java/org/jabref/model/database/BibDatabaseContext.java +++ b/src/main/java/org/jabref/model/database/BibDatabaseContext.java @@ -13,6 +13,7 @@ import org.jabref.logic.shared.DatabaseLocation; import org.jabref.logic.shared.DatabaseSynchronizer; import org.jabref.logic.util.CoarseChangeFilter; +import org.jabref.logic.util.OS; import org.jabref.model.entry.BibEntry; import org.jabref.model.metadata.MetaData; import org.jabref.model.pdf.search.SearchFieldConstants; @@ -29,8 +30,6 @@ @AllowedToUseLogic("because it needs access to shared database features") public class BibDatabaseContext { - public static final String SEARCH_INDEX_BASE_PATH = "JabRef"; - private static final Logger LOGGER = LoggerFactory.getLogger(LibraryTab.class); private final BibDatabase database; @@ -223,7 +222,7 @@ public boolean hasEmptyEntries() { } public static Path getFulltextIndexBasePath() { - return Path.of(AppDirsFactory.getInstance().getUserDataDir(SEARCH_INDEX_BASE_PATH, SearchFieldConstants.VERSION, "org.jabref")); + return Path.of(AppDirsFactory.getInstance().getUserDataDir(OS.APP_DIR_APP_NAME, SearchFieldConstants.VERSION, OS.APP_DIR_APP_AUTHOR)); } public Path getFulltextIndexPath() { diff --git a/src/main/java/org/jabref/model/database/event/BibDatabaseContextChangedEvent.java b/src/main/java/org/jabref/model/database/event/BibDatabaseContextChangedEvent.java index d0c993f6e42..9db98e10dbf 100644 --- a/src/main/java/org/jabref/model/database/event/BibDatabaseContextChangedEvent.java +++ b/src/main/java/org/jabref/model/database/event/BibDatabaseContextChangedEvent.java @@ -5,9 +5,10 @@ import org.jabref.model.metadata.event.MetaDataChangedEvent; /** - * This Event is automatically fired at the same time as {@link EntriesEvent}, {@link GroupUpdatedEvent} or {@link MetaDataChangedEvent}. + * This event is automatically fired at the same time as {@link EntriesEvent}, {@link GroupUpdatedEvent}, or {@link MetaDataChangedEvent}, + * because all three inherit from this class. */ -public class BibDatabaseContextChangedEvent { +public abstract class BibDatabaseContextChangedEvent { // If the event has been filtered out private boolean filteredOut; diff --git a/src/main/java/org/jabref/model/entry/event/FieldChangedEvent.java b/src/main/java/org/jabref/model/entry/event/FieldChangedEvent.java index 3f4eda3838e..9165aa13f15 100644 --- a/src/main/java/org/jabref/model/entry/event/FieldChangedEvent.java +++ b/src/main/java/org/jabref/model/entry/event/FieldChangedEvent.java @@ -19,7 +19,7 @@ public class FieldChangedEvent extends EntryChangedEvent { * @param field Name of field which has been changed * @param oldValue old field value * @param newValue new field value - * @param location location Location affected by this event + * @param location Location affected by this event */ public FieldChangedEvent(BibEntry bibEntry, Field field, String newValue, String oldValue, EntriesEventSource location) { @@ -44,7 +44,7 @@ public FieldChangedEvent(BibEntry bibEntry, Field field, String newValue, String } /** - * @param location location Location affected by this event + * @param location Location affected by this event */ public FieldChangedEvent(FieldChange fieldChange, EntriesEventSource location) { super(fieldChange.getEntry(), location); diff --git a/src/main/java/org/jabref/model/metadata/event/MetaDataChangedEvent.java b/src/main/java/org/jabref/model/metadata/event/MetaDataChangedEvent.java index f910e50a47c..79650aa077f 100644 --- a/src/main/java/org/jabref/model/metadata/event/MetaDataChangedEvent.java +++ b/src/main/java/org/jabref/model/metadata/event/MetaDataChangedEvent.java @@ -4,7 +4,7 @@ import org.jabref.model.metadata.MetaData; /** - * {@link MetaDataChangedEvent} is fired when a tuple of meta data has been put or removed. + * {@link MetaDataChangedEvent} is fired when a tuple of metadata has been put or removed. */ public class MetaDataChangedEvent extends BibDatabaseContextChangedEvent { diff --git a/src/main/java/org/jabref/model/util/FileHelper.java b/src/main/java/org/jabref/model/util/FileHelper.java index 5eddefb724e..a55d70dff33 100644 --- a/src/main/java/org/jabref/model/util/FileHelper.java +++ b/src/main/java/org/jabref/model/util/FileHelper.java @@ -12,6 +12,7 @@ import java.util.Objects; import java.util.Optional; +import org.jabref.logic.util.io.FileUtil; import org.jabref.model.database.BibDatabaseContext; import org.jabref.preferences.FilePreferences; @@ -26,6 +27,10 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +/** + * @deprecated Please use {@link FileUtil} + */ +@Deprecated public class FileHelper { /** * MUST ALWAYS BE A SORTED ARRAY because it is used in a binary search diff --git a/src/main/java/org/jabref/preferences/JabRefPreferences.java b/src/main/java/org/jabref/preferences/JabRefPreferences.java index b954e883683..73d703848e1 100644 --- a/src/main/java/org/jabref/preferences/JabRefPreferences.java +++ b/src/main/java/org/jabref/preferences/JabRefPreferences.java @@ -529,7 +529,7 @@ private JabRefPreferences() { // SSL defaults.put(TRUSTSTORE_PATH, Path.of(AppDirsFactory.getInstance() - .getUserDataDir("JabRef", "ssl", "org.jabref")) + .getUserDataDir(OS.APP_DIR_APP_NAME, "ssl", OS.APP_DIR_APP_AUTHOR)) .resolve("truststore.jks").toString()); defaults.put(POS_X, 0); diff --git a/src/main/resources/l10n/JabRef_en.properties b/src/main/resources/l10n/JabRef_en.properties index a7964cd04ac..43a9c75a6dc 100644 --- a/src/main/resources/l10n/JabRef_en.properties +++ b/src/main/resources/l10n/JabRef_en.properties @@ -1765,7 +1765,7 @@ Error\ while\ generating\ fetch\ URL=Error while generating fetch URL Error\ while\ parsing\ ID\ list=Error while parsing ID list Unable\ to\ get\ PubMed\ IDs=Unable to get PubMed IDs Backup\ found=Backup found -A\ backup\ file\ for\ '%0'\ was\ found.=A backup file for '%0' was found. +A\ backup\ file\ for\ '%0'\ was\ found\ at\ '%1'.=A backup file for '%0' was found at '%1'. This\ could\ indicate\ that\ JabRef\ did\ not\ shut\ down\ cleanly\ last\ time\ the\ file\ was\ used.=This could indicate that JabRef did not shut down cleanly last time the file was used. Do\ you\ want\ to\ recover\ the\ library\ from\ the\ backup\ file?=Do you want to recover the library from the backup file? diff --git a/src/test/java/org/jabref/logic/autosaveandbackup/BackupManagerTest.java b/src/test/java/org/jabref/logic/autosaveandbackup/BackupManagerTest.java index 6195936ad35..a3b42ecf59d 100644 --- a/src/test/java/org/jabref/logic/autosaveandbackup/BackupManagerTest.java +++ b/src/test/java/org/jabref/logic/autosaveandbackup/BackupManagerTest.java @@ -1,37 +1,77 @@ package org.jabref.logic.autosaveandbackup; +import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.StandardCopyOption; + +import org.jabref.logic.util.BackupFileType; +import org.jabref.logic.util.io.BackupFileUtil; import org.junit.jupiter.api.Test; -import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertTrue; public class BackupManagerTest { @Test - public void autosaveFileNameIsCorrectlyGeneratedWithinTmpDirectory() { + public void backupFileNameIsCorrectlyGeneratedInAppDataDirectory() { Path bibPath = Path.of("tmp", "test.bib"); - Path savPath = BackupManager.getBackupPath(bibPath); - assertEquals(Path.of("tmp", "test.bib.sav"), savPath); + Path bakPath = BackupManager.getBackupPathForNewBackup(bibPath); + + // Pattern is "27182d3c--test.bib--", but the hashing is implemented differently on Linux than on Windows + assertNotEquals("", bakPath); } @Test - public void autosaveFileIsEqualForNonExistingBackup() throws Exception { + public void backupFileIsEqualForNonExistingBackup() throws Exception { Path originalFile = Path.of(BackupManagerTest.class.getResource("no-autosave.bib").toURI()); assertFalse(BackupManager.backupFileDiffers(originalFile)); } @Test public void backupFileIsEqual() throws Exception { + // Prepare test: Create backup file on "right" path + Path source = Path.of(BackupManagerTest.class.getResource("no-changes.bib.bak").toURI()); + Path target = BackupFileUtil.getPathForNewBackupFileAndCreateDirectory(Path.of(BackupManagerTest.class.getResource("no-changes.bib").toURI()), BackupFileType.BACKUP); + Files.copy(source, target, StandardCopyOption.REPLACE_EXISTING); + Path originalFile = Path.of(BackupManagerTest.class.getResource("no-changes.bib").toURI()); assertFalse(BackupManager.backupFileDiffers(originalFile)); } @Test public void backupFileDiffers() throws Exception { + // Prepare test: Create backup file on "right" path + Path source = Path.of(BackupManagerTest.class.getResource("changes.bib.bak").toURI()); + Path target = BackupFileUtil.getPathForNewBackupFileAndCreateDirectory(Path.of(BackupManagerTest.class.getResource("changes.bib").toURI()), BackupFileType.BACKUP); + Files.copy(source, target, StandardCopyOption.REPLACE_EXISTING); + Path originalFile = Path.of(BackupManagerTest.class.getResource("changes.bib").toURI()); assertTrue(BackupManager.backupFileDiffers(originalFile)); } + + @Test + public void correctBackupFileDeterminedForMultipleBakFiles() throws Exception { + // Prepare test: Create backup files on "right" path + + // most recent file does not have any changes + Path source = Path.of(BackupManagerTest.class.getResource("no-changes.bib.bak").toURI()); + Path target = BackupFileUtil.getPathForNewBackupFileAndCreateDirectory(Path.of(BackupManagerTest.class.getResource("no-changes.bib").toURI()), BackupFileType.BACKUP); + Files.copy(source, target, StandardCopyOption.REPLACE_EXISTING); + + // create "older" .bak files containing changes + for (int i = 0; i < 10; i++) { + source = Path.of(BackupManagerTest.class.getResource("changes.bib").toURI()); + Path directory = BackupFileUtil.getAppDataBackupDir(); + String timeSuffix = "2020-02-03--00.00.0" + Integer.toString(i); + String fileName = BackupFileUtil.getUniqueFilePrefix(Path.of(BackupManagerTest.class.getResource("no-changes.bib").toURI())) + "--no-changes.bib--" + timeSuffix + ".bak"; + target = directory.resolve(fileName); + Files.copy(source, target, StandardCopyOption.REPLACE_EXISTING); + } + + Path originalFile = Path.of(BackupManagerTest.class.getResource("no-changes.bib").toURI()); + assertFalse(BackupManager.backupFileDiffers(originalFile)); + } } diff --git a/src/test/java/org/jabref/logic/util/io/BackupFileUtilTest.java b/src/test/java/org/jabref/logic/util/io/BackupFileUtilTest.java new file mode 100644 index 00000000000..f44e6a45e1e --- /dev/null +++ b/src/test/java/org/jabref/logic/util/io/BackupFileUtilTest.java @@ -0,0 +1,44 @@ +package org.jabref.logic.util.io; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; + +import org.jabref.logic.util.BackupFileType; + +import org.junit.jupiter.api.Test; +import org.mockito.Answers; +import org.mockito.MockedStatic; +import org.mockito.Mockito; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; + +public class BackupFileUtilTest { + + @Test + void uniqueFilePrefix() { + // We cannot test for a concrete hash code, because hashing implementation differs from environment to environment + assertNotEquals("", BackupFileUtil.getUniqueFilePrefix(Path.of("test.bib"))); + } + + @Test + void getPathOfBackupFileAndCreateDirectoryReturnsAppDirectoryInCaseOfNoError() { + String start = BackupFileUtil.getAppDataBackupDir().toString(); + String result = BackupFileUtil.getPathForNewBackupFileAndCreateDirectory(Path.of("test.bib"), BackupFileType.BACKUP).toString(); + // We just check the prefix + assertEquals(start, result.substring(0, start.length())); + } + + @Test + void getPathOfBackupFileAndCreateDirectoryReturnsSameDirectoryInCaseOfException() { + try (MockedStatic files = Mockito.mockStatic(Files.class, Answers.RETURNS_DEEP_STUBS)) { + files.when(() -> Files.createDirectories(BackupFileUtil.getAppDataBackupDir())) + .thenThrow(new IOException()); + Path testPath = Path.of("tmp", "test.bib"); + Path result = BackupFileUtil.getPathForNewBackupFileAndCreateDirectory(testPath, BackupFileType.BACKUP); + // The intended fallback behavior is to put the .bak file in the same directory as the .bib file + assertEquals(Path.of("tmp", "test.bib.bak"), result); + } + } +} diff --git a/src/test/java/org/jabref/logic/util/io/FileUtilTest.java b/src/test/java/org/jabref/logic/util/io/FileUtilTest.java index ce6f126c045..8cb061fc99e 100644 --- a/src/test/java/org/jabref/logic/util/io/FileUtilTest.java +++ b/src/test/java/org/jabref/logic/util/io/FileUtilTest.java @@ -395,7 +395,7 @@ void testFindinPath() { } @Test - void testFindinListofPath() { + void testFindInListOfPath() { List paths = List.of(existingTestFile, otherExistingTestFile, rootDir); List resultPaths = List.of(existingTestFile, existingTestFile); List result = FileUtil.find("existingTestFile.txt", paths); diff --git a/src/test/resources/org/jabref/logic/autosaveandbackup/changes.bib.sav b/src/test/resources/org/jabref/logic/autosaveandbackup/changes.bib.bak similarity index 100% rename from src/test/resources/org/jabref/logic/autosaveandbackup/changes.bib.sav rename to src/test/resources/org/jabref/logic/autosaveandbackup/changes.bib.bak diff --git a/src/test/resources/org/jabref/logic/autosaveandbackup/no-changes.bib.sav b/src/test/resources/org/jabref/logic/autosaveandbackup/no-changes.bib.bak similarity index 100% rename from src/test/resources/org/jabref/logic/autosaveandbackup/no-changes.bib.sav rename to src/test/resources/org/jabref/logic/autosaveandbackup/no-changes.bib.bak