Bug 522287 - Combobox filter with filter list as baseCollection

Fixed issues in case the content of a list is cleared and replaced with
other content.

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

Change-Id: I58497cc486a6ae648dc36c0675861bd6295f9b56
diff --git a/org.eclipse.nebula.widgets.nattable.examples/src/org/eclipse/nebula/widgets/nattable/examples/_800_Integration/_818_SortableAllFilterPerformanceColumnGroupExample.java b/org.eclipse.nebula.widgets.nattable.examples/src/org/eclipse/nebula/widgets/nattable/examples/_800_Integration/_818_SortableAllFilterPerformanceColumnGroupExample.java
index b5d7051..7951f08 100644
--- a/org.eclipse.nebula.widgets.nattable.examples/src/org/eclipse/nebula/widgets/nattable/examples/_800_Integration/_818_SortableAllFilterPerformanceColumnGroupExample.java
+++ b/org.eclipse.nebula.widgets.nattable.examples/src/org/eclipse/nebula/widgets/nattable/examples/_800_Integration/_818_SortableAllFilterPerformanceColumnGroupExample.java
@@ -20,6 +20,7 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.atomic.AtomicBoolean;
 
 import org.eclipse.jface.bindings.keys.KeyStroke;
 import org.eclipse.jface.bindings.keys.ParseException;
@@ -168,6 +169,12 @@
 
     private ArrayList<Serializable> filterExcludes = new ArrayList<>();
 
+    private List<PersonWithAddress> mixedPersons = PersonService.getPersonsWithAddress(100);
+    // private List<PersonWithAddress> mixedPersons = createPersons(0);
+    private List<PersonWithAddress> alternativePersons = createAlternativePersons();
+
+    private AtomicBoolean alternativePersonsActive = new AtomicBoolean(false);
+
     public static void main(String[] args) {
         StandaloneNatExampleRunner.run(new _818_SortableAllFilterPerformanceColumnGroupExample());
     }
@@ -226,8 +233,7 @@
 
         final BodyLayerStack<PersonWithAddress> bodyLayerStack =
                 new BodyLayerStack<>(
-                        PersonService.getPersonsWithAddress(50),
-                        // createPersons(0),
+                        this.mixedPersons,
                         columnPropertyAccessor);
 
         // add some null and empty values to verify the correct handling
@@ -620,25 +626,14 @@
         replaceContentButton.addSelectionListener(new SelectionAdapter() {
             @Override
             public void widgetSelected(SelectionEvent e) {
-                Address address1 = new Address();
-                address1.setStreet("Some Street");
-                address1.setHousenumber(42);
-                address1.setPostalCode(12345);
-                address1.setCity("In the clouds");
-                PersonWithAddress ralph = new PersonWithAddress(42, "Ralph",
-                        "Wiggum", Gender.MALE, false, new Date(), address1);
-
-                Address address2 = new Address();
-                address2.setStreet("Evergreen Terrace");
-                address2.setHousenumber(742);
-                address2.setPostalCode(54321);
-                address2.setCity("Springfield");
-                PersonWithAddress lisa = new PersonWithAddress(23, "Lisa",
-                        "Simpson", Gender.FEMALE, false, new Date(), address2);
-
                 bodyLayerStack.getSortedList().clear();
-                bodyLayerStack.getSortedList().add(ralph);
-                bodyLayerStack.getSortedList().add(lisa);
+                if (_818_SortableAllFilterPerformanceColumnGroupExample.this.alternativePersonsActive.compareAndSet(true, false)) {
+                    bodyLayerStack.getSortedList().addAll(_818_SortableAllFilterPerformanceColumnGroupExample.this.mixedPersons);
+                } else {
+                    _818_SortableAllFilterPerformanceColumnGroupExample.this.alternativePersonsActive.set(true);
+                    bodyLayerStack.getSortedList().addAll(_818_SortableAllFilterPerformanceColumnGroupExample.this.alternativePersons);
+                }
+
             }
         });
 
@@ -1491,6 +1486,40 @@
         }
     }
 
+    private List<PersonWithAddress> createAlternativePersons() {
+        List<PersonWithAddress> result = new ArrayList<>();
+
+        for (int i = 0; i < 100; i++) {
+            Address address = new Address();
+            address.setStreet("Evergreen Terrace");
+            address.setHousenumber(732);
+            address.setPostalCode(54321);
+            address.setCity("Springfield");
+            result.add(new PersonWithAddress(i,
+                    "Ralph", "Wiggum", Gender.MALE, false, new Date(),
+                    address));
+            result.add(new PersonWithAddress(i,
+                    "Clancy", "Wiggum", Gender.MALE, true, new Date(),
+                    address));
+            result.add(new PersonWithAddress(i,
+                    "Sarah", "Wiggum", Gender.FEMALE, true, new Date(),
+                    address));
+        }
+
+        for (int i = 400; i < 500; i++) {
+            Address address = new Address();
+            address.setStreet("Fish Smell Drive");
+            address.setHousenumber(19);
+            address.setPostalCode(54321);
+            address.setCity("Springfield");
+            result.add(new PersonWithAddress(i,
+                    "Nelson", "Muntz", Gender.MALE, false, new Date(),
+                    address));
+        }
+
+        return result;
+    }
+
     private static List<PersonWithAddress> createPersons(int startId) {
         List<PersonWithAddress> result = new ArrayList<>();
 
diff --git a/org.eclipse.nebula.widgets.nattable.extension.glazedlists.test/src/org/eclipse/nebula/widgets/nattable/extension/glazedlists/filterrow/MixedComboBoxFilterRowHeaderIntegrationTest.java b/org.eclipse.nebula.widgets.nattable.extension.glazedlists.test/src/org/eclipse/nebula/widgets/nattable/extension/glazedlists/filterrow/MixedComboBoxFilterRowHeaderIntegrationTest.java
index ec56e42..a30020a 100644
--- a/org.eclipse.nebula.widgets.nattable.extension.glazedlists.test/src/org/eclipse/nebula/widgets/nattable/extension/glazedlists/filterrow/MixedComboBoxFilterRowHeaderIntegrationTest.java
+++ b/org.eclipse.nebula.widgets.nattable.extension.glazedlists.test/src/org/eclipse/nebula/widgets/nattable/extension/glazedlists/filterrow/MixedComboBoxFilterRowHeaderIntegrationTest.java
@@ -114,6 +114,9 @@
 
     private GlazedListsSortModel<PersonWithAddress> sortModel;
 
+    List<PersonWithAddress> values = createPersons(0);
+    List<PersonWithAddress> alternativeValues = createAlternativePersons();
+
     public void setupFixture(boolean handleListChanges, boolean caching) {
         // create a new ConfigRegistry which will be needed for GlazedLists
         // handling
@@ -146,13 +149,11 @@
         final IColumnPropertyAccessor<PersonWithAddress> columnPropertyAccessor =
                 new ExtendedReflectiveColumnPropertyAccessor<>(propertyNames);
 
-        List<PersonWithAddress> values = createPersons(0);
-
         // to enable the group by summary feature, the GroupByDataLayer needs to
         // know the ConfigRegistry
         this.bodyLayer =
                 new BodyLayerStack<>(
-                        values,
+                        this.values,
                         columnPropertyAccessor,
                         configRegistry);
 
@@ -1494,6 +1495,98 @@
         this.filterRowHeaderLayer.getFilterStrategy().clearStaticFilter();
     }
 
+    // the following test only covers the handleListChanges==true cases, as
+    // without it there is no listener for the structural changes
+    @ParameterizedTest(name = "listchanges={0}, caching={1}")
+    @CsvSource({
+            "true, false",
+            "true, true",
+    })
+    public void shouldUpdateComboBoxContentOnStructuralChanges(boolean handleListChanges, boolean caching) throws InterruptedException {
+        setupFixture(handleListChanges, caching);
+
+        // first load values
+        List<?> lastnames = this.filterRowComboBoxDataProvider.getValues(DataModelConstants.LASTNAME_COLUMN_POSITION, 0);
+        List<?> streets = this.filterRowComboBoxDataProvider.getValues(DataModelConstants.STREET_COLUMN_POSITION, 0);
+
+        assertTrue(ObjectUtils.collectionsEqual(LASTNAMES, lastnames));
+        assertTrue(ObjectUtils.collectionsEqual(STREETS, streets));
+
+        // change the collection
+        this.bodyLayer.getSortedList().clear();
+        this.bodyLayer.getSortedList().addAll(this.alternativeValues);
+
+        Thread.sleep(200);
+
+        lastnames = this.filterRowComboBoxDataProvider.getValues(DataModelConstants.LASTNAME_COLUMN_POSITION, 0);
+        streets = this.filterRowComboBoxDataProvider.getValues(DataModelConstants.STREET_COLUMN_POSITION, 0);
+
+        assertTrue(ObjectUtils.collectionsEqual(Arrays.asList("Muntz", "Wiggum"), lastnames));
+        assertTrue(ObjectUtils.collectionsEqual(Arrays.asList("Evergreen Terrace", "Fish Smell Drive"), streets));
+
+        // change the collection back
+        this.bodyLayer.getSortedList().clear();
+        this.bodyLayer.getSortedList().addAll(this.values);
+    }
+
+    // the following test only covers the handleListChanges==true and
+    // caching==true, as without it there is no listener for the structural
+    // changes and there is no cache update that triggers an update to the
+    // filter
+    @ParameterizedTest(name = "listchanges={0}, caching={1}")
+    @CsvSource({
+            "true, true",
+    })
+    public void shouldUpdateFilterAndComboBoxContentOnStructuralChangesWithAppliedFilter(boolean handleListChanges, boolean caching) throws InterruptedException {
+        setupFixture(handleListChanges, caching);
+
+        // first load values
+        List<?> lastnames = this.filterRowComboBoxDataProvider.getValues(DataModelConstants.LASTNAME_COLUMN_POSITION, 0);
+        Object lastnameFilter = this.filterRowHeaderLayer.getDataValueByPosition(DataModelConstants.LASTNAME_COLUMN_POSITION, 1);
+        List<?> streets = this.filterRowComboBoxDataProvider.getValues(DataModelConstants.STREET_COLUMN_POSITION, 0);
+
+        // apply a filter
+        this.natTable.doCommand(new UpdateDataCommand(this.natTable, 2, 1, new ArrayList<>(Arrays.asList("Simpson"))));
+
+        Thread.sleep(200);
+
+        lastnames = this.filterRowComboBoxDataProvider.getValues(DataModelConstants.LASTNAME_COLUMN_POSITION, 0);
+        lastnameFilter = this.filterRowHeaderLayer.getDataValueByPosition(DataModelConstants.LASTNAME_COLUMN_POSITION, 1);
+        assertTrue(ObjectUtils.collectionsEqual(LASTNAMES, lastnames));
+        assertTrue(ObjectUtils.collectionsEqual(Arrays.asList("Simpson"), (Collection<?>) lastnameFilter), "the reduced collection");
+        assertFalse(
+                ComboBoxFilterUtils.isAllSelected(
+                        DataModelConstants.LASTNAME_COLUMN_POSITION,
+                        lastnameFilter,
+                        this.filterRowComboBoxDataProvider),
+                "all values selected");
+
+        // change the collection
+        this.bodyLayer.getSortedList().clear();
+        this.bodyLayer.getSortedList().addAll(this.alternativeValues);
+
+        Thread.sleep(200);
+
+        lastnames = this.filterRowComboBoxDataProvider.getValues(DataModelConstants.LASTNAME_COLUMN_POSITION, 0);
+        lastnameFilter = this.filterRowHeaderLayer.getDataValueByPosition(DataModelConstants.LASTNAME_COLUMN_POSITION, 1);
+        streets = this.filterRowComboBoxDataProvider.getValues(DataModelConstants.STREET_COLUMN_POSITION, 0);
+
+        assertTrue(ObjectUtils.collectionsEqual(Arrays.asList("Muntz", "Wiggum"), lastnames));
+        assertTrue(ObjectUtils.collectionsEqual(Arrays.asList("Evergreen Terrace", "Fish Smell Drive"), streets));
+
+        assertTrue(ObjectUtils.collectionsEqual(Arrays.asList("Muntz", "Wiggum"), (Collection<?>) lastnameFilter), "the reduced collection");
+        assertTrue(
+                ComboBoxFilterUtils.isAllSelected(
+                        DataModelConstants.LASTNAME_COLUMN_POSITION,
+                        lastnameFilter,
+                        this.filterRowComboBoxDataProvider),
+                "not all values selected");
+
+        // change the collection back
+        this.bodyLayer.getSortedList().clear();
+        this.bodyLayer.getSortedList().addAll(this.values);
+    }
+
     private static List<String> LASTNAMES = Arrays.asList("Simpson", "Flanders", "Leonard", "Carlson", "Lovejoy", null);
     private static List<String> STREETS = Arrays.asList("Evergreen Terrace", "South Street", "Main Street");
     private static List<String> CITIES = Arrays.asList("Springfield", "Shelbyville", "Ogdenville");
@@ -1618,6 +1711,40 @@
 
     }
 
+    private static List<PersonWithAddress> createAlternativePersons() {
+        List<PersonWithAddress> result = new ArrayList<>();
+
+        for (int i = 0; i < 10; i++) {
+            Address address = new Address();
+            address.setStreet("Evergreen Terrace");
+            address.setHousenumber(732);
+            address.setPostalCode(54321);
+            address.setCity("Springfield");
+            result.add(new PersonWithAddress(i,
+                    "Ralph", "Wiggum", Gender.MALE, false, new Date(),
+                    address));
+            result.add(new PersonWithAddress(i,
+                    "Clancy", "Wiggum", Gender.MALE, true, new Date(),
+                    address));
+            result.add(new PersonWithAddress(i,
+                    "Sarah", "Wiggum", Gender.FEMALE, true, new Date(),
+                    address));
+        }
+
+        for (int i = 40; i < 50; i++) {
+            Address address = new Address();
+            address.setStreet("Fish Smell Drive");
+            address.setHousenumber(19);
+            address.setPostalCode(54321);
+            address.setCity("Springfield");
+            result.add(new PersonWithAddress(i,
+                    "Nelson", "Muntz", Gender.MALE, false, new Date(),
+                    address));
+        }
+
+        return result;
+    }
+
     static class BodyLayerStack<T> extends AbstractLayerTransform {
 
         private final EventList<T> eventList;
diff --git a/org.eclipse.nebula.widgets.nattable.extension.glazedlists/src/org/eclipse/nebula/widgets/nattable/extension/glazedlists/filterrow/GlazedListsFilterRowComboBoxDataProvider.java b/org.eclipse.nebula.widgets.nattable.extension.glazedlists/src/org/eclipse/nebula/widgets/nattable/extension/glazedlists/filterrow/GlazedListsFilterRowComboBoxDataProvider.java
index 644834d..2d54d4f 100644
--- a/org.eclipse.nebula.widgets.nattable.extension.glazedlists/src/org/eclipse/nebula/widgets/nattable/extension/glazedlists/filterrow/GlazedListsFilterRowComboBoxDataProvider.java
+++ b/org.eclipse.nebula.widgets.nattable.extension.glazedlists/src/org/eclipse/nebula/widgets/nattable/extension/glazedlists/filterrow/GlazedListsFilterRowComboBoxDataProvider.java
@@ -13,7 +13,10 @@
 package org.eclipse.nebula.widgets.nattable.extension.glazedlists.filterrow;
 
 import java.util.Collection;
+import java.util.concurrent.ScheduledFuture;
 import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
 
 import org.eclipse.nebula.widgets.nattable.data.IColumnAccessor;
 import org.eclipse.nebula.widgets.nattable.edit.event.DataUpdateEvent;
@@ -50,10 +53,12 @@
     private static final Logger LOG = LoggerFactory.getLogger(GlazedListsFilterRowComboBoxDataProvider.class);
 
     private static final Scheduler SCHEDULER = new Scheduler("GlazedListsFilterRowComboBoxDataProvider"); //$NON-NLS-1$
+    private final ScheduledFuture<?> future;
 
-    private AtomicBoolean changeHandlingProcessing = new AtomicBoolean(false);
+    private AtomicBoolean eventsToProcess = new AtomicBoolean(false);
 
     private EventList<T> baseEventList;
+    private final ReadWriteLock cacheLock = new ReentrantReadWriteLock();
 
     /**
      * @param bodyLayer
@@ -109,20 +114,30 @@
         } else {
             LOG.error("baseCollection is not of type EventList. List changes can not be tracked."); //$NON-NLS-1$
         }
+
+        // Start the event conflation thread
+        this.future = SCHEDULER.scheduleAtFixedRate(() -> {
+            if (this.cachingEnabled
+                    && GlazedListsFilterRowComboBoxDataProvider.this.eventsToProcess.compareAndSet(true, false)) {
+                clearCache(true);
+            }
+        }, 0L, 100L);
     }
 
     @Override
     public void listChanged(ListEvent<T> listChanges) {
-        if (!this.changeHandlingProcessing.getAndSet(true)) {
-            // a new row was added or a row was deleted
-            SCHEDULER.schedule(() -> {
-                try {
-                    clearCache(true);
-                } finally {
-                    GlazedListsFilterRowComboBoxDataProvider.this.changeHandlingProcessing.set(false);
-                }
-            }, 100);
+        this.cacheLock.readLock().lock();
+        try {
+            // if the list is cleared, we drop the previous collection cache
+            // state
+            if (this.cachingEnabled && this.baseEventList.size() == 0) {
+                setLastFilter(-1, null);
+            }
+        } finally {
+            this.cacheLock.readLock().unlock();
         }
+
+        this.eventsToProcess.set(true);
     }
 
     @Override
@@ -146,6 +161,7 @@
     @Override
     public void dispose() {
         super.dispose();
+        SCHEDULER.unschedule(this.future);
         SCHEDULER.shutdownNow();
     }