diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b63f3508ca..6df019fe891 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,12 +27,13 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve ### Fixed +- We greatly improved the performance of the overall application and many operations. [#5071](https://github.com/JabRef/jabref/issues/5071) - We fixed an issue where sort by priority was broken. [#6222](https://github.com/JabRef/jabref/issues/6222) - We fixed an issue where opening a library from the recent libraries menu was not possible. [#5939](https://github.com/JabRef/jabref/issues/5939) -- We fixed an issue with inconsistent capitalization of file extensions when downloading files [#6115](https://github.com/JabRef/jabref/issues/6115) +- We fixed an issue with inconsistent capitalization of file extensions when downloading files. [#6115](https://github.com/JabRef/jabref/issues/6115) - We fixed the display of language and encoding in the preferences dialog. [#6130](https://github.com/JabRef/jabref/pull/6130) - We fixed an issue where search full-text documents downloaded files with same name, overwriting existing files. [#6174](https://github.com/JabRef/jabref/pull/6174) -- We fixe an issue where custom jstyles for Open/LibreOffice where not saved correctly [#6170](https://github.com/JabRef/jabref/issues/6170) +- We fixe an issue where custom jstyles for Open/LibreOffice where not saved correctly. [#6170](https://github.com/JabRef/jabref/issues/6170) ### Removed diff --git a/src/main/java/org/jabref/gui/groups/GroupNodeViewModel.java b/src/main/java/org/jabref/gui/groups/GroupNodeViewModel.java index a20278ae64f..4f40d4bb670 100644 --- a/src/main/java/org/jabref/gui/groups/GroupNodeViewModel.java +++ b/src/main/java/org/jabref/gui/groups/GroupNodeViewModel.java @@ -8,8 +8,8 @@ import javafx.beans.binding.Bindings; import javafx.beans.binding.BooleanBinding; +import javafx.beans.binding.IntegerBinding; import javafx.beans.property.SimpleBooleanProperty; -import javafx.beans.property.SimpleIntegerProperty; import javafx.collections.FXCollections; import javafx.collections.ListChangeListener; import javafx.collections.ObservableList; @@ -28,7 +28,6 @@ import org.jabref.gui.util.TaskExecutor; import org.jabref.logic.groups.DefaultGroupsFactory; import org.jabref.logic.layout.format.LatexToUnicodeFormatter; -import org.jabref.logic.util.DelayTaskThrottler; import org.jabref.model.FieldChange; import org.jabref.model.database.BibDatabaseContext; import org.jabref.model.entry.BibEntry; @@ -50,7 +49,7 @@ public class GroupNodeViewModel { private final BibDatabaseContext databaseContext; private final StateManager stateManager; private final GroupTreeNode groupNode; - private final SimpleIntegerProperty hits; + private final ObservableList matchedEntries = FXCollections.observableArrayList(); private final SimpleBooleanProperty hasChildren; private final SimpleBooleanProperty expandedProperty = new SimpleBooleanProperty(); private final BooleanBinding anySelectedEntriesMatched; @@ -58,7 +57,6 @@ public class GroupNodeViewModel { private final TaskExecutor taskExecutor; private final CustomLocalDragboard localDragBoard; private final ObservableList entriesList; - private final DelayTaskThrottler throttler; public GroupNodeViewModel(BibDatabaseContext databaseContext, StateManager stateManager, TaskExecutor taskExecutor, GroupTreeNode groupNode, CustomLocalDragboard localDragBoard) { this.databaseContext = Objects.requireNonNull(databaseContext); @@ -82,8 +80,7 @@ public GroupNodeViewModel(BibDatabaseContext databaseContext, StateManager state } hasChildren = new SimpleBooleanProperty(); hasChildren.bind(Bindings.isNotEmpty(children)); - hits = new SimpleIntegerProperty(0); - calculateNumberOfMatches(); + updateMatchedEntries(); expandedProperty.set(groupNode.getGroup().isExpanded()); expandedProperty.addListener((observable, oldValue, newValue) -> groupNode.getGroup().setExpanded(newValue)); @@ -91,7 +88,6 @@ public GroupNodeViewModel(BibDatabaseContext databaseContext, StateManager state // The wrapper created by the FXCollections will set a weak listener on the wrapped list. This weak listener gets garbage collected. Hence, we need to maintain a reference to this list. entriesList = databaseContext.getDatabase().getEntries(); entriesList.addListener(this::onDatabaseChanged); - throttler = taskExecutor.createThrottler(1000); ObservableList selectedEntriesMatchStatus = EasyBind.map(stateManager.getSelectedEntries(), groupNode::matches); anySelectedEntriesMatched = BindingsHelper.any(selectedEntriesMatchStatus, matched -> matched); @@ -157,8 +153,8 @@ public String getDescription() { return groupNode.getGroup().getDescription().orElse(""); } - public SimpleIntegerProperty getHits() { - return hits; + public IntegerBinding getHits() { + return Bindings.size(matchedEntries); } @Override @@ -184,7 +180,7 @@ public String toString() { ", children=" + children + ", databaseContext=" + databaseContext + ", groupNode=" + groupNode + - ", hits=" + hits + + ", matchedEntries=" + matchedEntries + '}'; } @@ -222,16 +218,44 @@ public GroupTreeNode getGroupNode() { * Gets invoked if an entry in the current database changes. */ private void onDatabaseChanged(ListChangeListener.Change change) { - throttler.schedule(this::calculateNumberOfMatches); + while (change.next()) { + if (change.wasPermutated()) { + // Nothing to do, as permutation doesn't change matched entries + } else if (change.wasUpdated()) { + for (BibEntry changedEntry : change.getList().subList(change.getFrom(), change.getTo())) { + if (groupNode.matches(changedEntry)) { + if (!matchedEntries.contains(changedEntry)) { + matchedEntries.add(changedEntry); + } + } else { + matchedEntries.remove(changedEntry); + } + } + } else { + for (BibEntry removedEntry : change.getRemoved()) { + matchedEntries.remove(removedEntry); + } + for (BibEntry addedEntry : change.getAddedSubList()) { + if (groupNode.matches(addedEntry)) { + if (!matchedEntries.contains(addedEntry)) { + matchedEntries.add(addedEntry); + } + } + } + } + } } - private void calculateNumberOfMatches() { + private void updateMatchedEntries() { // We calculate the new hit value // We could be more intelligent and try to figure out the new number of hits based on the entry change // for example, a previously matched entry gets removed -> hits = hits - 1 BackgroundTask - .wrap(() -> groupNode.calculateNumberOfMatches(databaseContext.getDatabase())) - .onSuccess(hits::setValue) + .wrap(() -> groupNode.findMatches(databaseContext.getDatabase())) + .onSuccess(entries -> { + matchedEntries.clear(); + matchedEntries.addAll(entries); + }) .executeWith(taskExecutor); } diff --git a/src/main/java/org/jabref/model/entry/BibEntry.java b/src/main/java/org/jabref/model/entry/BibEntry.java index d4c69cdfcdd..db66d82b3c6 100644 --- a/src/main/java/org/jabref/model/entry/BibEntry.java +++ b/src/main/java/org/jabref/model/entry/BibEntry.java @@ -528,8 +528,8 @@ public Optional setField(Field field, String value, EntriesEventSou changed = true; - fields.put(field, value.intern()); invalidateFieldCache(field); + fields.put(field, value.intern()); FieldChange change = new FieldChange(this, field, oldValue, value); if (isNewField) { @@ -574,8 +574,8 @@ public Optional clearField(Field field, EntriesEventSource eventSou changed = true; - fields.remove(field); invalidateFieldCache(field); + fields.remove(field); FieldChange change = new FieldChange(this, field, oldValue.get(), null); eventBus.post(new FieldAddedOrRemovedEvent(change, eventSource)); diff --git a/src/main/java/org/jabref/model/groups/GroupTreeNode.java b/src/main/java/org/jabref/model/groups/GroupTreeNode.java index f0f0b9d4745..85795e36194 100644 --- a/src/main/java/org/jabref/model/groups/GroupTreeNode.java +++ b/src/main/java/org/jabref/model/groups/GroupTreeNode.java @@ -218,24 +218,26 @@ public GroupTreeNode copyNode() { } /** - * Determines the number of entries in the specified list which are matched by this group. + * Determines the entries in the specified list which are matched by this group. + * * @param entries list of entries to be searched - * @return number of hits + * @return matched entries */ - public long calculateNumberOfMatches(List entries) { + public List findMatches(List entries) { SearchMatcher matcher = getSearchMatcher(); return entries.stream() .filter(matcher::isMatch) - .count(); + .collect(Collectors.toList()); } /** - * Determines the number of entries in the specified database which are matched by this group. + * Determines the entries in the specified database which are matched by this group. + * * @param database database to be searched - * @return number of hits + * @return matched entries */ - public long calculateNumberOfMatches(BibDatabase database) { - return calculateNumberOfMatches(database.getEntries()); + public List findMatches(BibDatabase database) { + return findMatches(database.getEntries()); } /** diff --git a/src/test/java/org/jabref/model/groups/GroupTreeNodeTest.java b/src/test/java/org/jabref/model/groups/GroupTreeNodeTest.java index 36fb12c924c..05886e41c76 100644 --- a/src/test/java/org/jabref/model/groups/GroupTreeNodeTest.java +++ b/src/test/java/org/jabref/model/groups/GroupTreeNodeTest.java @@ -95,7 +95,7 @@ private static AbstractGroup getExplict(String name) { * A */ /* - public GroupTreeNode getNodeAsChild(TreeNodeMock root) { + GroupTreeNode getNodeAsChild(TreeNodeMock root) { root.addChild(new TreeNodeMock()); root.addChild(new TreeNodeMock()); TreeNodeMock node = new TreeNodeMock(); @@ -109,7 +109,7 @@ public static GroupTreeNode getRoot() { } @BeforeEach - public void setUp() throws Exception { + void setUp() throws Exception { entries.clear(); entry = new BibEntry(); entries.add(entry); @@ -118,7 +118,7 @@ public void setUp() throws Exception { } /* - public GroupTreeNode getNodeInComplexTree() { + GroupTreeNode getNodeInComplexTree() { return getNodeInComplexTree(new TreeNodeMock()); } */ @@ -128,14 +128,14 @@ private GroupTreeNode getNodeInSimpleTree() { } @Test - public void getSearchRuleForIndependentGroupReturnsGroupAsMatcher() { + void getSearchRuleForIndependentGroupReturnsGroupAsMatcher() { GroupTreeNode node = GroupTreeNode .fromGroup(new ExplicitGroup("node", GroupHierarchyType.INDEPENDENT, ',')); assertEquals(node.getGroup(), node.getSearchMatcher()); } @Test - public void getSearchRuleForRefiningGroupReturnsParentAndGroupAsMatcher() { + void getSearchRuleForRefiningGroupReturnsParentAndGroupAsMatcher() { GroupTreeNode parent = GroupTreeNode .fromGroup( new ExplicitGroup("parent", GroupHierarchyType.INDEPENDENT, ',')); @@ -149,7 +149,7 @@ public void getSearchRuleForRefiningGroupReturnsParentAndGroupAsMatcher() { } @Test - public void getSearchRuleForIncludingGroupReturnsGroupOrSubgroupAsMatcher() { + void getSearchRuleForIncludingGroupReturnsGroupOrSubgroupAsMatcher() { GroupTreeNode node = GroupTreeNode.fromGroup(new ExplicitGroup("node", GroupHierarchyType.INCLUDING, ',')); GroupTreeNode child = node.addSubgroup(new ExplicitGroup("child", GroupHierarchyType.INDEPENDENT, ',')); @@ -160,48 +160,48 @@ public void getSearchRuleForIncludingGroupReturnsGroupOrSubgroupAsMatcher() { } @Test - public void numberOfHitsReturnsZeroForEmptyList() throws Exception { - assertEquals(0, getNodeInSimpleTree().calculateNumberOfMatches(Collections.emptyList())); + void findMatchesReturnsEmptyForEmptyList() throws Exception { + assertEquals(Collections.emptyList(), getNodeInSimpleTree().findMatches(Collections.emptyList())); } @Test - public void numberOfHitsMatchesOneEntry() throws Exception { + void findMatchesOneEntry() throws Exception { GroupTreeNode parent = getNodeInSimpleTree(); GroupTreeNode node = parent.addSubgroup( new WordKeywordGroup("node", GroupHierarchyType.INDEPENDENT, StandardField.AUTHOR, "author2", true, ',', false)); - assertEquals(1, node.calculateNumberOfMatches(entries)); + assertEquals(1, node.findMatches(entries).size()); } @Test - public void numberOfHitsMatchesMultipleEntries() throws Exception { + void findMatchesMultipleEntries() throws Exception { GroupTreeNode parent = getNodeInSimpleTree(); GroupTreeNode node = parent.addSubgroup( new WordKeywordGroup("node", GroupHierarchyType.INDEPENDENT, StandardField.AUTHOR, "author1", true, ',', false)); - assertEquals(2, node.calculateNumberOfMatches(entries)); + assertEquals(2, node.findMatches(entries).size()); } @Test - public void numberOfHitsWorksForRefiningGroups() throws Exception { + void findMatchesWorksForRefiningGroups() throws Exception { GroupTreeNode grandParent = getNodeInSimpleTree(); GroupTreeNode parent = grandParent.addSubgroup( new WordKeywordGroup("node", GroupHierarchyType.INDEPENDENT, StandardField.AUTHOR, "author2", true, ',', false)); GroupTreeNode node = parent.addSubgroup( new WordKeywordGroup("node", GroupHierarchyType.REFINING, StandardField.AUTHOR, "author1", true, ',', false)); - assertEquals(1, node.calculateNumberOfMatches(entries)); + assertEquals(1, node.findMatches(entries).size()); } @Test - public void numberOfHitsWorksForHierarchyOfIndependentGroups() throws Exception { + void findMatchesWorksForHierarchyOfIndependentGroups() throws Exception { GroupTreeNode grandParent = getNodeInSimpleTree(); GroupTreeNode parent = grandParent.addSubgroup( new WordKeywordGroup("node", GroupHierarchyType.INDEPENDENT, StandardField.AUTHOR, "author2", true, ',', false)); GroupTreeNode node = parent.addSubgroup( new WordKeywordGroup("node", GroupHierarchyType.INDEPENDENT, StandardField.AUTHOR, "author1", true, ',', false)); - assertEquals(2, node.calculateNumberOfMatches(entries)); + assertEquals(2, node.findMatches(entries).size()); } @Test - public void setGroupChangesUnderlyingGroup() throws Exception { + void setGroupChangesUnderlyingGroup() throws Exception { GroupTreeNode node = getNodeInSimpleTree(); AbstractGroup newGroup = new ExplicitGroup("NewGroup", GroupHierarchyType.INDEPENDENT, ','); @@ -211,7 +211,7 @@ public void setGroupChangesUnderlyingGroup() throws Exception { } @Test - public void setGroupAddsPreviousAssignmentsExplicitToExplicit() throws Exception { + void setGroupAddsPreviousAssignmentsExplicitToExplicit() throws Exception { ExplicitGroup oldGroup = new ExplicitGroup("OldGroup", GroupHierarchyType.INDEPENDENT, ','); oldGroup.add(entry); GroupTreeNode node = GroupTreeNode.fromGroup(oldGroup); @@ -223,7 +223,7 @@ public void setGroupAddsPreviousAssignmentsExplicitToExplicit() throws Exception } @Test - public void setGroupWithFalseDoesNotAddsPreviousAssignments() throws Exception { + void setGroupWithFalseDoesNotAddsPreviousAssignments() throws Exception { ExplicitGroup oldGroup = new ExplicitGroup("OldGroup", GroupHierarchyType.INDEPENDENT, ','); oldGroup.add(entry); GroupTreeNode node = GroupTreeNode.fromGroup(oldGroup); @@ -235,7 +235,7 @@ public void setGroupWithFalseDoesNotAddsPreviousAssignments() throws Exception { } @Test - public void setGroupAddsOnlyPreviousAssignments() throws Exception { + void setGroupAddsOnlyPreviousAssignments() throws Exception { ExplicitGroup oldGroup = new ExplicitGroup("OldGroup", GroupHierarchyType.INDEPENDENT, ','); assertFalse(oldGroup.isMatch(entry)); GroupTreeNode node = GroupTreeNode.fromGroup(oldGroup); @@ -247,7 +247,7 @@ public void setGroupAddsOnlyPreviousAssignments() throws Exception { } @Test - public void setGroupExplicitToSearchDoesNotKeepPreviousAssignments() throws Exception { + void setGroupExplicitToSearchDoesNotKeepPreviousAssignments() throws Exception { ExplicitGroup oldGroup = new ExplicitGroup("OldGroup", GroupHierarchyType.INDEPENDENT, ','); oldGroup.add(entry); GroupTreeNode node = GroupTreeNode.fromGroup(oldGroup); @@ -259,7 +259,7 @@ public void setGroupExplicitToSearchDoesNotKeepPreviousAssignments() throws Exce } @Test - public void setGroupExplicitToExplicitIsRenameAndSoRemovesPreviousAssignment() throws Exception { + void setGroupExplicitToExplicitIsRenameAndSoRemovesPreviousAssignment() throws Exception { ExplicitGroup oldGroup = new ExplicitGroup("OldGroup", GroupHierarchyType.INDEPENDENT, ','); oldGroup.add(entry); GroupTreeNode node = GroupTreeNode.fromGroup(oldGroup); @@ -271,7 +271,7 @@ public void setGroupExplicitToExplicitIsRenameAndSoRemovesPreviousAssignment() t } @Test - public void getChildByPathFindsCorrectChildInSecondLevel() throws Exception { + void getChildByPathFindsCorrectChildInSecondLevel() throws Exception { GroupTreeNode root = getRoot(); GroupTreeNode child = getNodeInSimpleTree(root); @@ -279,7 +279,7 @@ public void getChildByPathFindsCorrectChildInSecondLevel() throws Exception { } @Test - public void getPathSimpleTree() throws Exception { + void getPathSimpleTree() throws Exception { GroupTreeNode node = getNodeInSimpleTree(); assertEquals("ExplicitParent > ExplicitNode", node.getPath());