diff --git a/src/main/java/org/jabref/logic/ai/chatting/chathistory/ChatHistoryService.java b/src/main/java/org/jabref/logic/ai/chatting/chathistory/ChatHistoryService.java index 4b8020fb239..acbde14c3a3 100644 --- a/src/main/java/org/jabref/logic/ai/chatting/chathistory/ChatHistoryService.java +++ b/src/main/java/org/jabref/logic/ai/chatting/chathistory/ChatHistoryService.java @@ -1,10 +1,8 @@ package org.jabref.logic.ai.chatting.chathistory; import java.util.Comparator; -import java.util.HashMap; import java.util.HashSet; import java.util.List; -import java.util.Map; import java.util.Optional; import java.util.TreeMap; @@ -18,7 +16,6 @@ import org.jabref.logic.citationkeypattern.CitationKeyPatternPreferences; import org.jabref.model.database.BibDatabaseContext; import org.jabref.model.entry.BibEntry; -import org.jabref.model.entry.event.FieldAddedOrRemovedEvent; import org.jabref.model.entry.event.FieldChangedEvent; import org.jabref.model.entry.field.InternalField; import org.jabref.model.groups.AbstractGroup; @@ -26,7 +23,6 @@ import com.airhacks.afterburner.injection.Injector; import com.google.common.eventbus.Subscribe; -import com.tobiasdiez.easybind.EasyBind; import dev.langchain4j.data.message.ChatMessage; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -67,14 +63,14 @@ private record ChatHistoryManagementRecord(Optional bibDatab // When you compare {@link BibEntry} instances, they are compared by value, not by reference. // And when you store {@link BibEntry} instances in a {@link HashMap}, an old hash may be stored when the {@link BibEntry} is changed. // See also ADR-38. - private final TreeMap bibEntriesChatHistory = new TreeMap<>(new Comparator() { - @Override - public int compare(BibEntry o1, BibEntry o2) { - return o1.getId().compareTo(o2.getId()); - } - }); + private final TreeMap bibEntriesChatHistory = new TreeMap<>(Comparator.comparing(BibEntry::getId)); - private final Map groupsChatHistory = new HashMap<>(); + // We use {@link TreeMap} for group chat history for the same reason as for {@link BibEntry}ies. + private final TreeMap groupsChatHistory = new TreeMap<>((o1, o2) -> { + // The most important thing is to catch equality/non-equality. + // For "less" or "bigger" comparison, we will fall back to group names. + return o1 == o2 ? 0 : o1.getGroup().getName().compareTo(o2.getGroup().getName()); + }); public ChatHistoryService(CitationKeyPatternPreferences citationKeyPatternPreferences, ChatHistoryStorage implementation) { @@ -95,11 +91,17 @@ private void configureHistoryTransfer() { } private void configureHistoryTransfer(BibDatabaseContext bibDatabaseContext) { - bibDatabaseContext.getMetaData().getGroups().ifPresent(groupTreeNode -> { - groupTreeNode.iterateOverTree().forEach(groupNode -> { + bibDatabaseContext.getMetaData().getGroups().ifPresent(rootGroupTreeNode -> { + rootGroupTreeNode.iterateOverTree().forEach(groupNode -> { groupNode.getGroup().nameProperty().addListener((observable, oldValue, newValue) -> { if (newValue != null && oldValue != null) { - transferGroupHistory(bibDatabaseContext, oldValue, newValue); + transferGroupHistory(bibDatabaseContext, groupNode, oldValue, newValue); + } + }); + + groupNode.getGroupProperty().addListener((obs, oldValue, newValue) -> { + if (oldValue != null && newValue != null) { + transferGroupHistory(bibDatabaseContext, groupNode, oldValue.getName(), newValue.getName()); } }); }); @@ -251,18 +253,19 @@ public void close() { implementation.commit(); } - private void transferGroupHistory(BibDatabaseContext bibDatabaseContext, String oldName, String newName) { + private void transferGroupHistory(BibDatabaseContext bibDatabaseContext, GroupTreeNode groupTreeNode, String oldName, String newName) { if (bibDatabaseContext.getDatabasePath().isEmpty()) { LOGGER.warn("Could not transfer chat history of group {} (old name: {}): database path is empty.", newName, oldName); return; } - List chatMessages = implementation.loadMessagesForGroup(bibDatabaseContext.getDatabasePath().get(), oldName); + List chatMessages = groupsChatHistory.computeIfAbsent(groupTreeNode, + e -> new ChatHistoryManagementRecord(Optional.of(bibDatabaseContext), FXCollections.observableArrayList())).chatHistory; implementation.storeMessagesForGroup(bibDatabaseContext.getDatabasePath().get(), oldName, List.of()); implementation.storeMessagesForGroup(bibDatabaseContext.getDatabasePath().get(), newName, chatMessages); } - private void transferEntryHistory(BibDatabaseContext bibDatabaseContext, String oldCitationKey, String newCitationKey) { + private void transferEntryHistory(BibDatabaseContext bibDatabaseContext, BibEntry entry, String oldCitationKey, String newCitationKey) { // TODO: This method does not check if the citation key is valid. if (bibDatabaseContext.getDatabasePath().isEmpty()) { @@ -270,7 +273,8 @@ private void transferEntryHistory(BibDatabaseContext bibDatabaseContext, String return; } - List chatMessages = implementation.loadMessagesForEntry(bibDatabaseContext.getDatabasePath().get(), oldCitationKey); + List chatMessages = bibEntriesChatHistory.computeIfAbsent(entry, + e -> new ChatHistoryManagementRecord(Optional.of(bibDatabaseContext), FXCollections.observableArrayList())).chatHistory; implementation.storeMessagesForGroup(bibDatabaseContext.getDatabasePath().get(), oldCitationKey, List.of()); implementation.storeMessagesForEntry(bibDatabaseContext.getDatabasePath().get(), newCitationKey, chatMessages); } @@ -288,7 +292,7 @@ void listen(FieldChangedEvent e) { return; } - transferEntryHistory(bibDatabaseContext, e.getOldValue(), e.getNewValue()); + transferEntryHistory(bibDatabaseContext, e.getBibEntry(), e.getOldValue(), e.getNewValue()); } } } diff --git a/src/main/java/org/jabref/model/groups/GroupTreeNode.java b/src/main/java/org/jabref/model/groups/GroupTreeNode.java index c7885b0d06a..6e5b3284a68 100644 --- a/src/main/java/org/jabref/model/groups/GroupTreeNode.java +++ b/src/main/java/org/jabref/model/groups/GroupTreeNode.java @@ -8,6 +8,9 @@ import java.util.Optional; import java.util.stream.Collectors; +import javafx.beans.property.ObjectProperty; +import javafx.beans.property.SimpleObjectProperty; + import org.jabref.model.FieldChange; import org.jabref.model.TreeNode; import org.jabref.model.database.BibDatabase; @@ -22,7 +25,7 @@ public class GroupTreeNode extends TreeNode { private static final String PATH_DELIMITER = " > "; - private AbstractGroup group; + private ObjectProperty groupProperty = new SimpleObjectProperty<>(); /** * Creates this node and associates the specified group with it. @@ -44,7 +47,11 @@ public static GroupTreeNode fromGroup(AbstractGroup group) { * @return the group associated with this node */ public AbstractGroup getGroup() { - return group; + return groupProperty.get(); + } + + public ObjectProperty getGroupProperty() { + return groupProperty; } /** @@ -55,7 +62,7 @@ public AbstractGroup getGroup() { */ @Deprecated public void setGroup(AbstractGroup newGroup) { - this.group = Objects.requireNonNull(newGroup); + this.groupProperty.set(Objects.requireNonNull(newGroup)); } /** @@ -69,7 +76,7 @@ public void setGroup(AbstractGroup newGroup) { public List setGroup(AbstractGroup newGroup, boolean shouldKeepPreviousAssignments, boolean shouldRemovePreviousAssignments, List entriesInDatabase) { AbstractGroup oldGroup = getGroup(); - group = Objects.requireNonNull(newGroup); + groupProperty.set(Objects.requireNonNull(newGroup)); List changes = new ArrayList<>(); boolean shouldRemoveFromOldGroup = shouldRemovePreviousAssignments && (oldGroup instanceof GroupEntryChanger); @@ -94,17 +101,17 @@ public List setGroup(AbstractGroup newGroup, boolean shouldKeepPrev * Creates a {@link SearchMatcher} that matches entries of this group and that takes the hierarchical information into account. I.e., it finds elements contained in this nodes group, or the union of those elements in its own group and its children's groups (recursively), or the intersection of the elements in its own group and its parent's group (depending on the hierarchical settings stored in the involved groups) */ public SearchMatcher getSearchMatcher() { - return getSearchMatcher(group.getHierarchicalContext()); + return getSearchMatcher(getGroup().getHierarchicalContext()); } private SearchMatcher getSearchMatcher(GroupHierarchyType originalContext) { - final GroupHierarchyType context = group.getHierarchicalContext(); + final GroupHierarchyType context = getGroup().getHierarchicalContext(); if (context == GroupHierarchyType.INDEPENDENT) { - return group; + return getGroup(); } MatcherSet searchRule = MatcherSets.build( context == GroupHierarchyType.REFINING ? MatcherSets.MatcherType.AND : MatcherSets.MatcherType.OR); - searchRule.addRule(group); + searchRule.addRule(getGroup()); if ((context == GroupHierarchyType.INCLUDING) && (originalContext != GroupHierarchyType.REFINING)) { for (GroupTreeNode child : getChildren()) { searchRule.addRule(child.getSearchMatcher(originalContext)); @@ -126,13 +133,13 @@ public boolean equals(Object o) { return false; } GroupTreeNode that = (GroupTreeNode) o; - return Objects.equals(group, that.group) && + return Objects.equals(getGroup(), that.getGroup()) && Objects.equals(getChildren(), that.getChildren()); } @Override public int hashCode() { - return Objects.hash(group); + return Objects.hash(getGroup()); } /** @@ -147,11 +154,11 @@ public List getContainingGroups(List entries, boolean r // Add myself if I contain the entries if (requireAll) { - if (this.group.containsAll(entries)) { + if (this.getGroup().containsAll(entries)) { groups.add(this); } } else { - if (this.group.containsAny(entries)) { + if (this.getGroup().containsAny(entries)) { groups.add(this); } } @@ -197,7 +204,7 @@ public List getMatchingGroups(List entries) { public List getEntriesInGroup(List entries) { List result = new ArrayList<>(); for (BibEntry entry : entries) { - if (this.group.contains(entry)) { + if (this.getGroup().contains(entry)) { result.add(entry); } } @@ -210,7 +217,7 @@ public List getEntriesInGroup(List entries) { * @return String the name of the group */ public String getName() { - return group.getName(); + return getGroup().getName(); } public GroupTreeNode addSubgroup(AbstractGroup subgroup) { @@ -221,7 +228,7 @@ public GroupTreeNode addSubgroup(AbstractGroup subgroup) { @Override public GroupTreeNode copyNode() { - return GroupTreeNode.fromGroup(group); + return GroupTreeNode.fromGroup(getGroup()); } /** @@ -269,7 +276,7 @@ public String getPath() { @Override public String toString() { return "GroupTreeNode{" + - "group=" + group + + "group=" + getGroup() + '}'; } @@ -322,11 +329,11 @@ public List removeEntriesFromGroup(List entries) { * Returns true if the underlying groups of both {@link GroupTreeNode}s is the same. */ public boolean isSameGroupAs(GroupTreeNode other) { - return Objects.equals(group, other.group); + return Objects.equals(getGroup(), other.getGroup()); } public boolean containsGroup(AbstractGroup other) { - if (this.group == other) { + if (this.getGroup() == other) { return true; } else { return this.getChildren().stream().anyMatch(child -> child.getGroup() == other);