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

Fixed a regression that could causes a ClassCastException on loadState()
in case of GroupBy configurations for different columns of different
types.

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

Change-Id: I2385c3d9e78cb9957d696c1ccf1c0e6cd06daec2

4 files changed, 154 insertions, 5 deletions

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 77c21f36..a1e92b60 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
@@ -19,7 +19,9 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Comparator;
+import java.util.Date;
 import java.util.List;
+import java.util.Properties;
 
 import org.eclipse.nebula.widgets.nattable.config.ConfigRegistry;
 import org.eclipse.nebula.widgets.nattable.config.DefaultComparator;
@@ -30,7 +32,7 @@ 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.filterrow.DefaultGlazedListsStaticFilterStrategy;
 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;
@@ -164,7 +166,7 @@ public class GroupByDataLayerTest {
                 new DefaultColumnHeaderDataLayer(columnHeaderDataProvider);
 
         this.filterRowDataProvider = new FilterRowDataProvider<>(
-                new DefaultGlazedListsFilterStrategy<>(
+                new DefaultGlazedListsStaticFilterStrategy<>(
                         this.filterList,
                         this.columnPropertyAccessor,
                         this.configRegistry),
@@ -1155,4 +1157,107 @@ public class GroupByDataLayerTest {
         assertTrue(stack.hasLabel("ROW_1"));
         assertFalse(stack.hasLabel(GroupByDataLayer.GROUP_BY_OBJECT));
     }
+
+    @Test
+    public void shouldSwitchGroupingTypesOnLoad() {
+        // change the birthday to get reliable results
+        @SuppressWarnings("deprecation")
+        Date temp1 = new Date(1978, 9, 13);
+        this.filterList.get(0).setBirthday(temp1);
+        this.filterList.get(1).setBirthday(temp1);
+        this.filterList.get(2).setBirthday(temp1);
+        this.filterList.get(3).setBirthday(temp1);
+        this.filterList.get(4).setBirthday(temp1);
+        this.filterList.get(5).setBirthday(temp1);
+
+        @SuppressWarnings("deprecation")
+        Date temp2 = new Date(1976, 1, 24);
+        this.filterList.get(6).setBirthday(temp2);
+        this.filterList.get(7).setBirthday(temp2);
+        this.filterList.get(8).setBirthday(temp2);
+        this.filterList.get(9).setBirthday(temp2);
+
+        @SuppressWarnings("deprecation")
+        Date temp3 = new Date(2012, 0, 19);
+        this.filterList.get(10).setBirthday(temp3);
+        this.filterList.get(11).setBirthday(temp3);
+        this.filterList.get(12).setBirthday(temp3);
+        this.filterList.get(13).setBirthday(temp3);
+        this.filterList.get(14).setBirthday(temp3);
+        this.filterList.get(15).setBirthday(temp3);
+        this.filterList.get(16).setBirthday(temp3);
+        this.filterList.get(17).setBirthday(temp3);
+
+        // add a static filter so a list change is propagated on update()
+        addFilterCapability();
+        ((DefaultGlazedListsStaticFilterStrategy<Person>) this.filterRowDataProvider.getFilterStrategy()).addStaticFilter(item -> !item.getFirstName().equals("Ned"));
+
+        assertEquals(16, this.filterList.size());
+
+        // groupBy lastname
+        this.groupByModel.addGroupByColumnIndex(1);
+        // 16 data rows + 2 GroupBy rows
+        assertEquals(18, this.dataLayer.getRowCount());
+
+        Object o = this.dataLayer.getTreeList().get(0);
+        assertTrue(o instanceof GroupByObject, "Object is not a GroupByObject");
+        assertEquals("Flanders", ((GroupByObject) o).getValue());
+        o = this.dataLayer.getTreeList().get(7);
+        assertTrue(o instanceof GroupByObject, "Object is not a GroupByObject");
+        assertEquals("Simpson", ((GroupByObject) o).getValue());
+
+        // save state
+        Properties props = new Properties();
+        this.groupByModel.saveState("lastname", props);
+
+        // clear grouping
+        this.groupByModel.clearGroupByColumnIndexes();
+
+        // group by birthday
+        this.groupByModel.addGroupByColumnIndex(5);
+        // 16 data rows + 3 GroupBy rows
+        assertEquals(19, this.dataLayer.getRowCount());
+
+        o = this.dataLayer.getTreeList().get(0);
+        assertTrue(o instanceof GroupByObject, "Object is not a GroupByObject");
+        assertEquals(temp2, ((GroupByObject) o).getValue());
+        o = this.dataLayer.getTreeList().get(5);
+        assertTrue(o instanceof GroupByObject, "Object is not a GroupByObject");
+        assertEquals(temp1, ((GroupByObject) o).getValue());
+        o = this.dataLayer.getTreeList().get(12);
+        assertTrue(o instanceof GroupByObject, "Object is not a GroupByObject");
+        assertEquals(temp3, ((GroupByObject) o).getValue());
+
+        // save state
+        this.groupByModel.saveState("birthday", props);
+
+        // clear grouping
+        this.groupByModel.clearGroupByColumnIndexes();
+
+        // load first state
+        this.groupByModel.loadState("lastname", props);
+
+        assertEquals(18, this.dataLayer.getRowCount());
+        o = this.dataLayer.getTreeList().get(0);
+        assertTrue(o instanceof GroupByObject, "Object is not a GroupByObject");
+        assertEquals("Flanders", ((GroupByObject) o).getValue());
+        o = this.dataLayer.getTreeList().get(7);
+        assertTrue(o instanceof GroupByObject, "Object is not a GroupByObject");
+        assertEquals("Simpson", ((GroupByObject) o).getValue());
+
+        // load second state
+        this.groupByModel.loadState("birthday", props);
+
+        assertEquals(19, this.dataLayer.getRowCount());
+        o = this.dataLayer.getTreeList().get(0);
+        assertTrue(o instanceof GroupByObject, "Object is not a GroupByObject");
+        assertEquals(temp2, ((GroupByObject) o).getValue());
+        o = this.dataLayer.getTreeList().get(5);
+        assertTrue(o instanceof GroupByObject, "Object is not a GroupByObject");
+        assertEquals(temp1, ((GroupByObject) o).getValue());
+        o = this.dataLayer.getTreeList().get(12);
+        assertTrue(o instanceof GroupByObject, "Object is not a GroupByObject");
+        assertEquals(temp3, ((GroupByObject) o).getValue());
+
+    }
 }
diff --git a/org.eclipse.nebula.widgets.nattable.extension.glazedlists/.settings/.api_filters b/org.eclipse.nebula.widgets.nattable.extension.glazedlists/.settings/.api_filters
new file mode 100644
index 00000000..29182a8f
--- /dev/null
+++ b/org.eclipse.nebula.widgets.nattable.extension.glazedlists/.settings/.api_filters
@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+<component id="org.eclipse.nebula.widgets.nattable.extension.glazedlists" version="2">
+    <resource path="src/org/eclipse/nebula/widgets/nattable/extension/glazedlists/groupBy/GroupByModel.java" type="org.eclipse.nebula.widgets.nattable.extension.glazedlists.groupBy.GroupByModel">
+        <filter id="336658481">
+            <message_arguments>
+                <message_argument value="org.eclipse.nebula.widgets.nattable.extension.glazedlists.groupBy.GroupByModel"/>
+                <message_argument value="LOAD_STATE_INDICATOR"/>
+            </message_arguments>
+        </filter>
+    </resource>
+</component>
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 488c73a8..e9b9ccd6 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
@@ -111,6 +111,10 @@ public class GroupByDataLayer<T> extends DataLayer implements Observer {
      */
     public static final String GROUP_BY_SUMMARY_COLUMN_PREFIX = "GROUP_BY_SUMMARY_COLUMN_"; //$NON-NLS-1$
     /**
+     * The {@link GroupByModel} used for grouping.
+     */
+    private final GroupByModel groupByModel;
+    /**
      * The underlying base EventList.
      */
     private final EventList<T> eventList;
@@ -467,7 +471,8 @@ public class GroupByDataLayer<T> extends DataLayer implements Observer {
         this.eventList = eventList;
         this.columnAccessor = columnAccessor;
 
-        groupByModel.addObserver(this);
+        this.groupByModel = groupByModel;
+        this.groupByModel.addObserver(this);
 
         this.groupByColumnAccessor = new GroupByColumnAccessor(columnAccessor);
 
@@ -630,6 +635,20 @@ public class GroupByDataLayer<T> extends DataLayer implements Observer {
 
     @Override
     public void update(Observable o, Object arg) {
+        List<Integer> indexes = new ArrayList<>();
+        if (GroupByModel.LOAD_STATE_INDICATOR.equals(arg)) {
+            // if the GroupByModel was loaded and not simply updated, there can
+            // be situations where the groupby indexes and the corresponding
+            // types per column are totally different. In this case we first
+            // need to perform a clear operation without the new groupby
+            // configuration, and then re-apply everything. Otherwise the filter
+            // operation might trigger a list update which then leads to
+            // ClassCastExceptions as the old GroupByObjects are still in the
+            // TreeList.
+            indexes.addAll(this.groupByModel.getGroupByColumnIndexes());
+            this.groupByModel.getGroupByColumnIndexes().clear();
+        }
+
         // 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)
@@ -663,6 +682,10 @@ public class GroupByDataLayer<T> extends DataLayer implements Observer {
             this.filterRowDataProvider.getFilterStrategy().applyFilter(original);
         }
 
+        if (GroupByModel.LOAD_STATE_INDICATOR.equals(arg)) {
+            this.groupByModel.getGroupByColumnIndexes().addAll(indexes);
+        }
+
         updateTree();
 
         // re-apply the filter after the tree update
diff --git a/org.eclipse.nebula.widgets.nattable.extension.glazedlists/src/org/eclipse/nebula/widgets/nattable/extension/glazedlists/groupBy/GroupByModel.java b/org.eclipse.nebula.widgets.nattable.extension.glazedlists/src/org/eclipse/nebula/widgets/nattable/extension/glazedlists/groupBy/GroupByModel.java
index 4e262667..f34c9de7 100644
--- a/org.eclipse.nebula.widgets.nattable.extension.glazedlists/src/org/eclipse/nebula/widgets/nattable/extension/glazedlists/groupBy/GroupByModel.java
+++ b/org.eclipse.nebula.widgets.nattable.extension.glazedlists/src/org/eclipse/nebula/widgets/nattable/extension/glazedlists/groupBy/GroupByModel.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2012, 2020 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
@@ -29,6 +29,15 @@ public class GroupByModel extends Observable implements IPersistable {
 
     public static final String PERSISTENCE_KEY_GROUP_BY_COLUMN_INDEXES = ".groupByColumnIndexes"; //$NON-NLS-1$
 
+    /**
+     * Argument that is passed to {@link #notifyObservers(Object)} to inform
+     * about a complete change. Needed to handle the tree update slightly
+     * different.
+     *
+     * @since 2.1
+     */
+    public static final String LOAD_STATE_INDICATOR = "LOAD_STATE"; //$NON-NLS-1$
+
     private List<Integer> groupByColumnIndexes = new ArrayList<>();
 
     /**
@@ -112,7 +121,8 @@ public class GroupByModel extends Observable implements IPersistable {
             }
         }
 
-        update();
+        setChanged();
+        notifyObservers(LOAD_STATE_INDICATOR);
     }
 
     /**