Fix: Only if .sav file has changes a recovery dialog is shown

CHANGELOG.md

@@ -16,6 +16,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
 - We improved the arXiv fetcher. Now it should find entries even more reliably and does no longer include the version (e.g `v1`) in the `eprint` field. [forum#1941](https://discourse.jabref.org/t/remove-version-in-arxiv-import/1941)
 - We moved the group search bar and the button "New group" from bottom to top position to make it more prominent. [#6112](https://github.com/JabRef/jabref/pull/6112)
 
+- When JabRef finds a `.sav` file without changes, there is no dialog asking for acceptance of changes anymore.
 
 ### Fixed
 

src/main/java/org/jabref/JabRefGUI.java

@@ -247,7 +247,7 @@ private void openLastEditedDatabases() {
                 continue;
             }
 
-            if (BackupManager.checkForBackupFile(dbFile.toPath())) {
+            if (BackupManager.backupFileDiffers(dbFile.toPath())) {
                 BackupUIManager.showRestoreBackupDialog(mainFrame.getDialogService(), dbFile.toPath());
             }
 

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

@@ -380,7 +380,7 @@ public boolean quit() {
         List<String> filenames = new ArrayList<>();
         for (int i = 0; i < tabbedPane.getTabs().size(); i++) {
             BasePanel panel = getBasePanelAt(i);
-            BibDatabaseContext context = panel.getBibDatabaseContext();
+            final BibDatabaseContext context = panel.getBibDatabaseContext();
 
             if (panel.isModified() && (context.getLocation() == DatabaseLocation.LOCAL)) {
                 tabbedPane.getSelectionModel().select(i);
@@ -1159,7 +1159,7 @@ private void closeTab(BasePanel panel) {
             return;
         }
 
-        BibDatabaseContext context = panel.getBibDatabaseContext();
+        final BibDatabaseContext context = panel.getBibDatabaseContext();
 
         if (panel.isModified() && (context.getLocation() == DatabaseLocation.LOCAL)) {
             if (confirmClose(panel)) {

src/main/java/org/jabref/gui/dialogs/BackupUIManager.java

@@ -13,7 +13,6 @@
 public class BackupUIManager {
 
     private BackupUIManager() {
-
     }
 
     public static void showRestoreBackupDialog(DialogService dialogService, Path originalPath) {

src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java

@@ -181,7 +181,7 @@ private ParserResult loadDatabase(Path file) throws Exception {
 
         Globals.prefs.put(JabRefPreferences.WORKING_DIRECTORY, fileToLoad.getParent().toString());
 
-        if (BackupManager.checkForBackupFile(fileToLoad)) {
+        if (BackupManager.backupFileDiffers(fileToLoad)) {
             BackupUIManager.showRestoreBackupDialog(dialogService, fileToLoad);
         }
 

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

@@ -30,14 +30,15 @@
 /**
  * Backups the given bib database file from {@link BibDatabaseContext} on every {@link BibDatabaseContextChangedEvent}.
  * An intelligent {@link ExecutorService} with a {@link BlockingQueue} prevents a high load while making backups and
- * rejects all redundant backup tasks.
- * This class does not manage the .bak file which is created when opening a database.
+ * rejects all redundant backup tasks. This class does not manage the .bak file which is created when opening a
+ * database.
  */
 public class BackupManager {
 
     private static final Logger LOGGER = LoggerFactory.getLogger(BackupManager.class);
 
-    private static final String BACKUP_EXTENSION = ".sav";
+    // This differs from org.jabref.logic.exporter.AtomicFileOutputStream.BACKUP_EXTENSION, which is used for copying the .bib away before overwriting on save.
+    private static final String AUTOSAVE_FILE_EXTENSION = ".sav";
 
     private static Set<BackupManager> runningInstances = new HashSet<>();
 
@@ -58,12 +59,12 @@ private BackupManager(BibDatabaseContext bibDatabaseContext, BibEntryTypesManage
     }
 
     static Path getBackupPath(Path originalPath) {
-        return FileUtil.addExtension(originalPath, BACKUP_EXTENSION);
+        return FileUtil.addExtension(originalPath, AUTOSAVE_FILE_EXTENSION);
     }
 
     /**
-     * Starts the BackupManager which is associated with the given {@link BibDatabaseContext}.
-     * As long as no database file is present in {@link BibDatabaseContext}, the {@link BackupManager} will do nothing.
+     * Starts the BackupManager which is associated with the given {@link BibDatabaseContext}. As long as no database
+     * file is present in {@link BibDatabaseContext}, the {@link BackupManager} will do nothing.
      *
      * @param bibDatabaseContext Associated {@link BibDatabaseContext}
      */
@@ -86,13 +87,27 @@ public static void shutdown(BibDatabaseContext bibDatabaseContext) {
     }
 
     /**
-     * Checks whether a backup file exists for the given database file.
+     * Checks whether a backup file exists for the given database file. If it exists, it is checked whether it is
+     * different from the original.
      *
-     * @param originalPath Path to the file a backup should be checked for.
+     * @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.
      */
-    public static boolean checkForBackupFile(Path originalPath) {
+    public static boolean backupFileDiffers(Path originalPath) {
         Path backupPath = getBackupPath(originalPath);
-        return Files.exists(backupPath) && !Files.isDirectory(backupPath);
+        if (!Files.exists(backupPath) || Files.isDirectory(backupPath)) {
+            return false;
+        }
+
+        try {
+            return !com.google.common.io.Files.equal(originalPath.toFile(), backupPath.toFile());
+        } catch (IOException e) {
+            LOGGER.debug("Could not compare original file and backup file.", e);
+            // User has to investigate in this case
+            return true;
+        }
     }
 
     /**
@@ -148,8 +163,8 @@ private void startBackupTask() {
     }
 
     /**
-     * Unregisters the BackupManager from the eventBus of {@link BibDatabaseContext} and deletes the backup file.
-     * This method should only be used when closing a database/JabRef legally.
+     * Unregisters the BackupManager from the eventBus of {@link BibDatabaseContext} and deletes the backup file. This
+     * method should only be used when closing a database/JabRef legally.
      */
     private void shutdown() {
         changeFilter.unregisterListener(this);

src/main/java/org/jabref/logic/citationstyle/CSLAdapter.java

@@ -84,7 +84,7 @@ private void initialize(String newStyle, CitationStyleOutputFormat newFormat) th
      */
     private static class JabRefItemDataProvider implements ItemDataProvider {
 
-        private final ArrayList<BibEntry> data = new ArrayList<>();
+        private final List<BibEntry> data = new ArrayList<>();
 
         /**
          * Converts the {@link BibEntry} into {@link CSLItemData}.

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

@@ -6,6 +6,8 @@
 import org.junit.jupiter.api.Test;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
 public class BackupManagerTest {
 
@@ -15,4 +17,22 @@ public void backupFileNameIsCorrectlyGeneratedWithinTmpDirectory() {
         Path savPath = BackupManager.getBackupPath(bibPath);
         assertEquals(Paths.get("tmp", "test.bib.sav"), savPath);
     }
+
+    @Test
+    public void backupFileIsEqualForNonExistingBackup() throws Exception {
+        Path originalFile = Path.of(BackupManagerTest.class.getResource("no-backup.bib").toURI());
+        assertFalse(BackupManager.backupFileDiffers(originalFile));
+    }
+
+    @Test
+    public void backupFileIsEqual() throws Exception {
+        Path originalFile = Path.of(BackupManagerTest.class.getResource("no-changes.bib").toURI());
+        assertFalse(BackupManager.backupFileDiffers(originalFile));
+    }
+
+    @Test
+    public void backupFileDiffers() throws Exception {
+        Path originalFile = Path.of(BackupManagerTest.class.getResource("changes.bib").toURI());
+        assertTrue(BackupManager.backupFileDiffers(originalFile));
+    }
 }

src/test/resources/org/jabref/logic/autosaveandbackup/changes.bib

@@ -0,0 +1,28 @@
+% Encoding: UTF-8
+
+@Article{GarridoKallstromKummEtAl2016,
+    author    = {Mario Garrido and Petter Kallstrom and Martin Kumm and Oscar Gustafsson},
+    title     = {{CORDIC} {II:} {A} New Improved {CORDIC} Algorithm},
+    journal   = {{IEEE} Trans. on Circuits and Systems},
+    year      = {2016},
+    volume    = {63-II},
+    number    = {2},
+    pages     = {186--190},
+    bibsource = {dblp computer science bibliography, http://dblp.org},
+    biburl    = {http://dblp.uni-trier.de/rec/bib/journals/tcas/GarridoKKG16},
+    doi       = {10.1109/TCSII.2015.2483422},
+    timestamp = {Mon, 08 Feb 2016 00:00:00 +0100},
+}
+
+@InProceedings{GustafssonJohansson2015,
+    author    = {Oscar Gustafsson and H{\aa}kan Johansson},
+    title     = {Decimation filters for high-speed delta-sigma modulators with passband constraints: General versus CIC-based {FIR} filters},
+    booktitle = {2015 {IEEE} International Symposium on Circuits and Systems, {ISCAS} 2015, Lisbon, Portugal, May 24-27, 2015},
+    year      = {2015},
+    pages     = {2205--2208},
+    publisher = {{IEEE}},
+    bibsource = {dblp computer science bibliography, http://dblp.org},
+    biburl    = {http://dblp.uni-trier.de/rec/bib/conf/iscas/GustafssonJ15},
+    doi       = {10.1109/ISCAS.2015.7169119},
+    timestamp = {Wed, 05 Aug 2015 09:08:53 +0200},
+}

src/test/resources/org/jabref/logic/autosaveandbackup/no-backup.bib

@@ -0,0 +1,28 @@
+% Encoding: UTF-8
+
+@Article{GarridoKallstromKummEtAl2016,
+    author    = {Mario Garrido and Petter Kallstrom and Martin Kumm and Oscar Gustafsson},
+    title     = {{CORDIC} {II:} {A} New Improved {CORDIC} Algorithm},
+    journal   = {{IEEE} Trans. on Circuits and Systems},
+    year      = {2016},
+    volume    = {63-II},
+    number    = {2},
+    pages     = {186--190},
+    bibsource = {dblp computer science bibliography, http://dblp.org},
+    biburl    = {http://dblp.uni-trier.de/rec/bib/journals/tcas/GarridoKKG16},
+    doi       = {10.1109/TCSII.2015.2483422},
+    timestamp = {Mon, 08 Feb 2016 00:00:00 +0100},
+}
+
+@InProceedings{GustafssonJohansson2015,
+    author    = {Oscar Gustafsson and H{\aa}kan Johansson},
+    title     = {Decimation filters for high-speed delta-sigma modulators with passband constraints: General versus CIC-based {FIR} filters},
+    booktitle = {2015 {IEEE} International Symposium on Circuits and Systems, {ISCAS} 2015, Lisbon, Portugal, May 24-27, 2015},
+    year      = {2015},
+    pages     = {2205--2208},
+    publisher = {{IEEE}},
+    bibsource = {dblp computer science bibliography, http://dblp.org},
+    biburl    = {http://dblp.uni-trier.de/rec/bib/conf/iscas/GustafssonJ15},
+    doi       = {10.1109/ISCAS.2015.7169119},
+    timestamp = {Wed, 05 Aug 2015 09:08:53 +0200},
+}

src/test/resources/org/jabref/logic/autosaveandbackup/no-changes.bib

@@ -0,0 +1,15 @@
+% Encoding: UTF-8
+
+@Article{GarridoKallstromKummEtAl2016,
+  author    = {Mario Garrido and Petter Kallstrom and Martin Kumm and Oscar Gustafsson},
+  title     = {{CORDIC} {II:} {A} New Improved {CORDIC} Algorithm},
+  journal   = {{IEEE} Trans. on Circuits and Systems},
+  year      = {2016},
+  volume    = {63-II},
+  number    = {2},
+  pages     = {186--190},
+  bibsource = {dblp computer science bibliography, http://dblp.org},
+  biburl    = {http://dblp.uni-trier.de/rec/bib/journals/tcas/GarridoKKG16},
+  doi       = {10.1109/TCSII.2015.2483422},
+  timestamp = {Mon, 08 Feb 2016 00:00:00 +0100},
+}