From ce2d3537d6b506642f226c94ca12c52f73420f59 Mon Sep 17 00:00:00 2001 From: Siedlerchr Date: Mon, 20 May 2024 19:03:14 +0200 Subject: [PATCH] Draft for fixing sync with groups and shared db convert class to record --- src/main/java/org/jabref/gui/LibraryTab.java | 18 +++++ .../gui/groups/GroupTreeNodeViewModel.java | 72 ++++++++----------- .../jabref/gui/groups/GroupTreeViewModel.java | 22 ++++-- .../gui/groups/UndoableAddOrRemoveGroup.java | 8 +-- .../gui/groups/UndoableModifySubtree.java | 6 +- .../jabref/gui/groups/UndoableMoveGroup.java | 8 +-- .../gui/util/OptionalObjectProperty.java | 3 + .../bibtex/comparator/MetaDataDiffTest.java | 18 +++++ 8 files changed, 96 insertions(+), 59 deletions(-) diff --git a/src/main/java/org/jabref/gui/LibraryTab.java b/src/main/java/org/jabref/gui/LibraryTab.java index ff2c9e4ba20..3db3e635b9e 100644 --- a/src/main/java/org/jabref/gui/LibraryTab.java +++ b/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 groupsTreeChanged = new SimpleObjectProperty<>(); private final BooleanProperty nonUndoableChangeProperty = new SimpleBooleanProperty(false); private BibDatabaseContext bibDatabaseContext; @@ -427,6 +433,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()); + }); + } } /** @@ -890,6 +904,10 @@ public ObservableBooleanValue getLoading() { return loading; } + public ObjectProperty groupsTreeChangedProperty() { + return groupsTreeChanged; + } + public CountingUndoManager getUndoManager() { return undoManager; } diff --git a/src/main/java/org/jabref/gui/groups/GroupTreeNodeViewModel.java b/src/main/java/org/jabref/gui/groups/GroupTreeNodeViewModel.java index 4383ec85ebf..d61e955a62d 100644 --- a/src/main/java/org/jabref/gui/groups/GroupTreeNodeViewModel.java +++ b/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 getChildren() { List children = new ArrayList<>(); for (GroupTreeNode child : node.getChildren()) { @@ -51,7 +42,7 @@ public List getChildren() { return children; } - protected boolean printInItalics() { + private boolean printInItalics() { return node.getGroup().isDynamic(); } @@ -59,28 +50,30 @@ 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 "" + shortDescription + ""; } public boolean canAddEntries(List entries) { - return (getNode().getGroup() instanceof GroupEntryChanger) && !getNode().getGroup().containsAll(entries); + return (node().getGroup() instanceof GroupEntryChanger) && !node().getGroup().containsAll(entries); } public boolean canRemoveEntries(List 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 entries, UndoManager undoManager) { @@ -179,12 +167,12 @@ public List removeEntriesFromGroup(List 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 addEntriesToGroup(List entries) { } public void subscribeToDescendantChanged(Consumer subscriber) { - getNode().subscribeToDescendantChanged(node -> subscriber.accept(new GroupTreeNodeViewModel(node))); + node().subscribeToDescendantChanged(node -> subscriber.accept(new GroupTreeNodeViewModel(node))); } } diff --git a/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java b/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java index de0c579699b..c16d2e3f806 100644 --- a/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java +++ b/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 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(); }); }); } diff --git a/src/main/java/org/jabref/gui/groups/UndoableAddOrRemoveGroup.java b/src/main/java/org/jabref/gui/groups/UndoableAddOrRemoveGroup.java index 08ff81550f4..e5a6b45e5d6 100644 --- a/src/main/java/org/jabref/gui/groups/UndoableAddOrRemoveGroup.java +++ b/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) { diff --git a/src/main/java/org/jabref/gui/groups/UndoableModifySubtree.java b/src/main/java/org/jabref/gui/groups/UndoableModifySubtree.java index 8f92c717259..d38d200567c 100644 --- a/src/main/java/org/jabref/gui/groups/UndoableModifySubtree.java +++ b/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; } diff --git a/src/main/java/org/jabref/gui/groups/UndoableMoveGroup.java b/src/main/java/org/jabref/gui/groups/UndoableMoveGroup.java index c4f367d31f7..209a0660e7f 100644 --- a/src/main/java/org/jabref/gui/groups/UndoableMoveGroup.java +++ b/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); } } diff --git a/src/main/java/org/jabref/gui/util/OptionalObjectProperty.java b/src/main/java/org/jabref/gui/util/OptionalObjectProperty.java index a52d5654732..fb6daa5f8a1 100644 --- a/src/main/java/org/jabref/gui/util/OptionalObjectProperty.java +++ b/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() { } }); } + } diff --git a/src/test/java/org/jabref/logic/bibtex/comparator/MetaDataDiffTest.java b/src/test/java/org/jabref/logic/bibtex/comparator/MetaDataDiffTest.java index cd664323c96..32df0e54d89 100644 --- a/src/test/java/org/jabref/logic/bibtex/comparator/MetaDataDiffTest.java +++ b/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)); + } }