diff --git a/flow-data/src/main/java/com/vaadin/flow/data/provider/DataCommunicator.java b/flow-data/src/main/java/com/vaadin/flow/data/provider/DataCommunicator.java index c1b15a9a1b3..b691279457a 100644 --- a/flow-data/src/main/java/com/vaadin/flow/data/provider/DataCommunicator.java +++ b/flow-data/src/main/java/com/vaadin/flow/data/provider/DataCommunicator.java @@ -38,7 +38,6 @@ import com.vaadin.flow.dom.Element; import com.vaadin.flow.function.SerializableComparator; import com.vaadin.flow.function.SerializableConsumer; -import com.vaadin.flow.function.SerializablePredicate; import com.vaadin.flow.function.SerializableSupplier; import com.vaadin.flow.internal.ExecutionContext; import com.vaadin.flow.internal.JsonUtils; @@ -146,11 +145,8 @@ public EmptyDataProvider() { } /** - * Wraps the component's filter object with its lifespan, i.e. the - * information whether this filter is set permanently and used for any calls - * to data provider, or should it be used for just one requested range call - * to data provider, and then it will be erased and not took into account in - * the further calls. + * Wraps the component's filter object with the meta information whether + * this filter changing should trigger the item count change event. * * @param * filter's type @@ -160,17 +156,18 @@ public static final class Filter implements Serializable { // Serializability of filter is up to the application private F filterObject; - private boolean permanent; + private boolean notifyOnChange; /** - * Creates the filter object and sets it as a permanent by default. + * Creates the filter object and sets it notify item count change + * listeners by default. * * @param filterObject * filter object of a component */ public Filter(F filterObject) { this.filterObject = filterObject; - this.permanent = true; + this.notifyOnChange = true; } /** @@ -178,18 +175,16 @@ public Filter(F filterObject) { * * @param filterObject * filter object of a component - * @param permanent - * if {@code true}, then this filter is considered as a - * permanent and will be stored in the data communicator and - * used for any calls to data provider. Otherwise, it will be - * considered as a disposable filter, i.e. with a lifespan of - * just one requested range call to data provider, and then - * it will be erased and not took into account in the further - * calls. + * @param notifyOnChange + * if {@code true}, then the data communicator will fire the + * item count change event as soon as filter change modifies + * the item count. If {@code false}, the item count change + * event won't be fired, even if the item count will be + * changed as a result of filtering. */ - public Filter(F filterObject, boolean permanent) { + public Filter(F filterObject, boolean notifyOnChange) { this.filterObject = filterObject; - this.permanent = permanent; + this.notifyOnChange = notifyOnChange; } /** @@ -202,17 +197,17 @@ public F getFilterObject() { } /** - * Returns whether this filter is permanent or disposable. + * Returns whether to fire the item change event or not upon filter + * changing. * - * @return {@code true}, if this filter is considered as a permanent and - * will be stored in the data communicator and used for any - * calls to data provider. {@code false} if it's considered as a - * disposable filter, i.e. with a lifespan of just one requested - * range call to data provider, and then it will be erased and - * not took into account in the further calls. + * @return {@code true}, then the data communicator will fire the item + * count change event as soon as filter change modifies the item + * count. Returns {@code false}, the item count change event + * won't be fired, even if the item count will be changed as a + * result of filtering. */ - public boolean isPermanent() { - return permanent; + public boolean isNotifyOnChange() { + return notifyOnChange; } @Override @@ -222,13 +217,13 @@ public boolean equals(Object o) { if (o == null || getClass() != o.getClass()) return false; Filter filter1 = (Filter) o; - return permanent == filter1.permanent + return notifyOnChange == filter1.notifyOnChange && Objects.equals(filterObject, filter1.filterObject); } @Override public int hashCode() { - return Objects.hash(filterObject, permanent); + return Objects.hash(filterObject, notifyOnChange); } } @@ -387,28 +382,20 @@ public void confirmUpdate(int updateId) { * that the given data provider is queried for size and previous size * estimates are discarded. *

- * This method allows to define the lifespan for the given filter. It can be - * set as a permanent, i.e. it will be stored in the data communicator and - * used for any calls to data provider until further change through returned - * consumer. Otherwise, it will be considered as a disposable filter, i.e. - * with a lifespan of just one requested range call to data provider, and - * then it will be erased and not took into account in the further calls - * until further change through returned consumer. + * This method allows to define whether the data communicator notifies about + * changing of item count when it changes due to filtering. * * @param dataProvider * the data provider to set, not null * @param initialFilter * the initial filter value to use, or null to not * use any initial filter value - * @param permanentFilter - * if {@code true}, then the initial filter is considered as a - * permanent and will be stored in the data communicator and used - * for any calls to data provider until further change through - * returned consumer. Otherwise, it will be considered as a - * disposable filter, i.e. with a lifespan of just one requested - * range call to data provider, and then it will be erased and - * not took into account in the further calls until further - * change through returned consumer. + * @param notifiesOnChange + * if {@code true}, then the data communicator will fire the item + * count change event as soon as filter change modifies the item + * count. If {@code false}, the item count change event won't be + * fired, even if the item count will be changed as a result of + * filtering. * * @param * the filter type @@ -417,13 +404,13 @@ public void confirmUpdate(int updateId) { */ public SerializableConsumer> setDataProvider( DataProvider dataProvider, F initialFilter, - boolean permanentFilter) { + boolean notifiesOnChange) { Objects.requireNonNull(dataProvider, "data provider cannot be null"); removeFilteringAndSorting(); filter = initialFilter != null - ? new Filter<>(initialFilter, permanentFilter) + ? new Filter<>(initialFilter, notifiesOnChange) : null; countCallback = null; @@ -1152,8 +1139,6 @@ private void flush() { unregisterPassivatedKeys(); fireItemCountEvent(assumedSize); - - clearFilterIfDisposable(); } /** @@ -1163,16 +1148,17 @@ private void flush() { *

* * @param itemCount * item count to send */ private void fireItemCountEvent(int itemCount) { - final boolean permanentFilter = filter == null || filter.isPermanent(); + final boolean notify = filter == null || filter.isNotifyOnChange(); - if (lastSent != itemCount && permanentFilter) { + if (lastSent != itemCount && notify) { final Optional component = Element.get(stateNode) .getComponent(); component.ifPresent(value -> ComponentUtil.fireEvent(value, @@ -1357,26 +1343,6 @@ private JsonValue generateJson(T item) { return json; } - private void clearFilterIfDisposable() { - if (filter != null && !filter.isPermanent()) { - Optional component = Element.get(stateNode).getComponent(); - if (component.isPresent()) { - // Look up for the component's in-memory filter - Optional> componentFilter = DataViewUtils - .getComponentFilter(component.get()); - - // If the component's in-memory filter is present, then erase - // the client-side filter and re-apply the permanent - // component's filter - filter = componentFilter.map( - filterValue -> new Filter<>(filterValue, true)) - .orElse(null); - } else { - filter = null; - } - } - } - private void removeFilteringAndSorting() { Element.get(stateNode).getComponent().ifPresent( DataViewUtils::removeComponentFilterAndSortComparator); diff --git a/flow-data/src/test/java/com/vaadin/flow/data/provider/DataCommunicatorTest.java b/flow-data/src/test/java/com/vaadin/flow/data/provider/DataCommunicatorTest.java index 4121d96de3d..b74c0518258 100644 --- a/flow-data/src/test/java/com/vaadin/flow/data/provider/DataCommunicatorTest.java +++ b/flow-data/src/test/java/com/vaadin/flow/data/provider/DataCommunicatorTest.java @@ -1340,7 +1340,7 @@ public void setPageSize_setIncorrectPageSize_throws() { } @Test - public void filter_setFilterAsPermanent_shouldRetainFilter() { + public void filter_setFilterThroughFilterConsumer_shouldRetainFilterBetweenRequests() { SerializableConsumer> filterConsumer = dataCommunicator .setDataProvider(DataProvider.ofItems(new Item(1), new Item(2), new Item(3)), item -> item.id > 1); @@ -1352,7 +1352,7 @@ public void filter_setFilterAsPermanent_shouldRetainFilter() { fakeClientCommunication(); Assert.assertNotNull( - "Permanent filter should be retained after data request", + "Filter should be retained after data request", dataCommunicator.getFilter()); Assert.assertEquals("Unexpected items count", 2, @@ -1365,7 +1365,7 @@ public void filter_setFilterAsPermanent_shouldRetainFilter() { fakeClientCommunication(); Assert.assertNotNull( - "Permanent filter should be retained after data request", + "Filter should be retained after data request", dataCommunicator.getFilter()); Assert.assertEquals("Unexpected items count", 1, @@ -1373,52 +1373,7 @@ public void filter_setFilterAsPermanent_shouldRetainFilter() { } @Test - public void filter_setFilterAsDisposable_shouldDiscardFilterAfterFirstFlush() { - SerializableConsumer>> filterConsumer = dataCommunicator - .setDataProvider(DataProvider.ofItems(new Item(1), new Item(2), - new Item(3)), item -> item.id > 1, false); - - Assert.assertNotNull("Expected initial filter to be set", - dataCommunicator.getFilter()); - - dataCommunicator.setRequestedRange(0, 50); - fakeClientCommunication(); - - Assert.assertNull( - "Disposable filter should be discarded after data request", - dataCommunicator.getFilter()); - - Assert.assertEquals("Unexpected items count", 2, - dataCommunicator.getItemCount()); - - // Check that the filter change works properly - filterConsumer.accept( - new DataCommunicator.Filter<>(item -> item.id > 2, false)); - - dataCommunicator.setRequestedRange(0, 50); - fakeClientCommunication(); - - Assert.assertNull( - "Disposable filter should be discarded after data request", - dataCommunicator.getFilter()); - - Assert.assertEquals("Unexpected items count", 1, - dataCommunicator.getItemCount()); - - // Change to permanent and check that it's not discarded - filterConsumer.accept( - new DataCommunicator.Filter<>(item -> item.id > 2, true)); - - dataCommunicator.setRequestedRange(0, 50); - fakeClientCommunication(); - - Assert.assertNotNull( - "Permanent filter should be retained after data request", - dataCommunicator.getFilter()); - } - - @Test - public void filter_setFilterAsPermanent_firesItemChangeEvent() { + public void filter_setNotifyOnFilterChange_firesItemChangeEvent() { TestComponent testComponent = new TestComponent(element); AtomicBoolean eventTriggered = new AtomicBoolean(false); @@ -1436,16 +1391,16 @@ public void filter_setFilterAsPermanent_firesItemChangeEvent() { dataCommunicator.setRequestedRange(0, 50); fakeClientCommunication(); - Assert.assertTrue("Expected event to be triggered for permanent filter", + Assert.assertTrue("Expected event to be triggered", eventTriggered.get()); } @Test - public void filter_setFilterAsDisposable_doesNotFireItemChangeEvent() { + public void filter_skipNotifyOnFilterChange_doesNotFireItemChangeEvent() { TestComponent testComponent = new TestComponent(element); testComponent.addItemChangeListener(event -> Assert - .fail("Event triggering not expected for disposable filter")); + .fail("Event triggering not expected")); dataCommunicator.setDataProvider( DataProvider.ofItems(new Item(1), new Item(2), new Item(3)), @@ -1490,39 +1445,6 @@ public void setDataProvider_setNewDataProvider_filteringAndSortingRemoved() { new Item(0), listDataView.getItems().findFirst().orElse(null)); } - @Test - public void filter_clearFilterAfterFlush_componentFilterPreserved() { - final Item[] items = { new Item(1), new Item(2), new Item(3) }; - final SerializablePredicate disposableFilter = item -> item.id == 3; - final SerializablePredicate permanentFilter = item -> item.id == 2; - - dataCommunicator.setDataProvider(DataProvider.ofItems(items), - disposableFilter, false); - - AbstractListDataView listDataView = new AbstractListDataView( - dataCommunicator::getDataProvider, new TestComponent(element), - (filter, sorting) -> { - }) { - }; - - listDataView.setFilter(permanentFilter); - - dataCommunicator.setRequestedRange(0, 50); - fakeClientCommunication(); - - Assert.assertNotNull( - "Expected permanent filter to be preserved after flush", - dataCommunicator.getFilter()); - - @SuppressWarnings("unchecked") - SerializablePredicate filter = (SerializablePredicate) dataCommunicator - .getFilter(); - - Assert.assertArrayEquals("Unexpected permanent filter", - Arrays.stream(items).filter(permanentFilter).toArray(), - Arrays.stream(items).filter(filter).toArray()); - } - @Tag("test-component") private static class TestComponent extends Component {