From 9e62f59b36fb71329e752be0c6f039c27198d582 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Leif=20=C3=85strand?= Date: Thu, 15 Dec 2016 10:00:09 +0200 Subject: [PATCH] Combine and rename setFilter and applyFilter (#7963) setFilter is an unsuitable name since it looks like a setter even though it isn't one. It's therefore renamed to withFilter. The difference between setFilter and applyFilter is very confusing. The original reason for the distinction was that they had incompatible return types (Void vs F), but this is resolved by changing the non-appendable version to use ? instead of Void. One side effect of this change is that setFilter cannot be used on an appendable filter provider to create a wrapped data provider without further filtering support. This theorethical case is still supported by using convertFilter with a Void -> F converter. --- .../data/AppendableFilterDataProvider.java | 3 ++- .../com/vaadin/server/data/DataProvider.java | 2 +- .../vaadin/server/data/DataProviderWrapper.java | 4 +++- .../data/provider/DataProviderTestBase.java | 16 +--------------- .../data/provider/ListDataProviderTest.java | 16 ++++++++-------- .../combobox/ComboBoxFilteringTest.java | 2 +- 6 files changed, 16 insertions(+), 27 deletions(-) diff --git a/server/src/main/java/com/vaadin/server/data/AppendableFilterDataProvider.java b/server/src/main/java/com/vaadin/server/data/AppendableFilterDataProvider.java index 845488d4f1..c6e20acbe4 100644 --- a/server/src/main/java/com/vaadin/server/data/AppendableFilterDataProvider.java +++ b/server/src/main/java/com/vaadin/server/data/AppendableFilterDataProvider.java @@ -37,7 +37,8 @@ public interface AppendableFilterDataProvider extends DataProvider { * the applied filter; not {@code null} * @return new data provider with the filter applied */ - public default AppendableFilterDataProvider applyFilter(F filter) { + @Override + public default AppendableFilterDataProvider withFilter(F filter) { Objects.requireNonNull(filter, "The applied filter can't be null"); return DataProviderWrapper.chain(this, filter); } diff --git a/server/src/main/java/com/vaadin/server/data/DataProvider.java b/server/src/main/java/com/vaadin/server/data/DataProvider.java index 5c60ca0951..1a0360318b 100644 --- a/server/src/main/java/com/vaadin/server/data/DataProvider.java +++ b/server/src/main/java/com/vaadin/server/data/DataProvider.java @@ -111,7 +111,7 @@ public interface DataProvider extends Serializable { * the default filter * @return wrapped data provider with provided filter */ - public default DataProvider setFilter(F filter) { + public default DataProvider withFilter(F filter) { Objects.requireNonNull(filter, "Filter can't be null"); return DataProviderWrapper.filter(this, filter); } diff --git a/server/src/main/java/com/vaadin/server/data/DataProviderWrapper.java b/server/src/main/java/com/vaadin/server/data/DataProviderWrapper.java index a04d4a07bd..ba08ac4ef6 100644 --- a/server/src/main/java/com/vaadin/server/data/DataProviderWrapper.java +++ b/server/src/main/java/com/vaadin/server/data/DataProviderWrapper.java @@ -125,7 +125,7 @@ public abstract class DataProviderWrapper * This {@code DataProvider} will deliberately ignore any possible filters * from the Query. * - * @see DataProvider#setFilter(Object) + * @see DataProvider#withFilter(Object) * * @param dataProvider * the underlying data provider @@ -145,6 +145,8 @@ public abstract class DataProviderWrapper @Override protected F getFilter(Query query) { + assert !query.getFilter() + .isPresent() : "A filter is provided to a data provider that should be unfilterable"; return filter; } }; diff --git a/server/src/test/java/com/vaadin/server/data/provider/DataProviderTestBase.java b/server/src/test/java/com/vaadin/server/data/provider/DataProviderTestBase.java index d930162016..b6a6bfa11f 100644 --- a/server/src/test/java/com/vaadin/server/data/provider/DataProviderTestBase.java +++ b/server/src/test/java/com/vaadin/server/data/provider/DataProviderTestBase.java @@ -66,7 +66,7 @@ public abstract class DataProviderTestBase> createQuery( List> sortOrder, Comparator comp, SerializablePredicate filter) { - return new Query>(0, + return new Query<>(0, Integer.MAX_VALUE, sortOrder, comp, filter); } @@ -296,18 +296,4 @@ public abstract class DataProviderTestBase(fooFilter))); } - - @Test - public void filteringListDataProvider_defaultFilter() { - // Intentionally lost filter type. Not actually filterable anymore. - DataProvider filtered = dataProvider.setFilter(gt5Filter); - - Assert.assertEquals("Filter not applied, unexpected item count", - dataProvider.size(new Query<>(gt5Filter)), - filtered.size(new Query<>())); - - Assert.assertEquals("Further filtering succeeded", - filtered.size(new Query<>()), - filtered.size((Query) new Query<>(fooFilter))); - } } diff --git a/server/src/test/java/com/vaadin/server/data/provider/ListDataProviderTest.java b/server/src/test/java/com/vaadin/server/data/provider/ListDataProviderTest.java index a018148dfa..1141675ac1 100644 --- a/server/src/test/java/com/vaadin/server/data/provider/ListDataProviderTest.java +++ b/server/src/test/java/com/vaadin/server/data/provider/ListDataProviderTest.java @@ -24,11 +24,11 @@ public class ListDataProviderTest public void filteringListDataProvider_appliedFilters() { Assert.assertEquals("Filtering result differ", data.stream().filter(fooFilter).count(), - dataProvider.applyFilter(fooFilter).size(new Query<>())); + dataProvider.withFilter(fooFilter).size(new Query<>())); Assert.assertEquals("Chained filtering result differ", data.stream().filter(fooFilter.and(gt5Filter)).count(), - dataProvider.applyFilter(fooFilter) + dataProvider.withFilter(fooFilter) .size(new Query<>(gt5Filter))); } @@ -36,7 +36,7 @@ public class ListDataProviderTest public void filteringListDataProvider_chainedFilters() { Assert.assertEquals("Chained filtering result differ", data.stream().filter(fooFilter.and(gt5Filter)).count(), - dataProvider.applyFilter(fooFilter).applyFilter(gt5Filter) + dataProvider.withFilter(fooFilter).withFilter(gt5Filter) .size(new Query<>())); } @@ -55,26 +55,26 @@ public class ListDataProviderTest Assert.assertEquals("Chained filtering result differ", data.stream().filter(fooFilter.or(gt5Filter)).count(), - orFilteredDataProvider.applyFilter(fooFilter) - .applyFilter(gt5Filter).size(new Query<>())); + orFilteredDataProvider.withFilter(fooFilter) + .withFilter(gt5Filter).size(new Query<>())); } @Test public void filteringListDataProvider_appliedFilterAndConverter() { Assert.assertEquals("Filtering result differ with 'Foo'", data.stream().filter(gt5Filter.and(fooFilter)).count(), - dataProvider.applyFilter(gt5Filter).convertFilter( + dataProvider.withFilter(gt5Filter).convertFilter( text -> strBean -> strBean.getValue().equals(text)) .size(new Query<>("Foo"))); Assert.assertEquals("Filtering result differ with 'Xyz'", data.stream() .filter(gt5Filter.and(s -> s.getValue().equals("Xyz"))).count(), - dataProvider.applyFilter(gt5Filter).convertFilter( + dataProvider.withFilter(gt5Filter).convertFilter( text -> strBean -> strBean.getValue().equals(text)) .size(new Query<>("Xyz"))); Assert.assertEquals("No results should've been found", 0, - dataProvider.applyFilter(gt5Filter).convertFilter( + dataProvider.withFilter(gt5Filter).convertFilter( text -> strBean -> strBean.getValue().equals(text)) .size(new Query<>("Zyx"))); } diff --git a/server/src/test/java/com/vaadin/tests/server/component/combobox/ComboBoxFilteringTest.java b/server/src/test/java/com/vaadin/tests/server/component/combobox/ComboBoxFilteringTest.java index de03684307..9719cd43b8 100644 --- a/server/src/test/java/com/vaadin/tests/server/component/combobox/ComboBoxFilteringTest.java +++ b/server/src/test/java/com/vaadin/tests/server/component/combobox/ComboBoxFilteringTest.java @@ -143,7 +143,7 @@ public class ComboBoxFilteringTest { ListDataProvider ldp = DataProvider.create(getPersonArray()); // Same as above, but only showing a subset of the persons comboBox.setDataProvider(ldp - .applyFilter(person -> person.getFirstName().contains("nr")) + .withFilter(person -> person.getFirstName().contains("nr")) .convertFilter( text -> person -> person.getLastName().contains(text))); -- 2.39.5