Improve change detection (#5824)

* Add detailed view of metadata changes

* Fix KeyCollisionException

* Extract Globals.prefs

src/main/java/module-info.java

@@ -68,5 +68,11 @@
     requires org.jsoup;
     requires commons.csv;
     requires io.github.javadiffutils;
+    requires java.string.similarity;
+    requires ojdbc10;
+    requires org.postgresql.jdbc;
+    requires org.apache.commons.lang3;
+    requires org.antlr.antlr4.runtime;
     requires flowless;
+    requires org.apache.tika.core;
 }

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

@@ -44,7 +44,7 @@ public List<DatabaseChangeViewModel> scanForChanges() {
             // Start looking at changes.
             BibDatabaseDiff differences = BibDatabaseDiff.compare(database, databaseOnDisk);
             differences.getMetaDataDifferences().ifPresent(diff -> {
-                changes.add(new MetaDataChangeViewModel(diff));
+                changes.add(new MetaDataChangeViewModel(diff, Globals.prefs));
                 diff.getGroupDifferences().ifPresent(groupDiff -> changes.add(new GroupChangeViewModel(groupDiff)));
             });
             differences.getPreambleDifferences().ifPresent(diff -> changes.add(new PreambleChangeViewModel(diff)));
@@ -82,6 +82,6 @@ private DatabaseChangeViewModel createBibEntryDiff(BibEntryDiff diff) {
             return new EntryDeleteChangeViewModel(diff.getOriginalEntry());
         }
 
-        return new EntryChangeViewModel(diff.getOriginalEntry(), diff.getNewEntry(), database);
+        return new EntryChangeViewModel(diff.getOriginalEntry(), diff.getNewEntry());
     }
 }

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

@@ -13,30 +13,21 @@
 import org.jabref.model.database.BibDatabaseContext;
 import org.jabref.model.entry.BibEntry;
 
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
 class EntryChangeViewModel extends DatabaseChangeViewModel {
 
-    private static final Logger LOGGER = LoggerFactory.getLogger(EntryChangeViewModel.class);
-
     private final BibEntry oldEntry;
     private final BibEntry newEntry;
     private MergeEntries mergePanel;
 
-    private final BibDatabaseContext database;
-
-    public EntryChangeViewModel(BibEntry entry, BibEntry newEntry, BibDatabaseContext database) {
+    public EntryChangeViewModel(BibEntry entry, BibEntry newEntry) {
         super();
 
         this.oldEntry = entry;
         this.newEntry = newEntry;
-        this.database = database;
 
         name = entry.getCiteKeyOptional()
                     .map(key -> Localization.lang("Modified entry") + ": '" + key + '\'')
                     .orElse(Localization.lang("Modified entry"));
-
     }
 
     /**
@@ -55,9 +46,11 @@ public void setAccepted(boolean accepted) {
     @Override
     public void makeChange(BibDatabaseContext database, NamedCompound undoEdit) {
         database.getDatabase().removeEntry(oldEntry);
-        database.getDatabase().insertEntry(mergePanel.getMergeEntry());
+        BibEntry mergedEntry = mergePanel.getMergeEntry();
+        mergedEntry.setId(oldEntry.getId()); // Keep ID
+        database.getDatabase().insertEntry(mergedEntry);
         undoEdit.addEdit(new UndoableInsertEntries(database.getDatabase(), oldEntry));
-        undoEdit.addEdit(new UndoableInsertEntries(database.getDatabase(), mergePanel.getMergeEntry()));
+        undoEdit.addEdit(new UndoableInsertEntries(database.getDatabase(), mergedEntry));
     }
 
     @Override
@@ -68,7 +61,7 @@ public Node description() {
         header.getStyleClass().add("sectionHeader");
         container.getChildren().add(header);
         container.getChildren().add(mergePanel);
-        container.setMargin(mergePanel, new Insets(5, 5, 5, 5));
+        VBox.setMargin(mergePanel, new Insets(5, 5, 5, 5));
         return container;
     }
 }

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

@@ -2,39 +2,42 @@
 
 import javafx.scene.Node;
 import javafx.scene.control.Label;
+import javafx.scene.layout.VBox;
 
 import org.jabref.gui.undo.NamedCompound;
 import org.jabref.logic.bibtex.comparator.MetaDataDiff;
 import org.jabref.logic.l10n.Localization;
 import org.jabref.model.database.BibDatabaseContext;
-import org.jabref.model.metadata.MetaData;
+import org.jabref.preferences.JabRefPreferences;
 
 class MetaDataChangeViewModel extends DatabaseChangeViewModel {
 
-    private final MetaData newMetaData;
+    private final MetaDataDiff metaDataDiff;
+    private final JabRefPreferences preferences;
 
-    public MetaDataChangeViewModel(MetaDataDiff metaDataDiff) {
+    public MetaDataChangeViewModel(MetaDataDiff metaDataDiff, JabRefPreferences preferences) {
         super(Localization.lang("Metadata change"));
-        this.newMetaData = metaDataDiff.getNewMetaData();
+        this.metaDataDiff = metaDataDiff;
+        this.preferences = preferences;
     }
 
     @Override
     public Node description() {
+        VBox container = new VBox(15);
 
-        /*
-        // TODO: Show detailed description of the changes
-        StringBuilder sb = new StringBuilder(
-                "<html>" + Localization.lang("Changes have been made to the following metadata elements")
-                        + ":<p><br>&nbsp;&nbsp;");
-        sb.append(changes.stream().map(unit -> unit.key).collect(Collectors.joining("<br>&nbsp;&nbsp;")));
-        sb.append("</html>");
-        infoPane.setText(sb.toString());
-        */
-        return new Label(Localization.lang("Metadata change"));
+        Label header = new Label(Localization.lang("The following metadata changed:"));
+        header.getStyleClass().add("sectionHeader");
+        container.getChildren().add(header);
+
+        for (String change : metaDataDiff.getDifferences(preferences)) {
+            container.getChildren().add(new Label(change));
+        }
+
+        return container;
     }
 
     @Override
     public void makeChange(BibDatabaseContext database, NamedCompound undoEdit) {
-        database.setMetaData(newMetaData);
+        database.setMetaData(metaDataDiff.getNewMetaData());
     }
 }

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

@@ -2,16 +2,13 @@
 
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.nio.file.Paths;
 import java.sql.SQLException;
-import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Objects;
 import java.util.Optional;
-import java.util.stream.Collectors;
 
 import org.jabref.Globals;
 import org.jabref.gui.BasePanel;
@@ -32,7 +29,6 @@
 import org.jabref.logic.shared.exception.InvalidDBMSConnectionPropertiesException;
 import org.jabref.logic.shared.exception.NotASharedDatabaseException;
 import org.jabref.logic.util.StandardFileType;
-import org.jabref.model.database.BibDatabase;
 import org.jabref.model.database.shared.DatabaseNotSupportedException;
 import org.jabref.preferences.JabRefPreferences;
 
@@ -77,17 +73,13 @@ public static void performPostOpenActions(BasePanel panel, ParserResult result)
 
     @Override
     public void execute() {
-        List<Path> filesToOpen = new ArrayList<>();
-
         FileDialogConfiguration fileDialogConfiguration = new FileDialogConfiguration.Builder()
-                                                                                               .addExtensionFilter(StandardFileType.BIBTEX_DB)
-                                                                                               .withDefaultExtension(StandardFileType.BIBTEX_DB)
-                                                                                               .withInitialDirectory(getInitialDirectory())
-                                                                                               .build();
-
-        List<Path> chosenFiles = dialogService.showFileOpenDialogAndGetMultipleFiles(fileDialogConfiguration);
-        filesToOpen.addAll(chosenFiles);
+                .addExtensionFilter(StandardFileType.BIBTEX_DB)
+                .withDefaultExtension(StandardFileType.BIBTEX_DB)
+                .withInitialDirectory(getInitialDirectory())
+                .build();
 
+        List<Path> filesToOpen = dialogService.showFileOpenDialogAndGetMultipleFiles(fileDialogConfiguration);
         openFiles(filesToOpen, true);
     }
 
@@ -97,32 +89,20 @@ public void execute() {
      */
     private Path getInitialDirectory() {
         if (frame.getBasePanelCount() == 0) {
-            return getWorkingDirectoryPath();
+            return Globals.prefs.getWorkingDir();
         } else {
             Optional<Path> databasePath = frame.getCurrentBasePanel().getBibDatabaseContext().getDatabasePath();
-            return databasePath.map(p -> p.getParent()).orElse(getWorkingDirectoryPath());
+            return databasePath.map(Path::getParent).orElse(Globals.prefs.getWorkingDir());
         }
     }
 
-    private Path getWorkingDirectoryPath() {
-        return Paths.get(Globals.prefs.get(JabRefPreferences.WORKING_DIRECTORY));
-    }
-
     /**
      * Opens the given file. If null or 404, nothing happens
      *
      * @param file the file, may be null or not existing
      */
     public void openFile(Path file, boolean raisePanel) {
-        List<Path> filesToOpen = new ArrayList<>();
-        filesToOpen.add(file);
-        openFiles(filesToOpen, raisePanel);
-    }
-
-    public void openFilesAsStringList(List<String> fileNamesToOpen, boolean raisePanel) {
-        List<Path> filesToOpen = fileNamesToOpen.stream().map(Paths::get).collect(Collectors.toList());
-
-        openFiles(filesToOpen, raisePanel);
+        openFiles(Collections.singletonList(file), raisePanel);
     }
 
     /**
@@ -178,7 +158,6 @@ else if (toRaise != null) {
 
     /**
      * @param file the file, may be null or not existing
-     * @return
      */
     private void openTheFile(Path file, boolean raisePanel) {
         Objects.requireNonNull(file);
@@ -228,9 +207,6 @@ private ParserResult loadDatabase(Path file) throws Exception {
     }
 
     private BasePanel addNewDatabase(ParserResult result, final Path file, boolean raisePanel) {
-
-        BibDatabase database = result.getDatabase();
-
         if (result.hasWarnings()) {
             ParserResultWarningDialog.showParserResultWarningDialog(result, frame);
         }

src/main/java/org/jabref/logic/bibtex/comparator/MetaDataDiff.java

@@ -1,15 +1,22 @@
 package org.jabref.logic.bibtex.comparator;
 
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Objects;
 import java.util.Optional;
 
+import org.jabref.logic.l10n.Localization;
 import org.jabref.model.metadata.MetaData;
+import org.jabref.preferences.JabRefPreferences;
 
 public class MetaDataDiff {
 
     private final Optional<GroupDiff> groupDiff;
-    private MetaData newMetaData;
+    private final MetaData originalMetaData;
+    private final MetaData newMetaData;
 
     private MetaDataDiff(MetaData originalMetaData, MetaData newMetaData) {
+        this.originalMetaData = originalMetaData;
         this.newMetaData = newMetaData;
         this.groupDiff = GroupDiff.compare(originalMetaData, newMetaData);
     }
@@ -22,6 +29,51 @@ public static Optional<MetaDataDiff> compare(MetaData originalMetaData, MetaData
         }
     }
 
+    /**
+     * @implNote Should be kept in sync with {@link MetaData#equals(Object)}
+     */
+    public List<String> getDifferences(JabRefPreferences preferences) {
+        List<String> changes = new ArrayList<>();
+
+        if (originalMetaData.isProtected() != newMetaData.isProtected()) {
+            changes.add(Localization.lang("Library protection"));
+        }
+        if (!Objects.equals(originalMetaData.getGroups(), newMetaData.getGroups())) {
+            changes.add(Localization.lang("Modified groups tree"));
+        }
+        if (!Objects.equals(originalMetaData.getEncoding(), newMetaData.getEncoding())) {
+            changes.add(Localization.lang("Library encoding"));
+        }
+        if (!Objects.equals(originalMetaData.getSaveOrderConfig(), newMetaData.getSaveOrderConfig())) {
+            changes.add(Localization.lang("Save sort order"));
+        }
+        if (!Objects.equals(originalMetaData.getCiteKeyPattern(preferences.getKeyPattern()), newMetaData.getCiteKeyPattern(preferences.getKeyPattern()))) {
+            changes.add(Localization.lang("Key patterns"));
+        }
+        if (!Objects.equals(originalMetaData.getUserFileDirectories(), newMetaData.getUserFileDirectories())) {
+            changes.add(Localization.lang("User-specific file directory"));
+        }
+        if (!Objects.equals(originalMetaData.getLaTexFileDirectories(), newMetaData.getLaTexFileDirectories())) {
+            changes.add(Localization.lang("LaTex file directory"));
+        }
+        if (!Objects.equals(originalMetaData.getDefaultCiteKeyPattern(), newMetaData.getDefaultCiteKeyPattern())) {
+            changes.add(Localization.lang("Default pattern"));
+        }
+        if (!Objects.equals(originalMetaData.getSaveActions(), newMetaData.getSaveActions())) {
+            changes.add(Localization.lang("Save actions"));
+        }
+        if (originalMetaData.getMode() != newMetaData.getMode()) {
+            changes.add(Localization.lang("Library mode"));
+        }
+        if (!Objects.equals(originalMetaData.getDefaultFileDirectory(), newMetaData.getDefaultFileDirectory())) {
+            changes.add(Localization.lang("General file directory"));
+        }
+        if (!Objects.equals(originalMetaData.getContentSelectors(), newMetaData.getContentSelectors())) {
+            changes.add(Localization.lang("Content selectors"));
+        }
+        return changes;
+    }
+
     public MetaData getNewMetaData() {
         return newMetaData;
     }

src/main/java/org/jabref/logic/importer/util/MetaDataParser.java

@@ -102,7 +102,7 @@ public MetaData parse(MetaData metaData, Map<String, String> data, Character key
                     break;
                 default:
                     // Keep meta data items that we do not know in the file
-                    metaData.putUnkownMetaDataItem(entry.getKey(), value);
+                    metaData.putUnknownMetaDataItem(entry.getKey(), value);
             }
         }
         if (!defaultCiteKeyPattern.isEmpty() || !nonDefaultCiteKeyPatterns.isEmpty()) {

src/main/java/org/jabref/model/metadata/MetaData.java

@@ -294,7 +294,7 @@ public void unregisterListener(Object listener) {
         }
     }
 
-    private Optional<String> getDefaultCiteKeyPattern() {
+    public Optional<String> getDefaultCiteKeyPattern() {
         return Optional.ofNullable(defaultCiteKeyPattern);
     }
 
@@ -314,7 +314,7 @@ public Map<String, List<String>> getUnknownMetaData() {
         return Collections.unmodifiableMap(unkownMetaData);
     }
 
-    public void putUnkownMetaDataItem(String key, List<String> value) {
+    public void putUnknownMetaDataItem(String key, List<String> value) {
         Objects.requireNonNull(key);
         Objects.requireNonNull(value);
 
@@ -330,14 +330,16 @@ public boolean equals(Object o) {
             return false;
         }
         MetaData metaData = (MetaData) o;
-        return (isProtected == metaData.isProtected) && Objects.equals(groupsRoot, metaData.groupsRoot)
+        return (isProtected == metaData.isProtected)
+                && Objects.equals(groupsRoot, metaData.groupsRoot)
                 && Objects.equals(encoding, metaData.encoding)
                 && Objects.equals(saveOrderConfig, metaData.saveOrderConfig)
                 && Objects.equals(citeKeyPatterns, metaData.citeKeyPatterns)
                 && Objects.equals(userFileDirectory, metaData.userFileDirectory)
-               && Objects.equals(laTexFileDirectory, metaData.laTexFileDirectory)
+                && Objects.equals(laTexFileDirectory, metaData.laTexFileDirectory)
                 && Objects.equals(defaultCiteKeyPattern, metaData.defaultCiteKeyPattern)
-                && Objects.equals(saveActions, metaData.saveActions) && (mode == metaData.mode)
+                && Objects.equals(saveActions, metaData.saveActions)
+                && (mode == metaData.mode)
                 && Objects.equals(defaultFileDirectory, metaData.defaultFileDirectory)
                 && Objects.equals(contentSelectors, metaData.contentSelectors);
     }

src/main/resources/l10n/JabRef_en.properties

@@ -1023,7 +1023,7 @@ Formatter\ not\ found\:\ %0=Formatter not found: %0
 
 Could\ not\ save,\ file\ locked\ by\ another\ JabRef\ instance.=Could not save, file locked by another JabRef instance.
 Metadata\ change=Metadata change
-Changes\ have\ been\ made\ to\ the\ following\ metadata\ elements=Changes have been made to the following metadata elements
+The\ following\ metadata\ changed\:=The following metadata changed:
 
 Generate\ groups\ for\ author\ last\ names=Generate groups for author last names
 Generate\ groups\ from\ keywords\ in\ a\ BibTeX\ field=Generate groups from keywords in a BibTeX field
@@ -1187,6 +1187,7 @@ Five\ stars=Five stars
 Help\ on\ special\ fields=Help on special fields
 Keywords\ of\ selected\ entries=Keywords of selected entries
 Manage\ content\ selectors=Manage content selectors
+Content\ selectors=Content selectors
 Manage\ keywords=Manage keywords
 No\ priority\ information=No priority information
 No\ rank\ information=No rank information
@@ -1262,6 +1263,7 @@ Clear\ connection\ settings=Clear connection settings
 Open\ folder=Open folder
 Export\ entries\ ordered\ as\ specified=Export entries ordered as specified
 Export\ sort\ order=Export sort order
+Save\ sort\ order=Save sort order
 Export\ sorting=Export sorting
 Newline\ separator=Newline separator