Allow distinct configurable filter and wrapped filter types (#8322)

* Allow distinct configurable filter and wrapped filter types

This enables implementing the use case that was already described
towards the end of datamodel-providers.asciidoc.

Also swaps the order of the callback type parameters to make all three
match the order of the ConfigurableFilterDataProvider type parameters.

3 files changed, 44 insertions, 32 deletions

diff --git a/server/src/main/java/com/vaadin/data/provider/ConfigurableFilterDataProviderWrapper.java b/server/src/main/java/com/vaadin/data/provider/ConfigurableFilterDataProviderWrapper.java
index c6256787ce..b83fc98013 100644
--- a/server/src/main/java/com/vaadin/data/provider/ConfigurableFilterDataProviderWrapper.java
+++ b/server/src/main/java/com/vaadin/data/provider/ConfigurableFilterDataProviderWrapper.java
@@ -28,9 +28,11 @@ package com.vaadin.data.provider;
  *            the query filter type
  * @param <C>
  *            the configurable filter type
+ * @param <F>
+ *            the filter type of the wrapped data provider
  */
-public abstract class ConfigurableFilterDataProviderWrapper<T, Q, C>
-        extends DataProviderWrapper<T, Q, C>
+public abstract class ConfigurableFilterDataProviderWrapper<T, Q, C, F>
+        extends DataProviderWrapper<T, Q, F>
         implements ConfigurableFilterDataProvider<T, Q, C> {
 
     private C configuredFilter;
@@ -43,33 +45,36 @@ public abstract class ConfigurableFilterDataProviderWrapper<T, Q, C>
      *            the data provider to wrap, not <code>null</code>
      */
     public ConfigurableFilterDataProviderWrapper(
-            DataProvider<T, C> dataProvider) {
+            DataProvider<T, F> dataProvider) {
         super(dataProvider);
     }
 
     @Override
-    protected C getFilter(Query<T, Q> query) {
-        return query.getFilter().map(
-                queryFilter -> combineFilters(configuredFilter, queryFilter))
-                .orElse(configuredFilter);
+    protected F getFilter(Query<T, Q> query) {
+        Q queryFilter = query.getFilter().orElse(null);
+        if (configuredFilter == null && queryFilter == null) {
+            return null;
+        }
+        return combineFilters(queryFilter, configuredFilter);
     }
 
     /**
      * Combines the configured filter and the filter from the query into one
-     * filter instance that can be passed to the wrapped data provider. This
-     * method is called only if there is a query filter, otherwise the
-     * configured filter will be directly passed to the query.
-     *
+     * filter instance that can be passed to the wrapped data provider. Will not
+     * be called if the configured filter is <code>null</code> and the query has
+     * no filter.
+     * 
+     * @param queryFilter
+     *            the filter received through the query, or <code>null</code> if
+     *            no filter was provided in the query
      * @param configuredFilter
      *            the filter that this data provider is configured to use, or
      *            <code>null</code> if no filter has been configured
-     * @param queryFilter
-     *            the filter received through the query, not <code>null</code>
      * @return a filter that combines the two provided queries, or
      *         <code>null</code> to not pass any filter to the wrapped data
      *         provider
      */
-    protected abstract C combineFilters(C configuredFilter, Q queryFilter);
+    protected abstract F combineFilters(Q queryFilter, C configuredFilter);
 
     @Override
     public void setFilter(C filter) {
diff --git a/server/src/main/java/com/vaadin/data/provider/DataProvider.java b/server/src/main/java/com/vaadin/data/provider/DataProvider.java
index e641f0d057..380a1f4809 100644
--- a/server/src/main/java/com/vaadin/data/provider/DataProvider.java
+++ b/server/src/main/java/com/vaadin/data/provider/DataProvider.java
@@ -163,17 +163,18 @@ public interface DataProvider<T, F> extends Serializable {
      * @param filterCombiner
      *            a callback for combining and the configured filter with the
      *            filter from the query to get a filter to pass to the wrapped
-     *            provider. Will only be called if the query contains a filter.
-     *            Not <code>null</code>
+     *            provider. Either parameter might be <code>null</code>, but the
+     *            callback will not be invoked at all if both would be
+     *            <code>null</code>. Not <code>null</code>.
      *
      * @return a data provider with a configurable filter, not <code>null</code>
      */
-    public default <C> ConfigurableFilterDataProvider<T, C, F> withConfigurableFilter(
-            SerializableBiFunction<F, C, F> filterCombiner) {
-        return new ConfigurableFilterDataProviderWrapper<T, C, F>(this) {
+    public default <Q, C> ConfigurableFilterDataProvider<T, Q, C> withConfigurableFilter(
+            SerializableBiFunction<Q, C, F> filterCombiner) {
+        return new ConfigurableFilterDataProviderWrapper<T, Q, C, F>(this) {
             @Override
-            protected F combineFilters(F configuredFilter, C queryFilter) {
-                return filterCombiner.apply(configuredFilter, queryFilter);
+            protected F combineFilters(Q queryFilter, C configuredFilter) {
+                return filterCombiner.apply(queryFilter, configuredFilter);
             }
         };
     }
@@ -188,7 +189,7 @@ public interface DataProvider<T, F> extends Serializable {
      * @return a data provider with a configurable filter, not <code>null</code>
      */
     public default ConfigurableFilterDataProvider<T, Void, F> withConfigurableFilter() {
-        return withConfigurableFilter((configuredFilter, queryFilter) -> {
+        return withConfigurableFilter((queryFilter, configuredFilter) -> {
             assert queryFilter == null : "Filter from Void query must be null";
 
             return configuredFilter;
diff --git a/server/src/test/java/com/vaadin/data/provider/ConfigurableFilterDataProviderWrapperTest.java b/server/src/test/java/com/vaadin/data/provider/ConfigurableFilterDataProviderWrapperTest.java
index 115e4fd6bd..b2be27bd8d 100644
--- a/server/src/test/java/com/vaadin/data/provider/ConfigurableFilterDataProviderWrapperTest.java
+++ b/server/src/test/java/com/vaadin/data/provider/ConfigurableFilterDataProviderWrapperTest.java
@@ -29,13 +29,19 @@ public class ConfigurableFilterDataProviderWrapperTest {
             StrBean.generateRandomBeans(100));
     private ConfigurableFilterDataProvider<StrBean, Void, SerializablePredicate<StrBean>> configurableVoid = backEndProvider
             .withConfigurableFilter();
-    private ConfigurableFilterDataProvider<StrBean, SerializablePredicate<StrBean>, SerializablePredicate<StrBean>> configurablePredicate = backEndProvider
-            .withConfigurableFilter((configuredFilter, queryFilter) -> item -> {
-                if (configuredFilter != null && !configuredFilter.test(item)) {
+    private ConfigurableFilterDataProvider<StrBean, String, Integer> configurablePredicate = backEndProvider
+            .withConfigurableFilter((queryFilter, configuredFilter) -> item -> {
+                if (queryFilter != null
+                        && !item.getValue().equals(queryFilter)) {
                     return false;
                 }
 
-                return queryFilter.test(item);
+                if (configuredFilter != null
+                        && item.getId() < configuredFilter.intValue()) {
+                    return false;
+                }
+
+                return true;
             });
 
     @Test
@@ -60,9 +66,9 @@ public class ConfigurableFilterDataProviderWrapperTest {
 
     @Test
     public void predicate_setFilter() {
-        configurablePredicate.setFilter(xyzFilter);
+        configurablePredicate.setFilter(Integer.valueOf(50));
 
-        Assert.assertEquals("Set filter should be used", 1,
+        Assert.assertEquals("Set filter should be used", 49,
                 configurablePredicate.size(new Query<>()));
 
         configurablePredicate.setFilter(null);
@@ -74,7 +80,7 @@ public class ConfigurableFilterDataProviderWrapperTest {
     @Test
     public void predicate_queryFilter() {
         Assert.assertEquals("Query filter should be used", 1,
-                configurablePredicate.size(new Query<>(xyzFilter)));
+                configurablePredicate.size(new Query<>("Xyz")));
 
         Assert.assertEquals("null query filter should return all items", 100,
                 configurablePredicate.size(new Query<>()));
@@ -82,15 +88,15 @@ public class ConfigurableFilterDataProviderWrapperTest {
 
     @Test
     public void predicate_combinedFilters() {
-        configurablePredicate.setFilter(item -> item.getValue().equals("Foo"));
+        configurablePredicate.setFilter(Integer.valueOf(50));
 
         Assert.assertEquals("Both filters should be used", 0,
-                configurablePredicate.size(new Query<>(xyzFilter)));
+                configurablePredicate.size(new Query<>("Xyz")));
 
         configurablePredicate.setFilter(null);
 
         Assert.assertEquals("Only zyz filter should be used", 1,
-                configurablePredicate.size(new Query<>(xyzFilter)));
+                configurablePredicate.size(new Query<>("Xyz")));
     }
 
 }