diff --git a/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java b/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java index 5b3f0cd8863..11228558c48 100644 --- 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 true if backup file exists AND differs from originalPath. false is the * "default" return value in the good case. In the case of an exception true is returned to ensure that * the user checks the output. */ public static boolean backupFileDiffers(Path originalPath) { - Optional 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 index a3b42ecf59d..e99e3a3d8e8 100644 --- 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)); + } }