Add "discarded" flag to backups

CHANGELOG.md

@@ -62,7 +62,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
 ### Fixed
 
 - We fixed an issue where importing from XMP would fail for certain PDFs [#9383](https://github.com/JabRef/jabref/issues/9383)
-- We fixed the readability of the file field in the dark dark theme [#9340](https://github.com/JabRef/jabref/issues/9340)
+- We fixed the readability of the file field in the dark theme [#9340](https://github.com/JabRef/jabref/issues/9340)
 - We fixed that sorting of entries in the maintable by special fields is updated immediately [#9334](https://github.com/JabRef/jabref/issues/9334)
 - We fixed the Cleanup entries dialog is partially visible [#9223](https://github.com/JabRef/jabref/issues/9223)
 - We fixed the display of the "Customize Entry Types" dialogue title [#9198](https://github.com/JabRef/jabref/issues/9198)
@@ -93,6 +93,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
 - We fixed an issue where editing entry's "date" field in library mode "biblatex" causes an uncaught exception. [#8747](https://github.com/JabRef/jabref/issues/8747)
 - We fixed an issue where journal abbreviations would not abbreviate journal titles with escaped ampersands (\\&). [#8948](https://github.com/JabRef/jabref/issues/8948)
 - We fixed an issue where font size preferences did not apply correctly to preference dialog window and the menu bar. [#8386](https://github.com/JabRef/jabref/issues/8386) and [#9279](https://github.com/JabRef/jabref/issues/9279)
+- We fixed the behavior of "Discard changes" when reopening a modified library. [#9361](https://github.com/JabRef/jabref/issues/9361)
 - We fixed an issue that JabRef displayed the wrong group tree after loading. [koppor#637](https://github.com/koppor/jabref/issues/637)
 - We fixed an issue when using an unsafe character in the citation key, the auto-linking feature fails to link files. [#9267](https://github.com/JabRef/jabref/issues/9267)
 - We fixed an issue where a known journal's medline/dot-less abbreviation does not switch to the full name. [#9370](https://github.com/JabRef/jabref/issues/9370)

src/main/java/org/jabref/gui/JabRefFrame.java

@@ -1160,9 +1160,10 @@ public FileHistoryMenu getFileHistory() {
     }
 
     /**
-     * Ask if the user really wants to close the given database
+     * Ask if the user really wants to close the given database.
+     * Offers to save or discard the changes -- or return to the library
      *
-     * @return true if the user choose to close the database
+     * @return <code>true</code> if the user choose to close the database
      */
     private boolean confirmClose(LibraryTab libraryTab) {
         String filename = libraryTab.getBibDatabaseContext()
@@ -1173,15 +1174,24 @@ private boolean confirmClose(LibraryTab libraryTab) {
 
         ButtonType saveChanges = new ButtonType(Localization.lang("Save changes"), ButtonBar.ButtonData.YES);
         ButtonType discardChanges = new ButtonType(Localization.lang("Discard changes"), ButtonBar.ButtonData.NO);
-        ButtonType cancel = new ButtonType(Localization.lang("Return to library"), ButtonBar.ButtonData.CANCEL_CLOSE);
+        ButtonType returnToLibrary = new ButtonType(Localization.lang("Return to library"), ButtonBar.ButtonData.CANCEL_CLOSE);
 
         Optional<ButtonType> response = dialogService.showCustomButtonDialogAndWait(Alert.AlertType.CONFIRMATION,
                 Localization.lang("Save before closing"),
                 Localization.lang("Library '%0' has changed.", filename),
-                saveChanges, discardChanges, cancel);
+                saveChanges, discardChanges, returnToLibrary);
+
+        if (response.isEmpty()) {
+            return true;
+        }
+
+        ButtonType buttonType = response.get();
+
+        if (buttonType.equals(returnToLibrary)) {
+            return false;
+        }
 
-        if (response.isPresent() && response.get().equals(saveChanges)) {
-            // The user wants to save.
+        if (buttonType.equals(saveChanges)) {
             try {
                 SaveDatabaseAction saveAction = new SaveDatabaseAction(libraryTab, prefs, Globals.entryTypesManager);
                 if (saveAction.save()) {
@@ -1196,7 +1206,13 @@ private boolean confirmClose(LibraryTab libraryTab) {
             // Save was cancelled or an error occurred.
             return false;
         }
-        return response.isEmpty() || !response.get().equals(cancel);
+
+        if (buttonType.equals(discardChanges)) {
+            BackupManager.discardBackup(libraryTab.getBibDatabaseContext());
+            return true;
+        }
+
+        return false;
     }
 
     /**

src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java

@@ -66,7 +66,7 @@ public class BackupManager {
 
     private boolean needsBackup = true;
 
-    private BackupManager(BibDatabaseContext bibDatabaseContext, BibEntryTypesManager entryTypesManager, PreferencesService preferences) {
+    BackupManager(BibDatabaseContext bibDatabaseContext, BibEntryTypesManager entryTypesManager, PreferencesService preferences) {
         this.bibDatabaseContext = bibDatabaseContext;
         this.entryTypesManager = entryTypesManager;
         this.preferences = preferences;
@@ -105,6 +105,16 @@ public static BackupManager start(BibDatabaseContext bibDatabaseContext, BibEntr
         return backupManager;
     }
 
+    /**
+     * Marks the backup as discarded at the library which is associated with the given {@link BibDatabaseContext}.
+     *
+     * @param bibDatabaseContext Associated {@link BibDatabaseContext}
+     */
+    public static void discardBackup(BibDatabaseContext bibDatabaseContext) {
+        runningInstances.stream().filter(instance -> instance.bibDatabaseContext == bibDatabaseContext).forEach(
+                BackupManager::discardBackup);
+    }
+
     /**
      * Shuts down the BackupManager which is associated with the given {@link BibDatabaseContext}.
      *
@@ -120,13 +130,25 @@ public static void shutdown(BibDatabaseContext bibDatabaseContext) {
      * Checks whether a backup file exists for the given database file. If it exists, it is checked whether it is
      * newer and different from the original.
      *
+     * In case a discarded file is present, the method also returns <code>false</code>, See also {@link #discardBackup()}.
+     *
      * @param originalPath Path to the file a backup should be checked for. Example: jabref.bib.
      *
      * @return <code>true</code> if backup file exists AND differs from originalPath. <code>false</code> is the
-     * "default" return value in the good case. In the case of an exception <code>true</code> is returned to ensure that
-     * the user checks the output.
+     * "default" return value in the good case. In case a discarded file exists, <code>false</code> is returned, too.
+     * In the case of an exception <code>true</code> is returned to ensure that the user checks the output.
      */
     public static boolean backupFileDiffers(Path originalPath) {
+        Path discardedFile = determineDiscardedFile(originalPath);
+        if (Files.exists(discardedFile)) {
+            try {
+                Files.delete(discardedFile);
+            } catch (IOException e) {
+                LOGGER.error("Could not remove discarded file {}", discardedFile, e);
+                return true;
+            }
+            return false;
+        }
         return getLatestBackupPath(originalPath).map(latestBackupPath -> {
             FileTime latestBackupFileLastModifiedTime;
             try {
@@ -177,7 +199,7 @@ public static void restoreBackup(Path originalPath) {
         }
     }
 
-    private Optional<Path> determineBackupPathForNewBackup() {
+    Optional<Path> determineBackupPathForNewBackup() {
         return bibDatabaseContext.getDatabasePath().map(BackupManager::getBackupPathForNewBackup);
     }
 
@@ -188,7 +210,7 @@ private Optional<Path> determineBackupPathForNewBackup() {
      *
      * @param backupPath the path where the library should be backed up to
      */
-    private void performBackup(Path backupPath) {
+    void performBackup(Path backupPath) {
         if (!needsBackup) {
             return;
         }
@@ -226,6 +248,27 @@ private void performBackup(Path backupPath) {
         }
     }
 
+    private static Path determineDiscardedFile(Path file) {
+        return BackupFileUtil.getAppDataBackupDir().resolve(
+                BackupFileUtil.getUniqueFilePrefix(file) + "--" + file.getFileName() + "--discarded"
+        );
+    }
+
+    /**
+     * Marks the backups as discarded.
+     *
+     * 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());
+        try {
+            Files.createFile(path);
+        } catch (IOException e) {
+            LOGGER.info("Could not create backup file {}", path, e);
+        }
+    }
+
     private void logIfCritical(Path backupPath, IOException e) {
         Throwable innermostCause = e;
         while (innermostCause.getCause() != null) {

src/main/java/org/jabref/logic/util/BuildInfo.java

@@ -10,7 +10,7 @@
 
 public final class BuildInfo {
 
-    public static final String UNKNOWN_VERSION = "*unknown*";
+    public static final String UNKNOWN_VERSION = "UNKNOWN";
 
     public static final String OS = System.getProperty("os.name", UNKNOWN_VERSION);
     public static final String OS_VERSION = System.getProperty("os.version", UNKNOWN_VERSION).toLowerCase(Locale.ROOT);

src/main/java/org/jabref/logic/util/io/BackupFileUtil.java

@@ -69,6 +69,8 @@ public static Path getPathForNewBackupFileAndCreateDirectory(Path targetFile, Ba
 
     /**
      * 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<Path> getPathOfLatestExisingBackupFile(Path targetFile, BackupFileType fileType) {
         // The code is similar to "getPathForNewBackupFileAndCreateDirectory"

src/test/java/org/jabref/logic/autosaveandbackup/BackupManagerTestDiscarded.java

@@ -0,0 +1,109 @@
+package org.jabref.logic.autosaveandbackup;
+
+import java.io.IOException;
+import java.io.Writer;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+
+import org.jabref.logic.exporter.AtomicFileWriter;
+import org.jabref.logic.exporter.BibWriter;
+import org.jabref.logic.exporter.BibtexDatabaseWriter;
+import org.jabref.logic.exporter.SavePreferences;
+import org.jabref.model.database.BibDatabase;
+import org.jabref.model.database.BibDatabaseContext;
+import org.jabref.model.entry.BibEntry;
+import org.jabref.model.entry.BibEntryTypesManager;
+import org.jabref.model.entry.field.StandardField;
+import org.jabref.model.metadata.SaveOrderConfig;
+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.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.ArgumentMatchers.anyBoolean;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+/**
+ * Test for "discarded" flag
+ */
+class BackupManagerTestDiscarded {
+
+    private BibDatabaseContext bibDatabaseContext;
+    private BackupManager backupManager;
+    private Path testBib;
+    private SavePreferences savePreferences;
+    private PreferencesService preferencesService;
+    private BibEntryTypesManager bibEntryTypesManager;
+
+    @BeforeEach
+    public void setup(@TempDir Path tempDir) throws Exception {
+        Path backupDir = tempDir.resolve("backups");
+        Files.createDirectories(backupDir);
+
+        testBib = tempDir.resolve("test.bib");
+
+        bibDatabaseContext = new BibDatabaseContext(new BibDatabase());
+        bibDatabaseContext.setDatabasePath(testBib);
+
+        bibEntryTypesManager = new BibEntryTypesManager();
+
+        savePreferences = mock(SavePreferences.class, Answers.RETURNS_DEEP_STUBS);
+        when(savePreferences.shouldMakeBackup()).thenReturn(false);
+        when(savePreferences.getSaveOrder()).thenReturn(new SaveOrderConfig());
+        when(savePreferences.withMakeBackup(anyBoolean())).thenReturn(savePreferences);
+        when(savePreferences.shouldSaveInOriginalOrder()).thenReturn(true);
+
+        preferencesService = mock(PreferencesService.class, Answers.RETURNS_DEEP_STUBS);
+        when(preferencesService.getSavePreferences()).thenReturn(savePreferences);
+
+        saveDatabase();
+
+        backupManager = new BackupManager(bibDatabaseContext, bibEntryTypesManager, preferencesService);
+        makeBackup();
+    }
+
+    private void saveDatabase() throws IOException {
+        try (Writer writer = new AtomicFileWriter(testBib, StandardCharsets.UTF_8, false)) {
+            BibWriter bibWriter = new BibWriter(writer, bibDatabaseContext.getDatabase().getNewLineSeparator());
+            new BibtexDatabaseWriter(bibWriter, preferencesService.getGeneralPreferences(), savePreferences, bibEntryTypesManager)
+                    .saveDatabase(bibDatabaseContext);
+        }
+    }
+
+    private void databaseModification() {
+        bibDatabaseContext.getDatabase().insertEntry(new BibEntry().withField(StandardField.NOTE, "test"));
+    }
+
+    private void makeBackup() {
+        backupManager.determineBackupPathForNewBackup().ifPresent(backupManager::performBackup);
+    }
+
+    @Test
+    public void noDiscardingAChangeLeadsToNewerBackupBeReported() throws Exception {
+        databaseModification();
+        makeBackup();
+        assertTrue(BackupManager.backupFileDiffers(testBib));
+    }
+
+    @Test
+    public void noDiscardingASavedChange() throws Exception {
+        databaseModification();
+        makeBackup();
+        saveDatabase();
+        assertFalse(BackupManager.backupFileDiffers(testBib));
+    }
+
+    @Test
+    public void discardingAChangeLeadsToNewerBackupToBeIgnored() throws Exception {
+        databaseModification();
+        makeBackup();
+        backupManager.discardBackup();
+        assertFalse(BackupManager.backupFileDiffers(testBib));
+    }
+}

src/test/java/org/jabref/logic/util/BuildInfoTest.java

@@ -10,7 +10,7 @@ public class BuildInfoTest {
     @Test
     public void testDefaults() {
         BuildInfo buildInfo = new BuildInfo("asdf");
-        assertEquals("*unknown*", buildInfo.version.getFullVersion());
+        assertEquals("UNKNOWN", buildInfo.version.getFullVersion());
     }
 
     @Test