Fix changed on disk for ContentSelectors and groups

CHANGELOG.md

@@ -25,6 +25,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv
 ### Fixed
 
 - We fixed an issue where entry type with duplicate fields prevented opening existing libraries with custom entry types [#11127](https://github.com/JabRef/jabref/issues/11127)
+- We fixed an issue when the file was flagged as changed on disk in the case of content selectors or groups. [#9064](https://github.com/JabRef/jabref/issues/9064)
 - We fixed crash on opening the entry editor when auto completion is enabled. [#11188](https://github.com/JabRef/jabref/issues/11188)
 - We fixed the usage of the key binding for "Clear search" (default: <kbd>Escape</kbd>). [#10764](https://github.com/JabRef/jabref/issues/10764).
 

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

@@ -73,7 +73,7 @@ public DatabaseChangeDetailsView create(DatabaseChange databaseChange) {
         } else if (databaseChange instanceof BibTexStringRename stringRename) {
             return new BibTexStringRenameDetailsView(stringRename);
         } else if (databaseChange instanceof MetadataChange metadataChange) {
-            return new MetadataChangeDetailsView(metadataChange, preferencesService);
+            return new MetadataChangeDetailsView(metadataChange, preferencesService.getCitationKeyPatternPreferences().getKeyPatterns());
         } else if (databaseChange instanceof GroupChange groupChange) {
             return new GroupChangeDetailsView(groupChange);
         } else if (databaseChange instanceof PreambleChange preambleChange) {

src/main/java/org/jabref/gui/collab/metedatachange/MetadataChangeDetailsView.java

@@ -6,19 +6,19 @@
 
 import org.jabref.gui.collab.DatabaseChangeDetailsView;
 import org.jabref.logic.bibtex.comparator.MetaDataDiff;
+import org.jabref.logic.citationkeypattern.GlobalCitationKeyPatterns;
 import org.jabref.logic.l10n.Localization;
-import org.jabref.preferences.PreferencesService;
 
 public final class MetadataChangeDetailsView extends DatabaseChangeDetailsView {
 
-    public MetadataChangeDetailsView(MetadataChange metadataChange, PreferencesService preferencesService) {
+    public MetadataChangeDetailsView(MetadataChange metadataChange, GlobalCitationKeyPatterns globalCitationKeyPatterns) {
         VBox container = new VBox(15);
 
         Label header = new Label(Localization.lang("The following metadata changed:"));
         header.getStyleClass().add("sectionHeader");
         container.getChildren().add(header);
 
-        for (MetaDataDiff.Difference diff : metadataChange.getMetaDataDiff().getDifferences(preferencesService)) {
+        for (MetaDataDiff.Difference diff : metadataChange.getMetaDataDiff().getDifferences(globalCitationKeyPatterns)) {
             container.getChildren().add(new Label(getDifferenceString(diff.differenceType())));
             container.getChildren().add(new Label(diff.originalObject().toString()));
             container.getChildren().add(new Label(diff.newObject().toString()));
@@ -38,7 +38,7 @@ private String getDifferenceString(MetaDataDiff.DifferenceType changeType) {
         return switch (changeType) {
             case PROTECTED ->
                     Localization.lang("Library protection");
-            case GROUPS_ALTERED ->
+            case GROUPS ->
                     Localization.lang("Modified groups tree");
             case ENCODING ->
                     Localization.lang("Library encoding");

src/main/java/org/jabref/gui/libraryproperties/contentselectors/ContentSelectorViewModel.java

@@ -25,20 +25,18 @@
 import org.jabref.model.database.BibDatabaseContext;
 import org.jabref.model.entry.field.Field;
 import org.jabref.model.entry.field.FieldFactory;
-import org.jabref.model.entry.field.StandardField;
 import org.jabref.model.metadata.ContentSelector;
+import org.jabref.model.metadata.ContentSelectors;
 import org.jabref.model.metadata.MetaData;
 
 public class ContentSelectorViewModel implements PropertiesTabViewModel {
 
-    private static final List<Field> DEFAULT_FIELD_NAMES = List.of(StandardField.AUTHOR, StandardField.JOURNAL, StandardField.KEYWORDS, StandardField.PUBLISHER);
-
     private final MetaData metaData;
 
     private final DialogService dialogService;
 
     // The map from each field to its predefined strings ("keywords") that can be selected
-    private final Map<Field, List<String>> fieldKeywordsMap = new HashMap<>();
+    private Map<Field, List<String>> fieldKeywordsMap = new HashMap<>();
 
     private final ListProperty<Field> fields = new SimpleListProperty<>(FXCollections.observableArrayList());
     private final ListProperty<String> keywords = new SimpleListProperty<>(FXCollections.observableArrayList());
@@ -53,16 +51,14 @@ public class ContentSelectorViewModel implements PropertiesTabViewModel {
     @Override
     public void setValues() {
         // Populate field names keyword map
-        metaData.getContentSelectors().getContentSelectors().forEach(
-                existingContentSelector -> fieldKeywordsMap.put(existingContentSelector.getField(), new ArrayList<>(existingContentSelector.getValues()))
-        );
+        fieldKeywordsMap = ContentSelectors.getFieldKeywordsMap(metaData.getContentSelectors().getContentSelectors());
 
         // Populate Field names list
         List<Field> existingFields = new ArrayList<>(fieldKeywordsMap.keySet());
         fields.addAll(existingFields);
 
         if (fields.isEmpty()) {
-            DEFAULT_FIELD_NAMES.forEach(this::addFieldIfUnique);
+            ContentSelectors.DEFAULT_FIELD_NAMES.forEach(this::addFieldIfUnique);
         }
     }
 
@@ -71,7 +67,7 @@ public void storeSettings() {
         List<Field> metaDataFields = metaData.getContentSelectors().getFieldsWithSelectors();
         List<Field> fieldNamesToRemove;
 
-        if (isDefaultMap(fieldKeywordsMap)) {
+        if (ContentSelectors.isDefaultMap(fieldKeywordsMap)) {
             // Remove all fields of the content selector
             fieldNamesToRemove = metaData.getContentSelectorsSorted().stream().map(ContentSelector::getField).toList();
         } else {
@@ -82,24 +78,6 @@ public void storeSettings() {
         fieldNamesToRemove.forEach(metaData::clearContentSelectors);
     }
 
-    /**
-     * Checks whether the given map is the default map, i.e. contains only the default field names and no associated keywords.
-     */
-    private boolean isDefaultMap(Map<Field, List<String>> fieldKeywordsMap) {
-        if (fieldKeywordsMap.size() != DEFAULT_FIELD_NAMES.size()) {
-            return false;
-        }
-        for (Field field : DEFAULT_FIELD_NAMES) {
-            if (!fieldKeywordsMap.containsKey(field)) {
-                return false;
-            }
-            if (!fieldKeywordsMap.get(field).isEmpty()) {
-                return false;
-            }
-        }
-        return true;
-    }
-
     public ListProperty<Field> getFieldNamesBackingList() {
         return fields;
     }
@@ -225,7 +203,7 @@ private List<Field> determineFieldsToRemove() {
         // Remove all unset default fields
         result.addAll(fieldKeywordsMap.entrySet()
                                       .stream()
-                                      .filter(entry -> DEFAULT_FIELD_NAMES.contains(entry.getKey()) && entry.getValue().isEmpty()).map(Map.Entry::getKey)
+                                      .filter(entry -> ContentSelectors.DEFAULT_FIELD_NAMES.contains(entry.getKey()) && entry.getValue().isEmpty()).map(Map.Entry::getKey)
                                       .toList());
 
         return result;

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

@@ -2,19 +2,25 @@
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
 
+import org.jabref.logic.citationkeypattern.CitationKeyPattern;
+import org.jabref.logic.citationkeypattern.GlobalCitationKeyPatterns;
+import org.jabref.logic.groups.DefaultGroupsFactory;
+import org.jabref.model.entry.field.Field;
+import org.jabref.model.groups.GroupTreeNode;
+import org.jabref.model.metadata.ContentSelectors;
 import org.jabref.model.metadata.MetaData;
-import org.jabref.preferences.PreferencesService;
 
 public class MetaDataDiff {
     public enum DifferenceType {
         CONTENT_SELECTOR,
         DEFAULT_KEY_PATTERN,
         ENCODING,
         GENERAL_FILE_DIRECTORY,
-        GROUPS_ALTERED,
+        GROUPS,
         KEY_PATTERNS,
         LATEX_FILE_DIRECTORY,
         MODE,
@@ -41,28 +47,69 @@ public static Optional<MetaDataDiff> compare(MetaData originalMetaData, MetaData
         if (originalMetaData.equals(newMetaData)) {
             return Optional.empty();
         } else {
-            return Optional.of(new MetaDataDiff(originalMetaData, newMetaData));
+            MetaDataDiff diff = new MetaDataDiff(originalMetaData, newMetaData);
+            List<Difference> differences = diff.getDifferences(new GlobalCitationKeyPatterns(CitationKeyPattern.NULL_CITATION_KEY_PATTERN));
+            if (differences.isEmpty()) {
+                return Optional.empty();
+            }
+            return Optional.of(diff);
         }
     }
 
+    /**
+     * Checks if given content selectors are empty or default
+     */
+    private static boolean isDefaultContentSelectors(ContentSelectors contentSelectors) {
+        if (contentSelectors.getContentSelectors().isEmpty()) {
+            return true;
+        }
+        Map<Field, List<String>> fieldKeywordsMap = ContentSelectors.getFieldKeywordsMap(contentSelectors.getContentSelectors());
+        return ContentSelectors.isDefaultMap(fieldKeywordsMap);
+    }
+
     private void addToListIfDiff(List<Difference> changes, DifferenceType differenceType, Object originalObject, Object newObject) {
         if (!Objects.equals(originalObject, newObject)) {
+            if (differenceType == DifferenceType.CONTENT_SELECTOR) {
+                ContentSelectors originalContentSelectors = (ContentSelectors) originalObject;
+                ContentSelectors newContentSelectors = (ContentSelectors) newObject;
+                if (isDefaultContentSelectors(originalContentSelectors) && isDefaultContentSelectors(newContentSelectors)) {
+                    return;
+                }
+            }
+            if (differenceType == DifferenceType.GROUPS) {
+                Optional<GroupTreeNode> originalGroups = (Optional<GroupTreeNode>) originalObject;
+                Optional<GroupTreeNode> newGroups = (Optional<GroupTreeNode>) newObject;
+                if (isDefaultGroup(originalGroups) && isDefaultGroup(newGroups)) {
+                    return;
+                }
+            }
             changes.add(new Difference(differenceType, originalObject, newObject));
         }
     }
 
+    private boolean isDefaultGroup(Optional<GroupTreeNode> groups) {
+        if (groups.isEmpty()) {
+            return true;
+        }
+        GroupTreeNode groupRoot = groups.get();
+        if (!groupRoot.getChildren().isEmpty()) {
+            return false;
+        }
+        return groupRoot.getGroup().equals(DefaultGroupsFactory.getAllEntriesGroup());
+    }
+
     /**
      * Should be kept in sync with {@link MetaData#equals(Object)}
      */
-    public List<Difference> getDifferences(PreferencesService preferences) {
+    public List<Difference> getDifferences(GlobalCitationKeyPatterns globalCitationKeyPatterns) {
         List<Difference> changes = new ArrayList<>();
         addToListIfDiff(changes, DifferenceType.PROTECTED, originalMetaData.isProtected(), newMetaData.isProtected());
-        addToListIfDiff(changes, DifferenceType.GROUPS_ALTERED, originalMetaData.getGroups(), newMetaData.getGroups());
+        addToListIfDiff(changes, DifferenceType.GROUPS, originalMetaData.getGroups(), newMetaData.getGroups());
         addToListIfDiff(changes, DifferenceType.ENCODING, originalMetaData.getEncoding(), newMetaData.getEncoding());
         addToListIfDiff(changes, DifferenceType.SAVE_SORT_ORDER, originalMetaData.getSaveOrder(), newMetaData.getSaveOrder());
         addToListIfDiff(changes, DifferenceType.KEY_PATTERNS,
-                originalMetaData.getCiteKeyPatterns(preferences.getCitationKeyPatternPreferences().getKeyPatterns()),
-                newMetaData.getCiteKeyPatterns(preferences.getCitationKeyPatternPreferences().getKeyPatterns()));
+                originalMetaData.getCiteKeyPatterns(globalCitationKeyPatterns),
+                newMetaData.getCiteKeyPatterns(globalCitationKeyPatterns));
         addToListIfDiff(changes, DifferenceType.USER_FILE_DIRECTORY, originalMetaData.getUserFileDirectories(), newMetaData.getUserFileDirectories());
         addToListIfDiff(changes, DifferenceType.LATEX_FILE_DIRECTORY, originalMetaData.getLatexFileDirectories(), newMetaData.getLatexFileDirectories());
         addToListIfDiff(changes, DifferenceType.DEFAULT_KEY_PATTERN, originalMetaData.getDefaultCiteKeyPattern(), newMetaData.getDefaultCiteKeyPattern());
@@ -77,6 +124,9 @@ public MetaData getNewMetaData() {
         return newMetaData;
     }
 
+    /**
+     * Currently, the groups diff is contained here - and as entry in {@link #getDifferences(GlobalCitationKeyPatterns)}
+     */
     public Optional<GroupDiff> getGroupDifferences() {
         return groupDiff;
     }

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

@@ -4,16 +4,21 @@
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.Comparator;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 import java.util.Objects;
 import java.util.SortedSet;
 import java.util.TreeSet;
 
 import org.jabref.model.entry.field.Field;
 import org.jabref.model.entry.field.FieldProperty;
+import org.jabref.model.entry.field.StandardField;
 
 public class ContentSelectors {
 
+    public static final List<Field> DEFAULT_FIELD_NAMES = List.of(StandardField.AUTHOR, StandardField.JOURNAL, StandardField.KEYWORDS, StandardField.PUBLISHER);
+
     private final SortedSet<ContentSelector> contentSelectors;
 
     public ContentSelectors() {
@@ -113,4 +118,30 @@ public String toString() {
                 ", fieldsWithSelectors=" + getFieldsWithSelectors() +
                 '}';
     }
+
+    /**
+     * Checks whether the given map is the default map, i.e. contains only the default field names and no associated keywords.
+     */
+    public static boolean isDefaultMap(Map<Field, List<String>> fieldKeywordsMap) {
+        if (fieldKeywordsMap.size() != ContentSelectors.DEFAULT_FIELD_NAMES.size()) {
+            return false;
+        }
+        for (Field field : ContentSelectors.DEFAULT_FIELD_NAMES) {
+            if (!fieldKeywordsMap.containsKey(field)) {
+                return false;
+            }
+            if (!fieldKeywordsMap.get(field).isEmpty()) {
+                return false;
+            }
+        }
+        return true;
+    }
+
+    public static Map<Field, List<String>> getFieldKeywordsMap(SortedSet<ContentSelector> contentSelectors) {
+        final Map<Field, List<String>> fieldKeywordsMap = new HashMap<>();
+        contentSelectors.forEach(
+                existingContentSelector -> fieldKeywordsMap.put(existingContentSelector.getField(), new ArrayList<>(existingContentSelector.getValues()))
+        );
+        return fieldKeywordsMap;
+    }
 }

src/test/java/org/jabref/logic/bibtex/comparator/MetaDataDiffTest.java

@@ -3,14 +3,18 @@
 import java.util.List;
 import java.util.Optional;
 
+import org.jabref.logic.groups.DefaultGroupsFactory;
 import org.jabref.model.entry.field.StandardField;
+import org.jabref.model.groups.ExplicitGroup;
+import org.jabref.model.groups.GroupHierarchyType;
+import org.jabref.model.groups.GroupTreeNode;
 import org.jabref.model.metadata.ContentSelector;
 import org.jabref.model.metadata.MetaData;
 
-import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotEquals;
 
 public class MetaDataDiffTest {
     @Test
@@ -24,10 +28,9 @@ public void compareWithSameContentSelectorsDoesNotReportAnyDiffs() {
     }
 
     @Test
-    @Disabled
     public void defaultSettingEqualsEmptySetting() {
         MetaData one = new MetaData();
-        // Field list is from {@link org.jabref.gui.libraryproperties.contentselectors.ContentSelectorViewModel.DEFAULT_FIELD_NAMES}
+        // Field list is from {@link org.jabref.model.metadata.ContentSelectors.DEFAULT_FIELD_NAMES}
         one.addContentSelector(new ContentSelector(StandardField.AUTHOR, List.of()));
         one.addContentSelector(new ContentSelector(StandardField.JOURNAL, List.of()));
         one.addContentSelector(new ContentSelector(StandardField.PUBLISHER, List.of()));
@@ -36,4 +39,25 @@ public void defaultSettingEqualsEmptySetting() {
 
         assertEquals(Optional.empty(), MetaDataDiff.compare(one, two));
     }
+
+    @Test
+    public void allEntriesGroupIgnored() {
+        MetaData one = new MetaData();
+        one.setGroups(GroupTreeNode.fromGroup(DefaultGroupsFactory.getAllEntriesGroup()));
+        MetaData two = new MetaData();
+
+        assertEquals(Optional.empty(), MetaDataDiff.compare(one, two));
+    }
+
+    @Test
+    public void allEntriesGroupContainingGroupNotIgnored() {
+        MetaData one = new MetaData();
+        GroupTreeNode root = GroupTreeNode.fromGroup(DefaultGroupsFactory.getAllEntriesGroup());
+        root.addSubgroup(new ExplicitGroup("ExplicitA", GroupHierarchyType.INCLUDING, ','));
+        one.setGroups(root);
+
+        MetaData two = new MetaData();
+
+        assertNotEquals(Optional.empty(), MetaDataDiff.compare(one, two));
+    }
 }