Include check for TimeStamp (#9089)

JabRef · Aug 24, 2022 · f6928d2 · f6928d2
1 parent a255311
commit f6928d2
Show file tree

Hide file tree

Showing 2 changed files with 68 additions and 21 deletions.
diff --git a/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java b/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java
@@ -7,6 +7,7 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.StandardCopyOption;
+import java.nio.file.attribute.FileTime;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Optional;
@@ -117,26 +118,45 @@ 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
-     * different from the original.
+     * newer and different from the original.
      *
      * @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 backupFileDiffers(Path originalPath) {
-        Optional<Path> backupPath = getLatestBackupPath(originalPath);
-        if (backupPath.isEmpty()) {
-            return false;
-        }
-
-        try {
-            return Files.mismatch(originalPath, backupPath.get()) != -1L;
-        } catch (IOException e) {
-            LOGGER.debug("Could not compare original file and backup file.", e);
-            // User has to investigate in this case
-            return true;
-        }
+        return getLatestBackupPath(originalPath).map(latestBackupPath -> {
+            FileTime latestBackupFileLastModifiedTime;
+            try {
+                 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
+                return false;
+            }
+            FileTime currentFileLastModifiedTime;
+            try {
+                currentFileLastModifiedTime = Files.getLastModifiedTime(originalPath);
+            } catch (IOException e) {
+                LOGGER.debug("Could not get timestamp of current file file {}", originalPath, e);
+                // If we cannot get the timestamp, we do show any warning
+                return false;
+            }
+            if (latestBackupFileLastModifiedTime.compareTo(currentFileLastModifiedTime) <= 0) {
+                // Backup is older than current file
+                // We treat the backup as non-different (even if it could differ)
+                return false;
+            }
+            try {
+                return Files.mismatch(originalPath, latestBackupPath) != -1L;
+            } catch (IOException e) {
+                LOGGER.debug("Could not compare original file and backup file.", e);
+                // User has to investigate in this case
+                return true;
+            }
+        }).orElse(false);
     }
 
     /**

diff --git a/src/test/java/org/jabref/logic/autosaveandbackup/BackupManagerTest.java b/src/test/java/org/jabref/logic/autosaveandbackup/BackupManagerTest.java
@@ -3,6 +3,7 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.StandardCopyOption;
+import java.nio.file.attribute.FileTime;
 
 import org.jabref.logic.util.BackupFileType;
 import org.jabref.logic.util.io.BackupFileUtil;
@@ -54,24 +55,50 @@ public void backupFileDiffers() throws Exception {
 
     @Test
     public void correctBackupFileDeterminedForMultipleBakFiles() throws Exception {
-        // Prepare test: Create backup files on "right" path
+        Path noChangesBib = Path.of(BackupManagerTest.class.getResource("no-changes.bib").toURI());
+        Path noChangesBibBak = Path.of(BackupManagerTest.class.getResource("no-changes.bib.bak").toURI());
 
+        // Prepare test: Create backup files on "right" path
         // most recent file does not have any changes
-        Path source = Path.of(BackupManagerTest.class.getResource("no-changes.bib.bak").toURI());
-        Path target = BackupFileUtil.getPathForNewBackupFileAndCreateDirectory(Path.of(BackupManagerTest.class.getResource("no-changes.bib").toURI()), BackupFileType.BACKUP);
-        Files.copy(source, target, StandardCopyOption.REPLACE_EXISTING);
+        Path target = BackupFileUtil.getPathForNewBackupFileAndCreateDirectory(noChangesBib, BackupFileType.BACKUP);
+        Files.copy(noChangesBibBak, target, StandardCopyOption.REPLACE_EXISTING);
 
         // create "older" .bak files containing changes
         for (int i = 0; i < 10; i++) {
-            source = Path.of(BackupManagerTest.class.getResource("changes.bib").toURI());
+            Path changesBibBak = Path.of(BackupManagerTest.class.getResource("changes.bib").toURI());
             Path directory = BackupFileUtil.getAppDataBackupDir();
             String timeSuffix = "2020-02-03--00.00.0" + Integer.toString(i);
-            String fileName = BackupFileUtil.getUniqueFilePrefix(Path.of(BackupManagerTest.class.getResource("no-changes.bib").toURI())) + "--no-changes.bib--" + timeSuffix + ".bak";
+            String fileName = BackupFileUtil.getUniqueFilePrefix(noChangesBib) + "--no-changes.bib--" + timeSuffix + ".bak";
             target = directory.resolve(fileName);
-            Files.copy(source, target, StandardCopyOption.REPLACE_EXISTING);
+            Files.copy(changesBibBak, target, StandardCopyOption.REPLACE_EXISTING);
         }
 
-        Path originalFile = Path.of(BackupManagerTest.class.getResource("no-changes.bib").toURI());
+        Path originalFile = noChangesBib;
         assertFalse(BackupManager.backupFileDiffers(originalFile));
     }
+
+    @Test
+    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);
+        Files.copy(changesBibBak, target, StandardCopyOption.REPLACE_EXISTING);
+
+        assertTrue(BackupManager.backupFileDiffers(changesBib));
+    }
+
+    @Test
+    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);
+        Files.copy(changesBibBak, target, StandardCopyOption.REPLACE_EXISTING);
+
+        // Make .bak file very old
+        Files.setLastModifiedTime(target, FileTime.fromMillis(0));
+
+        assertFalse(BackupManager.backupFileDiffers(changesBib));
+    }
 }