Bug 581321 - [GroupBy] list order changed after grouping a filtered list

Signed-off-by: Dirk Fauth <dirk.fauth@googlemail.com>

Change-Id: I549d26ad661f64aa8fdedca372ad373a516a804e

8 files changed, 142 insertions, 9 deletions

diff --git a/org.eclipse.nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/filterrow/FilterRowDataProvider.java b/org.eclipse.nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/filterrow/FilterRowDataProvider.java
index ee9cb59b..eea4bf30 100644
--- a/org.eclipse.nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/filterrow/FilterRowDataProvider.java
+++ b/org.eclipse.nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/filterrow/FilterRowDataProvider.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2012, 2022 Original authors and others.
+ * Copyright (c) 2012, 2023 Original authors and others.
  *
  * This program and the accompanying materials are made
  * available under the terms of the Eclipse Public License 2.0
@@ -336,4 +336,15 @@ public class FilterRowDataProvider<T> implements IDataProvider, IPersistable {
         this.columnHeaderLayer.fireLayerEvent(new FilterAppliedEvent(this.columnHeaderLayer));
     }
 
+    /**
+     *
+     * @return The {@link IFilterStrategy} to which the set filter value should
+     *         be applied.
+     *
+     * @since 2.1
+     */
+    public IFilterStrategy<T> getFilterStrategy() {
+        return this.filterStrategy;
+    }
+
 }
diff --git a/org.eclipse.nebula.widgets.nattable.examples/src/org/eclipse/nebula/widgets/nattable/examples/_800_Integration/_808_SortableGroupByWithFilterExample.java b/org.eclipse.nebula.widgets.nattable.examples/src/org/eclipse/nebula/widgets/nattable/examples/_800_Integration/_808_SortableGroupByWithFilterExample.java
index 008758a9..bd2080ae 100644
--- a/org.eclipse.nebula.widgets.nattable.examples/src/org/eclipse/nebula/widgets/nattable/examples/_800_Integration/_808_SortableGroupByWithFilterExample.java
+++ b/org.eclipse.nebula.widgets.nattable.examples/src/org/eclipse/nebula/widgets/nattable/examples/_800_Integration/_808_SortableGroupByWithFilterExample.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2013, 2021 Dirk Fauth and others.
+ * Copyright (c) 2013, 2023 Dirk Fauth and others.
  *
  * This program and the accompanying materials are made
  * available under the terms of the Eclipse Public License 2.0
@@ -189,6 +189,9 @@ public class _808_SortableGroupByWithFilterExample extends AbstractNatExample {
                         columnHeaderDataLayer.getDataProvider(),
                         configRegistry);
 
+        bodyLayerStack.getBodyDataLayer().enableFilterSupport(
+                filterRowHeaderLayer.getFilterRowDataLayer().getFilterRowDataProvider());
+
         // build the row header layer
         IDataProvider rowHeaderDataProvider =
                 new DefaultRowHeaderDataProvider(bodyLayerStack.getBodyDataProvider());
diff --git a/org.eclipse.nebula.widgets.nattable.examples/src/org/eclipse/nebula/widgets/nattable/examples/_800_Integration/_810_SortableGroupByFilterColumnGroupAndFreezeExample.java b/org.eclipse.nebula.widgets.nattable.examples/src/org/eclipse/nebula/widgets/nattable/examples/_800_Integration/_810_SortableGroupByFilterColumnGroupAndFreezeExample.java
index b4f1ba21..f087af9a 100644
--- a/org.eclipse.nebula.widgets.nattable.examples/src/org/eclipse/nebula/widgets/nattable/examples/_800_Integration/_810_SortableGroupByFilterColumnGroupAndFreezeExample.java
+++ b/org.eclipse.nebula.widgets.nattable.examples/src/org/eclipse/nebula/widgets/nattable/examples/_800_Integration/_810_SortableGroupByFilterColumnGroupAndFreezeExample.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2013, 2021 Dirk Fauth and others.
+ * Copyright (c) 2013, 2023 Dirk Fauth and others.
  *
  * This program and the accompanying materials are made
  * available under the terms of the Eclipse Public License 2.0
@@ -230,6 +230,9 @@ public class _810_SortableGroupByFilterColumnGroupAndFreezeExample extends Abstr
                         columnHeaderDataLayer.getDataProvider(),
                         configRegistry);
 
+        bodyLayerStack.getBodyDataLayer().enableFilterSupport(
+                filterRowHeaderLayer.getFilterRowDataLayer().getFilterRowDataProvider());
+
         // Row header
         // Adding the specialized DefaultSummaryRowHeaderDataProvider to
         // indicate the summary row in the row header
diff --git a/org.eclipse.nebula.widgets.nattable.examples/src/org/eclipse/nebula/widgets/nattable/examples/_800_Integration/_813_SortableGroupByWithComboBoxFilterExample.java b/org.eclipse.nebula.widgets.nattable.examples/src/org/eclipse/nebula/widgets/nattable/examples/_800_Integration/_813_SortableGroupByWithComboBoxFilterExample.java
index 706fb1dc..845712cf 100644
--- a/org.eclipse.nebula.widgets.nattable.examples/src/org/eclipse/nebula/widgets/nattable/examples/_800_Integration/_813_SortableGroupByWithComboBoxFilterExample.java
+++ b/org.eclipse.nebula.widgets.nattable.examples/src/org/eclipse/nebula/widgets/nattable/examples/_800_Integration/_813_SortableGroupByWithComboBoxFilterExample.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2016, 2021 Dirk Fauth and others.
+ * Copyright (c) 2016, 2023 Dirk Fauth and others.
  *
  * This program and the accompanying materials are made
  * available under the terms of the Eclipse Public License 2.0
@@ -227,6 +227,9 @@ public class _813_SortableGroupByWithComboBoxFilterExample extends AbstractNatEx
                         columnHeaderDataProvider,
                         configRegistry);
 
+        bodyLayerStack.getBodyDataLayer().enableFilterSupport(
+                filterRowHeaderLayer.getFilterRowDataLayer().getFilterRowDataProvider());
+
         // build the row header layer
         IDataProvider rowHeaderDataProvider =
                 new DefaultRowHeaderDataProvider(bodyLayerStack.getBodyDataProvider());
diff --git a/org.eclipse.nebula.widgets.nattable.examples/src/org/eclipse/nebula/widgets/nattable/examples/_800_Integration/_814_EditableSortableGroupByWithFilterExample.java b/org.eclipse.nebula.widgets.nattable.examples/src/org/eclipse/nebula/widgets/nattable/examples/_800_Integration/_814_EditableSortableGroupByWithFilterExample.java
index 471e1609..0a33f3e3 100644
--- a/org.eclipse.nebula.widgets.nattable.examples/src/org/eclipse/nebula/widgets/nattable/examples/_800_Integration/_814_EditableSortableGroupByWithFilterExample.java
+++ b/org.eclipse.nebula.widgets.nattable.examples/src/org/eclipse/nebula/widgets/nattable/examples/_800_Integration/_814_EditableSortableGroupByWithFilterExample.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2013, 2021 Dirk Fauth and others.
+ * Copyright (c) 2013, 2023 Dirk Fauth and others.
  *
  * This program and the accompanying materials are made
  * available under the terms of the Eclipse Public License 2.0
@@ -262,6 +262,9 @@ public class _814_EditableSortableGroupByWithFilterExample extends AbstractNatEx
                         columnHeaderDataLayer.getDataProvider(),
                         configRegistry);
 
+        bodyLayerStack.getBodyDataLayer().enableFilterSupport(
+                filterRowHeaderLayer.getFilterRowDataLayer().getFilterRowDataProvider());
+
         // build the row header layer
         IDataProvider rowHeaderDataProvider =
                 new DefaultRowHeaderDataProvider(bodyLayerStack.getBodyDataProvider());
diff --git a/org.eclipse.nebula.widgets.nattable.examples/src/org/eclipse/nebula/widgets/nattable/examples/_800_Integration/_815_SortableGroupByFilterPerformanceColumnGroupAndFreezeExample.java b/org.eclipse.nebula.widgets.nattable.examples/src/org/eclipse/nebula/widgets/nattable/examples/_800_Integration/_815_SortableGroupByFilterPerformanceColumnGroupAndFreezeExample.java
index a88f27d4..f4c74702 100644
--- a/org.eclipse.nebula.widgets.nattable.examples/src/org/eclipse/nebula/widgets/nattable/examples/_800_Integration/_815_SortableGroupByFilterPerformanceColumnGroupAndFreezeExample.java
+++ b/org.eclipse.nebula.widgets.nattable.examples/src/org/eclipse/nebula/widgets/nattable/examples/_800_Integration/_815_SortableGroupByFilterPerformanceColumnGroupAndFreezeExample.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2019, 2021 Dirk Fauth and others.
+ * Copyright (c) 2019, 2023 Dirk Fauth and others.
  *
  * This program and the accompanying materials are made
  * available under the terms of the Eclipse Public License 2.0
@@ -225,6 +225,9 @@ public class _815_SortableGroupByFilterPerformanceColumnGroupAndFreezeExample ex
                         columnHeaderDataLayer.getDataProvider(),
                         configRegistry);
 
+        bodyLayerStack.getBodyDataLayer().enableFilterSupport(
+                filterRowHeaderLayer.getFilterRowDataLayer().getFilterRowDataProvider());
+
         // Row header
         // Adding the specialized DefaultSummaryRowHeaderDataProvider to
         // indicate the summary row in the row header
diff --git a/org.eclipse.nebula.widgets.nattable.extension.glazedlists.test/src/org/eclipse/nebula/widgets/nattable/extension/glazedlists/groupby/GroupByDataLayerTest.java b/org.eclipse.nebula.widgets.nattable.extension.glazedlists.test/src/org/eclipse/nebula/widgets/nattable/extension/glazedlists/groupby/GroupByDataLayerTest.java
index e615a46c..ced5ef10 100644
--- a/org.eclipse.nebula.widgets.nattable.extension.glazedlists.test/src/org/eclipse/nebula/widgets/nattable/extension/glazedlists/groupby/GroupByDataLayerTest.java
+++ b/org.eclipse.nebula.widgets.nattable.extension.glazedlists.test/src/org/eclipse/nebula/widgets/nattable/extension/glazedlists/groupby/GroupByDataLayerTest.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2014, 2022 Dirk Fauth and others.
+ * Copyright (c) 2014, 2023 Dirk Fauth and others.
  *
  * This program and the accompanying materials are made
  * available under the terms of the Eclipse Public License 2.0
@@ -26,15 +26,20 @@ import org.eclipse.nebula.widgets.nattable.config.DefaultComparator;
 import org.eclipse.nebula.widgets.nattable.data.IColumnPropertyAccessor;
 import org.eclipse.nebula.widgets.nattable.data.IDataProvider;
 import org.eclipse.nebula.widgets.nattable.data.ReflectiveColumnPropertyAccessor;
+import org.eclipse.nebula.widgets.nattable.data.convert.DefaultDisplayConverter;
 import org.eclipse.nebula.widgets.nattable.dataset.person.Person;
 import org.eclipse.nebula.widgets.nattable.dataset.person.PersonService;
 import org.eclipse.nebula.widgets.nattable.extension.glazedlists.GlazedListsSortModel;
+import org.eclipse.nebula.widgets.nattable.extension.glazedlists.filterrow.DefaultGlazedListsFilterStrategy;
 import org.eclipse.nebula.widgets.nattable.extension.glazedlists.groupBy.GroupByComparator;
 import org.eclipse.nebula.widgets.nattable.extension.glazedlists.groupBy.GroupByConfigAttributes;
 import org.eclipse.nebula.widgets.nattable.extension.glazedlists.groupBy.GroupByDataLayer;
 import org.eclipse.nebula.widgets.nattable.extension.glazedlists.groupBy.GroupByModel;
 import org.eclipse.nebula.widgets.nattable.extension.glazedlists.groupBy.GroupByObject;
 import org.eclipse.nebula.widgets.nattable.extension.glazedlists.groupBy.summary.SummationGroupBySummaryProvider;
+import org.eclipse.nebula.widgets.nattable.filterrow.FilterRowDataProvider;
+import org.eclipse.nebula.widgets.nattable.filterrow.TextMatchingMode;
+import org.eclipse.nebula.widgets.nattable.filterrow.config.FilterRowConfigAttributes;
 import org.eclipse.nebula.widgets.nattable.grid.data.DefaultColumnHeaderDataProvider;
 import org.eclipse.nebula.widgets.nattable.grid.layer.DefaultColumnHeaderDataLayer;
 import org.eclipse.nebula.widgets.nattable.layer.DataLayer;
@@ -50,6 +55,7 @@ import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
 import ca.odell.glazedlists.EventList;
+import ca.odell.glazedlists.FilterList;
 import ca.odell.glazedlists.GlazedLists;
 import ca.odell.glazedlists.SortedList;
 import ca.odell.glazedlists.event.ListEvent;
@@ -67,6 +73,9 @@ public class GroupByDataLayerTest {
     SortedList<Person> sortedList;
     ISortModel sortModel;
 
+    FilterList<Person> filterList;
+    FilterRowDataProvider<Person> filterRowDataProvider;
+
     static final String MY_LABEL = "myLabel";
 
     // property names of the Person class
@@ -77,10 +86,11 @@ public class GroupByDataLayerTest {
         this.groupByModel = new GroupByModel();
         EventList<Person> eventList = GlazedLists.eventList(PersonService.getFixedPersons());
         this.sortedList = new SortedList<>(eventList, null);
+        this.filterList = new FilterList<>(this.sortedList);
 
         this.columnPropertyAccessor = new ReflectiveColumnPropertyAccessor<>(this.propertyNames);
 
-        this.dataLayer = new GroupByDataLayer<>(this.groupByModel, this.sortedList, this.columnPropertyAccessor, this.configRegistry);
+        this.dataLayer = new GroupByDataLayer<>(this.groupByModel, this.filterList, this.columnPropertyAccessor, this.configRegistry);
         this.dataLayer.setConfigLabelAccumulator(new ColumnLabelAccumulator());
     }
 
@@ -147,6 +157,31 @@ public class GroupByDataLayerTest {
                 DefaultComparator.getInstance());
     }
 
+    void addFilterCapability() {
+        IDataProvider columnHeaderDataProvider =
+                new DefaultColumnHeaderDataProvider(this.propertyNames);
+        DataLayer columnHeaderDataLayer =
+                new DefaultColumnHeaderDataLayer(columnHeaderDataProvider);
+
+        this.filterRowDataProvider = new FilterRowDataProvider<>(
+                new DefaultGlazedListsFilterStrategy<>(
+                        this.filterList,
+                        this.columnPropertyAccessor,
+                        this.configRegistry),
+                columnHeaderDataLayer,
+                columnHeaderDataProvider,
+                this.configRegistry);
+
+        this.configRegistry.registerConfigAttribute(
+                FilterRowConfigAttributes.FILTER_DISPLAY_CONVERTER,
+                new DefaultDisplayConverter());
+        this.configRegistry.registerConfigAttribute(
+                FilterRowConfigAttributes.TEXT_MATCHING_MODE,
+                TextMatchingMode.CONTAINS);
+
+        this.dataLayer.enableFilterSupport(this.filterRowDataProvider);
+    }
+
     @Test
     public void testOneLevelGrouping() {
         assertEquals(18, this.dataLayer.getRowCount());
@@ -1071,4 +1106,36 @@ public class GroupByDataLayerTest {
         assertEquals(28, this.dataLayer.getRowCount());
     }
 
+    @Test
+    public void shouldKeepInitialOrderOnFilteredGrouping() {
+        addFilterCapability();
+
+        assertEquals(18, this.dataLayer.getRowCount());
+        assertEquals("Homer", this.dataLayer.getDataValue(0, 0));
+
+        // apply a filter
+        this.filterRowDataProvider.setDataValue(0, 0, "Homer");
+        assertEquals(3, this.dataLayer.getRowCount());
+
+        // group by last name
+        this.groupByModel.addGroupByColumnIndex(1);
+
+        assertEquals(4, this.dataLayer.getRowCount());
+        Object o = this.dataLayer.getTreeList().get(0);
+        assertTrue(o instanceof GroupByObject, "Object is not a GroupByObject");
+        assertEquals("Simpson", ((GroupByObject) o).getValue());
+
+        // ungroup lastname
+        this.groupByModel.removeGroupByColumnIndex(1);
+        assertEquals(3, this.dataLayer.getRowCount());
+
+        // remove the filter
+        this.filterRowDataProvider.setDataValue(0, 0, null);
+        assertEquals(18, this.dataLayer.getRowCount());
+
+        // Homer should be still at the first position
+        // without setting the FilterRowDataProvider to the GroupByDataLayer,
+        // this will fail
+        assertEquals("Homer", this.dataLayer.getDataValue(0, 0));
+    }
 }
diff --git a/org.eclipse.nebula.widgets.nattable.extension.glazedlists/src/org/eclipse/nebula/widgets/nattable/extension/glazedlists/groupBy/GroupByDataLayer.java b/org.eclipse.nebula.widgets.nattable.extension.glazedlists/src/org/eclipse/nebula/widgets/nattable/extension/glazedlists/groupBy/GroupByDataLayer.java
index c565ca43..017f5178 100644
--- a/org.eclipse.nebula.widgets.nattable.extension.glazedlists/src/org/eclipse/nebula/widgets/nattable/extension/glazedlists/groupBy/GroupByDataLayer.java
+++ b/org.eclipse.nebula.widgets.nattable.extension.glazedlists/src/org/eclipse/nebula/widgets/nattable/extension/glazedlists/groupBy/GroupByDataLayer.java
@@ -21,6 +21,7 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Comparator;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
@@ -39,6 +40,7 @@ import org.eclipse.nebula.widgets.nattable.data.ListDataProvider;
 import org.eclipse.nebula.widgets.nattable.extension.glazedlists.groupBy.summary.IGroupBySummaryProvider;
 import org.eclipse.nebula.widgets.nattable.extension.glazedlists.tree.GlazedListTreeData;
 import org.eclipse.nebula.widgets.nattable.extension.glazedlists.tree.GlazedListTreeRowModel;
+import org.eclipse.nebula.widgets.nattable.filterrow.FilterRowDataProvider;
 import org.eclipse.nebula.widgets.nattable.layer.DataLayer;
 import org.eclipse.nebula.widgets.nattable.layer.IUniqueIndexLayer;
 import org.eclipse.nebula.widgets.nattable.layer.LabelStack;
@@ -155,6 +157,12 @@ public class GroupByDataLayer<T> extends DataLayer implements Observer {
     private Matcher<Object> groupByMatcher = item -> item instanceof GroupByObject;
 
     /**
+     * The {@link FilterRowDataProvider} that is used in the composition. Needed
+     * to be able to re-apply possible filter states on tree updates.
+     */
+    private FilterRowDataProvider<T> filterRowDataProvider;
+
+    /**
      * Create a new {@link GroupByDataLayer} with the given configuration that:
      * <ul>
      * <li>uses the default <code>GroupByExpansionModel</code> which shows all
@@ -539,6 +547,17 @@ public class GroupByDataLayer<T> extends DataLayer implements Observer {
     }
 
     /**
+     * @param provider
+     *            The {@link FilterRowDataProvider} that is used in the
+     *            composition. Needed to be able to re-apply possible filter
+     *            states on tree updates.
+     * @since 2.1
+     */
+    public void enableFilterSupport(FilterRowDataProvider<T> provider) {
+        this.filterRowDataProvider = provider;
+    }
+
+    /**
      *
      * @param comparator
      *            The {@link IGroupByComparator} that is necessary to create the
@@ -613,7 +632,7 @@ public class GroupByDataLayer<T> extends DataLayer implements Observer {
         // if we know the sort model, we need to clear the sort model to avoid
         // strange side effects while updating the tree structure (e.g. not
         // applied sorting although showing the sort indicator)
-        // for better user experience we remember the sort state and reapply it
+        // for better user experience we remember the sort state and re-apply it
         // after the tree update
         List<Integer> sortedIndexes = Collections.emptyList();
         List<SortDirectionEnum> sortDirections = new ArrayList<>();
@@ -625,8 +644,29 @@ public class GroupByDataLayer<T> extends DataLayer implements Observer {
             this.treeFormat.getSortModel().clear();
         }
 
+        // if we know the filter row data provider, we need to remove the filter
+        // to avoid strange side effects while updating the tree structure (e.g.
+        // previously filtered items are not back at the original position but
+        // will be moved to the end of list after the filter is removed)
+        // for better user experience we remember the filter state and re-apply
+        // it after the tree update
+        Map<Integer, Object> filterCopy = Collections.emptyMap();
+        if (this.filterRowDataProvider != null) {
+            Map<Integer, Object> original = this.filterRowDataProvider.getFilterIndexToObjectMap();
+            filterCopy = new HashMap<>(original);
+            original.clear();
+            this.filterRowDataProvider.getFilterStrategy().applyFilter(original);
+        }
+
         updateTree();
 
+        // re-apply the filter after the tree update
+        if (this.filterRowDataProvider != null) {
+            Map<Integer, Object> original = this.filterRowDataProvider.getFilterIndexToObjectMap();
+            original.putAll(filterCopy);
+            this.filterRowDataProvider.getFilterStrategy().applyFilter(original);
+        }
+
         // re-apply the sorting after the tree update
         if (this.treeFormat.getSortModel() != null) {
             for (int i = 0; i < sortedIndexes.size(); i++) {