From 18d74480d0c9bae1d845f58900dd675155f251a6 Mon Sep 17 00:00:00 2001 From: Tobias Diez Date: Fri, 24 Feb 2017 13:34:35 +0100 Subject: [PATCH 1/4] Add "Remove group and subgroups" option Issue #1407 is fixed in the process --- CHANGELOG.md | 2 +- .../java/org/jabref/gui/DialogService.java | 14 ++++++++++++-- .../java/org/jabref/gui/FXDialogService.java | 13 +++++++++++-- .../gui/groups/GroupTreeController.java | 4 ++++ .../jabref/gui/groups/GroupTreeViewModel.java | 19 +++++++++++++++++++ 5 files changed, 47 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ed83bb94f8d..4e11dae738d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,7 +46,7 @@ We refer to [GitHub issues](https://github.com/JabRef/jabref/issues) by using `# - ArXiV fetcher now checks similarity of entry when using DOI retrieval to avoid false positives [#2575](https://github.com/JabRef/jabref/issues/2575) - Sciencedirect/Elsevier fetcher is now able to scrape new HTML structure [#2576](https://github.com/JabRef/jabref/issues/2576) - Fixed the synchronization logic of keywords and special fields and vice versa [#2580](https://github.com/JabRef/jabref/issues/2580) - + - Fixed a display issue when removing a group with a long name [#1407](https://github.com/JabRef/jabref/issues/1407) ### Removed diff --git a/src/main/java/org/jabref/gui/DialogService.java b/src/main/java/org/jabref/gui/DialogService.java index f9bbf9c5578..117767217e7 100644 --- a/src/main/java/org/jabref/gui/DialogService.java +++ b/src/main/java/org/jabref/gui/DialogService.java @@ -66,9 +66,19 @@ default void showErrorDialogAndWait(Exception exception) { * a OK and Cancel Button. To create a confirmation dialog with custom * buttons see also {@link #showCustomButtonDialogAndWait(Alert.AlertType, String, String, ButtonType...)} * - * @return Optional with the pressed Button as ButtonType + * @return true if the use clicked "OK" otherwise false + */ + boolean showConfirmationDialogAndWait(String title, String content); + + /** + * Create and display a new confirmation dialog. + * It will include a blue question icon on the left and + * a OK (with given label) and Cancel Button. To create a confirmation dialog with custom + * buttons see also {@link #showCustomButtonDialogAndWait(Alert.AlertType, String, String, ButtonType...)} + * + * @return true if the use clicked "OK" otherwise false */ - Optional showConfirmationDialogAndWait(String title, String content); + boolean showConfirmationDialogAndWait(String title, String content, String okButtonLabel); /** * This will create and display a new dialog of the specified diff --git a/src/main/java/org/jabref/gui/FXDialogService.java b/src/main/java/org/jabref/gui/FXDialogService.java index 6cd1863fc77..972fb7b2429 100644 --- a/src/main/java/org/jabref/gui/FXDialogService.java +++ b/src/main/java/org/jabref/gui/FXDialogService.java @@ -5,6 +5,7 @@ import java.util.Optional; import javafx.scene.control.Alert.AlertType; +import javafx.scene.control.ButtonBar; import javafx.scene.control.ButtonType; import javafx.scene.control.DialogPane; import javafx.stage.FileChooser; @@ -65,9 +66,17 @@ public void showErrorDialogAndWait(String message) { } @Override - public Optional showConfirmationDialogAndWait(String title, String content) { + public boolean showConfirmationDialogAndWait(String title, String content) { FXDialog alert = createDialog(AlertType.CONFIRMATION, title, content); - return alert.showAndWait(); + return alert.showAndWait().filter(buttonType -> buttonType == ButtonType.OK).isPresent(); + } + + @Override + public boolean showConfirmationDialogAndWait(String title, String content, String okButtonLabel) { + FXDialog alert = createDialog(AlertType.CONFIRMATION, title, content); + ButtonType okButtonType = new ButtonType(okButtonLabel, ButtonBar.ButtonData.OK_DONE); + alert.getButtonTypes().setAll(ButtonType.CANCEL, okButtonType); + return alert.showAndWait().filter(buttonType -> buttonType == okButtonType).isPresent(); } @Override diff --git a/src/main/java/org/jabref/gui/groups/GroupTreeController.java b/src/main/java/org/jabref/gui/groups/GroupTreeController.java index 590a8b76820..6813c4d4cbf 100644 --- a/src/main/java/org/jabref/gui/groups/GroupTreeController.java +++ b/src/main/java/org/jabref/gui/groups/GroupTreeController.java @@ -130,7 +130,11 @@ private ContextMenu createContextMenuForGroup(GroupNodeViewModel group) { MenuItem addSubgroup = new MenuItem(Localization.lang("Add subgroup")); addSubgroup.setOnAction(event -> viewModel.addNewSubgroup(group)); + MenuItem removeGroupAndSubgroups = new MenuItem(Localization.lang("Remove group and subgroups")); + removeGroupAndSubgroups.setOnAction(event -> viewModel.removeGroupAndSubgroups(group)); + menu.getItems().add(addSubgroup); + menu.getItems().add(removeGroupAndSubgroups); return menu; } diff --git a/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java b/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java index 6e7ed7e35b1..1f1c4308b26 100644 --- a/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java +++ b/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java @@ -94,4 +94,23 @@ public void addNewSubgroup(GroupNodeViewModel parent) { dialogService.notify(Localization.lang("Added group \"%0\".", group.getName())); }); } + + /** + * Removes the specified group and its subgroups (after asking for confirmation). + */ + public void removeGroupAndSubgroups(GroupNodeViewModel group) { + boolean confirmed = dialogService.showConfirmationDialogAndWait( + Localization.lang("Remove group and subgroups"), + Localization.lang("Remove group \"%0\" and its subgroups?", group.getDisplayName()), + Localization.lang("Remove")); + if (confirmed) { + // TODO: Add undo + //final UndoableAddOrRemoveGroup undo = new UndoableAddOrRemoveGroup(groupsRoot, node, UndoableAddOrRemoveGroup.REMOVE_NODE_AND_CHILDREN); + //panel.getUndoManager().addEdit(undo); + + group.getGroupNode().removeFromParent(); + + dialogService.notify(Localization.lang("Removed group \"%0\" and its subgroups.", group.getDisplayName())); + } + } } From 89a95defa130accd5b163a7a3bca14c2e11dae5c Mon Sep 17 00:00:00 2001 From: Tobias Diez Date: Fri, 24 Feb 2017 13:41:04 +0100 Subject: [PATCH 2/4] Remove old code --- .../org/jabref/gui/groups/GroupSelector.java | 34 ++----------------- 1 file changed, 2 insertions(+), 32 deletions(-) diff --git a/src/main/java/org/jabref/gui/groups/GroupSelector.java b/src/main/java/org/jabref/gui/groups/GroupSelector.java index f94467717e3..ffb17c98e12 100644 --- a/src/main/java/org/jabref/gui/groups/GroupSelector.java +++ b/src/main/java/org/jabref/gui/groups/GroupSelector.java @@ -102,7 +102,6 @@ public class GroupSelector extends SidePaneComponent implements TreeSelectionLis private final AbstractAction editGroupAction = new EditGroupAction(); private final NodeAction editGroupPopupAction = new EditGroupAction(); private final NodeAction addGroupPopupAction = new AddGroupAction(); - private final NodeAction removeGroupAndSubgroupsPopupAction = new RemoveGroupAndSubgroupsAction(); private final NodeAction removeSubgroupsPopupAction = new RemoveSubgroupsAction(); private final NodeAction removeGroupKeepSubgroupsPopupAction = new RemoveGroupKeepSubgroupsAction(); private final NodeAction moveNodeUpPopupAction = new MoveNodeUpAction(); @@ -307,7 +306,6 @@ private void definePopup() { groupsContextMenu.add(editGroupPopupAction); groupsContextMenu.add(addGroupPopupAction); groupsContextMenu.addSeparator(); - groupsContextMenu.add(removeGroupAndSubgroupsPopupAction); groupsContextMenu.add(removeGroupKeepSubgroupsPopupAction); groupsContextMenu.add(removeSubgroupsPopupAction); groupsContextMenu.addSeparator(); @@ -383,7 +381,6 @@ private void showPopup(MouseEvent e) { final TreePath path = groupsTree.getPathForLocation(e.getPoint().x, e.getPoint().y); addGroupPopupAction.setEnabled(true); editGroupPopupAction.setEnabled(path != null); - removeGroupAndSubgroupsPopupAction.setEnabled(path != null); removeGroupKeepSubgroupsPopupAction.setEnabled(path != null); moveSubmenu.setEnabled(path != null); expandSubtreePopupAction.setEnabled(path != null); @@ -396,7 +393,6 @@ private void showPopup(MouseEvent e) { if (path != null) { // some path dependent enabling/disabling GroupTreeNodeViewModel node = (GroupTreeNodeViewModel) path.getLastPathComponent(); editGroupPopupAction.setNode(node); - removeGroupAndSubgroupsPopupAction.setNode(node); removeSubgroupsPopupAction.setNode(node); removeGroupKeepSubgroupsPopupAction.setNode(node); expandSubtreePopupAction.setNode(node); @@ -407,13 +403,13 @@ private void showPopup(MouseEvent e) { if (node.canBeEdited()) { editGroupPopupAction.setEnabled(false); addGroupPopupAction.setEnabled(false); - removeGroupAndSubgroupsPopupAction.setEnabled(false); + //removeGroupAndSubgroupsPopupAction.setEnabled(false); removeGroupKeepSubgroupsPopupAction.setEnabled(false); } else { editGroupPopupAction.setEnabled(true); addGroupPopupAction.setEnabled(true); addGroupPopupAction.setNode(node); - removeGroupAndSubgroupsPopupAction.setEnabled(true); + //removeGroupAndSubgroupsPopupAction.setEnabled(true); removeGroupKeepSubgroupsPopupAction.setEnabled(true); } expandSubtreePopupAction @@ -452,7 +448,6 @@ private void showPopup(MouseEvent e) { } else { editGroupPopupAction.setNode(null); addGroupPopupAction.setNode(null); - removeGroupAndSubgroupsPopupAction.setNode(null); removeSubgroupsPopupAction.setNode(null); removeGroupKeepSubgroupsPopupAction.setNode(null); moveNodeUpPopupAction.setNode(null); @@ -983,31 +978,6 @@ public void actionPerformed(ActionEvent e) { } } - private class RemoveGroupAndSubgroupsAction extends NodeAction { - - public RemoveGroupAndSubgroupsAction() { - super(Localization.lang("Remove group and subgroups")); - } - - @Override - public void actionPerformed(ActionEvent e) { - final GroupTreeNodeViewModel node = getNodeToUse(); - final AbstractGroup group = node.getNode().getGroup(); - int conf = JOptionPane.showConfirmDialog(frame, - Localization.lang("Remove group \"%0\" and its subgroups?", group.getName()), - Localization.lang("Remove group and subgroups"), JOptionPane.YES_NO_OPTION); - if (conf == JOptionPane.YES_OPTION) { - final UndoableAddOrRemoveGroup undo = new UndoableAddOrRemoveGroup(groupsRoot, node, - UndoableAddOrRemoveGroup.REMOVE_NODE_AND_CHILDREN); - node.getNode().removeFromParent(); - // Store undo information. - panel.getUndoManager().addEdit(undo); - panel.markBaseChanged(); - frame.output(Localization.lang("Removed group \"%0\" and its subgroups.", group.getName())); - } - } - } - private class RemoveSubgroupsAction extends NodeAction { public RemoveSubgroupsAction() { From 22c75d11c650cc6b4cce4c3506f04bb52344bd85 Mon Sep 17 00:00:00 2001 From: Tobias Diez Date: Fri, 24 Feb 2017 17:41:57 +0100 Subject: [PATCH 3/4] Implement #1904: filter groups --- CHANGELOG.md | 1 + .../jabref/gui/groups/GroupNodeViewModel.java | 21 +- .../java/org/jabref/gui/groups/GroupTree.css | 2 +- .../java/org/jabref/gui/groups/GroupTree.fxml | 31 +- .../gui/groups/GroupTreeController.java | 44 +- .../jabref/gui/groups/GroupTreeViewModel.java | 23 +- .../jabref/gui/util/RecursiveTreeItem.java | 55 ++- .../gui/groups/GroupNodeViewModelTest.java | 16 + .../gui/util/RecursiveTreeItemTest.java | 58 +++ .../java/org/jabref/model/TreeNodeTest.java | 402 +++++++----------- .../org/jabref/model/TreeNodeTestData.java | 121 ++++++ 11 files changed, 470 insertions(+), 304 deletions(-) create mode 100644 src/test/java/org/jabref/gui/util/RecursiveTreeItemTest.java create mode 100644 src/test/java/org/jabref/model/TreeNodeTestData.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e11dae738d..4f8adcf3500 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ We refer to [GitHub issues](https://github.com/JabRef/jabref/issues) by using `# - Redesigned group panel. - Number of matched entries is always shown. - The background color of the hit counter signals whether the group contains all/any of the entries selected in the main table. + - Added a possibility to filter the groups panel [#1904](https://github.com/JabRef/jabref/issues/1904) - Removed edit mode. - Redesigned about dialog. - Redesigned key bindings dialog. diff --git a/src/main/java/org/jabref/gui/groups/GroupNodeViewModel.java b/src/main/java/org/jabref/gui/groups/GroupNodeViewModel.java index 61d8f72742f..578465c411c 100644 --- a/src/main/java/org/jabref/gui/groups/GroupNodeViewModel.java +++ b/src/main/java/org/jabref/gui/groups/GroupNodeViewModel.java @@ -27,6 +27,7 @@ import org.jabref.model.groups.AllEntriesGroup; import org.jabref.model.groups.AutomaticGroup; import org.jabref.model.groups.GroupTreeNode; +import org.jabref.model.strings.StringUtil; import com.google.common.eventbus.Subscribe; import org.fxmisc.easybind.EasyBind; @@ -136,13 +137,8 @@ public boolean equals(Object o) { GroupNodeViewModel that = (GroupNodeViewModel) o; - if (isRoot != that.isRoot) return false; - if (!displayName.equals(that.displayName)) return false; - if (!iconCode.equals(that.iconCode)) return false; - if (!children.equals(that.children)) return false; - if (!databaseContext.equals(that.databaseContext)) return false; if (!groupNode.equals(that.groupNode)) return false; - return hits.getValue().equals(that.hits.getValue()); + return true; } @Override @@ -160,14 +156,7 @@ public String toString() { @Override public int hashCode() { - int result = displayName.hashCode(); - result = 31 * result + (isRoot ? 1 : 0); - result = 31 * result + iconCode.hashCode(); - result = 31 * result + children.hashCode(); - result = 31 * result + databaseContext.hashCode(); - result = 31 * result + groupNode.hashCode(); - result = 31 * result + hits.hashCode(); - return result; + return groupNode.hashCode(); } public String getIconCode() { @@ -207,4 +196,8 @@ public GroupTreeNode addSubgroup(AbstractGroup subgroup) { void toggleExpansion() { expandedProperty().set(!expandedProperty().get()); } + + boolean isMatchedBy(String searchString) { + return StringUtil.isBlank(searchString) || getDisplayName().contains(searchString); + } } diff --git a/src/main/java/org/jabref/gui/groups/GroupTree.css b/src/main/java/org/jabref/gui/groups/GroupTree.css index 0c36d609d15..284c08f7f68 100644 --- a/src/main/java/org/jabref/gui/groups/GroupTree.css +++ b/src/main/java/org/jabref/gui/groups/GroupTree.css @@ -107,7 +107,7 @@ -fx-translate-x: -5px; } -#buttonBarBottom { +#barBottom { -fx-background-color: #dadad8; -fx-border-color: dimgray; -fx-border-width: 1 0 0 0; diff --git a/src/main/java/org/jabref/gui/groups/GroupTree.fxml b/src/main/java/org/jabref/gui/groups/GroupTree.fxml index 571694c45af..f0ce56026e6 100644 --- a/src/main/java/org/jabref/gui/groups/GroupTree.fxml +++ b/src/main/java/org/jabref/gui/groups/GroupTree.fxml @@ -6,6 +6,8 @@ + + @@ -24,18 +26,21 @@ - - - - - + + + + + + + + diff --git a/src/main/java/org/jabref/gui/groups/GroupTreeController.java b/src/main/java/org/jabref/gui/groups/GroupTreeController.java index 6813c4d4cbf..a38f63a68d0 100644 --- a/src/main/java/org/jabref/gui/groups/GroupTreeController.java +++ b/src/main/java/org/jabref/gui/groups/GroupTreeController.java @@ -1,7 +1,11 @@ package org.jabref.gui.groups; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; + import javax.inject.Inject; +import javafx.beans.property.ObjectProperty; import javafx.css.PseudoClass; import javafx.event.ActionEvent; import javafx.fxml.FXML; @@ -9,6 +13,7 @@ import javafx.scene.control.Control; import javafx.scene.control.MenuItem; import javafx.scene.control.SelectionModel; +import javafx.scene.control.TextField; import javafx.scene.control.TreeItem; import javafx.scene.control.TreeTableColumn; import javafx.scene.control.TreeTableRow; @@ -24,14 +29,21 @@ import org.jabref.gui.util.ViewModelTreeTableCellFactory; import org.jabref.logic.l10n.Localization; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.controlsfx.control.textfield.CustomTextField; +import org.controlsfx.control.textfield.TextFields; import org.fxmisc.easybind.EasyBind; public class GroupTreeController extends AbstractController { + private static final Log LOGGER = LogFactory.getLog(GroupTreeController.class); + @FXML private TreeTableView groupTree; @FXML private TreeTableColumn mainColumn; @FXML private TreeTableColumn numberColumn; @FXML private TreeTableColumn disclosureNodeColumn; + @FXML private CustomTextField searchField; @Inject private StateManager stateManager; @Inject private DialogService dialogService; @@ -41,15 +53,23 @@ public void initialize() { viewModel = new GroupTreeViewModel(stateManager, dialogService); // Set-up bindings - groupTree.rootProperty().bind( - EasyBind.map(viewModel.rootGroupProperty(), - group -> new RecursiveTreeItem<>(group, GroupNodeViewModel::getChildren, GroupNodeViewModel::expandedProperty)) - ); viewModel.selectedGroupProperty().bind( EasyBind.monadic(groupTree.selectionModelProperty()) .flatMap(SelectionModel::selectedItemProperty) .selectProperty(TreeItem::valueProperty) ); + viewModel.filterTextProperty().bind(searchField.textProperty()); + searchField.textProperty().addListener((observable, oldValue, newValue) -> { + }); + + groupTree.rootProperty().bind( + EasyBind.map(viewModel.rootGroupProperty(), + group -> new RecursiveTreeItem<>( + group, + GroupNodeViewModel::getChildren, + GroupNodeViewModel::expandedProperty, + viewModel.filterPredicateProperty())) + ); // Icon and group name mainColumn.setCellValueFactory(cellData -> cellData.getValue().valueProperty()); @@ -122,6 +142,9 @@ public void initialize() { return row; }); + + // Filter text field + setupClearButtonField(searchField); } private ContextMenu createContextMenuForGroup(GroupNodeViewModel group) { @@ -141,4 +164,17 @@ private ContextMenu createContextMenuForGroup(GroupNodeViewModel group) { public void addNewGroup(ActionEvent actionEvent) { viewModel.addNewGroupToRoot(); } + + /** + * Workaround taken from https://bitbucket.org/controlsfx/controlsfx/issues/330/making-textfieldssetupclearbuttonfield + */ + private void setupClearButtonField(CustomTextField customTextField) { + try { + Method m = TextFields.class.getDeclaredMethod("setupClearButtonField", TextField.class, ObjectProperty.class); + m.setAccessible(true); + m.invoke(null, customTextField, customTextField.rightProperty()); + } catch (NoSuchMethodException | IllegalAccessException | InvocationTargetException ex) { + LOGGER.error("Failed to decorate text field with clear button", ex); + } + } } diff --git a/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java b/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java index 1f1c4308b26..cb1cf36cd21 100644 --- a/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java +++ b/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java @@ -2,9 +2,13 @@ import java.util.Objects; import java.util.Optional; +import java.util.function.Predicate; +import javafx.beans.binding.Bindings; import javafx.beans.property.ObjectProperty; import javafx.beans.property.SimpleObjectProperty; +import javafx.beans.property.SimpleStringProperty; +import javafx.beans.property.StringProperty; import org.jabref.gui.AbstractViewModel; import org.jabref.gui.DialogService; @@ -21,18 +25,23 @@ public class GroupTreeViewModel extends AbstractViewModel { private final ObjectProperty selectedGroup = new SimpleObjectProperty<>(); private final StateManager stateManager; private final DialogService dialogService; + private final ObjectProperty> filterPredicate = new SimpleObjectProperty<>(); + private final StringProperty filterText = new SimpleStringProperty(); private Optional currentDatabase; public GroupTreeViewModel(StateManager stateManager, DialogService dialogService) { this.stateManager = Objects.requireNonNull(stateManager); this.dialogService = Objects.requireNonNull(dialogService); - // Init - onActiveDatabaseChanged(stateManager.activeDatabaseProperty().getValue()); - // Register listener stateManager.activeDatabaseProperty().addListener((observable, oldValue, newValue) -> onActiveDatabaseChanged(newValue)); selectedGroup.addListener((observable, oldValue, newValue) -> onSelectedGroupChanged(newValue)); + + // Set-up bindings + filterPredicate.bind(Bindings.createObjectBinding(() -> group -> group.isMatchedBy(filterText.get()), filterText)); + + // Init + onActiveDatabaseChanged(stateManager.activeDatabaseProperty().getValue()); } public ObjectProperty rootGroupProperty() { @@ -43,6 +52,14 @@ public ObjectProperty selectedGroupProperty() { return selectedGroup; } + public ObjectProperty> filterPredicateProperty() { + return filterPredicate; + } + + public StringProperty filterTextProperty() { + return filterText; + } + /** * Gets invoked if the user selects a different group. * We need to notify the {@link StateManager} about this change so that the main table gets updated. diff --git a/src/main/java/org/jabref/gui/util/RecursiveTreeItem.java b/src/main/java/org/jabref/gui/util/RecursiveTreeItem.java index 2714455e99f..059e5ccb883 100644 --- a/src/main/java/org/jabref/gui/util/RecursiveTreeItem.java +++ b/src/main/java/org/jabref/gui/util/RecursiveTreeItem.java @@ -1,11 +1,17 @@ package org.jabref.gui.util; import java.util.List; +import java.util.function.Predicate; import java.util.stream.Collectors; +import javafx.beans.binding.Bindings; import javafx.beans.property.BooleanProperty; +import javafx.beans.property.ObjectProperty; +import javafx.beans.property.SimpleObjectProperty; +import javafx.beans.value.ObservableValue; import javafx.collections.ListChangeListener; import javafx.collections.ObservableList; +import javafx.collections.transformation.FilteredList; import javafx.scene.Node; import javafx.scene.control.TreeItem; import javafx.util.Callback; @@ -17,20 +23,29 @@ public class RecursiveTreeItem extends TreeItem { private final Callback expandedProperty; private Callback> childrenFactory; + private ObjectProperty> filter = new SimpleObjectProperty<>(); + private FilteredList children; public RecursiveTreeItem(final T value, Callback> func) { - this(value, func, null); + this(value, func, null, null); } - public RecursiveTreeItem(final T value, Callback> func, Callback expandedProperty) { - this(value, (Node) null, func, expandedProperty); + public RecursiveTreeItem(final T value, Callback> func, Callback expandedProperty, ObservableValue> filter) { + this(value, null, func, expandedProperty, filter); } - public RecursiveTreeItem(final T value, Node graphic, Callback> func, Callback expandedProperty) { + public RecursiveTreeItem(final T value, Callback> func, ObservableValue> filter) { + this(value, null, func, null, filter); + } + + private RecursiveTreeItem(final T value, Node graphic, Callback> func, Callback expandedProperty, ObservableValue> filter) { super(value, graphic); this.childrenFactory = func; this.expandedProperty = expandedProperty; + if (filter != null) { + this.filter.bind(filter); + } if(value != null) { addChildrenListener(value); @@ -40,7 +55,7 @@ public RecursiveTreeItem(final T value, Node graphic, Callback{ if(newValue != null){ addChildrenListener(newValue); - bindExpandedProperty(value, expandedProperty); + bindExpandedProperty(newValue, expandedProperty); } }); } @@ -52,17 +67,14 @@ private void bindExpandedProperty(T value, Callback expanded } private void addChildrenListener(T value){ - final ObservableList children = childrenFactory.call(value); + children = new FilteredList<>(childrenFactory.call(value)); + children.predicateProperty().bind(Bindings.createObjectBinding(() -> this::showNode, filter)); - children.forEach(child -> RecursiveTreeItem.this.getChildren().add(new RecursiveTreeItem<>(child, getGraphic(), childrenFactory, expandedProperty))); + children.forEach(this::addAsChild); children.addListener((ListChangeListener) change -> { while(change.next()){ - if(change.wasAdded()){ - change.getAddedSubList().forEach(t -> RecursiveTreeItem.this.getChildren().add(new RecursiveTreeItem<>(t, getGraphic(), childrenFactory, expandedProperty))); - } - if(change.wasRemoved()){ change.getRemoved().forEach(t->{ final List> itemsToRemove = RecursiveTreeItem.this.getChildren().stream().filter(treeItem -> treeItem.getValue().equals(t)).collect(Collectors.toList()); @@ -71,7 +83,28 @@ private void addChildrenListener(T value){ }); } + if (change.wasAdded()) { + change.getAddedSubList().forEach(this::addAsChild); + } } }); } + + private boolean addAsChild(T child) { + return RecursiveTreeItem.this.getChildren().add(new RecursiveTreeItem<>(child, getGraphic(), childrenFactory, expandedProperty, filter)); + } + + private boolean showNode(T t) { + if (filter.get() == null) { + return true; + } + + if (filter.get().test(t)) { + // Node is directly matched -> so show it + return true; + } + + // Are there children (or children of children...) that are matched? If yes we also need to show this node + return childrenFactory.call(t).stream().anyMatch(this::showNode); + } } diff --git a/src/test/java/org/jabref/gui/groups/GroupNodeViewModelTest.java b/src/test/java/org/jabref/gui/groups/GroupNodeViewModelTest.java index 07e626f5144..2b71b6bc3f1 100644 --- a/src/test/java/org/jabref/gui/groups/GroupNodeViewModelTest.java +++ b/src/test/java/org/jabref/gui/groups/GroupNodeViewModelTest.java @@ -12,6 +12,7 @@ import org.junit.Test; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -19,12 +20,17 @@ public class GroupNodeViewModelTest { private StateManager stateManager; private BibDatabaseContext databaseContext; + private GroupNodeViewModel viewModel; @Before public void setUp() throws Exception { stateManager = mock(StateManager.class); when(stateManager.getSelectedEntries()).thenReturn(FXCollections.emptyObservableList()); databaseContext = new BibDatabaseContext(); + + viewModel = getViewModelForGroup( + new WordKeywordGroup("Test group", GroupHierarchyType.INDEPENDENT, "test", "search", true, ',', false)); + } @Test @@ -34,6 +40,16 @@ public void getDisplayNameConvertsLatexToUnicode() throws Exception { assertEquals("β", viewModel.getDisplayName()); } + @Test + public void alwaysMatchedByEmptySearchString() throws Exception { + assertTrue(viewModel.isMatchedBy("")); + } + + @Test + public void isMatchedIfContainsPartOfSearchString() throws Exception { + assertTrue(viewModel.isMatchedBy("est")); + } + private GroupNodeViewModel getViewModelForGroup(AbstractGroup group) { return new GroupNodeViewModel(databaseContext, stateManager, group); } diff --git a/src/test/java/org/jabref/gui/util/RecursiveTreeItemTest.java b/src/test/java/org/jabref/gui/util/RecursiveTreeItemTest.java new file mode 100644 index 00000000000..6f06b4012db --- /dev/null +++ b/src/test/java/org/jabref/gui/util/RecursiveTreeItemTest.java @@ -0,0 +1,58 @@ +package org.jabref.gui.util; + +import java.util.Collections; +import java.util.function.Predicate; +import java.util.stream.Collectors; + +import javafx.beans.property.ObjectProperty; +import javafx.beans.property.SimpleObjectProperty; +import javafx.scene.control.TreeItem; + +import org.jabref.model.TreeNode; +import org.jabref.model.TreeNodeTestData; + +import org.junit.Before; +import org.junit.Test; + +import static org.junit.Assert.assertEquals; + +public class RecursiveTreeItemTest { + + private RecursiveTreeItem rootTreeItem; + private TreeNodeTestData.TreeNodeMock root; + private ObjectProperty> filterPredicate; + private TreeNodeTestData.TreeNodeMock node; + + @Before + public void setUp() throws Exception { + root = new TreeNodeTestData.TreeNodeMock(); + node = TreeNodeTestData.getNodeInSimpleTree(root); + node.setName("test node"); + + filterPredicate = new SimpleObjectProperty<>(); + + rootTreeItem = new RecursiveTreeItem<>(root, TreeNode::getChildren, filterPredicate); + } + + @Test + public void addsAllChildrenNodes() throws Exception { + assertEquals(root.getChildren(), rootTreeItem.getChildren().stream().map(TreeItem::getValue).collect(Collectors.toList())); + } + + @Test + public void addsAllChildrenOfChildNode() throws Exception { + assertEquals( + root.getChildAt(1).get().getChildren(), + rootTreeItem.getChildren().get(1).getChildren().stream().map(TreeItem::getValue).collect(Collectors.toList())); + } + + @Test + public void respectsFilter() throws Exception { + filterPredicate.setValue(item -> item.getName().contains("test")); + + assertEquals(Collections.singletonList(node.getParent().get()), rootTreeItem.getChildren().stream().map(TreeItem::getValue).collect(Collectors.toList())); + assertEquals( + Collections.singletonList(node), + rootTreeItem.getChildren().get(0).getChildren().stream().map(TreeItem::getValue).collect(Collectors.toList())); + } +} diff --git a/src/test/java/org/jabref/model/TreeNodeTest.java b/src/test/java/org/jabref/model/TreeNodeTest.java index fb7a336a18f..841dfaeab29 100644 --- a/src/test/java/org/jabref/model/TreeNodeTest.java +++ b/src/test/java/org/jabref/model/TreeNodeTest.java @@ -21,88 +21,7 @@ public class TreeNodeTest { @Mock - Consumer subscriber; - - /** - * Gets the marked node in the following tree: - * Root - * A - * A (= parent) - * B (<-- this) - */ - private TreeNodeMock getNodeInSimpleTree(TreeNodeMock root) { - root.addChild(new TreeNodeMock()); - TreeNodeMock parent = new TreeNodeMock(); - root.addChild(parent); - TreeNodeMock node = new TreeNodeMock(); - parent.addChild(node); - return node; - } - - private TreeNodeMock getNodeInSimpleTree() { - return getNodeInSimpleTree(new TreeNodeMock()); - } - - /** - * Gets the marked node in the following tree: - * Root - * A - * A - * A (= grand parent) - * B - * B (= parent) - * C (<-- this) - * D (= child) - * C - * C - * C - * B - * B - * A - */ - private TreeNodeMock getNodeInComplexTree(TreeNodeMock root) { - root.addChild(new TreeNodeMock()); - root.addChild(new TreeNodeMock()); - TreeNodeMock grandParent = new TreeNodeMock(); - root.addChild(grandParent); - root.addChild(new TreeNodeMock()); - - grandParent.addChild(new TreeNodeMock()); - TreeNodeMock parent = new TreeNodeMock(); - grandParent.addChild(parent); - grandParent.addChild(new TreeNodeMock()); - grandParent.addChild(new TreeNodeMock()); - - TreeNodeMock node = new TreeNodeMock(); - parent.addChild(node); - parent.addChild(new TreeNodeMock()); - parent.addChild(new TreeNodeMock()); - parent.addChild(new TreeNodeMock()); - - node.addChild(new TreeNodeMock()); - return node; - } - - private TreeNodeMock getNodeInComplexTree() { - return getNodeInComplexTree(new TreeNodeMock()); - } - - /** - * Gets the marked in the following tree: - * Root - * A - * A - * A (<- this) - * A - */ - private TreeNodeMock getNodeAsChild(TreeNodeMock root) { - root.addChild(new TreeNodeMock()); - root.addChild(new TreeNodeMock()); - TreeNodeMock node = new TreeNodeMock(); - root.addChild(node); - root.addChild(new TreeNodeMock()); - return node; - } + Consumer subscriber; @Test(expected = UnsupportedOperationException.class) public void constructorChecksThatClassImplementsCorrectInterface() { @@ -111,13 +30,13 @@ public void constructorChecksThatClassImplementsCorrectInterface() { @Test public void constructorExceptsCorrectImplementation() { - TreeNodeMock treeNode = new TreeNodeMock(); + TreeNodeTestData.TreeNodeMock treeNode = new TreeNodeTestData.TreeNodeMock(); assertNotNull(treeNode); } @Test public void newTreeNodeHasNoParentOrChildren() { - TreeNodeMock treeNode = new TreeNodeMock(); + TreeNodeTestData.TreeNodeMock treeNode = new TreeNodeTestData.TreeNodeMock(); assertEquals(Optional.empty(), treeNode.getParent()); assertEquals(Collections.emptyList(), treeNode.getChildren()); assertNotNull(treeNode); @@ -125,126 +44,126 @@ public void newTreeNodeHasNoParentOrChildren() { @Test public void getIndexedPathFromRootReturnsEmptyListForRoot() { - TreeNodeMock root = new TreeNodeMock(); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); assertEquals(Collections.emptyList(), root.getIndexedPathFromRoot()); } @Test public void getIndexedPathFromRootSimplePath() { - assertEquals(Arrays.asList(1, 0), getNodeInSimpleTree().getIndexedPathFromRoot()); + assertEquals(Arrays.asList(1, 0), TreeNodeTestData.getNodeInSimpleTree().getIndexedPathFromRoot()); } @Test public void getIndexedPathFromRootComplexPath() { - assertEquals(Arrays.asList(2, 1, 0), getNodeInComplexTree().getIndexedPathFromRoot()); + assertEquals(Arrays.asList(2, 1, 0), TreeNodeTestData.getNodeInComplexTree().getIndexedPathFromRoot()); } @Test public void getDescendantSimplePath() { - TreeNodeMock root = new TreeNodeMock(); - TreeNodeMock node = getNodeInSimpleTree(root); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.TreeNodeMock node = TreeNodeTestData.getNodeInSimpleTree(root); assertEquals(node, root.getDescendant(Arrays.asList(1, 0)).get()); } @Test public void getDescendantComplexPath() { - TreeNodeMock root = new TreeNodeMock(); - TreeNodeMock node = getNodeInComplexTree(root); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.TreeNodeMock node = TreeNodeTestData.getNodeInComplexTree(root); assertEquals(node, root.getDescendant(Arrays.asList(2, 1, 0)).get()); } @Test public void getDescendantNonExistentReturnsEmpty() { - TreeNodeMock root = new TreeNodeMock(); - getNodeInComplexTree(root); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.getNodeInComplexTree(root); assertEquals(Optional.empty(), root.getDescendant(Arrays.asList(1, 100, 0))); } @Test(expected = UnsupportedOperationException.class) public void getPositionInParentForRootThrowsException() { - TreeNodeMock root = new TreeNodeMock(); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); root.getPositionInParent(); } @Test public void getPositionInParentSimpleTree() { - TreeNodeMock root = new TreeNodeMock(); - TreeNodeMock node = getNodeAsChild(root); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.TreeNodeMock node = TreeNodeTestData.getNodeAsChild(root); assertEquals(2, node.getPositionInParent()); } @Test public void getIndexOfNonExistentChildReturnsEmpty() { - TreeNodeMock root = new TreeNodeMock(); - assertEquals(Optional.empty(), root.getIndexOfChild(new TreeNodeMock())); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); + assertEquals(Optional.empty(), root.getIndexOfChild(new TreeNodeTestData.TreeNodeMock())); } @Test public void getIndexOfChild() { - TreeNodeMock root = new TreeNodeMock(); - TreeNodeMock node = getNodeAsChild(root); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.TreeNodeMock node = TreeNodeTestData.getNodeAsChild(root); assertEquals((Integer)2, root.getIndexOfChild(node).get()); } @Test public void getLevelOfRoot() { - TreeNodeMock root = new TreeNodeMock(); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); assertEquals(0, root.getLevel()); } @Test public void getLevelInSimpleTree() { - assertEquals(2, getNodeInSimpleTree().getLevel()); + assertEquals(2, TreeNodeTestData.getNodeInSimpleTree().getLevel()); } @Test public void getLevelInComplexTree() { - assertEquals(3, getNodeInComplexTree().getLevel()); + assertEquals(3, TreeNodeTestData.getNodeInComplexTree().getLevel()); } @Test public void getChildCountInSimpleTree() { - TreeNodeMock root = new TreeNodeMock(); - getNodeInSimpleTree(root); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.getNodeInSimpleTree(root); assertEquals(2, root.getNumberOfChildren()); } @Test public void getChildCountInComplexTree() { - TreeNodeMock root = new TreeNodeMock(); - getNodeInComplexTree(root); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.getNodeInComplexTree(root); assertEquals(4, root.getNumberOfChildren()); } @Test public void moveToAddsAsLastChildInSimpleTree() { - TreeNodeMock root = new TreeNodeMock(); - TreeNodeMock node = getNodeInSimpleTree(root); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.TreeNodeMock node = TreeNodeTestData.getNodeInSimpleTree(root); node.moveTo(root); assertEquals((Integer)2, root.getIndexOfChild(node).get()); } @Test public void moveToAddsAsLastChildInComplexTree() { - TreeNodeMock root = new TreeNodeMock(); - TreeNodeMock node = getNodeInComplexTree(root); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.TreeNodeMock node = TreeNodeTestData.getNodeInComplexTree(root); node.moveTo(root); assertEquals((Integer)4, root.getIndexOfChild(node).get()); } @Test public void moveToChangesParent() { - TreeNodeMock root = new TreeNodeMock(); - TreeNodeMock node = getNodeInSimpleTree(root); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.TreeNodeMock node = TreeNodeTestData.getNodeInSimpleTree(root); node.moveTo(root); assertEquals(root, node.getParent().get()); } @Test public void moveToInSameLevelAddsAtEnd() { - TreeNodeMock root = new TreeNodeMock(); - TreeNodeMock child1 = new TreeNodeMock(); - TreeNodeMock child2 = new TreeNodeMock(); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.TreeNodeMock child1 = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.TreeNodeMock child2 = new TreeNodeTestData.TreeNodeMock(); root.addChild(child1); root.addChild(child2); @@ -255,10 +174,10 @@ public void moveToInSameLevelAddsAtEnd() { @Test public void getPathFromRootInSimpleTree() { - TreeNodeMock root = new TreeNodeMock(); - TreeNodeMock node = getNodeInSimpleTree(root); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.TreeNodeMock node = TreeNodeTestData.getNodeInSimpleTree(root); - List path = node.getPathFromRoot(); + List path = node.getPathFromRoot(); assertEquals(3, path.size()); assertEquals(root, path.get(0)); assertEquals(node, path.get(2)); @@ -266,10 +185,10 @@ public void getPathFromRootInSimpleTree() { @Test public void getPathFromRootInComplexTree() { - TreeNodeMock root = new TreeNodeMock(); - TreeNodeMock node = getNodeInComplexTree(root); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.TreeNodeMock node = TreeNodeTestData.getNodeInComplexTree(root); - List path = node.getPathFromRoot(); + List path = node.getPathFromRoot(); assertEquals(4, path.size()); assertEquals(root, path.get(0)); assertEquals(node, path.get(3)); @@ -277,149 +196,149 @@ public void getPathFromRootInComplexTree() { @Test public void getPreviousSiblingReturnsCorrect() { - TreeNodeMock root = new TreeNodeMock(); - root.addChild(new TreeNodeMock()); - TreeNodeMock previous = new TreeNodeMock(); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); + root.addChild(new TreeNodeTestData.TreeNodeMock()); + TreeNodeTestData.TreeNodeMock previous = new TreeNodeTestData.TreeNodeMock(); root.addChild(previous); - TreeNodeMock node = new TreeNodeMock(); + TreeNodeTestData.TreeNodeMock node = new TreeNodeTestData.TreeNodeMock(); root.addChild(node); - root.addChild(new TreeNodeMock()); + root.addChild(new TreeNodeTestData.TreeNodeMock()); assertEquals(previous, node.getPreviousSibling().get()); } @Test public void getPreviousSiblingForRootReturnsEmpty() { - TreeNodeMock root = new TreeNodeMock(); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); assertEquals(Optional.empty(), root.getPreviousSibling()); } @Test public void getPreviousSiblingForNonexistentReturnsEmpty() { - TreeNodeMock root = new TreeNodeMock(); - TreeNodeMock node = new TreeNodeMock(); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.TreeNodeMock node = new TreeNodeTestData.TreeNodeMock(); root.addChild(node); assertEquals(Optional.empty(), node.getPreviousSibling()); } @Test public void getNextSiblingReturnsCorrect() { - TreeNodeMock root = new TreeNodeMock(); - root.addChild(new TreeNodeMock()); - TreeNodeMock node = new TreeNodeMock(); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); + root.addChild(new TreeNodeTestData.TreeNodeMock()); + TreeNodeTestData.TreeNodeMock node = new TreeNodeTestData.TreeNodeMock(); root.addChild(node); - TreeNodeMock next = new TreeNodeMock(); + TreeNodeTestData.TreeNodeMock next = new TreeNodeTestData.TreeNodeMock(); root.addChild(next); - root.addChild(new TreeNodeMock()); + root.addChild(new TreeNodeTestData.TreeNodeMock()); assertEquals(next, node.getNextSibling().get()); } @Test public void getNextSiblingForRootReturnsEmpty() { - TreeNodeMock root = new TreeNodeMock(); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); assertEquals(Optional.empty(), root.getNextSibling()); } @Test public void getNextSiblingForNonexistentReturnsEmpty() { - TreeNodeMock root = new TreeNodeMock(); - TreeNodeMock node = new TreeNodeMock(); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.TreeNodeMock node = new TreeNodeTestData.TreeNodeMock(); root.addChild(node); assertEquals(Optional.empty(), node.getPreviousSibling()); } @Test public void getParentReturnsCorrect() { - TreeNodeMock root = new TreeNodeMock(); - TreeNodeMock node = getNodeAsChild(root); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.TreeNodeMock node = TreeNodeTestData.getNodeAsChild(root); assertEquals(root, node.getParent().get()); } @Test public void getParentForRootReturnsEmpty() { - TreeNodeMock root = new TreeNodeMock(); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); assertEquals(Optional.empty(), root.getParent()); } @Test public void getChildAtReturnsCorrect() { - TreeNodeMock root = new TreeNodeMock(); - TreeNodeMock node = getNodeAsChild(root); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.TreeNodeMock node = TreeNodeTestData.getNodeAsChild(root); assertEquals(node, root.getChildAt(2).get()); } @Test public void getChildAtInvalidIndexReturnsEmpty() { - TreeNodeMock root = new TreeNodeMock(); - root.addChild(new TreeNodeMock()); - root.addChild(new TreeNodeMock()); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); + root.addChild(new TreeNodeTestData.TreeNodeMock()); + root.addChild(new TreeNodeTestData.TreeNodeMock()); assertEquals(Optional.empty(), root.getChildAt(10)); } @Test public void getRootReturnsTrueForRoot() { - TreeNodeMock root = new TreeNodeMock(); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); assertTrue(root.isRoot()); } @Test public void getRootReturnsFalseForChild() { - assertFalse(getNodeInSimpleTree().isRoot()); + assertFalse(TreeNodeTestData.getNodeInSimpleTree().isRoot()); } @Test public void nodeIsAncestorOfItself() { - TreeNodeMock root = new TreeNodeMock(); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); assertTrue(root.isAncestorOf(root)); } @Test public void isAncestorOfInSimpleTree() { - TreeNodeMock root = new TreeNodeMock(); - TreeNodeMock node = getNodeInSimpleTree(root); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.TreeNodeMock node = TreeNodeTestData.getNodeInSimpleTree(root); assertTrue(root.isAncestorOf(node)); } @Test public void isAncestorOfInComplexTree() { - TreeNodeMock root = new TreeNodeMock(); - TreeNodeMock node = getNodeInComplexTree(root); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.TreeNodeMock node = TreeNodeTestData.getNodeInComplexTree(root); assertTrue(root.isAncestorOf(node)); } @Test public void getRootOfSingleNode() { - TreeNodeMock root = new TreeNodeMock(); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); assertEquals(root, root.getRoot()); } @Test public void getRootInSimpleTree() { - TreeNodeMock root = new TreeNodeMock(); - TreeNodeMock node = getNodeInSimpleTree(root); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.TreeNodeMock node = TreeNodeTestData.getNodeInSimpleTree(root); assertEquals(root, node.getRoot()); } @Test public void getRootInComplexTree() { - TreeNodeMock root = new TreeNodeMock(); - TreeNodeMock node = getNodeInComplexTree(root); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.TreeNodeMock node = TreeNodeTestData.getNodeInComplexTree(root); assertEquals(root, node.getRoot()); } @Test public void isLeafIsCorrectForRootWithoutChildren() { - TreeNodeMock root = new TreeNodeMock(); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); assertTrue(root.isLeaf()); } @Test public void removeFromParentSetsParentToEmpty() { - TreeNodeMock root = new TreeNodeMock(); - TreeNodeMock node = getNodeAsChild(root); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.TreeNodeMock node = TreeNodeTestData.getNodeAsChild(root); node.removeFromParent(); assertEquals(Optional.empty(), node.getParent()); @@ -427,8 +346,8 @@ public void removeFromParentSetsParentToEmpty() { @Test public void removeFromParentRemovesNodeFromChildrenCollection() { - TreeNodeMock root = new TreeNodeMock(); - TreeNodeMock node = getNodeAsChild(root); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.TreeNodeMock node = TreeNodeTestData.getNodeAsChild(root); node.removeFromParent(); assertFalse(root.getChildren().contains(node)); @@ -436,8 +355,8 @@ public void removeFromParentRemovesNodeFromChildrenCollection() { @Test public void removeAllChildrenSetsParentOfChildToEmpty() { - TreeNodeMock root = new TreeNodeMock(); - TreeNodeMock node = getNodeAsChild(root); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.TreeNodeMock node = TreeNodeTestData.getNodeAsChild(root); root.removeAllChildren(); assertEquals(Optional.empty(), node.getParent()); @@ -445,8 +364,8 @@ public void removeAllChildrenSetsParentOfChildToEmpty() { @Test public void removeAllChildrenRemovesAllNodesFromChildrenCollection() { - TreeNodeMock root = new TreeNodeMock(); - getNodeAsChild(root); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.getNodeAsChild(root); root.removeAllChildren(); assertEquals(Collections.emptyList(), root.getChildren()); @@ -454,8 +373,8 @@ public void removeAllChildrenRemovesAllNodesFromChildrenCollection() { @Test public void getFirstChildAtReturnsCorrect() { - TreeNodeMock root = new TreeNodeMock(); - TreeNodeMock node = new TreeNodeMock(); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.TreeNodeMock node = new TreeNodeTestData.TreeNodeMock(); root.addChild(node); assertEquals(node, root.getFirstChild().get()); @@ -463,30 +382,30 @@ public void getFirstChildAtReturnsCorrect() { @Test public void getFirstChildAtLeafReturnsEmpty() { - TreeNodeMock root = new TreeNodeMock(); - TreeNodeMock leaf = getNodeAsChild(root); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.TreeNodeMock leaf = TreeNodeTestData.getNodeAsChild(root); assertEquals(Optional.empty(), leaf.getFirstChild()); } @Test public void isNodeDescendantInFirstLevel() { - TreeNodeMock root = new TreeNodeMock(); - TreeNodeMock child = getNodeAsChild(root); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.TreeNodeMock child = TreeNodeTestData.getNodeAsChild(root); assertTrue(root.isNodeDescendant(child)); } @Test public void isNodeDescendantInComplex() { - TreeNodeMock root = new TreeNodeMock(); - TreeNodeMock descendant = getNodeInComplexTree(root); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.TreeNodeMock descendant = TreeNodeTestData.getNodeInComplexTree(root); assertTrue(root.isNodeDescendant(descendant)); } @Test public void getChildrenReturnsAllChildren() { - TreeNodeMock root = new TreeNodeMock(); - TreeNodeMock child1 = new TreeNodeMock(); - TreeNodeMock child2 = new TreeNodeMock(); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.TreeNodeMock child1 = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.TreeNodeMock child2 = new TreeNodeTestData.TreeNodeMock(); root.addChild(child1); root.addChild(child2); @@ -495,8 +414,8 @@ public void getChildrenReturnsAllChildren() { @Test public void removeChildSetsParentToEmpty() { - TreeNodeMock root = new TreeNodeMock(); - TreeNodeMock node = getNodeAsChild(root); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.TreeNodeMock node = TreeNodeTestData.getNodeAsChild(root); root.removeChild(node); assertEquals(Optional.empty(), node.getParent()); @@ -504,8 +423,8 @@ public void removeChildSetsParentToEmpty() { @Test public void removeChildRemovesNodeFromChildrenCollection() { - TreeNodeMock root = new TreeNodeMock(); - TreeNodeMock node = getNodeAsChild(root); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.TreeNodeMock node = TreeNodeTestData.getNodeAsChild(root); root.removeChild(node); assertFalse(root.getChildren().contains(node)); @@ -513,8 +432,8 @@ public void removeChildRemovesNodeFromChildrenCollection() { @Test public void removeChildIndexSetsParentToEmpty() { - TreeNodeMock root = new TreeNodeMock(); - TreeNodeMock node = getNodeAsChild(root); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.TreeNodeMock node = TreeNodeTestData.getNodeAsChild(root); root.removeChild(2); assertEquals(Optional.empty(), node.getParent()); @@ -522,8 +441,8 @@ public void removeChildIndexSetsParentToEmpty() { @Test public void removeChildIndexRemovesNodeFromChildrenCollection() { - TreeNodeMock root = new TreeNodeMock(); - TreeNodeMock node = getNodeAsChild(root); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.TreeNodeMock node = TreeNodeTestData.getNodeAsChild(root); root.removeChild(2); assertFalse(root.getChildren().contains(node)); @@ -531,18 +450,18 @@ public void removeChildIndexRemovesNodeFromChildrenCollection() { @Test(expected = UnsupportedOperationException.class) public void addThrowsExceptionIfNodeHasParent() { - TreeNodeMock root = new TreeNodeMock(); - TreeNodeMock node = getNodeAsChild(root); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.TreeNodeMock node = TreeNodeTestData.getNodeAsChild(root); root.addChild(node); } @Test public void moveAllChildrenToAddsAtSpecifiedPosition() { - TreeNodeMock root = new TreeNodeMock(); - TreeNodeMock node = new TreeNodeMock(); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.TreeNodeMock node = new TreeNodeTestData.TreeNodeMock(); root.addChild(node); - TreeNodeMock child1 = new TreeNodeMock(); - TreeNodeMock child2 = new TreeNodeMock(); + TreeNodeTestData.TreeNodeMock child1 = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.TreeNodeMock child2 = new TreeNodeTestData.TreeNodeMock(); node.addChild(child1); node.addChild(child2); @@ -552,11 +471,11 @@ public void moveAllChildrenToAddsAtSpecifiedPosition() { @Test public void moveAllChildrenToChangesParent() { - TreeNodeMock root = new TreeNodeMock(); - TreeNodeMock node = new TreeNodeMock(); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.TreeNodeMock node = new TreeNodeTestData.TreeNodeMock(); root.addChild(node); - TreeNodeMock child1 = new TreeNodeMock(); - TreeNodeMock child2 = new TreeNodeMock(); + TreeNodeTestData.TreeNodeMock child1 = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.TreeNodeMock child2 = new TreeNodeTestData.TreeNodeMock(); node.addChild(child1); node.addChild(child2); @@ -567,18 +486,18 @@ public void moveAllChildrenToChangesParent() { @Test(expected = UnsupportedOperationException.class) public void moveAllChildrenToDescendantThrowsException() { - TreeNodeMock root = new TreeNodeMock(); - TreeNodeMock node = getNodeAsChild(root); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.TreeNodeMock node = TreeNodeTestData.getNodeAsChild(root); root.moveAllChildrenTo(node, 0); } @Test public void sortChildrenSortsInFirstLevel() { - TreeNodeMock root = new TreeNodeMock(); - TreeNodeMock child1 = new TreeNodeMock("a"); - TreeNodeMock child2 = new TreeNodeMock("b"); - TreeNodeMock child3 = new TreeNodeMock("c"); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.TreeNodeMock child1 = new TreeNodeTestData.TreeNodeMock("a"); + TreeNodeTestData.TreeNodeMock child2 = new TreeNodeTestData.TreeNodeMock("b"); + TreeNodeTestData.TreeNodeMock child3 = new TreeNodeTestData.TreeNodeMock("c"); root.addChild(child2); root.addChild(child3); root.addChild(child1); @@ -589,11 +508,11 @@ public void sortChildrenSortsInFirstLevel() { @Test public void sortChildrenRecursiveSortsInDeeperLevel() { - TreeNodeMock root = new TreeNodeMock(); - TreeNodeMock node = getNodeInSimpleTree(root); - TreeNodeMock child1 = new TreeNodeMock("a"); - TreeNodeMock child2 = new TreeNodeMock("b"); - TreeNodeMock child3 = new TreeNodeMock("c"); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.TreeNodeMock node = TreeNodeTestData.getNodeInSimpleTree(root); + TreeNodeTestData.TreeNodeMock child1 = new TreeNodeTestData.TreeNodeMock("a"); + TreeNodeTestData.TreeNodeMock child2 = new TreeNodeTestData.TreeNodeMock("b"); + TreeNodeTestData.TreeNodeMock child3 = new TreeNodeTestData.TreeNodeMock("c"); node.addChild(child2); node.addChild(child3); node.addChild(child1); @@ -604,10 +523,10 @@ public void sortChildrenRecursiveSortsInDeeperLevel() { @Test public void copySubtreeCopiesChildren() { - TreeNodeMock root = new TreeNodeMock(); - TreeNodeMock node = getNodeAsChild(root); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.TreeNodeMock node = TreeNodeTestData.getNodeAsChild(root); - TreeNodeMock copiedRoot = root.copySubtree(); + TreeNodeTestData.TreeNodeMock copiedRoot = root.copySubtree(); assertEquals(Optional.empty(), copiedRoot.getParent()); assertFalse(copiedRoot.getChildren().contains(node)); assertEquals(root.getNumberOfChildren(), copiedRoot.getNumberOfChildren()); @@ -615,20 +534,20 @@ public void copySubtreeCopiesChildren() { @Test public void addChildSomewhereInTreeInvokesChangeEvent() { - TreeNodeMock root = new TreeNodeMock(); - TreeNodeMock node = getNodeInComplexTree(root); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.TreeNodeMock node = TreeNodeTestData.getNodeInComplexTree(root); root.subscribeToDescendantChanged(subscriber); - node.addChild(new TreeNodeMock()); + node.addChild(new TreeNodeTestData.TreeNodeMock()); verify(subscriber).accept(node); } @Test public void moveNodeSomewhereInTreeInvokesChangeEvent() { - TreeNodeMock root = new TreeNodeMock(); - TreeNodeMock node = getNodeInComplexTree(root); - TreeNodeMock oldParent = node.getParent().get(); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.TreeNodeMock node = TreeNodeTestData.getNodeInComplexTree(root); + TreeNodeTestData.TreeNodeMock oldParent = node.getParent().get(); root.subscribeToDescendantChanged(subscriber); @@ -639,9 +558,9 @@ public void moveNodeSomewhereInTreeInvokesChangeEvent() { @Test public void removeChildSomewhereInTreeInvokesChangeEvent() { - TreeNodeMock root = new TreeNodeMock(); - TreeNodeMock node = getNodeInComplexTree(root); - TreeNodeMock child = node.addChild(new TreeNodeMock()); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.TreeNodeMock node = TreeNodeTestData.getNodeInComplexTree(root); + TreeNodeTestData.TreeNodeMock child = node.addChild(new TreeNodeTestData.TreeNodeMock()); root.subscribeToDescendantChanged(subscriber); @@ -651,9 +570,9 @@ public void removeChildSomewhereInTreeInvokesChangeEvent() { @Test public void removeChildIndexSomewhereInTreeInvokesChangeEvent() { - TreeNodeMock root = new TreeNodeMock(); - TreeNodeMock node = getNodeInComplexTree(root); - node.addChild(new TreeNodeMock()); + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.TreeNodeMock node = TreeNodeTestData.getNodeInComplexTree(root); + node.addChild(new TreeNodeTestData.TreeNodeMock()); root.subscribeToDescendantChanged(subscriber); @@ -661,49 +580,16 @@ public void removeChildIndexSomewhereInTreeInvokesChangeEvent() { verify(subscriber).accept(node); } - /** - * This is just a dummy class deriving from TreeNode so that we can test the generic class - */ - private static class TreeNodeMock extends TreeNode { - - private final String name; - - public TreeNodeMock() { - this(""); - } - - public TreeNodeMock(String name) { - super(TreeNodeMock.class); - this.name = name; - } - - public String getName() { - return name; - } - - @Override - public String toString() { - return "TreeNodeMock{" + - "name='" + name + '\'' + - '}'; - } - - @Override - public TreeNodeMock copyNode() { - return new TreeNodeMock(name); - } - } - - private static class WrongTreeNodeImplementation extends TreeNode { + private static class WrongTreeNodeImplementation extends TreeNode { // This class is a wrong derived class of TreeNode // since it does not extends TreeNode // See test constructorChecksThatClassImplementsCorrectInterface public WrongTreeNodeImplementation() { - super(TreeNodeMock.class); + super(TreeNodeTestData.TreeNodeMock.class); } @Override - public TreeNodeMock copyNode() { + public TreeNodeTestData.TreeNodeMock copyNode() { return null; } } diff --git a/src/test/java/org/jabref/model/TreeNodeTestData.java b/src/test/java/org/jabref/model/TreeNodeTestData.java new file mode 100644 index 00000000000..939e9629292 --- /dev/null +++ b/src/test/java/org/jabref/model/TreeNodeTestData.java @@ -0,0 +1,121 @@ +package org.jabref.model; + +public class TreeNodeTestData { + /** + * Gets the marked node in the following tree: + * Root + * A + * A (= parent) + * B (<-- this) + */ + public static TreeNodeMock getNodeInSimpleTree(TreeNodeMock root) { + root.addChild(new TreeNodeMock()); + TreeNodeMock parent = new TreeNodeMock(); + root.addChild(parent); + TreeNodeMock node = new TreeNodeMock(); + parent.addChild(node); + return node; + } + + public static TreeNodeMock getNodeInSimpleTree() { + return getNodeInSimpleTree(new TreeNodeMock()); + } + + /** + * Gets the marked node in the following tree: + * Root + * A + * A + * A (= grand parent) + * B + * B (= parent) + * C (<-- this) + * D (= child) + * C + * C + * C + * B + * B + * A + */ + public static TreeNodeMock getNodeInComplexTree(TreeNodeMock root) { + root.addChild(new TreeNodeMock()); + root.addChild(new TreeNodeMock()); + TreeNodeMock grandParent = new TreeNodeMock(); + root.addChild(grandParent); + root.addChild(new TreeNodeMock()); + + grandParent.addChild(new TreeNodeMock()); + TreeNodeMock parent = new TreeNodeMock(); + grandParent.addChild(parent); + grandParent.addChild(new TreeNodeMock()); + grandParent.addChild(new TreeNodeMock()); + + TreeNodeMock node = new TreeNodeMock(); + parent.addChild(node); + parent.addChild(new TreeNodeMock()); + parent.addChild(new TreeNodeMock()); + parent.addChild(new TreeNodeMock()); + + node.addChild(new TreeNodeMock()); + return node; + } + + public static TreeNodeMock getNodeInComplexTree() { + return getNodeInComplexTree(new TreeNodeMock()); + } + + /** + * Gets the marked in the following tree: + * Root + * A + * A + * A (<- this) + * A + */ + public static TreeNodeMock getNodeAsChild(TreeNodeMock root) { + root.addChild(new TreeNodeMock()); + root.addChild(new TreeNodeMock()); + TreeNodeMock node = new TreeNodeMock(); + root.addChild(node); + root.addChild(new TreeNodeMock()); + return node; + } + + /** + * This is just a dummy class deriving from TreeNode so that we can test the generic class + */ + public static class TreeNodeMock extends TreeNode { + + private String name; + + public TreeNodeMock() { + this(""); + } + + public TreeNodeMock(String name) { + super(TreeNodeMock.class); + this.name = name; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + @Override + public String toString() { + return "TreeNodeMock{" + + "name='" + name + '\'' + + '}'; + } + + @Override + public TreeNodeMock copyNode() { + return new TreeNodeMock(name); + } + } +} From 699ae73f17fba7f66e2f597bb6dd793097568d0d Mon Sep 17 00:00:00 2001 From: Tobias Diez Date: Fri, 3 Mar 2017 21:41:25 +0100 Subject: [PATCH 4/4] Entries are filtered again correctly --- src/main/java/org/jabref/gui/groups/GroupTreeController.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/main/java/org/jabref/gui/groups/GroupTreeController.java b/src/main/java/org/jabref/gui/groups/GroupTreeController.java index 298cb85e44d..a38f63a68d0 100644 --- a/src/main/java/org/jabref/gui/groups/GroupTreeController.java +++ b/src/main/java/org/jabref/gui/groups/GroupTreeController.java @@ -34,7 +34,6 @@ import org.controlsfx.control.textfield.CustomTextField; import org.controlsfx.control.textfield.TextFields; import org.fxmisc.easybind.EasyBind; -import org.fxmisc.easybind.monadic.PropertyBinding; public class GroupTreeController extends AbstractController { @@ -54,7 +53,7 @@ public void initialize() { viewModel = new GroupTreeViewModel(stateManager, dialogService); // Set-up bindings - viewModel.selectedGroupProperty().bindBidirectional( + viewModel.selectedGroupProperty().bind( EasyBind.monadic(groupTree.selectionModelProperty()) .flatMap(SelectionModel::selectedItemProperty) .selectProperty(TreeItem::valueProperty)