Skip to content

Commit

Permalink
fix: preserve filter to not lose it between page requests (#9421)
Browse files Browse the repository at this point in the history 
Data communicator's filter removal, which has been introduced for components with internal filtering (like a ComboBox), may lead to filter loss during the items scrolling. This fix reverts the filter removal and the components should implement this feature on their side, if necessary.

Related-to: vaadin/flow-components#388
  • Loading branch information
@mshabarov
mshabarov authored Nov 18, 2020
1 parent 19fe386 commit f3be09a
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 158 deletions.
112 changes: 39 additions & 73 deletions flow-data/src/main/java/com/vaadin/flow/data/provider/DataCommunicator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 <F>
* filter's type
Expand All @@ -160,36 +156,35 @@ public static final class Filter<F> 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;
}

/**
* Creates the filter object and sets its lifespan.
*
* @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;
}

/**
Expand All @@ -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
Expand All @@ -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);
}
}

Expand Down Expand Up @@ -387,28 +382,20 @@ public void confirmUpdate(int updateId) {
* that the given data provider is queried for size and previous size
* estimates are discarded.
* <p>
* 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 <code>null</code>
* @param initialFilter
* the initial filter value to use, or <code>null</code> 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 <F>
* the filter type
Expand All @@ -417,13 +404,13 @@ public void confirmUpdate(int updateId) {
*/
public <F> SerializableConsumer<Filter<F>> setDataProvider(
DataProvider<T, F> 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;
Expand Down Expand Up @@ -1152,8 +1139,6 @@ private void flush() {
unregisterPassivatedKeys();

fireItemCountEvent(assumedSize);

clearFilterIfDisposable();
}

/**
Expand All @@ -1163,16 +1148,17 @@ private void flush() {
* <ul>
* <li>the passed item count differs from the item count passed on the
* previous call of this method</li>
* <li>Current component's filter is permanent</li>
* <li>Current component's filter set up to fire the event upon filtering
* changes</li>
* </ul>
*
* @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> component = Element.get(stateNode)
.getComponent();
component.ifPresent(value -> ComponentUtil.fireEvent(value,
Expand Down Expand Up @@ -1357,26 +1343,6 @@ private JsonValue generateJson(T item) {
return json;
}

private void clearFilterIfDisposable() {
if (filter != null && !filter.isPermanent()) {
Optional<Component> component = Element.get(stateNode).getComponent();
if (component.isPresent()) {
// Look up for the component's in-memory filter
Optional<SerializablePredicate<T>> 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);
Expand Down
92 changes: 7 additions & 85 deletions flow-data/src/test/java/com/vaadin/flow/data/provider/DataCommunicatorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1340,7 +1340,7 @@ public void setPageSize_setIncorrectPageSize_throws() {
}

@Test
public void filter_setFilterAsPermanent_shouldRetainFilter() {
public void filter_setFilterThroughFilterConsumer_shouldRetainFilterBetweenRequests() {
SerializableConsumer<SerializablePredicate<Item>> filterConsumer = dataCommunicator
.setDataProvider(DataProvider.ofItems(new Item(1), new Item(2),
new Item(3)), item -> item.id > 1);
Expand All @@ -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,
Expand All @@ -1365,60 +1365,15 @@ 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,
dataCommunicator.getItemCount());
}

@Test
public void filter_setFilterAsDisposable_shouldDiscardFilterAfterFirstFlush() {
SerializableConsumer<DataCommunicator.Filter<SerializablePredicate<Item>>> 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);
Expand All @@ -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)),
Expand Down Expand Up @@ -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<Item> disposableFilter = item -> item.id == 3;
final SerializablePredicate<Item> permanentFilter = item -> item.id == 2;

dataCommunicator.setDataProvider(DataProvider.ofItems(items),
disposableFilter, false);

AbstractListDataView<Item> listDataView = new AbstractListDataView<Item>(
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<Item> filter = (SerializablePredicate<Item>) 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 {

Expand Down

0 comments on commit f3be09a

Please sign in to comment.