From 2ff3e9b949d83af16ab9f0379f6c48f6e6bfb971 Mon Sep 17 00:00:00 2001 From: Siedlerchr Date: Tue, 25 Apr 2023 21:23:52 +0200 Subject: [PATCH 01/17] Configure backup path in prefs TODO: Enable/disable backup --- src/main/java/org/jabref/gui/JabRefFrame.java | 4 +- src/main/java/org/jabref/gui/LibraryTab.java | 2 +- .../gui/backup/BackupResolverDialog.java | 2 +- .../GenerateCitationKeyAction.java | 6 +- .../jabref/gui/dialogs/BackupUIManager.java | 4 +- .../gui/exporter/SaveDatabaseAction.java | 2 +- .../importer/actions/OpenDatabaseAction.java | 3 +- .../jabref/gui/preferences/file/FileTab.fxml | 19 ++++++ .../jabref/gui/preferences/file/FileTab.java | 15 ++++- .../preferences/file/FileTabViewModel.java | 36 +++++++++- .../autosaveandbackup/BackupManager.java | 58 ++++++++-------- .../jabref/logic/util/io/BackupFileUtil.java | 67 ++++++++++--------- .../jabref/preferences/FilePreferences.java | 32 ++++++++- .../jabref/preferences/JabRefPreferences.java | 14 +++- .../BackupManagerDiscardedTest.java | 11 +-- .../autosaveandbackup/BackupManagerTest.java | 34 ++++++---- .../logic/util/io/BackupFileUtilTest.java | 16 ++++- 17 files changed, 230 insertions(+), 95 deletions(-) diff --git a/src/main/java/org/jabref/gui/JabRefFrame.java b/src/main/java/org/jabref/gui/JabRefFrame.java index 24cb8f44581..d3d0b10172e 100644 --- a/src/main/java/org/jabref/gui/JabRefFrame.java +++ b/src/main/java/org/jabref/gui/JabRefFrame.java @@ -490,7 +490,7 @@ public boolean quit() { context.clearDBMSSynchronizer(); } AutosaveManager.shutdown(context); - BackupManager.shutdown(context); + BackupManager.shutdown(context, prefs); context.getDatabasePath().map(Path::toAbsolutePath).map(Path::toString).ifPresent(filenames::add); } @@ -1312,7 +1312,7 @@ private void closeTab(LibraryTab libraryTab) { removeTab(libraryTab); } AutosaveManager.shutdown(context); - BackupManager.shutdown(context); + BackupManager.shutdown(context, prefs); } private void removeTab(LibraryTab libraryTab) { diff --git a/src/main/java/org/jabref/gui/LibraryTab.java b/src/main/java/org/jabref/gui/LibraryTab.java index 6edf841e0df..7373ed667f4 100644 --- a/src/main/java/org/jabref/gui/LibraryTab.java +++ b/src/main/java/org/jabref/gui/LibraryTab.java @@ -705,7 +705,7 @@ private void saveDividerLocation(Number position) { public void cleanUp() { changeMonitor.ifPresent(DatabaseChangeMonitor::unregister); AutosaveManager.shutdown(bibDatabaseContext); - BackupManager.shutdown(bibDatabaseContext); + BackupManager.shutdown(bibDatabaseContext, preferencesService); } /** diff --git a/src/main/java/org/jabref/gui/backup/BackupResolverDialog.java b/src/main/java/org/jabref/gui/backup/BackupResolverDialog.java index bdd2fbd2d16..e806ff0d736 100644 --- a/src/main/java/org/jabref/gui/backup/BackupResolverDialog.java +++ b/src/main/java/org/jabref/gui/backup/BackupResolverDialog.java @@ -32,7 +32,7 @@ public BackupResolverDialog(Path originalPath) { getDialogPane().setMinHeight(180); getDialogPane().getButtonTypes().setAll(RESTORE_FROM_BACKUP, REVIEW_BACKUP, IGNORE_BACKUP); - Optional backupPathOpt = BackupFileUtil.getPathOfLatestExistingBackupFile(originalPath, BackupFileType.BACKUP); + Optional backupPathOpt = BackupFileUtil.yyyyyyyyyy(originalPath, BackupFileType.BACKUP); String backupFilename = backupPathOpt.map(Path::getFileName).map(Path::toString).orElse(Localization.lang("File not found")); String content = new StringBuilder() .append(Localization.lang("A backup file for '%0' was found at [%1]", diff --git a/src/main/java/org/jabref/gui/citationkeypattern/GenerateCitationKeyAction.java b/src/main/java/org/jabref/gui/citationkeypattern/GenerateCitationKeyAction.java index 1cc32a198c1..5cc07a31c71 100644 --- a/src/main/java/org/jabref/gui/citationkeypattern/GenerateCitationKeyAction.java +++ b/src/main/java/org/jabref/gui/citationkeypattern/GenerateCitationKeyAction.java @@ -54,7 +54,7 @@ public void execute() { checkOverwriteKeysChosen(); if (!this.isCanceled) { - BackgroundTask backgroundTask = this.generateKeysInBackground(); + BackgroundTask backgroundTask = this.generateKeysInBackground(); backgroundTask.showToUser(true); backgroundTask.titleProperty().set(Localization.lang("Autogenerate citation keys")); backgroundTask.messageProperty().set(Localization.lang("%0/%1 entries", 0, entries.size())); @@ -93,8 +93,8 @@ private void checkOverwriteKeysChosen() { } } - private BackgroundTask generateKeysInBackground() { - return new BackgroundTask() { + private BackgroundTask generateKeysInBackground() { + return new BackgroundTask<>() { private NamedCompound compound; @Override diff --git a/src/main/java/org/jabref/gui/dialogs/BackupUIManager.java b/src/main/java/org/jabref/gui/dialogs/BackupUIManager.java index ceba43dd088..a872034b547 100644 --- a/src/main/java/org/jabref/gui/dialogs/BackupUIManager.java +++ b/src/main/java/org/jabref/gui/dialogs/BackupUIManager.java @@ -41,7 +41,7 @@ public static Optional showRestoreBackupDialog(DialogService dialo var actionOpt = showBackupResolverDialog(dialogService, originalPath); return actionOpt.flatMap(action -> { if (action == BackupResolverDialog.RESTORE_FROM_BACKUP) { - BackupManager.restoreBackup(originalPath); + BackupManager.restoreBackup(originalPath, preferencesService.getFilePreferences().getBackupDirectory()); return Optional.empty(); } else if (action == BackupResolverDialog.REVIEW_BACKUP) { return showReviewBackupDialog(dialogService, originalPath, preferencesService); @@ -63,7 +63,7 @@ private static Optional showReviewBackupDialog(DialogService dialo // This will be modified by using the `DatabaseChangesResolverDialog`. BibDatabaseContext originalDatabase = originalParserResult.getDatabaseContext(); - Path backupPath = BackupFileUtil.getPathOfLatestExistingBackupFile(originalPath, BackupFileType.BACKUP).orElseThrow(); + Path backupPath = BackupFileUtil.yyyyyyyyyy(originalPath, BackupFileType.BACKUP).orElseThrow(); BibDatabaseContext backupDatabase = OpenDatabase.loadDatabase(backupPath, importFormatPreferences, new DummyFileUpdateMonitor()).getDatabaseContext(); DatabaseChangeResolverFactory changeResolverFactory = new DatabaseChangeResolverFactory(dialogService, originalDatabase, preferencesService.getBibEntryPreferences()); diff --git a/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java b/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java index 84bf9346e23..b5b74e60cc4 100644 --- a/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java +++ b/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java @@ -116,7 +116,7 @@ boolean saveAs(Path file, SaveDatabaseMode mode) { final Path oldFile = databasePath.get(); context.setDatabasePath(oldFile); AutosaveManager.shutdown(context); - BackupManager.shutdown(context); + BackupManager.shutdown(context, this.preferences); } // Set new location 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 403359dd390..75ee983ec31 100644 --- a/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java +++ b/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java @@ -192,9 +192,10 @@ private ParserResult loadDatabase(Path file) throws Exception { dialogService.notify(Localization.lang("Opening") + ": '" + file + "'"); preferencesService.getFilePreferences().setWorkingDirectory(fileToLoad.getParent()); + Path backupDir = preferencesService.getFilePreferences().getBackupDirectory(); ParserResult parserResult = null; - if (BackupManager.backupFileDiffers(fileToLoad)) { + if (BackupManager.backupFileDiffers(fileToLoad, backupDir)) { // In case the backup differs, ask the user what to do. // In case the user opted for restoring a backup, the content of the backup is contained in parserResult. parserResult = BackupUIManager.showRestoreBackupDialog(dialogService, fileToLoad, preferencesService).orElse(null); diff --git a/src/main/java/org/jabref/gui/preferences/file/FileTab.fxml b/src/main/java/org/jabref/gui/preferences/file/FileTab.fxml index 59b1d7f0e90..4847b6654d0 100644 --- a/src/main/java/org/jabref/gui/preferences/file/FileTab.fxml +++ b/src/main/java/org/jabref/gui/preferences/file/FileTab.fxml @@ -6,8 +6,12 @@ + + + + @@ -42,4 +46,19 @@ + + + + + + diff --git a/src/main/java/org/jabref/gui/preferences/file/FileTab.java b/src/main/java/org/jabref/gui/preferences/file/FileTab.java index 88c3768df07..d24d74a368d 100644 --- a/src/main/java/org/jabref/gui/preferences/file/FileTab.java +++ b/src/main/java/org/jabref/gui/preferences/file/FileTab.java @@ -6,7 +6,7 @@ import javafx.scene.control.RadioButton; import javafx.scene.control.TextField; -import org.jabref.gui.Globals; +import org.jabref.gui.DialogService; import org.jabref.gui.actions.ActionFactory; import org.jabref.gui.actions.StandardActions; import org.jabref.gui.help.HelpAction; @@ -16,6 +16,7 @@ import org.jabref.logic.l10n.Localization; import com.airhacks.afterburner.views.ViewLoader; +import jakarta.inject.Inject; public class FileTab extends AbstractPreferenceTabView implements PreferencesTab { @@ -25,10 +26,14 @@ public class FileTab extends AbstractPreferenceTabView impleme @FXML private RadioButton resolveStrings; @FXML private TextField resolveStringsForFields; @FXML private CheckBox alwaysReformatBib; + @FXML private CheckBox createBackup; + @FXML private TextField backupDirectory; @FXML private CheckBox autosaveLocalLibraries; @FXML private Button autosaveLocalLibrariesHelp; + @Inject private DialogService dialogService; + public FileTab() { ViewLoader.view(this) .root(this) @@ -36,7 +41,7 @@ public FileTab() { } public void initialize() { - this.viewModel = new FileTabViewModel(preferencesService.getImportExportPreferences()); + this.viewModel = new FileTabViewModel(preferencesService.getImportExportPreferences(), preferencesService.getFilePreferences(), dialogService); openLastStartup.selectedProperty().bindBidirectional(viewModel.openLastStartupProperty()); noWrapFiles.textProperty().bindBidirectional(viewModel.noWrapFilesProperty()); @@ -49,7 +54,7 @@ public void initialize() { alwaysReformatBib.selectedProperty().bindBidirectional(viewModel.alwaysReformatBibProperty()); autosaveLocalLibraries.selectedProperty().bindBidirectional(viewModel.autosaveLocalLibrariesProperty()); - ActionFactory actionFactory = new ActionFactory(Globals.getKeyPrefs()); + ActionFactory actionFactory = new ActionFactory(preferencesService.getKeyBindingRepository()); actionFactory.configureIconButton(StandardActions.HELP, new HelpAction(HelpFile.AUTOSAVE, dialogService), autosaveLocalLibrariesHelp); } @@ -57,4 +62,8 @@ public void initialize() { public String getTabName() { return Localization.lang("File"); } + + public void backupFileDirBrowse() { + viewModel.backupFileDirBrowse(); + } } diff --git a/src/main/java/org/jabref/gui/preferences/file/FileTabViewModel.java b/src/main/java/org/jabref/gui/preferences/file/FileTabViewModel.java index 3981f4d2c82..30849fd5437 100644 --- a/src/main/java/org/jabref/gui/preferences/file/FileTabViewModel.java +++ b/src/main/java/org/jabref/gui/preferences/file/FileTabViewModel.java @@ -1,11 +1,16 @@ package org.jabref.gui.preferences.file; +import java.nio.file.Path; + import javafx.beans.property.BooleanProperty; import javafx.beans.property.SimpleBooleanProperty; import javafx.beans.property.SimpleStringProperty; import javafx.beans.property.StringProperty; +import org.jabref.gui.DialogService; import org.jabref.gui.preferences.PreferenceTabViewModel; +import org.jabref.gui.util.DirectoryDialogConfiguration; +import org.jabref.preferences.FilePreferences; import org.jabref.preferences.ImportExportPreferences; public class FileTabViewModel implements PreferenceTabViewModel { @@ -18,10 +23,17 @@ public class FileTabViewModel implements PreferenceTabViewModel { private final BooleanProperty alwaysReformatBibProperty = new SimpleBooleanProperty(); private final BooleanProperty autosaveLocalLibraries = new SimpleBooleanProperty(); + private final BooleanProperty createBackupProperty = new SimpleBooleanProperty(); + private final StringProperty backupDirectoryProperty = new SimpleStringProperty(""); + private final ImportExportPreferences importExportPreferences; + private final FilePreferences filePreferences; + private final DialogService dialogService; - FileTabViewModel(ImportExportPreferences importExportPreferences) { + FileTabViewModel(ImportExportPreferences importExportPreferences, FilePreferences filePreferences, DialogService dialogService) { this.importExportPreferences = importExportPreferences; + this.filePreferences = filePreferences; + this.dialogService = dialogService; } @Override @@ -34,6 +46,9 @@ public void setValues() { resolveStringsForFieldsProperty.setValue(importExportPreferences.getResolvableFields()); alwaysReformatBibProperty.setValue(importExportPreferences.shouldAlwaysReformatOnSave()); autosaveLocalLibraries.setValue(importExportPreferences.shouldAutoSave()); + + createBackupProperty.setValue(filePreferences.shouldCreateBackup()); + backupDirectoryProperty.setValue(filePreferences.getBackupDirectory().toString()); } @Override @@ -44,6 +59,9 @@ public void storeSettings() { importExportPreferences.setResolvableFields(resolveStringsForFieldsProperty.getValue().trim()); importExportPreferences.setAlwaysReformatOnSave(alwaysReformatBibProperty.getValue()); importExportPreferences.setAutoSave(autosaveLocalLibraries.getValue()); + + filePreferences.createBackupProperty().setValue(createBackupProperty.getValue()); + filePreferences.backupDirectoryProperty().setValue(Path.of(backupDirectoryProperty.getValue())); } // General @@ -78,4 +96,20 @@ public BooleanProperty alwaysReformatBibProperty() { public BooleanProperty autosaveLocalLibrariesProperty() { return autosaveLocalLibraries; } + + public BooleanProperty createBackupProperty() { + return this.createBackupProperty; + } + + public StringProperty backupDirectoryProperty() { + return this.backupDirectoryProperty; + } + + public void backupFileDirBrowse() { + DirectoryDialogConfiguration dirDialogConfiguration = + new DirectoryDialogConfiguration.Builder().withInitialDirectory(Path.of(backupDirectoryProperty().getValue())).build(); + dialogService.showDirectorySelectionDialog(dirDialogConfiguration) + .ifPresent(dir -> backupDirectoryProperty.setValue(dir.toString())); + + } } diff --git a/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java b/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java index 789671c0b14..727a47f0201 100644 --- a/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java +++ b/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java @@ -58,7 +58,7 @@ public class BackupManager { private final ScheduledThreadPoolExecutor executor; private final CoarseChangeFilter changeFilter; private final BibEntryTypesManager entryTypesManager; - + private final Path backupPath; // Contains a list of all backup paths // During a write, the less recent backup file is deleted @@ -70,6 +70,7 @@ public class BackupManager { this.bibDatabaseContext = bibDatabaseContext; this.entryTypesManager = entryTypesManager; this.preferences = preferences; + this.backupPath = preferences.getFilePreferences().getBackupDirectory(); this.executor = new ScheduledThreadPoolExecutor(2); changeFilter = new CoarseChangeFilter(bibDatabaseContext); @@ -79,15 +80,15 @@ public class BackupManager { /** * Determines the most recent backup file name */ - static Path getBackupPathForNewBackup(Path originalPath) { - return BackupFileUtil.getPathForNewBackupFileAndCreateDirectory(originalPath, BackupFileType.BACKUP); + static Path getBackupPathForNewBackup(Path originalPath, Path backupDir) { + return BackupFileUtil.getPathForNewBackupFileAndCreateDirectory(originalPath, BackupFileType.BACKUP, backupDir); } /** * Determines the most recent existing backup file name */ - static Optional getLatestBackupPath(Path originalPath) { - return BackupFileUtil.getPathOfLatestExistingBackupFile(originalPath, BackupFileType.BACKUP); + static Optional getLatestBackupPath(Path originalPath, Path backupDir) { + return BackupFileUtil.getPathOfLatestExistingBackupFile(originalPath, BackupFileType.BACKUP, backupDir); } /** @@ -100,7 +101,7 @@ static Optional getLatestBackupPath(Path originalPath) { */ public static BackupManager start(BibDatabaseContext bibDatabaseContext, BibEntryTypesManager entryTypesManager, PreferencesService preferences) { BackupManager backupManager = new BackupManager(bibDatabaseContext, entryTypesManager, preferences); - backupManager.startBackupTask(); + backupManager.startBackupTask(preferences.getFilePreferences().getBackupDirectory()); runningInstances.add(backupManager); return backupManager; } @@ -112,7 +113,7 @@ public static BackupManager start(BibDatabaseContext bibDatabaseContext, BibEntr */ public static void discardBackup(BibDatabaseContext bibDatabaseContext) { runningInstances.stream().filter(instance -> instance.bibDatabaseContext == bibDatabaseContext).forEach( - BackupManager::discardBackup); + BackupManager::discardBackup); } /** @@ -120,9 +121,12 @@ public static void discardBackup(BibDatabaseContext bibDatabaseContext) { * * @param bibDatabaseContext Associated {@link BibDatabaseContext} */ - public static void shutdown(BibDatabaseContext bibDatabaseContext) { + public static void shutdown(BibDatabaseContext bibDatabaseContext, PreferencesService preferencesService) { + + Path backupDir = preferencesService.getFilePreferences().getBackupDirectory(); + runningInstances.stream().filter(instance -> instance.bibDatabaseContext == bibDatabaseContext).forEach( - BackupManager::shutdown); + manager -> manager.shutdown(backupDir)); runningInstances.removeIf(instance -> instance.bibDatabaseContext == bibDatabaseContext); } @@ -138,7 +142,7 @@ public static void shutdown(BibDatabaseContext bibDatabaseContext) { * "default" return value in the good case. In case a discarded file exists, false is returned, too. * In the case of an exception true is returned to ensure that the user checks the output. */ - public static boolean backupFileDiffers(Path originalPath) { + public static boolean backupFileDiffers(Path originalPath, Path backupDir) { Path discardedFile = determineDiscardedFile(originalPath); if (Files.exists(discardedFile)) { try { @@ -149,10 +153,10 @@ public static boolean backupFileDiffers(Path originalPath) { } return false; } - return getLatestBackupPath(originalPath).map(latestBackupPath -> { + return getLatestBackupPath(originalPath, backupDir).map(latestBackupPath -> { FileTime latestBackupFileLastModifiedTime; try { - latestBackupFileLastModifiedTime = Files.getLastModifiedTime(latestBackupPath); + latestBackupFileLastModifiedTime = Files.getLastModifiedTime(latestBackupPath); } catch (IOException e) { LOGGER.debug("Could not get timestamp of backup file {}", latestBackupPath, e); // If we cannot get the timestamp, we do show any warning @@ -186,8 +190,8 @@ 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) { - Optional backupPath = getLatestBackupPath(originalPath); + public static void restoreBackup(Path originalPath, Path backupDir) { + Optional backupPath = getLatestBackupPath(originalPath, backupDir); if (backupPath.isEmpty()) { LOGGER.error("There is no backup file"); return; @@ -199,8 +203,8 @@ public static void restoreBackup(Path originalPath) { } } - Optional determineBackupPathForNewBackup() { - return bibDatabaseContext.getDatabasePath().map(BackupManager::getBackupPathForNewBackup); + Optional determineBackupPathForNewBackup(Path backupDir) { + return bibDatabaseContext.getDatabasePath().map(path -> BackupManager.getBackupPathForNewBackup(path, backupDir)); } /** @@ -237,7 +241,7 @@ void performBackup(Path backupPath) { try (Writer writer = new AtomicFileWriter(backupPath, encoding, false)) { BibWriter bibWriter = new BibWriter(writer, bibDatabaseContext.getDatabase().getNewLineSeparator()); new BibtexDatabaseWriter(bibWriter, generalPreferences, savePreferences, entryTypesManager) - .saveDatabase(bibDatabaseContext); + .saveDatabase(bibDatabaseContext); backupFilesQueue.add(backupPath); // We wrote the file successfully @@ -250,8 +254,7 @@ void performBackup(Path backupPath) { private static Path determineDiscardedFile(Path file) { return BackupFileUtil.getAppDataBackupDir().resolve( - BackupFileUtil.getUniqueFilePrefix(file) + "--" + file.getFileName() + "--discarded" - ); + BackupFileUtil.getUniqueFilePrefix(file) + "--" + file.getFileName() + "--discarded"); } /** @@ -289,15 +292,15 @@ public synchronized void listen(@SuppressWarnings("unused") BibDatabaseContextCh } } - private void startBackupTask() { + private void startBackupTask(Path backupDir) { 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); + // We need to determine the backup path on each action, because we use the timestamp in the filename + () -> determineBackupPathForNewBackup(backupDir).ifPresent(path -> this.performBackup(backupDir)), + DELAY_BETWEEN_BACKUP_ATTEMPTS_IN_SECONDS, + DELAY_BETWEEN_BACKUP_ATTEMPTS_IN_SECONDS, + TimeUnit.SECONDS); } private void fillQueue() { @@ -323,13 +326,14 @@ private void fillQueue() { /** * Unregisters the BackupManager from the eventBus of {@link BibDatabaseContext}. * This method should only be used when closing a database/JabRef in a normal way. + * @param path */ - private void shutdown() { + private void shutdown(Path backupDir) { changeFilter.unregisterListener(this); changeFilter.shutdown(); executor.shutdown(); // Ensure that backup is a recent one - determineBackupPathForNewBackup().ifPresent(this::performBackup); + determineBackupPathForNewBackup(backupDir).ifPresent(this::performBackup); } } diff --git a/src/main/java/org/jabref/logic/util/io/BackupFileUtil.java b/src/main/java/org/jabref/logic/util/io/BackupFileUtil.java index c378386c1ed..c20c133fd67 100644 --- a/src/main/java/org/jabref/logic/util/io/BackupFileUtil.java +++ b/src/main/java/org/jabref/logic/util/io/BackupFileUtil.java @@ -48,12 +48,13 @@ public static Path getAppDataBackupDir() { * (and configured in the preferences as "make backups") *

*/ - public static Path getPathForNewBackupFileAndCreateDirectory(Path targetFile, BackupFileType fileType) { + + public static Path getPathForNewBackupFileAndCreateDirectory(Path targetFile, BackupFileType fileType, Path backupDir) { 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(); + Path directory = backupDir; try { Files.createDirectories(directory); } catch (IOException e) { @@ -66,41 +67,45 @@ public static Path getPathForNewBackupFileAndCreateDirectory(Path targetFile, Ba return directory.resolve(fileName); } - /** - * Finds the latest backup (.sav). If it does not exist, an empty optional is returned - * - * @param targetFile the full path of the file to backup - */ - public static Optional getPathOfLatestExistingBackupFile(Path targetFile, BackupFileType fileType) { - // The code is similar to "getPathForNewBackupFileAndCreateDirectory" + public static Optional getPathOfLatestExistingBackupFile(Path targetFile, BackupFileType fileType, Path backupDir) { + // The code is similar to "getPathForNewBackupFileAndCreateDirectory" - String extension = "." + fileType.getExtensions().get(0); + 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 { + Path directory = backupDir; + 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; } - // 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; + /** + * Finds the latest backup (.sav). If it does not exist, an empty optional is returned + * + * @param targetFile the full path of the file to backup + */ + public static Optional yyyyyyyyyy(Path targetFile, BackupFileType fileType) { + return getPathOfLatestExistingBackupFile(targetFile, fileType, Path.of("")); } /** diff --git a/src/main/java/org/jabref/preferences/FilePreferences.java b/src/main/java/org/jabref/preferences/FilePreferences.java index 264d778aeda..f204005afed 100644 --- a/src/main/java/org/jabref/preferences/FilePreferences.java +++ b/src/main/java/org/jabref/preferences/FilePreferences.java @@ -31,6 +31,8 @@ public class FilePreferences { private final BooleanProperty fulltextIndexLinkedFiles = new SimpleBooleanProperty(); private final ObjectProperty workingDirectory = new SimpleObjectProperty<>(); private final ObservableSet externalFileTypes = FXCollections.observableSet(new TreeSet<>(Comparator.comparing(ExternalFileType::getName))); + private final BooleanProperty createBackup = new SimpleBooleanProperty(); + private final ObjectProperty backupDiretory = new SimpleObjectProperty<>(); public FilePreferences(String user, String mainFileDirectory, @@ -40,7 +42,9 @@ public FilePreferences(String user, boolean downloadLinkedFiles, boolean fulltextIndexLinkedFiles, Path workingDirectory, - Set externalFileTypes) { + Set externalFileTypes, + boolean createBackup, + Path backupDirectory) { this.user.setValue(user); this.mainFileDirectory.setValue(mainFileDirectory); this.storeFilesRelativeToBibFile.setValue(storeFilesRelativeToBibFile); @@ -50,6 +54,8 @@ public FilePreferences(String user, this.fulltextIndexLinkedFiles.setValue(fulltextIndexLinkedFiles); this.workingDirectory.setValue(workingDirectory); this.externalFileTypes.addAll(externalFileTypes); + this.createBackup.setValue(createBackup); + this.backupDiretory.setValue(backupDirectory); } public String getUser() { @@ -147,4 +153,28 @@ public void setWorkingDirectory(Path workingDirectory) { public ObservableSet getExternalFileTypes() { return this.externalFileTypes; } + + public void setCreateBackup(boolean createBackup) { + this.createBackup.set(createBackup); + } + + public boolean shouldCreateBackup() { + return this.createBackup.getValue(); + } + + public BooleanProperty createBackupProperty() { + return this.createBackup; + } + + public ObjectProperty backupDirectoryProperty() { + return this.backupDiretory; + } + + public void setBackupDirectory(Path backupPath) { + this.backupDiretory.set(backupPath); + } + + public Path getBackupDirectory() { + return this.backupDiretory.getValue(); + } } diff --git a/src/main/java/org/jabref/preferences/JabRefPreferences.java b/src/main/java/org/jabref/preferences/JabRefPreferences.java index bbc182f9497..ed841e85977 100644 --- a/src/main/java/org/jabref/preferences/JabRefPreferences.java +++ b/src/main/java/org/jabref/preferences/JabRefPreferences.java @@ -101,6 +101,7 @@ import org.jabref.logic.util.OS; import org.jabref.logic.util.Version; import org.jabref.logic.util.io.AutoLinkPreferences; +import org.jabref.logic.util.io.BackupFileUtil; import org.jabref.logic.util.io.FileHistory; import org.jabref.logic.xmp.XmpPreferences; import org.jabref.model.database.BibDatabaseMode; @@ -193,6 +194,7 @@ public class JabRefPreferences implements PreferencesService { public static final String SIZE_X = "mainWindowSizeX"; public static final String POS_Y = "mainWindowPosY"; public static final String POS_X = "mainWindowPosX"; + public static final String LAST_EDITED = "lastEdited"; public static final String OPEN_LAST_EDITED = "openLastEdited"; public static final String LAST_FOCUSED = "lastFocused"; @@ -201,6 +203,8 @@ public class JabRefPreferences implements PreferencesService { public static final String LAST_USED_EXPORT = "lastUsedExport"; public static final String EXPORT_WORKING_DIRECTORY = "exportWorkingDirectory"; public static final String WORKING_DIRECTORY = "workingDirectory"; + public static final String BACKUP_DIRECTORY = "backupDirectory"; + public static final String CREATE_BACKUP = "createBackup"; public static final String KEYWORD_SEPARATOR = "groupKeywordSeparator"; public static final String AUTO_ASSIGN_GROUP = "autoAssignGroup"; @@ -587,6 +591,9 @@ private JabRefPreferences() { defaults.put(USE_XMP_PRIVACY_FILTER, Boolean.FALSE); defaults.put(WORKING_DIRECTORY, USER_HOME); defaults.put(EXPORT_WORKING_DIRECTORY, USER_HOME); + + defaults.put(CREATE_BACKUP, Boolean.TRUE); + // Remembers working directory of last import defaults.put(IMPORT_WORKING_DIRECTORY, USER_HOME); defaults.put(PREFS_EXPORT_PATH, USER_HOME); @@ -2172,8 +2179,9 @@ public FilePreferences getFilePreferences() { getBoolean(DOWNLOAD_LINKED_FILES), getBoolean(FULLTEXT_INDEX_LINKED_FILES), Path.of(get(WORKING_DIRECTORY)), - ExternalFileTypes.fromString(get(EXTERNAL_FILE_TYPES)) - ); + ExternalFileTypes.fromString(get(EXTERNAL_FILE_TYPES)), + getBoolean(CREATE_BACKUP), + getPath(BACKUP_DIRECTORY, BackupFileUtil.getAppDataBackupDir())); EasyBind.listen(filePreferences.mainFileDirectoryProperty(), (obs, oldValue, newValue) -> put(MAIN_FILE_DIRECTORY, newValue)); EasyBind.listen(filePreferences.storeFilesRelativeToBibFileProperty(), (obs, oldValue, newValue) -> putBoolean(STORE_RELATIVE_TO_BIB, newValue)); @@ -2184,6 +2192,8 @@ public FilePreferences getFilePreferences() { EasyBind.listen(filePreferences.workingDirectoryProperty(), (obs, oldValue, newValue) -> put(WORKING_DIRECTORY, newValue.toString())); filePreferences.getExternalFileTypes().addListener((SetChangeListener) c -> put(EXTERNAL_FILE_TYPES, ExternalFileTypes.toStringList(filePreferences.getExternalFileTypes()))); + EasyBind.listen(filePreferences.createBackupProperty(), (obs, oldValue, newValue) -> putBoolean(CREATE_BACKUP, newValue)); + EasyBind.listen(filePreferences.backupDirectoryProperty(), (obs, oldValue, newValue) -> put(BACKUP_DIRECTORY, newValue.toString())); return filePreferences; } diff --git a/src/test/java/org/jabref/logic/autosaveandbackup/BackupManagerDiscardedTest.java b/src/test/java/org/jabref/logic/autosaveandbackup/BackupManagerDiscardedTest.java index 85507f6c5c8..5a05afe1c24 100644 --- a/src/test/java/org/jabref/logic/autosaveandbackup/BackupManagerDiscardedTest.java +++ b/src/test/java/org/jabref/logic/autosaveandbackup/BackupManagerDiscardedTest.java @@ -40,10 +40,11 @@ class BackupManagerDiscardedTest { private SavePreferences savePreferences; private PreferencesService preferencesService; private BibEntryTypesManager bibEntryTypesManager; + private Path backupDir; @BeforeEach public void setup(@TempDir Path tempDir) throws Exception { - Path backupDir = tempDir.resolve("backups"); + this.backupDir = tempDir.resolve("backups"); Files.createDirectories(backupDir); testBib = tempDir.resolve("test.bib"); @@ -81,14 +82,14 @@ private void databaseModification() { } private void makeBackup() { - backupManager.determineBackupPathForNewBackup().ifPresent(backupManager::performBackup); + backupManager.determineBackupPathForNewBackup(backupDir).ifPresent(path -> backupManager.performBackup(backupDir)); } @Test public void noDiscardingAChangeLeadsToNewerBackupBeReported() throws Exception { databaseModification(); makeBackup(); - assertTrue(BackupManager.backupFileDiffers(testBib)); + assertTrue(BackupManager.backupFileDiffers(testBib, backupDir)); } @Test @@ -96,7 +97,7 @@ public void noDiscardingASavedChange() throws Exception { databaseModification(); makeBackup(); saveDatabase(); - assertFalse(BackupManager.backupFileDiffers(testBib)); + assertFalse(BackupManager.backupFileDiffers(testBib, backupDir)); } @Test @@ -104,6 +105,6 @@ public void discardingAChangeLeadsToNewerBackupToBeIgnored() throws Exception { databaseModification(); makeBackup(); backupManager.discardBackup(); - assertFalse(BackupManager.backupFileDiffers(testBib)); + assertFalse(BackupManager.backupFileDiffers(testBib, backupDir)); } } diff --git a/src/test/java/org/jabref/logic/autosaveandbackup/BackupManagerTest.java b/src/test/java/org/jabref/logic/autosaveandbackup/BackupManagerTest.java index e99e3a3d8e8..5ad86ed2c32 100644 --- a/src/test/java/org/jabref/logic/autosaveandbackup/BackupManagerTest.java +++ b/src/test/java/org/jabref/logic/autosaveandbackup/BackupManagerTest.java @@ -8,7 +8,9 @@ import org.jabref.logic.util.BackupFileType; import org.jabref.logic.util.io.BackupFileUtil; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotEquals; @@ -16,10 +18,18 @@ public class BackupManagerTest { + Path backupDir; + + @BeforeEach + void setup(@TempDir Path tempDir) { + backupDir = tempDir.resolve("backup"); + } + @Test public void backupFileNameIsCorrectlyGeneratedInAppDataDirectory() { Path bibPath = Path.of("tmp", "test.bib"); - Path bakPath = BackupManager.getBackupPathForNewBackup(bibPath); + backupDir = BackupFileUtil.getAppDataBackupDir(); + Path bakPath = BackupManager.getBackupPathForNewBackup(bibPath, backupDir); // Pattern is "27182d3c--test.bib--", but the hashing is implemented differently on Linux than on Windows assertNotEquals("", bakPath); @@ -28,29 +38,29 @@ public void backupFileNameIsCorrectlyGeneratedInAppDataDirectory() { @Test public void backupFileIsEqualForNonExistingBackup() throws Exception { Path originalFile = Path.of(BackupManagerTest.class.getResource("no-autosave.bib").toURI()); - assertFalse(BackupManager.backupFileDiffers(originalFile)); + assertFalse(BackupManager.backupFileDiffers(originalFile, backupDir)); } @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); + Path target = BackupFileUtil.getPathForNewBackupFileAndCreateDirectory(Path.of(BackupManagerTest.class.getResource("no-changes.bib").toURI()), BackupFileType.BACKUP, backupDir); Files.copy(source, target, StandardCopyOption.REPLACE_EXISTING); Path originalFile = Path.of(BackupManagerTest.class.getResource("no-changes.bib").toURI()); - assertFalse(BackupManager.backupFileDiffers(originalFile)); + assertFalse(BackupManager.backupFileDiffers(originalFile, backupDir)); } @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); + Path target = BackupFileUtil.getPathForNewBackupFileAndCreateDirectory(Path.of(BackupManagerTest.class.getResource("changes.bib").toURI()), BackupFileType.BACKUP, backupDir); Files.copy(source, target, StandardCopyOption.REPLACE_EXISTING); Path originalFile = Path.of(BackupManagerTest.class.getResource("changes.bib").toURI()); - assertTrue(BackupManager.backupFileDiffers(originalFile)); + assertTrue(BackupManager.backupFileDiffers(originalFile, backupDir)); } @Test @@ -60,7 +70,7 @@ public void correctBackupFileDeterminedForMultipleBakFiles() throws Exception { // Prepare test: Create backup files on "right" path // most recent file does not have any changes - Path target = BackupFileUtil.getPathForNewBackupFileAndCreateDirectory(noChangesBib, BackupFileType.BACKUP); + Path target = BackupFileUtil.getPathForNewBackupFileAndCreateDirectory(noChangesBib, BackupFileType.BACKUP, backupDir); Files.copy(noChangesBibBak, target, StandardCopyOption.REPLACE_EXISTING); // create "older" .bak files containing changes @@ -74,7 +84,7 @@ public void correctBackupFileDeterminedForMultipleBakFiles() throws Exception { } Path originalFile = noChangesBib; - assertFalse(BackupManager.backupFileDiffers(originalFile)); + assertFalse(BackupManager.backupFileDiffers(originalFile, backupDir)); } @Test @@ -82,10 +92,10 @@ public void bakFileWithNewerTimeStampLeadsToDiff() throws Exception { Path changesBib = Path.of(BackupManagerTest.class.getResource("changes.bib").toURI()); Path changesBibBak = Path.of(BackupManagerTest.class.getResource("changes.bib.bak").toURI()); - Path target = BackupFileUtil.getPathForNewBackupFileAndCreateDirectory(changesBib, BackupFileType.BACKUP); + Path target = BackupFileUtil.getPathForNewBackupFileAndCreateDirectory(changesBib, BackupFileType.BACKUP, backupDir); Files.copy(changesBibBak, target, StandardCopyOption.REPLACE_EXISTING); - assertTrue(BackupManager.backupFileDiffers(changesBib)); + assertTrue(BackupManager.backupFileDiffers(changesBib, backupDir)); } @Test @@ -93,12 +103,12 @@ public void bakFileWithOlderTimeStampDoesNotLeadToDiff() throws Exception { Path changesBib = Path.of(BackupManagerTest.class.getResource("changes.bib").toURI()); Path changesBibBak = Path.of(BackupManagerTest.class.getResource("changes.bib.bak").toURI()); - Path target = BackupFileUtil.getPathForNewBackupFileAndCreateDirectory(changesBib, BackupFileType.BACKUP); + Path target = BackupFileUtil.getPathForNewBackupFileAndCreateDirectory(changesBib, BackupFileType.BACKUP, backupDir); Files.copy(changesBibBak, target, StandardCopyOption.REPLACE_EXISTING); // Make .bak file very old Files.setLastModifiedTime(target, FileTime.fromMillis(0)); - assertFalse(BackupManager.backupFileDiffers(changesBib)); + assertFalse(BackupManager.backupFileDiffers(changesBib, backupDir)); } } diff --git a/src/test/java/org/jabref/logic/util/io/BackupFileUtilTest.java b/src/test/java/org/jabref/logic/util/io/BackupFileUtilTest.java index f44e6a45e1e..c705be1f09c 100644 --- a/src/test/java/org/jabref/logic/util/io/BackupFileUtilTest.java +++ b/src/test/java/org/jabref/logic/util/io/BackupFileUtilTest.java @@ -6,7 +6,9 @@ import org.jabref.logic.util.BackupFileType; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; import org.mockito.Answers; import org.mockito.MockedStatic; import org.mockito.Mockito; @@ -16,6 +18,13 @@ public class BackupFileUtilTest { + Path backupDir; + + @BeforeEach + void setup(@TempDir Path tempDir) { + backupDir = tempDir.resolve("backup"); + } + @Test void uniqueFilePrefix() { // We cannot test for a concrete hash code, because hashing implementation differs from environment to environment @@ -25,18 +34,21 @@ void uniqueFilePrefix() { @Test void getPathOfBackupFileAndCreateDirectoryReturnsAppDirectoryInCaseOfNoError() { String start = BackupFileUtil.getAppDataBackupDir().toString(); - String result = BackupFileUtil.getPathForNewBackupFileAndCreateDirectory(Path.of("test.bib"), BackupFileType.BACKUP).toString(); + backupDir = BackupFileUtil.getAppDataBackupDir(); + String result = BackupFileUtil.getPathForNewBackupFileAndCreateDirectory(Path.of("test.bib"), BackupFileType.BACKUP, backupDir).toString(); // We just check the prefix assertEquals(start, result.substring(0, start.length())); } @Test void getPathOfBackupFileAndCreateDirectoryReturnsSameDirectoryInCaseOfException() { + + backupDir = BackupFileUtil.getAppDataBackupDir(); 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); + Path result = BackupFileUtil.getPathForNewBackupFileAndCreateDirectory(testPath, BackupFileType.BACKUP, backupDir); // 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); } From ddca12a2b4597e45da22a9e8ebb7e2fefbe92209 Mon Sep 17 00:00:00 2001 From: Siedlerchr Date: Fri, 28 Apr 2023 22:05:57 +0200 Subject: [PATCH 02/17] bind ui to prefs, fix making of backups in correct dir TODO: test flag if createBackup --- src/main/java/org/jabref/gui/JabRefFrame.java | 6 +-- src/main/java/org/jabref/gui/LibraryTab.java | 4 +- .../java/org/jabref/gui/StateManager.java | 4 +- .../gui/backup/BackupResolverDialog.java | 4 +- .../jabref/gui/dialogs/BackupUIManager.java | 10 ++--- .../gui/exporter/SaveDatabaseAction.java | 2 +- .../jabref/gui/preferences/file/FileTab.java | 3 ++ .../preferences/file/FileTabViewModel.java | 1 - .../autosaveandbackup/BackupManager.java | 43 +++++++++---------- .../jabref/logic/util/io/BackupFileUtil.java | 9 ---- .../BackupManagerDiscardedTest.java | 4 +- 11 files changed, 41 insertions(+), 49 deletions(-) diff --git a/src/main/java/org/jabref/gui/JabRefFrame.java b/src/main/java/org/jabref/gui/JabRefFrame.java index d3d0b10172e..307302a3dd9 100644 --- a/src/main/java/org/jabref/gui/JabRefFrame.java +++ b/src/main/java/org/jabref/gui/JabRefFrame.java @@ -490,7 +490,7 @@ public boolean quit() { context.clearDBMSSynchronizer(); } AutosaveManager.shutdown(context); - BackupManager.shutdown(context, prefs); + BackupManager.shutdown(context, prefs.getFilePreferences().getBackupDirectory(), prefs.getFilePreferences().shouldCreateBackup()); context.getDatabasePath().map(Path::toAbsolutePath).map(Path::toString).ifPresent(filenames::add); } @@ -1235,7 +1235,7 @@ private boolean confirmClose(LibraryTab libraryTab) { } if (buttonType.equals(discardChanges)) { - BackupManager.discardBackup(libraryTab.getBibDatabaseContext()); + BackupManager.discardBackup(libraryTab.getBibDatabaseContext(), prefs.getFilePreferences().getBackupDirectory()); return true; } @@ -1312,7 +1312,7 @@ private void closeTab(LibraryTab libraryTab) { removeTab(libraryTab); } AutosaveManager.shutdown(context); - BackupManager.shutdown(context, prefs); + BackupManager.shutdown(context, prefs.getFilePreferences().getBackupDirectory(), prefs.getFilePreferences().shouldCreateBackup()); } private void removeTab(LibraryTab libraryTab) { diff --git a/src/main/java/org/jabref/gui/LibraryTab.java b/src/main/java/org/jabref/gui/LibraryTab.java index 7373ed667f4..98259e67127 100644 --- a/src/main/java/org/jabref/gui/LibraryTab.java +++ b/src/main/java/org/jabref/gui/LibraryTab.java @@ -289,7 +289,7 @@ public void installAutosaveManagerAndBackupManager() { AutosaveManager autosaveManager = AutosaveManager.start(bibDatabaseContext); autosaveManager.registerListener(new AutosaveUiManager(this)); } - if (isDatabaseReadyForBackup(bibDatabaseContext)) { + if (isDatabaseReadyForBackup(bibDatabaseContext) && preferencesService.getFilePreferences().shouldCreateBackup()) { BackupManager.start(bibDatabaseContext, Globals.entryTypesManager, preferencesService); } } @@ -705,7 +705,7 @@ private void saveDividerLocation(Number position) { public void cleanUp() { changeMonitor.ifPresent(DatabaseChangeMonitor::unregister); AutosaveManager.shutdown(bibDatabaseContext); - BackupManager.shutdown(bibDatabaseContext, preferencesService); + BackupManager.shutdown(bibDatabaseContext, preferencesService.getFilePreferences().getBackupDirectory(), preferencesService.getFilePreferences().shouldCreateBackup()); } /** diff --git a/src/main/java/org/jabref/gui/StateManager.java b/src/main/java/org/jabref/gui/StateManager.java index fffab647dd0..76d696aad4b 100644 --- a/src/main/java/org/jabref/gui/StateManager.java +++ b/src/main/java/org/jabref/gui/StateManager.java @@ -62,7 +62,7 @@ public class StateManager { private final OptionalObjectProperty activeSearchQuery = OptionalObjectProperty.empty(); private final ObservableMap searchResultMap = FXCollections.observableHashMap(); private final OptionalObjectProperty focusOwner = OptionalObjectProperty.empty(); - private final ObservableList>> backgroundTasks = FXCollections.observableArrayList(task -> new Observable[] {task.getValue().progressProperty(), task.getValue().runningProperty()}); + private final ObservableList, Task>> backgroundTasks = FXCollections.observableArrayList(task -> new Observable[] {task.getValue().progressProperty(), task.getValue().runningProperty()}); private final EasyBinding anyTaskRunning = EasyBind.reduce(backgroundTasks, tasks -> tasks.map(Pair::getValue).anyMatch(Task::isRunning)); private final EasyBinding anyTasksThatWillNotBeRecoveredRunning = EasyBind.reduce(backgroundTasks, tasks -> tasks.anyMatch(task -> !task.getKey().willBeRecoveredAutomatically() && task.getValue().isRunning())); private final EasyBinding tasksProgress = EasyBind.reduce(backgroundTasks, tasks -> tasks.map(Pair::getValue).filter(Task::isRunning).mapToDouble(Task::getProgress).average().orElse(1)); @@ -169,7 +169,7 @@ public ObservableList> getBackgroundTasks() { return EasyBind.map(backgroundTasks, Pair::getValue); } - public void addBackgroundTask(BackgroundTask backgroundTask, Task task) { + public void addBackgroundTask(BackgroundTask backgroundTask, Task task) { this.backgroundTasks.add(0, new Pair<>(backgroundTask, task)); } diff --git a/src/main/java/org/jabref/gui/backup/BackupResolverDialog.java b/src/main/java/org/jabref/gui/backup/BackupResolverDialog.java index e806ff0d736..de43a067eb8 100644 --- a/src/main/java/org/jabref/gui/backup/BackupResolverDialog.java +++ b/src/main/java/org/jabref/gui/backup/BackupResolverDialog.java @@ -26,13 +26,13 @@ public class BackupResolverDialog extends FXDialog { private static final Logger LOGGER = LoggerFactory.getLogger(BackupResolverDialog.class); - public BackupResolverDialog(Path originalPath) { + public BackupResolverDialog(Path originalPath, Path backupDir) { super(AlertType.CONFIRMATION, Localization.lang("Backup found"), true); setHeaderText(null); getDialogPane().setMinHeight(180); getDialogPane().getButtonTypes().setAll(RESTORE_FROM_BACKUP, REVIEW_BACKUP, IGNORE_BACKUP); - Optional backupPathOpt = BackupFileUtil.yyyyyyyyyy(originalPath, BackupFileType.BACKUP); + Optional backupPathOpt = BackupFileUtil.getPathOfLatestExistingBackupFile(originalPath, BackupFileType.BACKUP, backupDir); String backupFilename = backupPathOpt.map(Path::getFileName).map(Path::toString).orElse(Localization.lang("File not found")); String content = new StringBuilder() .append(Localization.lang("A backup file for '%0' was found at [%1]", diff --git a/src/main/java/org/jabref/gui/dialogs/BackupUIManager.java b/src/main/java/org/jabref/gui/dialogs/BackupUIManager.java index a872034b547..4a820615a48 100644 --- a/src/main/java/org/jabref/gui/dialogs/BackupUIManager.java +++ b/src/main/java/org/jabref/gui/dialogs/BackupUIManager.java @@ -38,7 +38,7 @@ private BackupUIManager() { } public static Optional showRestoreBackupDialog(DialogService dialogService, Path originalPath, PreferencesService preferencesService) { - var actionOpt = showBackupResolverDialog(dialogService, originalPath); + var actionOpt = showBackupResolverDialog(dialogService, originalPath, preferencesService.getFilePreferences().getBackupDirectory()); return actionOpt.flatMap(action -> { if (action == BackupResolverDialog.RESTORE_FROM_BACKUP) { BackupManager.restoreBackup(originalPath, preferencesService.getFilePreferences().getBackupDirectory()); @@ -50,20 +50,20 @@ public static Optional showRestoreBackupDialog(DialogService dialo }); } - private static Optional showBackupResolverDialog(DialogService dialogService, Path originalPath) { - return DefaultTaskExecutor.runInJavaFXThread(() -> dialogService.showCustomDialogAndWait(new BackupResolverDialog(originalPath))); + private static Optional showBackupResolverDialog(DialogService dialogService, Path originalPath, Path backupDir) { + return DefaultTaskExecutor.runInJavaFXThread(() -> dialogService.showCustomDialogAndWait(new BackupResolverDialog(originalPath, backupDir))); } private static Optional showReviewBackupDialog(DialogService dialogService, Path originalPath, PreferencesService preferencesService) { try { - ImportFormatPreferences importFormatPreferences = Globals.prefs.getImportFormatPreferences(); + ImportFormatPreferences importFormatPreferences = preferencesService.getImportFormatPreferences(); // The database of the originalParserResult will be modified ParserResult originalParserResult = OpenDatabase.loadDatabase(originalPath, importFormatPreferences, Globals.getFileUpdateMonitor()); // This will be modified by using the `DatabaseChangesResolverDialog`. BibDatabaseContext originalDatabase = originalParserResult.getDatabaseContext(); - Path backupPath = BackupFileUtil.yyyyyyyyyy(originalPath, BackupFileType.BACKUP).orElseThrow(); + Path backupPath = BackupFileUtil.getPathOfLatestExistingBackupFile(originalPath, BackupFileType.BACKUP, preferencesService.getFilePreferences().getBackupDirectory()).orElseThrow(); BibDatabaseContext backupDatabase = OpenDatabase.loadDatabase(backupPath, importFormatPreferences, new DummyFileUpdateMonitor()).getDatabaseContext(); DatabaseChangeResolverFactory changeResolverFactory = new DatabaseChangeResolverFactory(dialogService, originalDatabase, preferencesService.getBibEntryPreferences()); diff --git a/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java b/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java index b5b74e60cc4..a0944d0961d 100644 --- a/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java +++ b/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java @@ -116,7 +116,7 @@ boolean saveAs(Path file, SaveDatabaseMode mode) { final Path oldFile = databasePath.get(); context.setDatabasePath(oldFile); AutosaveManager.shutdown(context); - BackupManager.shutdown(context, this.preferences); + BackupManager.shutdown(context, this.preferences.getFilePreferences().getBackupDirectory(), preferences.getFilePreferences().shouldCreateBackup()); } // Set new location diff --git a/src/main/java/org/jabref/gui/preferences/file/FileTab.java b/src/main/java/org/jabref/gui/preferences/file/FileTab.java index d24d74a368d..fbb76edaba5 100644 --- a/src/main/java/org/jabref/gui/preferences/file/FileTab.java +++ b/src/main/java/org/jabref/gui/preferences/file/FileTab.java @@ -54,6 +54,9 @@ public void initialize() { alwaysReformatBib.selectedProperty().bindBidirectional(viewModel.alwaysReformatBibProperty()); autosaveLocalLibraries.selectedProperty().bindBidirectional(viewModel.autosaveLocalLibrariesProperty()); + createBackup.selectedProperty().bindBidirectional(viewModel.createBackupProperty()); + backupDirectory.textProperty().bindBidirectional(viewModel.backupDirectoryProperty()); + ActionFactory actionFactory = new ActionFactory(preferencesService.getKeyBindingRepository()); actionFactory.configureIconButton(StandardActions.HELP, new HelpAction(HelpFile.AUTOSAVE, dialogService), autosaveLocalLibrariesHelp); } diff --git a/src/main/java/org/jabref/gui/preferences/file/FileTabViewModel.java b/src/main/java/org/jabref/gui/preferences/file/FileTabViewModel.java index 30849fd5437..6e9ac6109a6 100644 --- a/src/main/java/org/jabref/gui/preferences/file/FileTabViewModel.java +++ b/src/main/java/org/jabref/gui/preferences/file/FileTabViewModel.java @@ -110,6 +110,5 @@ public void backupFileDirBrowse() { new DirectoryDialogConfiguration.Builder().withInitialDirectory(Path.of(backupDirectoryProperty().getValue())).build(); dialogService.showDirectorySelectionDialog(dirDialogConfiguration) .ifPresent(dir -> backupDirectoryProperty.setValue(dir.toString())); - } } diff --git a/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java b/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java index 727a47f0201..bf3f9365b66 100644 --- a/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java +++ b/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java @@ -58,20 +58,20 @@ public class BackupManager { private final ScheduledThreadPoolExecutor executor; private final CoarseChangeFilter changeFilter; private final BibEntryTypesManager entryTypesManager; - private final Path backupPath; // 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 = false; + private boolean needsBackup = true; + private final boolean createBackup; BackupManager(BibDatabaseContext bibDatabaseContext, BibEntryTypesManager entryTypesManager, PreferencesService preferences) { this.bibDatabaseContext = bibDatabaseContext; this.entryTypesManager = entryTypesManager; this.preferences = preferences; - this.backupPath = preferences.getFilePreferences().getBackupDirectory(); this.executor = new ScheduledThreadPoolExecutor(2); + this.createBackup = preferences.getFilePreferences().shouldCreateBackup(); changeFilter = new CoarseChangeFilter(bibDatabaseContext); changeFilter.registerListener(this); @@ -111,9 +111,8 @@ public static BackupManager start(BibDatabaseContext bibDatabaseContext, BibEntr * * @param bibDatabaseContext Associated {@link BibDatabaseContext} */ - public static void discardBackup(BibDatabaseContext bibDatabaseContext) { - runningInstances.stream().filter(instance -> instance.bibDatabaseContext == bibDatabaseContext).forEach( - BackupManager::discardBackup); + public static void discardBackup(BibDatabaseContext bibDatabaseContext, Path backupDir) { + runningInstances.stream().filter(instance -> instance.bibDatabaseContext == bibDatabaseContext).forEach(backupManager -> backupManager.discardBackup(backupDir)); } /** @@ -121,12 +120,10 @@ public static void discardBackup(BibDatabaseContext bibDatabaseContext) { * * @param bibDatabaseContext Associated {@link BibDatabaseContext} */ - public static void shutdown(BibDatabaseContext bibDatabaseContext, PreferencesService preferencesService) { - - Path backupDir = preferencesService.getFilePreferences().getBackupDirectory(); - - runningInstances.stream().filter(instance -> instance.bibDatabaseContext == bibDatabaseContext).forEach( - manager -> manager.shutdown(backupDir)); + public static void shutdown(BibDatabaseContext bibDatabaseContext, Path backupDir, boolean createBackup) { + if (createBackup) { + runningInstances.stream().filter(instance -> instance.bibDatabaseContext == bibDatabaseContext).forEach(manager -> manager.shutdown(backupDir)); + } runningInstances.removeIf(instance -> instance.bibDatabaseContext == bibDatabaseContext); } @@ -143,7 +140,7 @@ public static void shutdown(BibDatabaseContext bibDatabaseContext, PreferencesSe * In the case of an exception true is returned to ensure that the user checks the output. */ public static boolean backupFileDiffers(Path originalPath, Path backupDir) { - Path discardedFile = determineDiscardedFile(originalPath); + Path discardedFile = determineDiscardedFile(originalPath, backupDir); if (Files.exists(discardedFile)) { try { Files.delete(discardedFile); @@ -212,6 +209,7 @@ Optional determineBackupPathForNewBackup(Path backupDir) { * * SIDE EFFECT: Deletes oldest backup file * + * * @param backupPath the path where the library should be backed up to */ void performBackup(Path backupPath) { @@ -252,9 +250,8 @@ void performBackup(Path backupPath) { } } - private static Path determineDiscardedFile(Path file) { - return BackupFileUtil.getAppDataBackupDir().resolve( - BackupFileUtil.getUniqueFilePrefix(file) + "--" + file.getFileName() + "--discarded"); + private static Path determineDiscardedFile(Path file, Path backupDir) { + return backupDir.resolve(BackupFileUtil.getUniqueFilePrefix(file) + "--" + file.getFileName() + "--discarded"); } /** @@ -263,8 +260,8 @@ private static Path determineDiscardedFile(Path file) { * We do not delete any files, because the user might want to recover old backup files. * Therefore, we mark discarded backups by a --discarded file. */ - public void discardBackup() { - Path path = determineDiscardedFile(bibDatabaseContext.getDatabasePath().get()); + public void discardBackup(Path backupDir) { + Path path = determineDiscardedFile(bibDatabaseContext.getDatabasePath().get(), backupDir); try { Files.createFile(path); } catch (IOException e) { @@ -297,7 +294,7 @@ private void startBackupTask(Path backupDir) { executor.scheduleAtFixedRate( // We need to determine the backup path on each action, because we use the timestamp in the filename - () -> determineBackupPathForNewBackup(backupDir).ifPresent(path -> this.performBackup(backupDir)), + () -> determineBackupPathForNewBackup(backupDir).ifPresent(path -> this.performBackup(path)), DELAY_BETWEEN_BACKUP_ATTEMPTS_IN_SECONDS, DELAY_BETWEEN_BACKUP_ATTEMPTS_IN_SECONDS, TimeUnit.SECONDS); @@ -326,14 +323,16 @@ private void fillQueue() { /** * Unregisters the BackupManager from the eventBus of {@link BibDatabaseContext}. * This method should only be used when closing a database/JabRef in a normal way. - * @param path + * @param path The backup directory */ private void shutdown(Path backupDir) { changeFilter.unregisterListener(this); changeFilter.shutdown(); executor.shutdown(); - // Ensure that backup is a recent one - determineBackupPathForNewBackup(backupDir).ifPresent(this::performBackup); + if (createBackup) { + // Ensure that backup is a recent one + determineBackupPathForNewBackup(backupDir).ifPresent(this::performBackup); + } } } diff --git a/src/main/java/org/jabref/logic/util/io/BackupFileUtil.java b/src/main/java/org/jabref/logic/util/io/BackupFileUtil.java index c20c133fd67..76b9ca753f3 100644 --- a/src/main/java/org/jabref/logic/util/io/BackupFileUtil.java +++ b/src/main/java/org/jabref/logic/util/io/BackupFileUtil.java @@ -99,15 +99,6 @@ public static Optional getPathOfLatestExistingBackupFile(Path targetFile, return mostRecentFile; } - /** - * Finds the latest backup (.sav). If it does not exist, an empty optional is returned - * - * @param targetFile the full path of the file to backup - */ - public static Optional yyyyyyyyyy(Path targetFile, BackupFileType fileType) { - return getPathOfLatestExistingBackupFile(targetFile, fileType, Path.of("")); - } - /** *

* Determines a unique file prefix. diff --git a/src/test/java/org/jabref/logic/autosaveandbackup/BackupManagerDiscardedTest.java b/src/test/java/org/jabref/logic/autosaveandbackup/BackupManagerDiscardedTest.java index 5a05afe1c24..b7e2e79da8c 100644 --- a/src/test/java/org/jabref/logic/autosaveandbackup/BackupManagerDiscardedTest.java +++ b/src/test/java/org/jabref/logic/autosaveandbackup/BackupManagerDiscardedTest.java @@ -82,7 +82,7 @@ private void databaseModification() { } private void makeBackup() { - backupManager.determineBackupPathForNewBackup(backupDir).ifPresent(path -> backupManager.performBackup(backupDir)); + backupManager.determineBackupPathForNewBackup(backupDir).ifPresent(path -> backupManager.performBackup(path)); } @Test @@ -104,7 +104,7 @@ public void noDiscardingASavedChange() throws Exception { public void discardingAChangeLeadsToNewerBackupToBeIgnored() throws Exception { databaseModification(); makeBackup(); - backupManager.discardBackup(); + backupManager.discardBackup(backupDir); assertFalse(BackupManager.backupFileDiffers(testBib, backupDir)); } } From 7c534b26001bc4a69a500193504b44fec869cb10 Mon Sep 17 00:00:00 2001 From: Siedlerchr Date: Fri, 28 Apr 2023 22:27:20 +0200 Subject: [PATCH 03/17] =?UTF-8?q?fix=20l1=C3=9Fn?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/main/resources/l10n/JabRef_en.properties | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main/resources/l10n/JabRef_en.properties b/src/main/resources/l10n/JabRef_en.properties index 8a955defa7a..8f54e71c313 100644 --- a/src/main/resources/l10n/JabRef_en.properties +++ b/src/main/resources/l10n/JabRef_en.properties @@ -2543,3 +2543,5 @@ Delete\ %0\ files=Delete %0 files Delete\ %0\ files\ permanently\ from\ disk,\ or\ just\ remove\ the\ files\ from\ the\ entry?\ Pressing\ Delete\ will\ delete\ the\ files\ permanently\ from\ disk.=Delete %0 files permanently from disk, or just remove the files from the entry? Pressing Delete will delete the files permanently from disk. Error\ accessing\ file\ '%0'.=Error accessing file '%0'. This\ operation\ requires\ selected\ linked\ files.=This operation requires selected linked files. + +Create\ backup=Create backup \ No newline at end of file From 473a5e06f3ddab771e4ed3d4bf22f5ae74342af6 Mon Sep 17 00:00:00 2001 From: Siedlerchr Date: Fri, 28 Apr 2023 22:31:26 +0200 Subject: [PATCH 04/17] checkstyle and l10n --- src/main/java/org/jabref/gui/dialogs/BackupUIManager.java | 2 +- .../java/org/jabref/logic/autosaveandbackup/BackupManager.java | 1 + src/test/java/org/jabref/logic/util/io/BackupFileUtilTest.java | 1 - 3 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/jabref/gui/dialogs/BackupUIManager.java b/src/main/java/org/jabref/gui/dialogs/BackupUIManager.java index 4a820615a48..b1c4981e744 100644 --- a/src/main/java/org/jabref/gui/dialogs/BackupUIManager.java +++ b/src/main/java/org/jabref/gui/dialogs/BackupUIManager.java @@ -56,7 +56,7 @@ private static Optional showBackupResolverDialog(DialogService dialo private static Optional showReviewBackupDialog(DialogService dialogService, Path originalPath, PreferencesService preferencesService) { try { - ImportFormatPreferences importFormatPreferences = preferencesService.getImportFormatPreferences(); + ImportFormatPreferences importFormatPreferences = preferencesService.getImportFormatPreferences(); // The database of the originalParserResult will be modified ParserResult originalParserResult = OpenDatabase.loadDatabase(originalPath, importFormatPreferences, Globals.getFileUpdateMonitor()); diff --git a/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java b/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java index bf3f9365b66..5ba5d436784 100644 --- a/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java +++ b/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java @@ -323,6 +323,7 @@ private void fillQueue() { /** * Unregisters the BackupManager from the eventBus of {@link BibDatabaseContext}. * This method should only be used when closing a database/JabRef in a normal way. + * * @param path The backup directory */ private void shutdown(Path backupDir) { diff --git a/src/test/java/org/jabref/logic/util/io/BackupFileUtilTest.java b/src/test/java/org/jabref/logic/util/io/BackupFileUtilTest.java index c705be1f09c..bedea46b580 100644 --- a/src/test/java/org/jabref/logic/util/io/BackupFileUtilTest.java +++ b/src/test/java/org/jabref/logic/util/io/BackupFileUtilTest.java @@ -42,7 +42,6 @@ void getPathOfBackupFileAndCreateDirectoryReturnsAppDirectoryInCaseOfNoError() { @Test void getPathOfBackupFileAndCreateDirectoryReturnsSameDirectoryInCaseOfException() { - backupDir = BackupFileUtil.getAppDataBackupDir(); try (MockedStatic files = Mockito.mockStatic(Files.class, Answers.RETURNS_DEEP_STUBS)) { files.when(() -> Files.createDirectories(BackupFileUtil.getAppDataBackupDir())) From 0bb9321a19041efd88b66e30774b0f6f6bdd06dd Mon Sep 17 00:00:00 2001 From: Siedlerchr Date: Fri, 28 Apr 2023 22:33:45 +0200 Subject: [PATCH 05/17] checkstyle --- .../java/org/jabref/logic/autosaveandbackup/BackupManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java b/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java index 5ba5d436784..733dadf6462 100644 --- a/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java +++ b/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java @@ -71,7 +71,7 @@ public class BackupManager { this.entryTypesManager = entryTypesManager; this.preferences = preferences; this.executor = new ScheduledThreadPoolExecutor(2); - this.createBackup = preferences.getFilePreferences().shouldCreateBackup(); + this.createBackup = preferences.getFilePreferences().shouldCreateBackup(); changeFilter = new CoarseChangeFilter(bibDatabaseContext); changeFilter.registerListener(this); From b935fde92cb20a8461ab4afddf78ee1f6666ba10 Mon Sep 17 00:00:00 2001 From: Siedlerchr Date: Mon, 1 May 2023 20:34:18 +0200 Subject: [PATCH 06/17] fix merge conflicts --- .../org/jabref/gui/preferences/file/FileTab.java | 2 +- .../gui/preferences/file/FileTabViewModel.java | 16 +++++++++++----- .../logic/autosaveandbackup/BackupManager.java | 12 +++++++----- 3 files changed, 19 insertions(+), 11 deletions(-) diff --git a/src/main/java/org/jabref/gui/preferences/file/FileTab.java b/src/main/java/org/jabref/gui/preferences/file/FileTab.java index 1e2424c5297..04c01de911b 100644 --- a/src/main/java/org/jabref/gui/preferences/file/FileTab.java +++ b/src/main/java/org/jabref/gui/preferences/file/FileTab.java @@ -40,7 +40,7 @@ public FileTab() { } public void initialize() { - this.viewModel = new FileTabViewModel(preferencesService.getImportExportPreferences(), preferencesService.getFieldPreferences()); + this.viewModel = new FileTabViewModel(preferencesService.getImportExportPreferences(), preferencesService.getFieldPreferences(), preferencesService.getFilePreferences(), dialogService); noWrapFiles.textProperty().bindBidirectional(viewModel.noWrapFilesProperty()); diff --git a/src/main/java/org/jabref/gui/preferences/file/FileTabViewModel.java b/src/main/java/org/jabref/gui/preferences/file/FileTabViewModel.java index c37c3762bbb..c64626c39ec 100644 --- a/src/main/java/org/jabref/gui/preferences/file/FileTabViewModel.java +++ b/src/main/java/org/jabref/gui/preferences/file/FileTabViewModel.java @@ -12,6 +12,7 @@ import org.jabref.gui.util.DirectoryDialogConfiguration; import org.jabref.logic.bibtex.FieldPreferences; import org.jabref.model.entry.field.FieldFactory; +import org.jabref.preferences.FilePreferences; import org.jabref.preferences.ImportExportPreferences; public class FileTabViewModel implements PreferenceTabViewModel { @@ -23,12 +24,17 @@ public class FileTabViewModel implements PreferenceTabViewModel { private final BooleanProperty alwaysReformatBibProperty = new SimpleBooleanProperty(); private final BooleanProperty autosaveLocalLibraries = new SimpleBooleanProperty(); + private final BooleanProperty createBackupProperty = new SimpleBooleanProperty(); + private final StringProperty backupDirectoryProperty = new SimpleStringProperty(""); + private final ImportExportPreferences importExportPreferences; private final FieldPreferences fieldPreferences; private final DialogService dialogService; + private final FilePreferences filePreferences; - FileTabViewModel(ImportExportPreferences importExportPreferences, FieldPreferences fieldPreferences) { + FileTabViewModel(ImportExportPreferences importExportPreferences, FieldPreferences fieldPreferences, FilePreferences filePreferences, DialogService dialogService) { this.importExportPreferences = importExportPreferences; + this.fieldPreferences = fieldPreferences; this.filePreferences = filePreferences; this.dialogService = dialogService; } @@ -43,8 +49,8 @@ public void setValues() { alwaysReformatBibProperty.setValue(importExportPreferences.shouldAlwaysReformatOnSave()); autosaveLocalLibraries.setValue(importExportPreferences.shouldAutoSave()); - createBackupProperty.setValue(fieldPreferences.shouldCreateBackup()); - backupDirectoryProperty.setValue(fieldPreferences.getBackupDirectory().toString()); + createBackupProperty.setValue(filePreferences.shouldCreateBackup()); + backupDirectoryProperty.setValue(filePreferences.getBackupDirectory().toString()); } @Override @@ -55,8 +61,8 @@ public void storeSettings() { importExportPreferences.setAlwaysReformatOnSave(alwaysReformatBibProperty.getValue()); importExportPreferences.setAutoSave(autosaveLocalLibraries.getValue()); - fieldPreferences.createBackupProperty().setValue(createBackupProperty.getValue()); - fieldPreferences.backupDirectoryProperty().setValue(Path.of(backupDirectoryProperty.getValue())); + filePreferences.createBackupProperty().setValue(createBackupProperty.getValue()); + filePreferences.backupDirectoryProperty().setValue(Path.of(backupDirectoryProperty.getValue())); } // ImportExport diff --git a/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java b/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java index 4c9588f6228..a22f977761a 100644 --- a/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java +++ b/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java @@ -64,12 +64,12 @@ public class BackupManager { private boolean needsBackup = false; + BackupManager(BibDatabaseContext bibDatabaseContext, BibEntryTypesManager entryTypesManager, PreferencesService preferences) { this.bibDatabaseContext = bibDatabaseContext; this.entryTypesManager = entryTypesManager; this.preferences = preferences; this.executor = new ScheduledThreadPoolExecutor(2); - this.createBackup = preferences.getFilePreferences().shouldCreateBackup(); changeFilter = new CoarseChangeFilter(bibDatabaseContext); changeFilter.registerListener(this); @@ -117,10 +117,11 @@ public static void discardBackup(BibDatabaseContext bibDatabaseContext, Path bac * Shuts down the BackupManager which is associated with the given {@link BibDatabaseContext}. * * @param bibDatabaseContext Associated {@link BibDatabaseContext} + * @param b + * @param path */ - public static void shutdown(BibDatabaseContext bibDatabaseContext) { - runningInstances.stream().filter(instance -> instance.bibDatabaseContext == bibDatabaseContext).forEach( - BackupManager::shutdown); + public static void shutdown(BibDatabaseContext bibDatabaseContext, Path path, boolean createBackup) { + runningInstances.stream().filter(instance -> instance.bibDatabaseContext == bibDatabaseContext).forEach(backupManager -> backupManager.shutdown(path, createBackup)); runningInstances.removeIf(instance -> instance.bibDatabaseContext == bibDatabaseContext); } @@ -328,8 +329,9 @@ private void fillQueue() { * This method should only be used when closing a database/JabRef in a normal way. * * @param path The backup directory + * @param createBackup If the backup manager should still perform a backup */ - private void shutdown(Path backupDir) { + private void shutdown(Path backupDir, boolean createBackup) { changeFilter.unregisterListener(this); changeFilter.shutdown(); executor.shutdown(); From 5dae3721f89884622da1edfeb0f9df04b7c71f61 Mon Sep 17 00:00:00 2001 From: Siedlerchr Date: Mon, 1 May 2023 21:00:47 +0200 Subject: [PATCH 07/17] add comment --- src/main/java/org/jabref/preferences/JabRefPreferences.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/org/jabref/preferences/JabRefPreferences.java b/src/main/java/org/jabref/preferences/JabRefPreferences.java index 08b5999c8ab..4494cd93b6f 100644 --- a/src/main/java/org/jabref/preferences/JabRefPreferences.java +++ b/src/main/java/org/jabref/preferences/JabRefPreferences.java @@ -2122,6 +2122,7 @@ public FilePreferences getFilePreferences() { Path.of(get(WORKING_DIRECTORY)), ExternalFileTypes.fromString(get(EXTERNAL_FILE_TYPES)), getBoolean(CREATE_BACKUP), + // We choose the data directory, because a ".bak" file should survive cache cleanups getPath(BACKUP_DIRECTORY, BackupFileUtil.getAppDataBackupDir())); EasyBind.listen(filePreferences.mainFileDirectoryProperty(), (obs, oldValue, newValue) -> put(MAIN_FILE_DIRECTORY, newValue)); From 2f447d9660c77c23ad2828677a225ddb6c383f4e Mon Sep 17 00:00:00 2001 From: Siedlerchr Date: Thu, 4 May 2023 20:56:32 +0200 Subject: [PATCH 08/17] fix checkstyle --- .../logic/autosaveandbackup/BackupManager.java | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java b/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java index a22f977761a..858eaae4824 100644 --- a/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java +++ b/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java @@ -61,10 +61,8 @@ public class BackupManager { // 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 = false; - BackupManager(BibDatabaseContext bibDatabaseContext, BibEntryTypesManager entryTypesManager, PreferencesService preferences) { this.bibDatabaseContext = bibDatabaseContext; this.entryTypesManager = entryTypesManager; @@ -117,11 +115,11 @@ public static void discardBackup(BibDatabaseContext bibDatabaseContext, Path bac * Shuts down the BackupManager which is associated with the given {@link BibDatabaseContext}. * * @param bibDatabaseContext Associated {@link BibDatabaseContext} - * @param b - * @param path + * @param createBackup True, if a backup should be created + * @param backupDir The path to the backup directory */ - public static void shutdown(BibDatabaseContext bibDatabaseContext, Path path, boolean createBackup) { - runningInstances.stream().filter(instance -> instance.bibDatabaseContext == bibDatabaseContext).forEach(backupManager -> backupManager.shutdown(path, createBackup)); + public static void shutdown(BibDatabaseContext bibDatabaseContext, Path backupDir, boolean createBackup) { + runningInstances.stream().filter(instance -> instance.bibDatabaseContext == bibDatabaseContext).forEach(backupManager -> backupManager.shutdown(backupDir, createBackup)); runningInstances.removeIf(instance -> instance.bibDatabaseContext == bibDatabaseContext); } @@ -328,7 +326,7 @@ private void fillQueue() { * Unregisters the BackupManager from the eventBus of {@link BibDatabaseContext}. * This method should only be used when closing a database/JabRef in a normal way. * - * @param path The backup directory + * @param backupDir The backup directory * @param createBackup If the backup manager should still perform a backup */ private void shutdown(Path backupDir, boolean createBackup) { From bf4c43dc8a56132d1edec2a5b29403856b4f5877 Mon Sep 17 00:00:00 2001 From: Carl Christian Snethlage <50491877+calixtus@users.noreply.github.com> Date: Thu, 4 May 2023 21:01:41 +0200 Subject: [PATCH 09/17] Moved backup settings to GeneralTab --- .../jabref/gui/preferences/file/FileTab.fxml | 21 +--------- .../jabref/gui/preferences/file/FileTab.java | 9 ----- .../preferences/file/FileTabViewModel.java | 27 ------------- .../gui/preferences/general/GeneralTab.fxml | 20 ++++++++++ .../gui/preferences/general/GeneralTab.java | 13 ++++++- .../general/GeneralTabViewModel.java | 38 +++++++++++++++++-- 6 files changed, 69 insertions(+), 59 deletions(-) diff --git a/src/main/java/org/jabref/gui/preferences/file/FileTab.fxml b/src/main/java/org/jabref/gui/preferences/file/FileTab.fxml index 52d7772599b..8a68749dd2f 100644 --- a/src/main/java/org/jabref/gui/preferences/file/FileTab.fxml +++ b/src/main/java/org/jabref/gui/preferences/file/FileTab.fxml @@ -6,12 +6,8 @@ - - - - @@ -42,19 +38,6 @@ - - - - - - + + diff --git a/src/main/java/org/jabref/gui/preferences/file/FileTab.java b/src/main/java/org/jabref/gui/preferences/file/FileTab.java index 04c01de911b..36bd53ff158 100644 --- a/src/main/java/org/jabref/gui/preferences/file/FileTab.java +++ b/src/main/java/org/jabref/gui/preferences/file/FileTab.java @@ -25,8 +25,6 @@ public class FileTab extends AbstractPreferenceTabView impleme @FXML private RadioButton resolveStrings; @FXML private TextField resolveStringsForFields; @FXML private CheckBox alwaysReformatBib; - @FXML private CheckBox createBackup; - @FXML private TextField backupDirectory; @FXML private CheckBox autosaveLocalLibraries; @FXML private Button autosaveLocalLibrariesHelp; @@ -52,9 +50,6 @@ public void initialize() { alwaysReformatBib.selectedProperty().bindBidirectional(viewModel.alwaysReformatBibProperty()); autosaveLocalLibraries.selectedProperty().bindBidirectional(viewModel.autosaveLocalLibrariesProperty()); - createBackup.selectedProperty().bindBidirectional(viewModel.createBackupProperty()); - backupDirectory.textProperty().bindBidirectional(viewModel.backupDirectoryProperty()); - ActionFactory actionFactory = new ActionFactory(preferencesService.getKeyBindingRepository()); actionFactory.configureIconButton(StandardActions.HELP, new HelpAction(HelpFile.AUTOSAVE, dialogService), autosaveLocalLibrariesHelp); } @@ -63,8 +58,4 @@ public void initialize() { public String getTabName() { return Localization.lang("File"); } - - public void backupFileDirBrowse() { - viewModel.backupFileDirBrowse(); - } } diff --git a/src/main/java/org/jabref/gui/preferences/file/FileTabViewModel.java b/src/main/java/org/jabref/gui/preferences/file/FileTabViewModel.java index c64626c39ec..da087e3bdcb 100644 --- a/src/main/java/org/jabref/gui/preferences/file/FileTabViewModel.java +++ b/src/main/java/org/jabref/gui/preferences/file/FileTabViewModel.java @@ -1,7 +1,5 @@ package org.jabref.gui.preferences.file; -import java.nio.file.Path; - import javafx.beans.property.BooleanProperty; import javafx.beans.property.SimpleBooleanProperty; import javafx.beans.property.SimpleStringProperty; @@ -9,7 +7,6 @@ import org.jabref.gui.DialogService; import org.jabref.gui.preferences.PreferenceTabViewModel; -import org.jabref.gui.util.DirectoryDialogConfiguration; import org.jabref.logic.bibtex.FieldPreferences; import org.jabref.model.entry.field.FieldFactory; import org.jabref.preferences.FilePreferences; @@ -24,9 +21,6 @@ public class FileTabViewModel implements PreferenceTabViewModel { private final BooleanProperty alwaysReformatBibProperty = new SimpleBooleanProperty(); private final BooleanProperty autosaveLocalLibraries = new SimpleBooleanProperty(); - private final BooleanProperty createBackupProperty = new SimpleBooleanProperty(); - private final StringProperty backupDirectoryProperty = new SimpleStringProperty(""); - private final ImportExportPreferences importExportPreferences; private final FieldPreferences fieldPreferences; private final DialogService dialogService; @@ -48,9 +42,6 @@ public void setValues() { resolveStringsForFieldsProperty.setValue(FieldFactory.serializeFieldsList(fieldPreferences.getResolvableFields())); alwaysReformatBibProperty.setValue(importExportPreferences.shouldAlwaysReformatOnSave()); autosaveLocalLibraries.setValue(importExportPreferences.shouldAutoSave()); - - createBackupProperty.setValue(filePreferences.shouldCreateBackup()); - backupDirectoryProperty.setValue(filePreferences.getBackupDirectory().toString()); } @Override @@ -60,9 +51,6 @@ public void storeSettings() { fieldPreferences.setResolvableFields(FieldFactory.parseFieldList(resolveStringsForFieldsProperty.getValue().trim())); importExportPreferences.setAlwaysReformatOnSave(alwaysReformatBibProperty.getValue()); importExportPreferences.setAutoSave(autosaveLocalLibraries.getValue()); - - filePreferences.createBackupProperty().setValue(createBackupProperty.getValue()); - filePreferences.backupDirectoryProperty().setValue(Path.of(backupDirectoryProperty.getValue())); } // ImportExport @@ -91,19 +79,4 @@ public BooleanProperty alwaysReformatBibProperty() { public BooleanProperty autosaveLocalLibrariesProperty() { return autosaveLocalLibraries; } - - public BooleanProperty createBackupProperty() { - return this.createBackupProperty; - } - - public StringProperty backupDirectoryProperty() { - return this.backupDirectoryProperty; - } - - public void backupFileDirBrowse() { - DirectoryDialogConfiguration dirDialogConfiguration = - new DirectoryDialogConfiguration.Builder().withInitialDirectory(Path.of(backupDirectoryProperty().getValue())).build(); - dialogService.showDirectorySelectionDialog(dirDialogConfiguration) - .ifPresent(dir -> backupDirectoryProperty.setValue(dir.toString())); - } } diff --git a/src/main/java/org/jabref/gui/preferences/general/GeneralTab.fxml b/src/main/java/org/jabref/gui/preferences/general/GeneralTab.fxml index a108e2fec39..d5b921ca7e0 100644 --- a/src/main/java/org/jabref/gui/preferences/general/GeneralTab.fxml +++ b/src/main/java/org/jabref/gui/preferences/general/GeneralTab.fxml @@ -1,12 +1,17 @@ + + + + + @@ -35,4 +40,19 @@ + + + + + + diff --git a/src/main/java/org/jabref/gui/preferences/general/GeneralTab.java b/src/main/java/org/jabref/gui/preferences/general/GeneralTab.java index 6a28b84db3a..85cf7f50af7 100644 --- a/src/main/java/org/jabref/gui/preferences/general/GeneralTab.java +++ b/src/main/java/org/jabref/gui/preferences/general/GeneralTab.java @@ -3,6 +3,7 @@ import javafx.fxml.FXML; import javafx.scene.control.CheckBox; import javafx.scene.control.ComboBox; +import javafx.scene.control.TextField; import org.jabref.gui.preferences.AbstractPreferenceTabView; import org.jabref.gui.preferences.PreferencesTab; @@ -24,6 +25,9 @@ public class GeneralTab extends AbstractPreferenceTabView i @FXML private CheckBox collectTelemetry; @FXML private CheckBox showAdvancedHints; + @FXML private CheckBox createBackup; + @FXML private TextField backupDirectory; + public GeneralTab() { ViewLoader.view(this) .root(this) @@ -36,7 +40,7 @@ public String getTabName() { } public void initialize() { - this.viewModel = new GeneralTabViewModel(dialogService, preferencesService.getGeneralPreferences(), preferencesService.getTelemetryPreferences()); + this.viewModel = new GeneralTabViewModel(preferencesService, dialogService); new ViewModelListCellFactory() .withText(Language::getDisplayName) @@ -56,5 +60,12 @@ public void initialize() { openLastStartup.selectedProperty().bindBidirectional(viewModel.openLastStartupProperty()); collectTelemetry.selectedProperty().bindBidirectional(viewModel.collectTelemetryProperty()); showAdvancedHints.selectedProperty().bindBidirectional(viewModel.showAdvancedHintsProperty()); + + createBackup.selectedProperty().bindBidirectional(viewModel.createBackupProperty()); + backupDirectory.textProperty().bindBidirectional(viewModel.backupDirectoryProperty()); + } + + public void backupFileDirBrowse() { + viewModel.backupFileDirBrowse(); } } diff --git a/src/main/java/org/jabref/gui/preferences/general/GeneralTabViewModel.java b/src/main/java/org/jabref/gui/preferences/general/GeneralTabViewModel.java index 68130266435..61cbeaeced9 100644 --- a/src/main/java/org/jabref/gui/preferences/general/GeneralTabViewModel.java +++ b/src/main/java/org/jabref/gui/preferences/general/GeneralTabViewModel.java @@ -1,5 +1,6 @@ package org.jabref.gui.preferences.general; +import java.nio.file.Path; import java.util.ArrayList; import java.util.Comparator; import java.util.List; @@ -10,15 +11,20 @@ import javafx.beans.property.SimpleBooleanProperty; import javafx.beans.property.SimpleListProperty; import javafx.beans.property.SimpleObjectProperty; +import javafx.beans.property.SimpleStringProperty; +import javafx.beans.property.StringProperty; import javafx.collections.FXCollections; import javafx.collections.transformation.SortedList; import org.jabref.gui.DialogService; import org.jabref.gui.preferences.PreferenceTabViewModel; +import org.jabref.gui.util.DirectoryDialogConfiguration; import org.jabref.logic.l10n.Language; import org.jabref.logic.l10n.Localization; import org.jabref.model.database.BibDatabaseMode; +import org.jabref.preferences.FilePreferences; import org.jabref.preferences.GeneralPreferences; +import org.jabref.preferences.PreferencesService; import org.jabref.preferences.TelemetryPreferences; public class GeneralTabViewModel implements PreferenceTabViewModel { @@ -34,17 +40,22 @@ public class GeneralTabViewModel implements PreferenceTabViewModel { private final BooleanProperty openLastStartupProperty = new SimpleBooleanProperty(); private final BooleanProperty showAdvancedHintsProperty = new SimpleBooleanProperty(); + private final BooleanProperty createBackupProperty = new SimpleBooleanProperty(); + private final StringProperty backupDirectoryProperty = new SimpleStringProperty(""); + private final DialogService dialogService; private final GeneralPreferences generalPreferences; private final TelemetryPreferences telemetryPreferences; + private final FilePreferences filePreferences; private final List restartWarning = new ArrayList<>(); @SuppressWarnings("ReturnValueIgnored") - public GeneralTabViewModel(DialogService dialogService, GeneralPreferences generalPreferences, TelemetryPreferences telemetryPreferences) { + public GeneralTabViewModel(PreferencesService preferencesService, DialogService dialogService) { this.dialogService = dialogService; - this.generalPreferences = generalPreferences; - this.telemetryPreferences = telemetryPreferences; + this.generalPreferences = preferencesService.getGeneralPreferences(); + this.telemetryPreferences = preferencesService.getTelemetryPreferences(); + this.filePreferences = preferencesService.getFilePreferences(); } public void setValues() { @@ -60,6 +71,9 @@ public void setValues() { collectTelemetryProperty.setValue(telemetryPreferences.shouldCollectTelemetry()); openLastStartupProperty.setValue(generalPreferences.shouldOpenLastEdited()); showAdvancedHintsProperty.setValue(generalPreferences.shouldShowAdvancedHints()); + + createBackupProperty.setValue(filePreferences.shouldCreateBackup()); + backupDirectoryProperty.setValue(filePreferences.getBackupDirectory().toString()); } public void storeSettings() { @@ -84,6 +98,9 @@ public void storeSettings() { generalPreferences.setShowAdvancedHints(showAdvancedHintsProperty.getValue()); telemetryPreferences.setCollectTelemetry(collectTelemetryProperty.getValue()); + + filePreferences.createBackupProperty().setValue(createBackupProperty.getValue()); + filePreferences.backupDirectoryProperty().setValue(Path.of(backupDirectoryProperty.getValue())); } @Override @@ -132,4 +149,19 @@ public BooleanProperty openLastStartupProperty() { public BooleanProperty showAdvancedHintsProperty() { return this.showAdvancedHintsProperty; } + + public BooleanProperty createBackupProperty() { + return this.createBackupProperty; + } + + public StringProperty backupDirectoryProperty() { + return this.backupDirectoryProperty; + } + + public void backupFileDirBrowse() { + DirectoryDialogConfiguration dirDialogConfiguration = + new DirectoryDialogConfiguration.Builder().withInitialDirectory(Path.of(backupDirectoryProperty().getValue())).build(); + dialogService.showDirectorySelectionDialog(dirDialogConfiguration) + .ifPresent(dir -> backupDirectoryProperty.setValue(dir.toString())); + } } From 9a270dcf2907e3d0cf3defdfd165a4314f861483 Mon Sep 17 00:00:00 2001 From: Carl Christian Snethlage <50491877+calixtus@users.noreply.github.com> Date: Thu, 4 May 2023 21:11:22 +0200 Subject: [PATCH 10/17] Refine ui --- .../java/org/jabref/gui/preferences/general/GeneralTab.fxml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/main/java/org/jabref/gui/preferences/general/GeneralTab.fxml b/src/main/java/org/jabref/gui/preferences/general/GeneralTab.fxml index d5b921ca7e0..8044c84f7e7 100644 --- a/src/main/java/org/jabref/gui/preferences/general/GeneralTab.fxml +++ b/src/main/java/org/jabref/gui/preferences/general/GeneralTab.fxml @@ -1,5 +1,6 @@ + @@ -54,5 +55,8 @@ + + + From de977791706ef424b596378eada3586213ebdb69 Mon Sep 17 00:00:00 2001 From: Siedlerchr Date: Thu, 4 May 2023 21:22:39 +0200 Subject: [PATCH 11/17] fix unit tests --- .../org/jabref/logic/autosaveandbackup/BackupManagerTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/jabref/logic/autosaveandbackup/BackupManagerTest.java b/src/test/java/org/jabref/logic/autosaveandbackup/BackupManagerTest.java index 5ad86ed2c32..3fdf7a456d0 100644 --- a/src/test/java/org/jabref/logic/autosaveandbackup/BackupManagerTest.java +++ b/src/test/java/org/jabref/logic/autosaveandbackup/BackupManagerTest.java @@ -76,7 +76,7 @@ public void correctBackupFileDeterminedForMultipleBakFiles() throws Exception { // create "older" .bak files containing changes for (int i = 0; i < 10; i++) { Path changesBibBak = Path.of(BackupManagerTest.class.getResource("changes.bib").toURI()); - Path directory = BackupFileUtil.getAppDataBackupDir(); + Path directory = backupDir; String timeSuffix = "2020-02-03--00.00.0" + Integer.toString(i); String fileName = BackupFileUtil.getUniqueFilePrefix(noChangesBib) + "--no-changes.bib--" + timeSuffix + ".bak"; target = directory.resolve(fileName); From cfebdbee8ab16b556993f119bc0e7793c977cc08 Mon Sep 17 00:00:00 2001 From: Siedlerchr Date: Thu, 4 May 2023 22:14:06 +0200 Subject: [PATCH 12/17] add tests --- .../autosaveandbackup/BackupManager.java | 2 +- .../autosaveandbackup/BackupManagerTest.java | 68 +++++++++++++++++++ 2 files changed, 69 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java b/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java index 858eaae4824..b50beb7a1d0 100644 --- a/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java +++ b/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java @@ -206,7 +206,7 @@ Optional determineBackupPathForNewBackup(Path backupDir) { * SIDE EFFECT: Deletes oldest backup file * * - * @param backupPath the path where the library should be backed up to + * @param backupPath the full path to the file where the library should be backed up to */ void performBackup(Path backupPath) { if (!needsBackup) { diff --git a/src/test/java/org/jabref/logic/autosaveandbackup/BackupManagerTest.java b/src/test/java/org/jabref/logic/autosaveandbackup/BackupManagerTest.java index 3fdf7a456d0..b124b8d3d7b 100644 --- a/src/test/java/org/jabref/logic/autosaveandbackup/BackupManagerTest.java +++ b/src/test/java/org/jabref/logic/autosaveandbackup/BackupManagerTest.java @@ -4,17 +4,32 @@ import java.nio.file.Path; import java.nio.file.StandardCopyOption; import java.nio.file.attribute.FileTime; +import java.util.Collections; +import java.util.List; +import java.util.Optional; import org.jabref.logic.util.BackupFileType; import org.jabref.logic.util.io.BackupFileUtil; +import org.jabref.model.database.BibDatabase; +import org.jabref.model.database.BibDatabaseContext; +import org.jabref.model.entry.BibEntryTypesManager; +import org.jabref.model.groups.event.GroupUpdatedEvent; +import org.jabref.model.metadata.MetaData; +import org.jabref.model.metadata.event.MetaDataChangedEvent; +import org.jabref.preferences.FilePreferences; +import org.jabref.preferences.PreferencesService; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; +import org.mockito.Answers; +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; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; public class BackupManagerTest { @@ -111,4 +126,57 @@ public void bakFileWithOlderTimeStampDoesNotLeadToDiff() throws Exception { assertFalse(BackupManager.backupFileDiffers(changesBib, backupDir)); } + + @Test + public void shouldNotCreateABackup(@TempDir Path customDir) throws Exception { + Path backupDir = customDir.resolve("subBackupDir"); + Files.createDirectories(backupDir); + + var database = new BibDatabaseContext(new BibDatabase()); + database.setDatabasePath(customDir.resolve("Bibfile.bib")); + + var preferences = mock(PreferencesService.class, Answers.RETURNS_DEEP_STUBS); + var filePreferences = mock(FilePreferences.class); + when(preferences.getFilePreferences()).thenReturn(filePreferences); + when(filePreferences.getBackupDirectory()).thenReturn(backupDir); + + BackupManager manager = BackupManager.start(database, mock(BibEntryTypesManager.class, Answers.RETURNS_DEEP_STUBS), preferences); + manager.listen(new MetaDataChangedEvent(new MetaData())); + + BackupManager.shutdown(database, backupDir, false); + + List files = Files.list(backupDir).toList(); + assertEquals(Collections.emptyList(), files); + } + + @Test + public void shouldCreateABackup(@TempDir Path customDir) throws Exception { + Path backupDir = customDir.resolve("subBackupDir"); + Files.createDirectories(backupDir); + + var database = new BibDatabaseContext(new BibDatabase()); + database.setDatabasePath(customDir.resolve("Bibfile.bib")); + + var preferences = mock(PreferencesService.class, Answers.RETURNS_DEEP_STUBS); + var filePreferences = mock(FilePreferences.class); + when(preferences.getFilePreferences()).thenReturn(filePreferences); + when(filePreferences.getBackupDirectory()).thenReturn(backupDir); + when(filePreferences.shouldCreateBackup()).thenReturn(true); + + BackupManager manager = BackupManager.start(database, mock(BibEntryTypesManager.class, Answers.RETURNS_DEEP_STUBS), preferences); + manager.listen(new MetaDataChangedEvent(new MetaData())); + + Optional fullBackupPath = manager.determineBackupPathForNewBackup(backupDir); + fullBackupPath.ifPresent(manager::performBackup); + manager.listen(new GroupUpdatedEvent(new MetaData())); + // We have to wait a bit so that the backup time differs and the path is different + Thread.sleep(600); + + BackupManager.shutdown(database, backupDir, true); + + List files = Files.list(backupDir).sorted().toList(); + assertEquals(2, files.size()); + // we only know the first backup path because the second one is created on shutdown + assertEquals(fullBackupPath.get(), files.get(0)); + } } From 243f73a4cc6574ae61981d177c02104ec1bc7f5e Mon Sep 17 00:00:00 2001 From: Siedlerchr Date: Thu, 4 May 2023 22:17:02 +0200 Subject: [PATCH 13/17] fix indentation --- .../jabref/logic/util/io/BackupFileUtil.java | 50 +++++++++---------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/src/main/java/org/jabref/logic/util/io/BackupFileUtil.java b/src/main/java/org/jabref/logic/util/io/BackupFileUtil.java index 76b9ca753f3..d64446b9949 100644 --- a/src/main/java/org/jabref/logic/util/io/BackupFileUtil.java +++ b/src/main/java/org/jabref/logic/util/io/BackupFileUtil.java @@ -68,37 +68,37 @@ public static Path getPathForNewBackupFileAndCreateDirectory(Path targetFile, Ba } public static Optional getPathOfLatestExistingBackupFile(Path targetFile, BackupFileType fileType, Path backupDir) { - // The code is similar to "getPathForNewBackupFileAndCreateDirectory" + // The code is similar to "getPathForNewBackupFileAndCreateDirectory" - String extension = "." + fileType.getExtensions().get(0); - - Path directory = backupDir; - 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(); - } - } + String extension = "." + fileType.getExtensions().get(0); - // 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); + Path directory = backupDir; + 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(); } - return mostRecentFile; } + // 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. From ad52f328205476e31f8a2ac723e7683d374ee8ce Mon Sep 17 00:00:00 2001 From: Siedlerchr Date: Thu, 4 May 2023 22:17:57 +0200 Subject: [PATCH 14/17] remove useless var --- src/main/java/org/jabref/logic/util/io/BackupFileUtil.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/jabref/logic/util/io/BackupFileUtil.java b/src/main/java/org/jabref/logic/util/io/BackupFileUtil.java index d64446b9949..d2b42eef98f 100644 --- a/src/main/java/org/jabref/logic/util/io/BackupFileUtil.java +++ b/src/main/java/org/jabref/logic/util/io/BackupFileUtil.java @@ -72,8 +72,7 @@ public static Optional getPathOfLatestExistingBackupFile(Path targetFile, String extension = "." + fileType.getExtensions().get(0); - Path directory = backupDir; - if (Files.notExists(directory)) { + if (Files.notExists(backupDir)) { // 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)) { @@ -87,7 +86,7 @@ public static Optional getPathOfLatestExistingBackupFile(Path targetFile, final String prefix = getUniqueFilePrefix(targetFile) + "--" + targetFile.getFileName(); Optional mostRecentFile; try { - mostRecentFile = Files.list(directory) + mostRecentFile = Files.list(backupDir) // just list the .sav belonging to the given targetFile .filter(p -> p.getFileName().toString().startsWith(prefix)) .sorted() From b9c1af2dbb1edf59f8eeab76e8df92afe9cc8ce0 Mon Sep 17 00:00:00 2001 From: Siedlerchr Date: Fri, 5 May 2023 19:46:07 +0200 Subject: [PATCH 15/17] disable backup dir when unchecked --- src/main/java/org/jabref/gui/preferences/general/GeneralTab.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/org/jabref/gui/preferences/general/GeneralTab.java b/src/main/java/org/jabref/gui/preferences/general/GeneralTab.java index 85cf7f50af7..0f3811c47cb 100644 --- a/src/main/java/org/jabref/gui/preferences/general/GeneralTab.java +++ b/src/main/java/org/jabref/gui/preferences/general/GeneralTab.java @@ -63,6 +63,7 @@ public void initialize() { createBackup.selectedProperty().bindBidirectional(viewModel.createBackupProperty()); backupDirectory.textProperty().bindBidirectional(viewModel.backupDirectoryProperty()); + backupDirectory.disableProperty().bind(viewModel.createBackupProperty().not()); } public void backupFileDirBrowse() { From 2df54e8085c45cd15eefeb4752659e571aab1218 Mon Sep 17 00:00:00 2001 From: Siedlerchr Date: Fri, 5 May 2023 20:12:19 +0200 Subject: [PATCH 16/17] remove test for second file --- .../org/jabref/logic/autosaveandbackup/BackupManagerTest.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/test/java/org/jabref/logic/autosaveandbackup/BackupManagerTest.java b/src/test/java/org/jabref/logic/autosaveandbackup/BackupManagerTest.java index b124b8d3d7b..7bf69d79f91 100644 --- a/src/test/java/org/jabref/logic/autosaveandbackup/BackupManagerTest.java +++ b/src/test/java/org/jabref/logic/autosaveandbackup/BackupManagerTest.java @@ -169,14 +169,12 @@ public void shouldCreateABackup(@TempDir Path customDir) throws Exception { Optional fullBackupPath = manager.determineBackupPathForNewBackup(backupDir); fullBackupPath.ifPresent(manager::performBackup); manager.listen(new GroupUpdatedEvent(new MetaData())); - // We have to wait a bit so that the backup time differs and the path is different - Thread.sleep(600); BackupManager.shutdown(database, backupDir, true); List files = Files.list(backupDir).sorted().toList(); - assertEquals(2, files.size()); // we only know the first backup path because the second one is created on shutdown + // due to timing issues we cannot test that reliable assertEquals(fullBackupPath.get(), files.get(0)); } } From 5547e35873af1c6293effdb2a3afc7c996d6b3f3 Mon Sep 17 00:00:00 2001 From: Siedlerchr Date: Fri, 5 May 2023 21:20:46 +0200 Subject: [PATCH 17/17] fix missing path parameter --- .../org/jabref/logic/autosaveandbackup/BackupManager.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java b/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java index dc8b7c3ab6c..599ac265fc9 100644 --- a/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java +++ b/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java @@ -292,7 +292,7 @@ public synchronized void listen(@SuppressWarnings("unused") BibDatabaseContextCh } private void startBackupTask(Path backupDir) { - fillQueue(); + fillQueue(backupDir); executor.scheduleAtFixedRate( // We need to determine the backup path on each action, because we use the timestamp in the filename @@ -302,8 +302,7 @@ private void startBackupTask(Path backupDir) { TimeUnit.SECONDS); } - private void fillQueue() { - Path backupDir = BackupFileUtil.getAppDataBackupDir(); + private void fillQueue(Path backupDir) { if (!Files.exists(backupDir)) { return; }