Skip to content

Commit

Permalink
Fix for #170
Browse files Browse the repository at this point in the history
  • Loading branch information
@InAnYan
InAnYan committed Sep 4, 2024
1 parent ccab8b2 commit 69d78f3
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 37 deletions.
42 changes: 23 additions & 19 deletions src/main/java/org/jabref/logic/ai/chatting/chathistory/ChatHistoryService.java
View file
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -18,15 +16,13 @@
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;
import org.jabref.model.groups.GroupTreeNode;

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;
Expand Down Expand Up @@ -67,14 +63,14 @@ private record ChatHistoryManagementRecord(Optional<BibDatabaseContext> 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<BibEntry, ChatHistoryManagementRecord> bibEntriesChatHistory = new TreeMap<>(new Comparator<BibEntry>() {
@Override
public int compare(BibEntry o1, BibEntry o2) {
return o1.getId().compareTo(o2.getId());
}
});
private final TreeMap<BibEntry, ChatHistoryManagementRecord> bibEntriesChatHistory = new TreeMap<>(Comparator.comparing(BibEntry::getId));

private final Map<GroupTreeNode, ChatHistoryManagementRecord> groupsChatHistory = new HashMap<>();
// We use {@link TreeMap} for group chat history for the same reason as for {@link BibEntry}ies.
private final TreeMap<GroupTreeNode, ChatHistoryManagementRecord> 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) {
Expand All @@ -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());
}
});
});
Expand Down Expand Up @@ -251,26 +253,28 @@ 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<ChatMessage> chatMessages = implementation.loadMessagesForGroup(bibDatabaseContext.getDatabasePath().get(), oldName);
List<ChatMessage> 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()) {
LOGGER.warn("Could not transfer chat history of entry {} (old key: {}): database path is empty.", newCitationKey, oldCitationKey);
return;
}

List<ChatMessage> chatMessages = implementation.loadMessagesForEntry(bibDatabaseContext.getDatabasePath().get(), oldCitationKey);
List<ChatMessage> 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);
}
Expand All @@ -288,7 +292,7 @@ void listen(FieldChangedEvent e) {
return;
}

transferEntryHistory(bibDatabaseContext, e.getOldValue(), e.getNewValue());
transferEntryHistory(bibDatabaseContext, e.getBibEntry(), e.getOldValue(), e.getNewValue());
}
}
}
43 changes: 25 additions & 18 deletions src/main/java/org/jabref/model/groups/GroupTreeNode.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -22,7 +25,7 @@
public class GroupTreeNode extends TreeNode<GroupTreeNode> {

private static final String PATH_DELIMITER = " > ";
private AbstractGroup group;
private ObjectProperty<AbstractGroup> groupProperty = new SimpleObjectProperty<>();

/**
* Creates this node and associates the specified group with it.
Expand All @@ -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<AbstractGroup> getGroupProperty() {
return groupProperty;
}

/**
Expand All @@ -55,7 +62,7 @@ public AbstractGroup getGroup() {
*/
@Deprecated
public void setGroup(AbstractGroup newGroup) {
this.group = Objects.requireNonNull(newGroup);
this.groupProperty.set(Objects.requireNonNull(newGroup));
}

/**
Expand All @@ -69,7 +76,7 @@ public void setGroup(AbstractGroup newGroup) {
public List<FieldChange> setGroup(AbstractGroup newGroup, boolean shouldKeepPreviousAssignments,
boolean shouldRemovePreviousAssignments, List<BibEntry> entriesInDatabase) {
AbstractGroup oldGroup = getGroup();
group = Objects.requireNonNull(newGroup);
groupProperty.set(Objects.requireNonNull(newGroup));

List<FieldChange> changes = new ArrayList<>();
boolean shouldRemoveFromOldGroup = shouldRemovePreviousAssignments && (oldGroup instanceof GroupEntryChanger);
Expand All @@ -94,17 +101,17 @@ public List<FieldChange> 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));
Expand All @@ -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());
}

/**
Expand All @@ -147,11 +154,11 @@ public List<GroupTreeNode> getContainingGroups(List<BibEntry> 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);
}
}
Expand Down Expand Up @@ -197,7 +204,7 @@ public List<GroupTreeNode> getMatchingGroups(List<BibEntry> entries) {
public List<BibEntry> getEntriesInGroup(List<BibEntry> entries) {
List<BibEntry> result = new ArrayList<>();
for (BibEntry entry : entries) {
if (this.group.contains(entry)) {
if (this.getGroup().contains(entry)) {
result.add(entry);
}
}
Expand All @@ -210,7 +217,7 @@ public List<BibEntry> getEntriesInGroup(List<BibEntry> entries) {
* @return String the name of the group
*/
public String getName() {
return group.getName();
return getGroup().getName();
}

public GroupTreeNode addSubgroup(AbstractGroup subgroup) {
Expand All @@ -221,7 +228,7 @@ public GroupTreeNode addSubgroup(AbstractGroup subgroup) {

@Override
public GroupTreeNode copyNode() {
return GroupTreeNode.fromGroup(group);
return GroupTreeNode.fromGroup(getGroup());
}

/**
Expand Down Expand Up @@ -269,7 +276,7 @@ public String getPath() {
@Override
public String toString() {
return "GroupTreeNode{" +
"group=" + group +
"group=" + getGroup() +
'}';
}

Expand Down Expand Up @@ -322,11 +329,11 @@ public List<FieldChange> removeEntriesFromGroup(List<BibEntry> 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);
Expand Down

0 comments on commit 69d78f3

Please sign in to comment.