Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: preserve filter for not losing it between page requests #9421

Merged
merged 1 commit into from
Nov 18, 2020
Merged

fix: preserve filter for not losing it between page requests #9421

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
112 changes: 39 additions & 73 deletions flow-data/src/main/java/com/vaadin/flow/data/provider/DataCommunicator.java
View file
Open in desktop
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
Open in desktop
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