Draft for fixing sync with groups and shared db

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

@@ -15,7 +15,9 @@
 import javafx.animation.PauseTransition;
 import javafx.application.Platform;
 import javafx.beans.property.BooleanProperty;
+import javafx.beans.property.ObjectProperty;
 import javafx.beans.property.SimpleBooleanProperty;
+import javafx.beans.property.SimpleObjectProperty;
 import javafx.beans.value.ObservableBooleanValue;
 import javafx.collections.ListChangeListener;
 import javafx.event.Event;
@@ -60,6 +62,7 @@
 import org.jabref.logic.citationstyle.CitationStyleCache;
 import org.jabref.logic.importer.ParserResult;
 import org.jabref.logic.importer.util.FileFieldParser;
+import org.jabref.logic.importer.util.MediaTypes;
 import org.jabref.logic.l10n.Localization;
 import org.jabref.logic.pdf.FileAnnotationCache;
 import org.jabref.logic.pdf.search.IndexingTaskManager;
@@ -84,6 +87,8 @@
 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.groups.event.GroupUpdatedEvent;
+import org.jabref.model.metadata.MetaData;
 import org.jabref.model.util.DirectoryMonitorManager;
 import org.jabref.model.util.FileUpdateMonitor;
 import org.jabref.preferences.PreferencesService;
@@ -114,6 +119,7 @@ private enum PanelMode { MAIN_TABLE, MAIN_TABLE_AND_ENTRY_EDITOR }
     private final StateManager stateManager;
     private final BibEntryTypesManager entryTypesManager;
     private final BooleanProperty changedProperty = new SimpleBooleanProperty(false);
+    private final ObjectProperty<MetaData> groupsTreeChanged = new SimpleObjectProperty<>();
     private final BooleanProperty nonUndoableChangeProperty = new SimpleBooleanProperty(false);
 
     private BibDatabaseContext bibDatabaseContext;
@@ -428,6 +434,14 @@ public void updateTabTitle(boolean isChanged) {
     @Subscribe
     public void listen(BibDatabaseContextChangedEvent event) {
         this.changedProperty.setValue(true);
+
+        // TODO: Check performance and maybe also re-add the event source check?
+        if (event instanceof GroupUpdatedEvent groupUpdatedEvent) {
+            DefaultTaskExecutor.runInJavaFXThread(() -> {
+                groupsTreeChanged.setValue(null);
+                groupsTreeChanged.setValue(groupUpdatedEvent.getMetaData());
+            });
+        }
     }
 
     /**
@@ -891,6 +905,10 @@ public ObservableBooleanValue getLoading() {
         return loading;
     }
 
+    public ObjectProperty<MetaData> groupsTreeChangedProperty() {
+        return groupsTreeChanged;
+    }
+
     public CountingUndoManager getUndoManager() {
         return undoManager;
     }

src/main/java/org/jabref/gui/groups/GroupTreeNodeViewModel.java

@@ -21,16 +21,11 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-public class GroupTreeNodeViewModel {
+public record GroupTreeNodeViewModel(
+        GroupTreeNode node) {
 
     private static final Logger LOGGER = LoggerFactory.getLogger(GroupTreeNodeViewModel.class);
 
-    private final GroupTreeNode node;
-
-    public GroupTreeNodeViewModel(GroupTreeNode node) {
-        this.node = node;
-    }
-
     @Override
     public String toString() {
         final StringBuilder sb = new StringBuilder("GroupTreeNodeViewModel{");
@@ -39,10 +34,6 @@ public String toString() {
         return sb.toString();
     }
 
-    public GroupTreeNode getNode() {
-        return node;
-    }
-
     public List<GroupTreeNodeViewModel> getChildren() {
         List<GroupTreeNodeViewModel> children = new ArrayList<>();
         for (GroupTreeNode child : node.getChildren()) {
@@ -51,36 +42,38 @@ public List<GroupTreeNodeViewModel> getChildren() {
         return children;
     }
 
-    protected boolean printInItalics() {
+    private boolean printInItalics() {
         return node.getGroup().isDynamic();
     }
 
     public String getDescription() {
         AbstractGroup group = node.getGroup();
         String shortDescription = "";
         boolean showDynamic = true;
-        if (group instanceof ExplicitGroup explicitGroup) {
-            shortDescription = GroupDescriptions.getShortDescriptionExplicitGroup(explicitGroup);
-        } else if (group instanceof KeywordGroup keywordGroup) {
-            shortDescription = GroupDescriptions.getShortDescriptionKeywordGroup(keywordGroup, showDynamic);
-        } else if (group instanceof SearchGroup searchGroup) {
-            shortDescription = GroupDescriptions.getShortDescription(searchGroup, showDynamic);
-        } else {
-            shortDescription = GroupDescriptions.getShortDescriptionAllEntriesGroup();
-        }
+        shortDescription = switch (group) {
+            case ExplicitGroup explicitGroup ->
+                    GroupDescriptions.getShortDescriptionExplicitGroup(explicitGroup);
+            case KeywordGroup keywordGroup ->
+                    GroupDescriptions.getShortDescriptionKeywordGroup(keywordGroup, showDynamic);
+            case SearchGroup searchGroup ->
+                    GroupDescriptions.getShortDescription(searchGroup, showDynamic);
+            case null,
+                 default ->
+                    GroupDescriptions.getShortDescriptionAllEntriesGroup();
+        };
         return "<html>" + shortDescription + "</html>";
     }
 
     public boolean canAddEntries(List<BibEntry> entries) {
-        return (getNode().getGroup() instanceof GroupEntryChanger) && !getNode().getGroup().containsAll(entries);
+        return (node().getGroup() instanceof GroupEntryChanger) && !node().getGroup().containsAll(entries);
     }
 
     public boolean canRemoveEntries(List<BibEntry> entries) {
-        return (getNode().getGroup() instanceof GroupEntryChanger) && getNode().getGroup().containsAny(entries);
+        return (node().getGroup() instanceof GroupEntryChanger) && node().getGroup().containsAny(entries);
     }
 
     public void sortChildrenByName(boolean recursive) {
-        getNode().sortChildren(
+        node().sortChildren(
                 (node1, node2) -> node1.getGroup().getName().compareToIgnoreCase(node2.getGroup().getName()),
                 recursive);
     }
@@ -98,38 +91,33 @@ public boolean equals(Object o) {
         return node.equals(viewModel.node);
     }
 
-    @Override
-    public int hashCode() {
-        return node.hashCode();
-    }
-
     public String getName() {
-        return getNode().getGroup().getName();
+        return node().getGroup().getName();
     }
 
     public boolean canBeEdited() {
-        return getNode().getGroup() instanceof AllEntriesGroup;
+        return node().getGroup() instanceof AllEntriesGroup;
     }
 
     public boolean canMoveUp() {
-        return (getNode().getPreviousSibling() != null)
-                && !(getNode().getGroup() instanceof AllEntriesGroup);
+        return (node().getPreviousSibling().isPresent())
+                && !(node().getGroup() instanceof AllEntriesGroup);
     }
 
     public boolean canMoveDown() {
-        return (getNode().getNextSibling() != null)
-                && !(getNode().getGroup() instanceof AllEntriesGroup);
+        return (node().getNextSibling().isPresent())
+                && !(node().getGroup() instanceof AllEntriesGroup);
     }
 
     public boolean canMoveLeft() {
-        return !(getNode().getGroup() instanceof AllEntriesGroup)
+        return !(node().getGroup() instanceof AllEntriesGroup)
                 // TODO: Null!
-                && !(getNode().getParent().get().getGroup() instanceof AllEntriesGroup);
+                && !(node().getParent().get().getGroup() instanceof AllEntriesGroup);
     }
 
     public boolean canMoveRight() {
-        return (getNode().getPreviousSibling() != null)
-                && !(getNode().getGroup() instanceof AllEntriesGroup);
+        return (node().getPreviousSibling().isPresent())
+                && !(node().getGroup() instanceof AllEntriesGroup);
     }
 
     public void changeEntriesTo(List<BibEntry> entries, UndoManager undoManager) {
@@ -179,12 +167,12 @@ public List<FieldChange> removeEntriesFromGroup(List<BibEntry> entries) {
     }
 
     public boolean isAllEntriesGroup() {
-        return getNode().getGroup() instanceof AllEntriesGroup;
+        return node().getGroup() instanceof AllEntriesGroup;
     }
 
     public void addNewGroup(AbstractGroup newGroup, CountingUndoManager undoManager) {
         GroupTreeNode newNode = GroupTreeNode.fromGroup(newGroup);
-        this.getNode().addChild(newNode);
+        this.node().addChild(newNode);
 
         UndoableAddOrRemoveGroup undo = new UndoableAddOrRemoveGroup(
                 this,
@@ -201,6 +189,6 @@ public List<FieldChange> addEntriesToGroup(List<BibEntry> entries) {
     }
 
     public void subscribeToDescendantChanged(Consumer<GroupTreeNodeViewModel> subscriber) {
-        getNode().subscribeToDescendantChanged(node -> subscriber.accept(new GroupTreeNodeViewModel(node)));
+        node().subscribeToDescendantChanged(node -> subscriber.accept(new GroupTreeNodeViewModel(node)));
     }
 }

src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java

@@ -23,10 +23,12 @@
 
 import org.jabref.gui.AbstractViewModel;
 import org.jabref.gui.DialogService;
+import org.jabref.gui.LibraryTab;
 import org.jabref.gui.StateManager;
 import org.jabref.gui.util.CustomLocalDragboard;
 import org.jabref.gui.util.TaskExecutor;
 import org.jabref.logic.l10n.Localization;
+import org.jabref.model.database.BibDatabase;
 import org.jabref.model.database.BibDatabaseContext;
 import org.jabref.model.entry.BibEntry;
 import org.jabref.model.groups.AbstractGroup;
@@ -83,15 +85,23 @@ public GroupTreeViewModel(StateManager stateManager, DialogService dialogService
         EasyBind.subscribe(stateManager.activeDatabaseProperty(), this::onActiveDatabaseChanged);
         EasyBind.subscribe(selectedGroups, this::onSelectedGroupChanged);
 
+
         // Set-up bindings
         filterPredicate.bind(EasyBind.map(filterText, text -> group -> group.isMatchedBy(text)));
-
+        var groupTreeChangedExternally = stateManager.activeTabProperty().flatMap(tab -> tab.map(LibraryTab::groupsTreeChangedProperty).orElse(new SimpleObjectProperty<>()));
+        EasyBind.subscribe(groupTreeChangedExternally, (metaData) -> {
+            if (metaData != null) {
+                System.out.println("Simulate database changed");
+                onActiveDatabaseChanged(stateManager.getActiveDatabase());
+            }
+        });
         // Init
-        refresh();
+        initRefreshListener();
     }
 
-    private void refresh() {
+    private void initRefreshListener() {
         onActiveDatabaseChanged(stateManager.activeDatabaseProperty().getValue());
+
     }
 
     public ObjectProperty<GroupNodeViewModel> rootGroupProperty() {
@@ -293,7 +303,7 @@ public void editGroup(GroupNodeViewModel oldGroup) {
                     dialogService.notify(Localization.lang("Modified group \"%0\".", group.getName()));
                     writeGroupChangesToMetaData();
                     // This is ugly but we have no proper update mechanism in place to propagate the changes, so redraw everything
-                    refresh();
+                    initRefreshListener();
                     return;
                 }
 
@@ -305,7 +315,7 @@ public void editGroup(GroupNodeViewModel oldGroup) {
                             database.getEntries());
 
                     writeGroupChangesToMetaData();
-                    refresh();
+                    initRefreshListener();
                     return;
                 }
 
@@ -379,7 +389,7 @@ public void editGroup(GroupNodeViewModel oldGroup) {
                 dialogService.notify(Localization.lang("Modified group \"%0\".", group.getName()));
                 writeGroupChangesToMetaData();
                 // This is ugly but we have no proper update mechanism in place to propagate the changes, so redraw everything
-                refresh();
+                initRefreshListener();
             });
         });
     }

src/main/java/org/jabref/gui/groups/UndoableAddOrRemoveGroup.java

@@ -71,13 +71,13 @@ public UndoableAddOrRemoveGroup(GroupTreeNodeViewModel groupsRoot,
         // storing a backup of the whole subtree is not required when children
         // are kept
         m_subtreeBackup = editType != UndoableAddOrRemoveGroup.REMOVE_NODE_KEEP_CHILDREN ?
-                editedNode.getNode()
+                editedNode.node()
                           .copySubtree()
-                : GroupTreeNode.fromGroup(editedNode.getNode().getGroup().deepCopy());
+                : GroupTreeNode.fromGroup(editedNode.node().getGroup().deepCopy());
         // remember path to edited node. this cannot be stored as a reference,
         // because the reference itself might change. the method below is more
         // robust.
-        m_pathToNode = editedNode.getNode().getIndexedPathFromRoot();
+        m_pathToNode = editedNode.node().getIndexedPathFromRoot();
     }
 
     @Override
@@ -108,7 +108,7 @@ public void redo() {
     }
 
     private void doOperation(boolean undo) {
-        GroupTreeNode cursor = m_groupsRootHandle.getNode();
+        GroupTreeNode cursor = m_groupsRootHandle.node();
         final int childIndex = m_pathToNode.getLast();
         // traverse path up to but last element
         for (int i = 0; i < (m_pathToNode.size() - 1); ++i) {

src/main/java/org/jabref/gui/groups/UndoableModifySubtree.java

@@ -32,9 +32,9 @@ public class UndoableModifySubtree extends AbstractUndoableJabRefEdit {
      */
     public UndoableModifySubtree(GroupTreeNodeViewModel groupRoot,
                                  GroupTreeNodeViewModel subtree, String name) {
-        m_subtreeBackup = subtree.getNode().copySubtree();
-        m_groupRoot = groupRoot.getNode();
-        m_subtreeRootPath = subtree.getNode().getIndexedPathFromRoot();
+        m_subtreeBackup = subtree.node().copySubtree();
+        m_groupRoot = groupRoot.node();
+        m_subtreeRootPath = subtree.node().getIndexedPathFromRoot();
         m_name = name;
     }
 

src/main/java/org/jabref/gui/groups/UndoableMoveGroup.java

@@ -33,19 +33,19 @@ public String getPresentationName() {
     public void undo() {
         super.undo();
 
-        GroupTreeNode newParent = root.getNode().getDescendant(pathToNewParent).get(); // TODO: NULL
+        GroupTreeNode newParent = root.node().getDescendant(pathToNewParent).get(); // TODO: NULL
         GroupTreeNode node = newParent.getChildAt(newChildIndex).get(); // TODO: Null
         // TODO: NULL
-        node.moveTo(root.getNode().getDescendant(pathToOldParent).get(), oldChildIndex);
+        node.moveTo(root.node().getDescendant(pathToOldParent).get(), oldChildIndex);
     }
 
     @Override
     public void redo() {
         super.redo();
 
-        GroupTreeNode oldParent = root.getNode().getDescendant(pathToOldParent).get(); // TODO: NULL
+        GroupTreeNode oldParent = root.node().getDescendant(pathToOldParent).get(); // TODO: NULL
         GroupTreeNode node = oldParent.getChildAt(oldChildIndex).get(); // TODO:Null
         // TODO: NULL
-        node.moveTo(root.getNode().getDescendant(pathToNewParent).get(), newChildIndex);
+        node.moveTo(root.node().getDescendant(pathToNewParent).get(), newChildIndex);
     }
 }

src/main/java/org/jabref/gui/util/OptionalObjectProperty.java

@@ -1,10 +1,12 @@
 package org.jabref.gui.util;
 
 import java.util.Optional;
+import java.util.function.Function;
 
 import javafx.beans.binding.BooleanExpression;
 import javafx.beans.binding.ObjectBinding;
 import javafx.beans.property.SimpleObjectProperty;
+import javafx.beans.value.ObservableValue;
 
 import com.tobiasdiez.easybind.PreboundBinding;
 
@@ -42,4 +44,5 @@ protected Boolean computeValue() {
             }
         });
     }
+
 }

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

@@ -60,4 +60,22 @@ public void allEntriesGroupContainingGroupNotIgnored() {
 
         assertNotEquals(Optional.empty(), MetaDataDiff.compare(one, two));
     }
+
+    @Test
+    public void metadataWithColoredGroupsShowsDiff() {
+        MetaData one = new MetaData();
+        GroupTreeNode root = GroupTreeNode.fromGroup(DefaultGroupsFactory.getAllEntriesGroup());
+        var group = new ExplicitGroup("ExplicitA", GroupHierarchyType.INCLUDING, ',');
+        group.setColor("0xffb399ff");
+        root.addSubgroup(group);
+        one.setGroups(root);
+
+        MetaData two = new MetaData();
+        GroupTreeNode root2 = GroupTreeNode.fromGroup(DefaultGroupsFactory.getAllEntriesGroup());
+        var group2 = new ExplicitGroup("ExplicitA", GroupHierarchyType.INCLUDING, ',');
+        root2.addSubgroup(group2);
+        two.setGroups(root2);
+
+        assertNotEquals(Optional.empty(), MetaDataDiff.compare(one, two));
+    }
 }