[WIP] Fix change on disk detection

build.gradle

@@ -221,6 +221,7 @@ dependencies {
     testCompile "org.testfx:testfx-core:4.0.17-alpha-SNAPSHOT"
     testCompile "org.testfx:testfx-junit5:4.0.17-alpha-SNAPSHOT"
     testCompile "org.hamcrest:hamcrest-library:2.2"
+    testCompile group: 'org.assertj', name: 'assertj-core', version: '3.14.0'
 
     checkstyle 'com.puppycrawl.tools:checkstyle:8.26'
     xjc group: 'org.glassfish.jaxb', name: 'jaxb-xjc', version: '2.3.2'

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

@@ -34,7 +34,6 @@
 import org.jabref.gui.bibtexkeypattern.GenerateBibtexKeyAction;
 import org.jabref.gui.cleanup.CleanupAction;
 import org.jabref.gui.collab.DatabaseChangeMonitor;
-import org.jabref.gui.collab.DatabaseChangePane;
 import org.jabref.gui.desktop.JabRefDesktop;
 import org.jabref.gui.edit.CopyBibTeXKeyAndLinkAction;
 import org.jabref.gui.edit.ReplaceStringAction;

src/main/java/org/jabref/gui/collab/ChangeScanner.java

@@ -1,33 +1,45 @@
 package org.jabref.gui.collab;
 
-import java.nio.file.Path;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.Comparator;
 import java.util.List;
 import java.util.Optional;
 
-import org.jabref.Globals;
 import org.jabref.logic.bibtex.DuplicateCheck;
 import org.jabref.logic.bibtex.comparator.BibDatabaseDiff;
 import org.jabref.logic.bibtex.comparator.BibEntryDiff;
 import org.jabref.logic.bibtex.comparator.BibStringDiff;
-import org.jabref.logic.importer.ImportFormatPreferences;
-import org.jabref.logic.importer.OpenDatabase;
-import org.jabref.logic.importer.ParserResult;
 import org.jabref.model.database.BibDatabaseContext;
 import org.jabref.model.entry.BibEntry;
 import org.jabref.model.entry.BibtexString;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 public class ChangeScanner {
 
-    private final Path referenceFile;
-    private final BibDatabaseContext database;
-    private final List<DatabaseChangeViewModel> changes = new ArrayList<>();
-    private BibDatabaseContext referenceDatabase;
+    private static final Logger LOGGER = LoggerFactory.getLogger(ChangeScanner.class);
+
+    private final BibDatabaseContext oldDatabase;
+    private final BibDatabaseContext newDatabase;
+
+    public ChangeScanner(BibDatabaseContext oldDatabase, BibDatabaseContext newDatabase) {
+        this.oldDatabase = oldDatabase;
+        this.newDatabase = newDatabase;
+    }
 
-    public ChangeScanner(BibDatabaseContext database, Path referenceFile) {
-        this.database = database;
-        this.referenceFile = referenceFile;
+    public List<DatabaseChangeViewModel> scanForChanges() {
+        List changes = new ArrayList();
+        BibDatabaseDiff differences = BibDatabaseDiff.compare(oldDatabase, newDatabase);
+        differences.getMetaDataDifferences().ifPresent(diff -> {
+            changes.add(new MetaDataChangeViewModel(diff));
+            diff.getGroupDifferences().ifPresent(groupDiff -> changes.add(new GroupChangeViewModel(groupDiff)));
+        });
+        differences.getPreambleDifferences().ifPresent(diff -> changes.add(new PreambleChangeViewModel(diff)));
+        differences.getBibStringDifferences().forEach(diff -> changes.add(createBibStringDiff(diff)));
+        differences.getEntryDifferences().forEach(diff -> changes.add(createBibEntryDiff(diff)));
+        return changes;
     }
 
     /**
@@ -40,46 +52,22 @@ private static BibEntry bestFit(BibEntry targetEntry, List<BibEntry> entries) {
                       .orElse(null);
     }
 
-    public List<DatabaseChangeViewModel> scanForChanges() {
-        database.getDatabasePath().ifPresent(diskdb -> {
-            // Parse the temporary file.
-            ImportFormatPreferences importFormatPreferences = Globals.prefs.getImportFormatPreferences();
-            ParserResult result = OpenDatabase.loadDatabase(referenceFile.toAbsolutePath().toString(), importFormatPreferences, Globals.getFileUpdateMonitor());
-            referenceDatabase = result.getDatabaseContext();
-
-            // Parse the modified file.
-            result = OpenDatabase.loadDatabase(diskdb.toAbsolutePath().toString(), importFormatPreferences, Globals.getFileUpdateMonitor());
-            BibDatabaseContext databaseOnDisk = result.getDatabaseContext();
-
-            // Start looking at changes.
-            BibDatabaseDiff differences = BibDatabaseDiff.compare(referenceDatabase, databaseOnDisk);
-            differences.getMetaDataDifferences().ifPresent(diff -> {
-                changes.add(new MetaDataChangeViewModel(diff));
-                diff.getGroupDifferences().ifPresent(groupDiff -> changes.add(new GroupChangeViewModel(groupDiff)));
-            });
-            differences.getPreambleDifferences().ifPresent(diff -> changes.add(new PreambleChangeViewModel(diff)));
-            differences.getBibStringDifferences().forEach(diff -> changes.add(createBibStringDiff(diff)));
-            differences.getEntryDifferences().forEach(diff -> changes.add(createBibEntryDiff(diff)));
-        });
-        return changes;
-    }
-
     private DatabaseChangeViewModel createBibStringDiff(BibStringDiff diff) {
         if (diff.getOriginalString() == null) {
             return new StringAddChangeViewModel(diff.getNewString());
         }
 
         if (diff.getNewString() == null) {
-            Optional<BibtexString> current = database.getDatabase().getStringByName(diff.getOriginalString().getName());
+            Optional<BibtexString> current = oldDatabase.getDatabase().getStringByName(diff.getOriginalString().getName());
             return new StringRemoveChangeViewModel(diff.getOriginalString(), current.orElse(null));
         }
 
         if (diff.getOriginalString().getName().equals(diff.getNewString().getName())) {
-            Optional<BibtexString> current = database.getDatabase().getStringByName(diff.getOriginalString().getName());
+            Optional<BibtexString> current = oldDatabase.getDatabase().getStringByName(diff.getOriginalString().getName());
             return new StringChangeViewModel(current.orElse(null), diff.getOriginalString(), diff.getNewString().getContent());
         }
 
-        Optional<BibtexString> current = database.getDatabase().getStringByName(diff.getOriginalString().getName());
+        Optional<BibtexString> current = oldDatabase.getDatabase().getStringByName(diff.getOriginalString().getName());
         return new StringNameChangeViewModel(current.orElse(null), diff.getOriginalString(), current.map(BibtexString::getName).orElse(""), diff.getNewString().getName());
     }
 
@@ -89,9 +77,9 @@ private DatabaseChangeViewModel createBibEntryDiff(BibEntryDiff diff) {
         }
 
         if (diff.getNewEntry() == null) {
-            return new EntryDeleteChangeViewModel(bestFit(diff.getOriginalEntry(), database.getEntries()), diff.getOriginalEntry());
+            return new EntryDeleteChangeViewModel(bestFit(diff.getOriginalEntry(), oldDatabase.getEntries()), diff.getOriginalEntry());
         }
 
-        return new EntryChangeViewModel(bestFit(diff.getOriginalEntry(), database.getEntries()), diff.getOriginalEntry(), diff.getNewEntry());
+        return new EntryChangeViewModel(bestFit(diff.getOriginalEntry(), oldDatabase.getEntries()), diff.getOriginalEntry(), diff.getNewEntry());
     }
 }

src/main/java/org/jabref/gui/collab/DatabaseChangeListener.java

@@ -1,7 +1,7 @@
 package org.jabref.gui.collab;
 
-import java.util.List;
+import org.jabref.logic.bibtex.comparator.BibDatabaseDiff;
 
 public interface DatabaseChangeListener {
-    void databaseChanged(List<DatabaseChangeViewModel> changes);
+    void databaseChanged(BibDatabaseDiff bibDatabaseDiff);
 }

src/main/java/org/jabref/gui/collab/DatabaseChangeMonitor.java

@@ -3,76 +3,101 @@
 import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.nio.file.StandardCopyOption;
 import java.util.ArrayList;
 import java.util.List;
 
+import org.jabref.Globals;
 import org.jabref.gui.util.BackgroundTask;
 import org.jabref.gui.util.TaskExecutor;
+import org.jabref.logic.bibtex.comparator.BibDatabaseDiff;
+import org.jabref.logic.importer.ImportFormatPreferences;
+import org.jabref.logic.importer.OpenDatabase;
+import org.jabref.logic.importer.ParserResult;
 import org.jabref.logic.util.io.FileUtil;
 import org.jabref.model.database.BibDatabaseContext;
+import org.jabref.model.util.DummyFileUpdateMonitor;
 import org.jabref.model.util.FileUpdateListener;
 import org.jabref.model.util.FileUpdateMonitor;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+/**
+ * An update monitor for a file-based library (.bib file). Has to be re-instantiated if the location of the bib file
+ * changes.
+ */
 public class DatabaseChangeMonitor implements FileUpdateListener {
     private static final Logger LOGGER = LoggerFactory.getLogger(DatabaseChangeMonitor.class);
 
-    private final BibDatabaseContext database;
+    private BibDatabaseContext referenceDatabase;
+    private Path databaseToBeMonitored;
+
     private final FileUpdateMonitor fileMonitor;
-    private final List<DatabaseChangeListener> listeners;
-    private Path referenceFile;
+    private final List<DatabaseChangeListener> listeners = new ArrayList<>();
     private TaskExecutor taskExecutor;
 
-    public DatabaseChangeMonitor(BibDatabaseContext database, FileUpdateMonitor fileMonitor, TaskExecutor taskExecutor) {
-        this.database = database;
+    /**
+     * @param databaseToBeMonitored The BibTeX database to be monitored
+     * @param fileMonitor           The update monitor where to register to get notified about a change
+     * @param taskExecutor          The scan for changes and notification is run with that task executor
+     */
+    public DatabaseChangeMonitor(BibDatabaseContext databaseToBeMonitored, FileUpdateMonitor fileMonitor, TaskExecutor taskExecutor) {
         this.fileMonitor = fileMonitor;
         this.taskExecutor = taskExecutor;
-        this.listeners = new ArrayList<>();
 
-        this.database.getDatabasePath().ifPresent(path -> {
+        databaseToBeMonitored.getDatabasePath().ifPresentOrElse(path -> {
+            this.databaseToBeMonitored = path;
+            loadReferenceDatabaseFromDatabaseToBeMonitored();
             try {
+                // as last step, register this class
                 fileMonitor.addListenerForFile(path, this);
-                referenceFile = Files.createTempFile("jabref", ".bib");
-                referenceFile.toFile().deleteOnExit();
-                setAsReference(path);
             } catch (IOException e) {
                 LOGGER.error("Error while trying to monitor " + path, e);
             }
+        }, () -> {
+            throw new IllegalStateException("Path has to be present");
         });
     }
 
     @Override
     public void fileUpdated() {
         // File on disk has changed, thus look for notable changes and notify listeners in case there are such changes
-        ChangeScanner scanner = new ChangeScanner(database, referenceFile);
-        BackgroundTask.wrap(scanner::scanForChanges)
-                      .onSuccess(changes -> {
-                          if (!changes.isEmpty()) {
-                              listeners.forEach(listener -> listener.databaseChanged(changes));
-                          }
-                      })
-                      .executeWith(taskExecutor);
+
+        BackgroundTask.wrap(() -> {
+            BibDatabaseDiff differences;
+            // no two threads may check for changes
+            synchronized (referenceDatabase) {
+                ParserResult result = OpenDatabase.loadDatabase(this.databaseToBeMonitored.toAbsolutePath().toString(), Globals.prefs.getImportFormatPreferences(), new DummyFileUpdateMonitor());
+                BibDatabaseContext databaseOnDisk = result.getDatabaseContext();
+                differences = BibDatabaseDiff.compare(this.referenceDatabase, databaseOnDisk);
+                this.referenceDatabase = databaseOnDisk;
+            }
+            return differences;
+        }).onSuccess(diff -> {
+            listeners.forEach(listener -> listener.databaseChanged(diff));
+        }).executeWith(taskExecutor);
     }
 
     public void addListener(DatabaseChangeListener listener) {
         listeners.add(listener);
     }
 
     public void unregister() {
-        database.getDatabasePath().ifPresent(file -> fileMonitor.removeListener(file, this));
+        fileMonitor.removeListener(this.databaseToBeMonitored, this);
     }
 
-    public void markExternalChangesAsResolved() {
-        markAsSaved();
+    private void loadReferenceDatabaseFromDatabaseToBeMonitored() {
+        // there is no clone of BibDatabaseContext - we do it "the hard way" und just reload the database
+        ImportFormatPreferences importFormatPreferences = Globals.prefs.getImportFormatPreferences();
+        ParserResult result = OpenDatabase.loadDatabase(this.databaseToBeMonitored.toAbsolutePath().toString(), importFormatPreferences, new DummyFileUpdateMonitor());
+        this.referenceDatabase = result.getDatabaseContext();
     }
 
+    /**
+     * Call this if the database to monitor was saved
+     */
     public void markAsSaved() {
-        database.getDatabasePath().ifPresent(this::setAsReference);
-    }
-
-    private void setAsReference(Path file) {
-        FileUtil.copyFile(file, referenceFile, true);
+        loadReferenceDatabaseFromDatabaseToBeMonitored();
     }
 }

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

@@ -28,7 +28,7 @@ public void listen(@SuppressWarnings("unused") AutosaveEvent event) {
         try {
             new SaveDatabaseAction(panel, Globals.prefs, Globals.entryTypesManager).save(SaveDatabaseAction.SaveDatabaseMode.SILENT);
         } catch (Throwable e) {
-            LOGGER.error("Problem occured while saving.", e);
+            LOGGER.error("Problem occurred while saving.", e);
         }
     }
 }

src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java

@@ -42,7 +42,7 @@
 /**
  * Action for the "Save" and "Save as" operations called from BasePanel. This class is also used for save operations
  * when closing a database or quitting the applications.
- *
+ * <p>
  * The save operation is loaded off of the GUI thread using {@link BackgroundTask}. Callers can query whether the
  * operation was canceled, or whether it was successful.
  */
@@ -69,12 +69,10 @@ public SaveDatabaseAction(BasePanel panel, JabRefPreferences prefs, BibEntryType
     }
 
     private boolean saveDatabase(Path file, boolean selectedOnly, Charset encoding, SavePreferences.DatabaseSaveType saveType) throws SaveException {
-        try {
-            SavePreferences preferences = prefs.loadForSaveFromPreferences()
-                                               .withEncoding(encoding)
-                                               .withSaveType(saveType);
-
-            AtomicFileWriter fileWriter = new AtomicFileWriter(file, preferences.getEncoding(), preferences.makeBackup());
+        SavePreferences preferences = prefs.loadForSaveFromPreferences()
+                                           .withEncoding(encoding)
+                                           .withSaveType(saveType);
+        try (AtomicFileWriter fileWriter = new AtomicFileWriter(file, preferences.getEncoding(), preferences.makeBackup())) {
             BibtexDatabaseWriter databaseWriter = new BibtexDatabaseWriter(fileWriter, preferences, entryTypesManager);
 
             if (selectedOnly) {
@@ -148,7 +146,7 @@ private boolean doSave() {
 
                 // Reset title of tab
                 frame.setTabTitle(panel, panel.getTabTitle(),
-                                  panel.getBibDatabaseContext().getDatabaseFile().get().getAbsolutePath());
+                        panel.getBibDatabaseContext().getDatabasePath().get().toAbsolutePath().toString());
                 frame.setWindowTitle();
                 frame.updateAllTabTitles();
             }