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

Fixed issues related to static and/or exclude filters applied
additionally.

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

Change-Id: I66eb378c2c143baff2223b7a88adb1a1c3021f79

5 files changed, 168 insertions, 29 deletions

diff --git a/org.eclipse.nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/filterrow/IActivatableFilterStrategy.java b/org.eclipse.nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/filterrow/IActivatableFilterStrategy.java
new file mode 100644
index 00000000..a511b259
--- /dev/null
+++ b/org.eclipse.nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/filterrow/IActivatableFilterStrategy.java
@@ -0,0 +1,46 @@
+/*******************************************************************************
+ * Copyright (c) 2023 Original authors and others.
+ *
+ * This program and the accompanying materials are made
+ * available under the terms of the Eclipse Public License 2.0
+ * which is available at https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Contributors:
+ *     Original authors and others - initial API and implementation
+ ******************************************************************************/
+package org.eclipse.nebula.widgets.nattable.filterrow;
+
+/**
+ * Extended {@link IFilterStrategy} that supports the activating and
+ * deactivating the filter logic. Useful for special filter logic like static or
+ * exclude filters.
+ *
+ * @param <T>
+ *            type of objects in the FilterList
+ *
+ * @since 2.1
+ */
+public interface IActivatableFilterStrategy<T> extends IFilterStrategy<T> {
+
+    /**
+     * Activate additional filter logic so it gets applied on the next filter
+     * operation, e.g. static or exclude filters. By default does nothing.
+     */
+    void activateFilterStrategy();
+
+    /**
+     * Deactivate additional filter logic so it does not get applied on the next
+     * filter operation, e.g. static or exclude filters. By default does
+     * nothing.
+     */
+    void deactivateFilterStrategy();
+
+    /**
+     *
+     * @return <code>true</code> if the additional filter logic provided by this
+     *         {@link IFilterStrategy} is active, <code>false</code> if not.
+     */
+    boolean isActive();
+}
diff --git a/org.eclipse.nebula.widgets.nattable.extension.glazedlists.test/src/org/eclipse/nebula/widgets/nattable/extension/glazedlists/filterrow/ComboBoxFilterRowHeaderCompositeIntegrationTest.java b/org.eclipse.nebula.widgets.nattable.extension.glazedlists.test/src/org/eclipse/nebula/widgets/nattable/extension/glazedlists/filterrow/ComboBoxFilterRowHeaderCompositeIntegrationTest.java
index 0652466c..1a080352 100644
--- a/org.eclipse.nebula.widgets.nattable.extension.glazedlists.test/src/org/eclipse/nebula/widgets/nattable/extension/glazedlists/filterrow/ComboBoxFilterRowHeaderCompositeIntegrationTest.java
+++ b/org.eclipse.nebula.widgets.nattable.extension.glazedlists.test/src/org/eclipse/nebula/widgets/nattable/extension/glazedlists/filterrow/ComboBoxFilterRowHeaderCompositeIntegrationTest.java
@@ -396,7 +396,7 @@ public class ComboBoxFilterRowHeaderCompositeIntegrationTest {
         this.bodyLayer.eventList.add(entry);
 
         // long start = System.currentTimeMillis();
-        boolean completed = countDown.await(5000, TimeUnit.MILLISECONDS);
+        boolean completed = countDown.await(10000, TimeUnit.MILLISECONDS);
         // long end = System.currentTimeMillis();
         // System.out.println("duration " + (end - start));
 
diff --git a/org.eclipse.nebula.widgets.nattable.extension.glazedlists/src/org/eclipse/nebula/widgets/nattable/extension/glazedlists/filterrow/ComboBoxGlazedListsWithExcludeFilterStrategy.java b/org.eclipse.nebula.widgets.nattable.extension.glazedlists/src/org/eclipse/nebula/widgets/nattable/extension/glazedlists/filterrow/ComboBoxGlazedListsWithExcludeFilterStrategy.java
index fcd89433..43198b23 100644
--- a/org.eclipse.nebula.widgets.nattable.extension.glazedlists/src/org/eclipse/nebula/widgets/nattable/extension/glazedlists/filterrow/ComboBoxGlazedListsWithExcludeFilterStrategy.java
+++ b/org.eclipse.nebula.widgets.nattable.extension.glazedlists/src/org/eclipse/nebula/widgets/nattable/extension/glazedlists/filterrow/ComboBoxGlazedListsWithExcludeFilterStrategy.java
@@ -105,11 +105,13 @@ public class ComboBoxGlazedListsWithExcludeFilterStrategy<T> extends ComboBoxGla
      */
     public void addExcludeFilter(final MatcherEditor<T> matcherEditor) {
         // add the new MatcherEditor to the CompositeMatcherEditor
-        this.filterLock.writeLock().lock();
-        try {
-            this.compositeMatcherEditor.getMatcherEditors().add(matcherEditor);
-        } finally {
-            this.filterLock.writeLock().unlock();
+        if (isActive()) {
+            this.filterLock.writeLock().lock();
+            try {
+                this.compositeMatcherEditor.getMatcherEditors().add(matcherEditor);
+            } finally {
+                this.filterLock.writeLock().unlock();
+            }
         }
 
         this.excludeMatcherEditor.put(matcherEditor.getMatcher(), matcherEditor);
@@ -123,7 +125,7 @@ public class ComboBoxGlazedListsWithExcludeFilterStrategy<T> extends ComboBoxGla
      */
     public void removeExcludeFilter(final Matcher<T> matcher) {
         MatcherEditor<T> removed = this.excludeMatcherEditor.remove(matcher);
-        if (removed != null) {
+        if (removed != null && isActive()) {
             this.filterLock.writeLock().lock();
             try {
                 this.compositeMatcherEditor.getMatcherEditors().remove(removed);
@@ -148,14 +150,46 @@ public class ComboBoxGlazedListsWithExcludeFilterStrategy<T> extends ComboBoxGla
      */
     public void clearExcludeFilter() {
         Collection<MatcherEditor<T>> excludeMatcher = this.excludeMatcherEditor.values();
-        if (!excludeMatcher.isEmpty()) {
+        if (!excludeMatcher.isEmpty() && isActive()) {
             this.filterLock.writeLock().lock();
             try {
                 this.compositeMatcherEditor.getMatcherEditors().removeAll(excludeMatcher);
             } finally {
                 this.filterLock.writeLock().unlock();
             }
-            this.excludeMatcherEditor.clear();
         }
+        this.excludeMatcherEditor.clear();
+    }
+
+    @Override
+    public void activateFilterStrategy() {
+        if (!isActive()) {
+            Collection<MatcherEditor<T>> excludeMatcher = this.excludeMatcherEditor.values();
+            if (!excludeMatcher.isEmpty()) {
+                this.filterLock.writeLock().lock();
+                try {
+                    this.compositeMatcherEditor.getMatcherEditors().addAll(excludeMatcher);
+                } finally {
+                    this.filterLock.writeLock().unlock();
+                }
+            }
+        }
+        super.activateFilterStrategy();
+    }
+
+    @Override
+    public void deactivateFilterStrategy() {
+        if (isActive()) {
+            Collection<MatcherEditor<T>> excludeMatcher = this.excludeMatcherEditor.values();
+            if (!excludeMatcher.isEmpty()) {
+                this.filterLock.writeLock().lock();
+                try {
+                    this.compositeMatcherEditor.getMatcherEditors().removeAll(excludeMatcher);
+                } finally {
+                    this.filterLock.writeLock().unlock();
+                }
+            }
+        }
+        super.deactivateFilterStrategy();
     }
 }
\ No newline at end of file
diff --git a/org.eclipse.nebula.widgets.nattable.extension.glazedlists/src/org/eclipse/nebula/widgets/nattable/extension/glazedlists/filterrow/DefaultGlazedListsStaticFilterStrategy.java b/org.eclipse.nebula.widgets.nattable.extension.glazedlists/src/org/eclipse/nebula/widgets/nattable/extension/glazedlists/filterrow/DefaultGlazedListsStaticFilterStrategy.java
index 612e9bc3..b45a8513 100644
--- a/org.eclipse.nebula.widgets.nattable.extension.glazedlists/src/org/eclipse/nebula/widgets/nattable/extension/glazedlists/filterrow/DefaultGlazedListsStaticFilterStrategy.java
+++ b/org.eclipse.nebula.widgets.nattable.extension.glazedlists/src/org/eclipse/nebula/widgets/nattable/extension/glazedlists/filterrow/DefaultGlazedListsStaticFilterStrategy.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
@@ -18,6 +18,7 @@ import java.util.Map;
 
 import org.eclipse.nebula.widgets.nattable.config.IConfigRegistry;
 import org.eclipse.nebula.widgets.nattable.data.IColumnAccessor;
+import org.eclipse.nebula.widgets.nattable.filterrow.IActivatableFilterStrategy;
 import org.eclipse.nebula.widgets.nattable.filterrow.IFilterStrategy;
 
 import ca.odell.glazedlists.FilterList;
@@ -34,10 +35,12 @@ import ca.odell.glazedlists.matchers.MatcherEditor;
  * @param <T>
  *            the type of the objects shown within the NatTable
  */
-public class DefaultGlazedListsStaticFilterStrategy<T> extends DefaultGlazedListsFilterStrategy<T> {
+public class DefaultGlazedListsStaticFilterStrategy<T> extends DefaultGlazedListsFilterStrategy<T> implements IActivatableFilterStrategy<T> {
 
     protected Map<Matcher<T>, MatcherEditor<T>> staticMatcherEditor = new HashMap<>();
 
+    private boolean active = true;
+
     /**
      * Create a new DefaultGlazedListsStaticFilterStrategy on top of the given
      * FilterList.
@@ -56,8 +59,10 @@ public class DefaultGlazedListsStaticFilterStrategy<T> extends DefaultGlazedList
      *            The IConfigRegistry necessary to retrieve filter specific
      *            configurations.
      */
-    public DefaultGlazedListsStaticFilterStrategy(FilterList<T> filterList,
-            IColumnAccessor<T> columnAccessor, IConfigRegistry configRegistry) {
+    public DefaultGlazedListsStaticFilterStrategy(
+            FilterList<T> filterList,
+            IColumnAccessor<T> columnAccessor,
+            IConfigRegistry configRegistry) {
         super(filterList, columnAccessor, configRegistry);
     }
 
@@ -83,9 +88,11 @@ public class DefaultGlazedListsStaticFilterStrategy<T> extends DefaultGlazedList
      *            The IConfigRegistry necessary to retrieve filter specific
      *            configurations.
      */
-    public DefaultGlazedListsStaticFilterStrategy(FilterList<T> filterList,
+    public DefaultGlazedListsStaticFilterStrategy(
+            FilterList<T> filterList,
             CompositeMatcherEditor<T> matcherEditor,
-            IColumnAccessor<T> columnAccessor, IConfigRegistry configRegistry) {
+            IColumnAccessor<T> columnAccessor,
+            IConfigRegistry configRegistry) {
         super(filterList, matcherEditor, columnAccessor, configRegistry);
     }
 
@@ -95,11 +102,13 @@ public class DefaultGlazedListsStaticFilterStrategy<T> extends DefaultGlazedList
     @Override
     public void applyFilter(Map<Integer, Object> filterIndexToObjectMap) {
         super.applyFilter(filterIndexToObjectMap);
-        this.filterLock.writeLock().lock();
-        try {
-            this.getMatcherEditor().getMatcherEditors().addAll(this.staticMatcherEditor.values());
-        } finally {
-            this.filterLock.writeLock().unlock();
+        if (this.active) {
+            this.filterLock.writeLock().lock();
+            try {
+                this.getMatcherEditor().getMatcherEditors().addAll(this.staticMatcherEditor.values());
+            } finally {
+                this.filterLock.writeLock().unlock();
+            }
         }
     }
 
@@ -125,11 +134,13 @@ public class DefaultGlazedListsStaticFilterStrategy<T> extends DefaultGlazedList
      */
     public void addStaticFilter(final MatcherEditor<T> matcherEditor) {
         // add the new MatcherEditor to the CompositeMatcherEditor
-        this.filterLock.writeLock().lock();
-        try {
-            this.getMatcherEditor().getMatcherEditors().add(matcherEditor);
-        } finally {
-            this.filterLock.writeLock().unlock();
+        if (isActive()) {
+            this.filterLock.writeLock().lock();
+            try {
+                this.getMatcherEditor().getMatcherEditors().add(matcherEditor);
+            } finally {
+                this.filterLock.writeLock().unlock();
+            }
         }
 
         // remember the MatcherEditor so it can be restored after new
@@ -145,7 +156,7 @@ public class DefaultGlazedListsStaticFilterStrategy<T> extends DefaultGlazedList
      */
     public void removeStaticFilter(final Matcher<T> matcher) {
         MatcherEditor<T> removed = this.staticMatcherEditor.remove(matcher);
-        if (removed != null) {
+        if (removed != null && isActive()) {
             this.filterLock.writeLock().lock();
             try {
                 this.getMatcherEditor().getMatcherEditors().remove(removed);
@@ -172,14 +183,53 @@ public class DefaultGlazedListsStaticFilterStrategy<T> extends DefaultGlazedList
      */
     public void clearStaticFilter() {
         Collection<MatcherEditor<T>> staticMatcher = this.staticMatcherEditor.values();
-        if (!staticMatcher.isEmpty()) {
+        if (!staticMatcher.isEmpty() && isActive()) {
             this.filterLock.writeLock().lock();
             try {
                 this.getMatcherEditor().getMatcherEditors().removeAll(staticMatcher);
             } finally {
                 this.filterLock.writeLock().unlock();
             }
-            this.staticMatcherEditor.clear();
         }
+        this.staticMatcherEditor.clear();
+    }
+
+    @Override
+    public void activateFilterStrategy() {
+        if (!this.active) {
+            this.active = true;
+
+            Collection<MatcherEditor<T>> staticMatcher = this.staticMatcherEditor.values();
+            if (!staticMatcher.isEmpty()) {
+                this.filterLock.writeLock().lock();
+                try {
+                    this.getMatcherEditor().getMatcherEditors().addAll(staticMatcher);
+                } finally {
+                    this.filterLock.writeLock().unlock();
+                }
+            }
+        }
+    }
+
+    @Override
+    public void deactivateFilterStrategy() {
+        if (this.active) {
+            this.active = false;
+
+            Collection<MatcherEditor<T>> staticMatcher = this.staticMatcherEditor.values();
+            if (!staticMatcher.isEmpty()) {
+                this.filterLock.writeLock().lock();
+                try {
+                    this.getMatcherEditor().getMatcherEditors().removeAll(staticMatcher);
+                } finally {
+                    this.filterLock.writeLock().unlock();
+                }
+            }
+        }
+    }
+
+    @Override
+    public boolean isActive() {
+        return this.active;
     }
 }
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 017f5178..488c73a8 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
@@ -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
@@ -41,6 +41,7 @@ import org.eclipse.nebula.widgets.nattable.extension.glazedlists.groupBy.summary
 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.filterrow.IActivatableFilterStrategy;
 import org.eclipse.nebula.widgets.nattable.layer.DataLayer;
 import org.eclipse.nebula.widgets.nattable.layer.IUniqueIndexLayer;
 import org.eclipse.nebula.widgets.nattable.layer.LabelStack;
@@ -652,6 +653,10 @@ public class GroupByDataLayer<T> extends DataLayer implements Observer {
         // it after the tree update
         Map<Integer, Object> filterCopy = Collections.emptyMap();
         if (this.filterRowDataProvider != null) {
+            if (this.filterRowDataProvider.getFilterStrategy() instanceof IActivatableFilterStrategy) {
+                ((IActivatableFilterStrategy<T>) this.filterRowDataProvider.getFilterStrategy()).deactivateFilterStrategy();
+            }
+
             Map<Integer, Object> original = this.filterRowDataProvider.getFilterIndexToObjectMap();
             filterCopy = new HashMap<>(original);
             original.clear();
@@ -662,6 +667,10 @@ public class GroupByDataLayer<T> extends DataLayer implements Observer {
 
         // re-apply the filter after the tree update
         if (this.filterRowDataProvider != null) {
+            if (this.filterRowDataProvider.getFilterStrategy() instanceof IActivatableFilterStrategy) {
+                ((IActivatableFilterStrategy<T>) this.filterRowDataProvider.getFilterStrategy()).activateFilterStrategy();
+            }
+
             Map<Integer, Object> original = this.filterRowDataProvider.getFilterIndexToObjectMap();
             original.putAll(filterCopy);
             this.filterRowDataProvider.getFilterStrategy().applyFilter(original);