Fix search groups

JabRef · Aug 25, 2024 · 4ca8afb · 4ca8afb
1 parent 193d533
commit 4ca8afb
Show file tree

Hide file tree

Showing 9 changed files with 106 additions and 59 deletions.
diff --git a/src/main/java/org/jabref/gui/groups/GroupNodeViewModel.java b/src/main/java/org/jabref/gui/groups/GroupNodeViewModel.java
@@ -59,7 +59,7 @@ public class GroupNodeViewModel {
     private final BibDatabaseContext databaseContext;
     private final StateManager stateManager;
     private final GroupTreeNode groupNode;
-    private final ObservableMap<Integer, BibEntry> matchedEntries = FXCollections.observableHashMap();
+    private final ObservableMap<Integer, BibEntry> matchedEntries;
     private final SimpleBooleanProperty hasChildren;
     private final SimpleBooleanProperty expandedProperty = new SimpleBooleanProperty();
     private final BooleanBinding anySelectedEntriesMatched;
@@ -94,6 +94,13 @@ public GroupNodeViewModel(BibDatabaseContext databaseContext, StateManager state
         if (groupNode.getGroup() instanceof TexGroup) {
             databaseContext.getMetaData().groupsBinding().addListener(new WeakInvalidationListener(onInvalidatedGroup));
         }
+
+        if (groupNode.getGroup() instanceof SearchGroup searchGroup) {
+            matchedEntries = searchGroup.getMatchedEntries();
+        } else {
+            matchedEntries = FXCollections.observableHashMap();
+        }
+
         hasChildren = new SimpleBooleanProperty();
         hasChildren.bind(Bindings.isNotEmpty(children));
         EasyBind.subscribe(preferencesService.getGroupsPreferences().displayGroupCountProperty(), shouldDisplay -> updateMatchedEntries());
@@ -231,8 +238,13 @@ public GroupTreeNode getGroupNode() {
 
     /**
      * Gets invoked if an entry in the current database changes.
+     *
+     * @implNote Search groups are updated in {@link org.jabref.gui.maintable.MainTableDataModel.LuceneIndexListener#updateSearchGroupsMatches(org.jabref.model.entry.BibEntry, org.jabref.model.groups.GroupTreeNode)}.
      */
     private void onDatabaseChanged(ListChangeListener.Change<? extends BibEntry> change) {
+        if (groupNode.getGroup() instanceof SearchGroup) {
+            return;
+        }
         while (change.next()) {
             if (change.wasPermutated()) {
                 // Nothing to do, as permutation doesn't change matched entries
@@ -388,7 +400,7 @@ public boolean canAddEntriesIn() {
             return true;
         } else if (group instanceof LastNameGroup || group instanceof RegexKeywordGroup) {
             return groupNode.getParent()
-                            .map(parent -> parent.getGroup())
+                            .map(GroupTreeNode::getGroup)
                             .map(groupParent -> groupParent instanceof AutomaticKeywordGroup || groupParent instanceof AutomaticPersonsGroup)
                             .orElse(false);
         } else if (group instanceof KeywordGroup) {
@@ -416,7 +428,7 @@ public boolean canBeDragged() {
         } else if (group instanceof KeywordGroup) {
             // KeywordGroup is parent of LastNameGroup, RegexKeywordGroup and WordKeywordGroup
             return groupNode.getParent()
-                            .map(parent -> parent.getGroup())
+                            .map(GroupTreeNode::getGroup)
                             .map(groupParent -> !(groupParent instanceof AutomaticKeywordGroup || groupParent instanceof AutomaticPersonsGroup))
                             .orElse(false);
         } else if (group instanceof SearchGroup) {
@@ -441,7 +453,7 @@ public boolean canAddGroupsIn() {
         } else if (group instanceof KeywordGroup) {
             // KeywordGroup is parent of LastNameGroup, RegexKeywordGroup and WordKeywordGroup
             return groupNode.getParent()
-                            .map(parent -> parent.getGroup())
+                            .map(GroupTreeNode::getGroup)
                             .map(groupParent -> !(groupParent instanceof AutomaticKeywordGroup || groupParent instanceof AutomaticPersonsGroup))
                             .orElse(false);
         } else if (group instanceof SearchGroup) {
@@ -466,7 +478,7 @@ public boolean canRemove() {
         } else if (group instanceof KeywordGroup) {
             // KeywordGroup is parent of LastNameGroup, RegexKeywordGroup and WordKeywordGroup
             return groupNode.getParent()
-                            .map(parent -> parent.getGroup())
+                            .map(GroupTreeNode::getGroup)
                             .map(groupParent -> !(groupParent instanceof AutomaticKeywordGroup || groupParent instanceof AutomaticPersonsGroup))
                             .orElse(false);
         } else if (group instanceof SearchGroup) {
@@ -491,7 +503,7 @@ public boolean isEditable() {
         } else if (group instanceof KeywordGroup) {
             // KeywordGroup is parent of LastNameGroup, RegexKeywordGroup and WordKeywordGroup
             return groupNode.getParent()
-                            .map(parent -> parent.getGroup())
+                            .map(GroupTreeNode::getGroup)
                             .map(groupParent -> !(groupParent instanceof AutomaticKeywordGroup || groupParent instanceof AutomaticPersonsGroup))
                             .orElse(false);
         } else if (group instanceof SearchGroup) {

diff --git a/src/main/java/org/jabref/gui/groups/GroupTreeNodeViewModel.java b/src/main/java/org/jabref/gui/groups/GroupTreeNodeViewModel.java
@@ -18,13 +18,7 @@
 import org.jabref.model.groups.KeywordGroup;
 import org.jabref.model.groups.SearchGroup;
 
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
 public class GroupTreeNodeViewModel {
-
-    private static final Logger LOGGER = LoggerFactory.getLogger(GroupTreeNodeViewModel.class);
-
     private final GroupTreeNode node;
 
     public GroupTreeNodeViewModel(GroupTreeNode node) {
@@ -33,10 +27,7 @@ public GroupTreeNodeViewModel(GroupTreeNode node) {
 
     @Override
     public String toString() {
-        final StringBuilder sb = new StringBuilder("GroupTreeNodeViewModel{");
-        sb.append("node=").append(node);
-        sb.append('}');
-        return sb.toString();
+        return "GroupTreeNodeViewModel{" + "node=" + node + '}';
     }
 
     public GroupTreeNode getNode() {
@@ -59,15 +50,17 @@ 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>";
     }
 
@@ -112,12 +105,12 @@ public boolean canBeEdited() {
     }
 
     public boolean canMoveUp() {
-        return (getNode().getPreviousSibling() != null)
+        return (getNode().getPreviousSibling().isPresent())
                 && !(getNode().getGroup() instanceof AllEntriesGroup);
     }
 
     public boolean canMoveDown() {
-        return (getNode().getNextSibling() != null)
+        return (getNode().getNextSibling().isPresent())
                 && !(getNode().getGroup() instanceof AllEntriesGroup);
     }
 
@@ -128,7 +121,7 @@ public boolean canMoveLeft() {
     }
 
     public boolean canMoveRight() {
-        return (getNode().getPreviousSibling() != null)
+        return (getNode().getPreviousSibling().isPresent())
                 && !(getNode().getGroup() instanceof AllEntriesGroup);
     }
 

diff --git a/src/main/java/org/jabref/gui/groups/GroupTreeView.java b/src/main/java/org/jabref/gui/groups/GroupTreeView.java
@@ -74,30 +74,24 @@
 public class GroupTreeView extends BorderPane {
 
     private static final Logger LOGGER = LoggerFactory.getLogger(GroupTreeView.class);
-
     private static final PseudoClass PSEUDOCLASS_ANYSELECTED = PseudoClass.getPseudoClass("any-selected");
     private static final PseudoClass PSEUDOCLASS_ALLSELECTED = PseudoClass.getPseudoClass("all-selected");
     private static final PseudoClass PSEUDOCLASS_ROOTELEMENT = PseudoClass.getPseudoClass("root");
     private static final PseudoClass PSEUDOCLASS_SUBELEMENT = PseudoClass.getPseudoClass("sub"); // > 1 deep
 
-    private static final double SCROLL_SPEED_UP = 3.0;
+    private final StateManager stateManager;
+    private final DialogService dialogService;
+    private final TaskExecutor taskExecutor;
+    private final PreferencesService preferencesService;
 
     private TreeTableView<GroupNodeViewModel> groupTree;
     private TreeTableColumn<GroupNodeViewModel, GroupNodeViewModel> mainColumn;
     private TreeTableColumn<GroupNodeViewModel, GroupNodeViewModel> numberColumn;
     private TreeTableColumn<GroupNodeViewModel, GroupNodeViewModel> expansionNodeColumn;
     private CustomTextField searchField;
-
-    private final StateManager stateManager;
-    private final DialogService dialogService;
-    private final TaskExecutor taskExecutor;
-    private final PreferencesService preferencesService;
-
     private GroupTreeViewModel viewModel;
     private CustomLocalDragboard localDragboard;
-
     private DragExpansionHandler dragExpansionHandler;
-
     private Timer scrollTimer;
     private double scrollVelocity = 0;
     private double scrollableAreaHeight;

diff --git a/src/main/java/org/jabref/gui/maintable/MainTableDataModel.java b/src/main/java/org/jabref/gui/maintable/MainTableDataModel.java
@@ -31,6 +31,7 @@
 import org.jabref.model.search.SearchQuery;
 import org.jabref.model.search.SearchResults;
 import org.jabref.model.search.event.IndexAddedOrUpdatedEvent;
+import org.jabref.model.search.event.IndexRemovedEvent;
 import org.jabref.model.search.event.IndexStartedEvent;
 import org.jabref.model.search.matchers.MatcherSet;
 import org.jabref.model.search.matchers.MatcherSets;
@@ -220,6 +221,9 @@ public void listen(IndexAddedOrUpdatedEvent indexAddedOrUpdatedEvent) {
                             viewModel.searchScoreProperty().set(0);
                             viewModel.hasFullTextResultsProperty().set(false);
                         }
+
+                        bibDatabaseContext.getMetaData().getGroups().ifPresent(root -> updateSearchGroupsMatches(entry, root));
+
                         updateEntrySearchMatch(viewModel, isMatched, isFloatingMode);
                         updateEntryGroupMatch(viewModel, groupsMatcher, groupsPreferences.getGroupViewMode().contains(GroupViewMode.INVERT), !groupsPreferences.getGroupViewMode().contains(GroupViewMode.FILTER));
                     }
@@ -230,17 +234,47 @@ public void listen(IndexAddedOrUpdatedEvent indexAddedOrUpdatedEvent) {
 
         @Subscribe
         public void listen(IndexStartedEvent indexStartedEvent) {
-            bibDatabaseContext.getMetaData().getGroups().ifPresent(this::setLuceneManagerForSearchGroups);
+            bibDatabaseContext.getMetaData().getGroups().ifPresent(this::setSearchGroupsMatches);
             updateSearchMatches(searchQueryProperty.get());
             updateGroupMatches(selectedGroupsProperty);
         }
 
-        private void setLuceneManagerForSearchGroups(GroupTreeNode root) {
+        private void setSearchGroupsMatches(GroupTreeNode root) {
+            for (GroupTreeNode node : root.getChildren()) {
+                if (node.getGroup() instanceof SearchGroup searchGroup) {
+                    SearchQuery query = searchGroup.getQuery();
+                    SearchResults searchResults = luceneManager.search(query);
+                    searchGroup.setMatchedEntries(searchResults.getMatchedEntries());
+                }
+                setSearchGroupsMatches(node);
+            }
+        }
+
+        private void updateSearchGroupsMatches(BibEntry entry, GroupTreeNode root) {
+            for (GroupTreeNode node : root.getChildren()) {
+                if (node.getGroup() instanceof SearchGroup searchGroup) {
+                    searchGroup.updateEntry(entry, luceneManager.isMatched(entry, searchGroup.getQuery()));
+                    searchGroup.updateEntry(entry, node.getSearchMatcher().isMatch(entry));
+                }
+                updateSearchGroupsMatches(entry, node);
+            }
+        }
+
+        @Subscribe
+        public void listen(IndexRemovedEvent indexRemovedEvent) {
+            BackgroundTask.wrap(() -> {
+                for (BibEntry entry : indexRemovedEvent.removedEntries()) {
+                    bibDatabaseContext.getMetaData().getGroups().ifPresent(root -> removeSearchGroupsMatches(entry, root));
+                }
+            }).executeWith(taskExecutor);
+        }
+
+        private void removeSearchGroupsMatches(BibEntry entry, GroupTreeNode root) {
             for (GroupTreeNode node : root.getChildren()) {
                 if (node.getGroup() instanceof SearchGroup searchGroup) {
-                    searchGroup.setLuceneManager(luceneManager);
+                    searchGroup.updateEntry(entry, false);
                 }
-                setLuceneManagerForSearchGroups(node);
+                removeSearchGroupsMatches(entry, node);
             }
         }
     }

diff --git a/src/main/java/org/jabref/logic/search/LuceneManager.java b/src/main/java/org/jabref/logic/search/LuceneManager.java
@@ -123,7 +123,7 @@ protected Object call() {
                 bibFieldsIndexer.removeFromIndex(entries, this);
                 return null;
             }
-        }.onFinished(() -> this.databaseContext.getDatabase().postEvent(new IndexRemovedEvent()))
+        }.onFinished(() -> this.databaseContext.getDatabase().postEvent(new IndexRemovedEvent(entries)))
         .showToUser(true).executeWith(taskExecutor);
 
         if (shouldIndexLinkedFiles.get()) {

diff --git a/src/main/java/org/jabref/model/groups/SearchGroup.java b/src/main/java/org/jabref/model/groups/SearchGroup.java
@@ -1,9 +1,12 @@
 package org.jabref.model.groups;
 
+import java.util.Collection;
 import java.util.EnumSet;
 import java.util.Objects;
 
-import org.jabref.logic.search.LuceneManager;
+import javafx.collections.FXCollections;
+import javafx.collections.ObservableMap;
+
 import org.jabref.model.entry.BibEntry;
 import org.jabref.model.search.SearchFlags;
 import org.jabref.model.search.SearchQuery;
@@ -18,8 +21,8 @@
 public class SearchGroup extends AbstractGroup {
 
     private static final Logger LOGGER = LoggerFactory.getLogger(SearchGroup.class);
+    private final ObservableMap<Integer, BibEntry> matchedEntries = FXCollections.observableHashMap();
     private final SearchQuery query;
-    private LuceneManager luceneManager;
 
     public SearchGroup(String name, GroupHierarchyType context, String searchExpression, EnumSet<SearchFlags> searchFlags) {
         super(name, context);
@@ -38,10 +41,6 @@ public EnumSet<SearchFlags> getSearchFlags() {
         return query.getSearchFlags();
     }
 
-    public void setLuceneManager(LuceneManager luceneManager) {
-        this.luceneManager = luceneManager;
-    }
-
     @Override
     public boolean equals(Object o) {
         if (this == o) {
@@ -58,10 +57,24 @@ public boolean equals(Object o) {
 
     @Override
     public boolean contains(BibEntry entry) {
-        if (luceneManager == null) {
-            return false;
+        return matchedEntries.containsKey(System.identityHashCode(entry));
+    }
+
+    public void setMatchedEntries(Collection<BibEntry> entries) {
+        matchedEntries.clear();
+        entries.forEach(entry -> matchedEntries.put(System.identityHashCode(entry), entry));
+    }
+
+    public void updateEntry(BibEntry entry, boolean matched) {
+        if (matched) {
+            matchedEntries.put(System.identityHashCode(entry), entry);
+        } else {
+            matchedEntries.remove(System.identityHashCode(entry));
         }
-        return luceneManager.isMatched(entry, query);
+    }
+
+    public ObservableMap<Integer, BibEntry> getMatchedEntries() {
+        return matchedEntries;
     }
 
     @Override

diff --git a/src/main/java/org/jabref/model/search/SearchResults.java b/src/main/java/org/jabref/model/search/SearchResults.java
@@ -5,6 +5,7 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 import org.jabref.model.entry.BibEntry;
 
@@ -46,8 +47,8 @@ public Map<String, List<SearchResult>> getFileSearchResultsForEntry(BibEntry ent
         return results;
     }
 
-    public Map<BibEntry, List<SearchResult>> getAllSearchResults() {
-        return searchResults;
+    public Set<BibEntry> getMatchedEntries() {
+        return searchResults.keySet();
     }
 
     public int getNumberOfResults() {

diff --git a/src/main/java/org/jabref/model/search/event/IndexRemovedEvent.java b/src/main/java/org/jabref/model/search/event/IndexRemovedEvent.java
@@ -1,4 +1,8 @@
 package org.jabref.model.search.event;
 
-public record IndexRemovedEvent() {
+import java.util.List;
+
+import org.jabref.model.entry.BibEntry;
+
+public record IndexRemovedEvent(List<BibEntry> removedEntries) {
 }
diff --git a/src/main/java/org/jabref/model/search/event/IndexUpdatedEvent.java b/src/main/java/org/jabref/model/search/event/IndexUpdatedEvent.java