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
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 77c21f3..a1e92b6 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 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.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 @@
new DefaultColumnHeaderDataLayer(columnHeaderDataProvider);
this.filterRowDataProvider = new FilterRowDataProvider<>(
- new DefaultGlazedListsFilterStrategy<>(
+ new DefaultGlazedListsStaticFilterStrategy<>(
this.filterList,
this.columnPropertyAccessor,
this.configRegistry),
@@ -1155,4 +1157,107 @@
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 0000000..29182a8
--- /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 488c73a..e9b9ccd 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 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 @@
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 @@
@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 @@
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 4e26266..f34c9de 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 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 @@
}
}
- update();
+ setChanged();
+ notifyObservers(LOAD_STATE_INDICATOR);
}
/**
