diff options
25 files changed, 784 insertions, 50 deletions
diff --git a/documentation/datamodel/datamodel-providers.asciidoc b/documentation/datamodel/datamodel-providers.asciidoc index 6b5fea2275..80b9fb9cb4 100644 --- a/documentation/datamodel/datamodel-providers.asciidoc +++ b/documentation/datamodel/datamodel-providers.asciidoc @@ -181,7 +181,7 @@ Button modifyPersonButton = new Button("Modify person", personToChange.setName("Changed person"); - dataProvider.refreshAll(); + dataProvider.refreshItem(personToChange); }); ---- @@ -572,3 +572,48 @@ public class PersonDataProvider } } ---- + +[[lazy-refresh]] +=== Refreshing + +When your application makes changes to the data that is in your backend, you might need to make sure all parts of the application are aware of these changes. +All data providers have the `refreshAll`and `refreshItem` methods. +These methods can be used when data in the backend has been updated. + +For example Spring Data gives you new instances with every request, and making changes to the repository will make old instances of the same object "stale". +In these cases you should inform any interested component by calling `dataProvider.refreshItem(newInstance)`. +This can work out of the box, if your beans have equals and hashCode implementations that check if the objects represent the same data. +Since that is not always the case, the user of a `CallbackDataProvider` can give it a `ValueProvider` that will provide a stable ID for the data objects. +This is usually a method reference, eg. `Person::getId`. + +As an example, our service interface has an update method that returns a new instance of the item. +Other functionality has been omitted to keep focus on the updating. + +[source, java] +---- +public interface PersonService { + Person save(Person person); +} +---- + +Part of the application code wants to update a persons name and save it to the backend. + +[source, java] +---- +PersonService service; +DataProvider<Person, String> allPersonsWithId = new CallbackDataProvider<>( + fetchCallback, sizeCallback, Person::getId); + +NativeSelect<Person> persons = new NativeSelect<>(); +persons.setDataProvider(allPersonsWithId); + +Button modifyPersonButton = new Button("Modify person", + clickEvent -> { + Person personToChange = persons.getValue(); + + personToChange.setName("Changed person"); + + Person newInstance = service.save(personToChange); + dataProvider.refreshItem(newInstance); +}); +---- diff --git a/server/src/main/java/com/vaadin/data/provider/AbstractDataProvider.java b/server/src/main/java/com/vaadin/data/provider/AbstractDataProvider.java index 3319bfb382..c87e18e1c2 100644 --- a/server/src/main/java/com/vaadin/data/provider/AbstractDataProvider.java +++ b/server/src/main/java/com/vaadin/data/provider/AbstractDataProvider.java @@ -18,6 +18,7 @@ package com.vaadin.data.provider; import java.lang.reflect.Method; import java.util.EventObject; +import com.vaadin.data.provider.DataChangeEvent.DataRefreshEvent; import com.vaadin.event.EventRouter; import com.vaadin.shared.Registration; @@ -39,14 +40,20 @@ public abstract class AbstractDataProvider<T, F> implements DataProvider<T, F> { private EventRouter eventRouter; @Override - public Registration addDataProviderListener(DataProviderListener listener) { + public Registration addDataProviderListener( + DataProviderListener<T> listener) { return addListener(DataChangeEvent.class, listener, DataProviderListener.class.getMethods()[0]); } @Override public void refreshAll() { - fireEvent(new DataChangeEvent(this)); + fireEvent(new DataChangeEvent<>(this)); + } + + @Override + public void refreshItem(T item) { + fireEvent(new DataRefreshEvent<>(this, item)); } /** @@ -65,7 +72,7 @@ public abstract class AbstractDataProvider<T, F> implements DataProvider<T, F> { * @return a registration for the listener */ protected Registration addListener(Class<?> eventType, - DataProviderListener listener, Method method) { + DataProviderListener<T> listener, Method method) { if (eventRouter == null) { eventRouter = new EventRouter(); } diff --git a/server/src/main/java/com/vaadin/data/provider/BackEndDataProvider.java b/server/src/main/java/com/vaadin/data/provider/BackEndDataProvider.java index 36d875a21a..b50b5c7fc8 100644 --- a/server/src/main/java/com/vaadin/data/provider/BackEndDataProvider.java +++ b/server/src/main/java/com/vaadin/data/provider/BackEndDataProvider.java @@ -71,5 +71,4 @@ public interface BackEndDataProvider<T, F> extends DataProvider<T, F> { default boolean isInMemory() { return false; } - } diff --git a/server/src/main/java/com/vaadin/data/provider/CallbackDataProvider.java b/server/src/main/java/com/vaadin/data/provider/CallbackDataProvider.java index d83c1f1934..2f68e57e06 100644 --- a/server/src/main/java/com/vaadin/data/provider/CallbackDataProvider.java +++ b/server/src/main/java/com/vaadin/data/provider/CallbackDataProvider.java @@ -18,6 +18,7 @@ package com.vaadin.data.provider; import java.util.Objects; import java.util.stream.Stream; +import com.vaadin.data.ValueProvider; import com.vaadin.server.SerializableFunction; import com.vaadin.server.SerializableToIntFunction; @@ -36,6 +37,7 @@ public class CallbackDataProvider<T, F> extends AbstractBackEndDataProvider<T, F> { private final SerializableFunction<Query<T, F>, Stream<T>> fetchCallback; private final SerializableToIntFunction<Query<T, F>> sizeCallback; + private final ValueProvider<T, Object> idGetter; /** * Constructs a new DataProvider to request data using callbacks for @@ -45,16 +47,40 @@ public class CallbackDataProvider<T, F> * function that returns a stream of items from the back end for * a query * @param sizeCallback - * function that returns the number of items in the back end for - * a query + * function that return the number of items in the back end for a + * query + * + * @see #CallbackDataProvider(SerializableFunction, + * SerializableToIntFunction, ValueProvider) */ public CallbackDataProvider( SerializableFunction<Query<T, F>, Stream<T>> fetchCallback, SerializableToIntFunction<Query<T, F>> sizeCallback) { - Objects.requireNonNull(fetchCallback, "Request function can't be null"); + this(fetchCallback, sizeCallback, t -> t); + } + + /** + * Constructs a new DataProvider to request data using callbacks for + * fetching and counting items in the back end. + * + * @param fetchCallBack + * function that requests data from back end based on query + * @param sizeCallback + * function that returns the amount of data in back end for query + * @param identifierGetter + * function that returns the identifier for a given item + */ + public CallbackDataProvider( + SerializableFunction<Query<T, F>, Stream<T>> fetchCallBack, + SerializableToIntFunction<Query<T, F>> sizeCallback, + ValueProvider<T, Object> identifierGetter) { + Objects.requireNonNull(fetchCallBack, "Request function can't be null"); Objects.requireNonNull(sizeCallback, "Size callback can't be null"); - this.fetchCallback = fetchCallback; + Objects.requireNonNull(identifierGetter, + "Identifier getter function can't be null"); + this.fetchCallback = fetchCallBack; this.sizeCallback = sizeCallback; + this.idGetter = identifierGetter; } @Override @@ -66,4 +92,12 @@ public class CallbackDataProvider<T, F> protected int sizeInBackEnd(Query<T, F> query) { return sizeCallback.applyAsInt(query); } + + @Override + public Object getId(T item) { + Object itemId = idGetter.apply(item); + assert itemId != null : "CallbackDataProvider got null as an id for item: " + + item; + return itemId; + } } diff --git a/server/src/main/java/com/vaadin/data/provider/DataChangeEvent.java b/server/src/main/java/com/vaadin/data/provider/DataChangeEvent.java index 86ad9235a6..93def4f810 100644 --- a/server/src/main/java/com/vaadin/data/provider/DataChangeEvent.java +++ b/server/src/main/java/com/vaadin/data/provider/DataChangeEvent.java @@ -16,18 +16,57 @@ package com.vaadin.data.provider; import java.util.EventObject; +import java.util.Objects; /** * An event fired when the data of a {@code DataProvider} changes. * - * * @see DataProviderListener * * @author Vaadin Ltd * @since 8.0 * + * + * @param <T> + * the data type */ -public class DataChangeEvent extends EventObject { +public class DataChangeEvent<T> extends EventObject { + + /** + * An event fired when a single item of a {@code DataProvider} has been + * updated. + * + * @param <T> + * the data type + */ + public static class DataRefreshEvent<T> extends DataChangeEvent<T> { + + private final T item; + + /** + * Creates a new data refresh event originating from the given data + * provider. + * + * @param source + * the data provider, not null + * @param item + * the updated item, not null + */ + public DataRefreshEvent(DataProvider<T, ?> source, T item) { + super(source); + Objects.requireNonNull(item, "Refreshed item can't be null"); + this.item = item; + } + + /** + * Gets the refreshed item. + * + * @return the refreshed item + */ + public T getItem() { + return item; + } + } /** * Creates a new {@code DataChangeEvent} event originating from the given @@ -36,13 +75,12 @@ public class DataChangeEvent extends EventObject { * @param source * the data provider, not null */ - public DataChangeEvent(DataProvider<?, ?> source) { + public DataChangeEvent(DataProvider<T, ?> source) { super(source); } @Override - public DataProvider<?, ?> getSource() { - return (DataProvider<?, ?>) super.getSource(); + public DataProvider<T, ?> getSource() { + return (DataProvider<T, ?>) super.getSource(); } - } diff --git a/server/src/main/java/com/vaadin/data/provider/DataCommunicator.java b/server/src/main/java/com/vaadin/data/provider/DataCommunicator.java index 021623555e..9080ad3442 100644 --- a/server/src/main/java/com/vaadin/data/provider/DataCommunicator.java +++ b/server/src/main/java/com/vaadin/data/provider/DataCommunicator.java @@ -27,6 +27,7 @@ import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; +import com.vaadin.data.provider.DataChangeEvent.DataRefreshEvent; import com.vaadin.server.AbstractExtension; import com.vaadin.server.KeyMapper; import com.vaadin.server.SerializableConsumer; @@ -465,17 +466,20 @@ public class DataCommunicator<T> extends AbstractExtension { * @param initialFilter * the initial filter value to use, or <code>null</code> to not * use any initial filter value + * + * @param <F> + * the filter type + * * @return a consumer that accepts a new filter value to use */ public <F> SerializableConsumer<F> setDataProvider( DataProvider<T, F> dataProvider, F initialFilter) { Objects.requireNonNull(dataProvider, "data provider cannot be null"); - filter = initialFilter; - this.dataProvider = dataProvider; - detachDataProviderListener(); dropAllData(); + this.dataProvider = dataProvider; + /* * This introduces behavior which influence on the client-server * communication: now the very first response to the client will always @@ -556,8 +560,18 @@ public class DataCommunicator<T> extends AbstractExtension { private void attachDataProviderListener() { dataProviderUpdateRegistration = getDataProvider() - .addDataProviderListener( - event -> getUI().access(() -> reset())); + .addDataProviderListener(event -> { + getUI().access(() -> { + if (event instanceof DataRefreshEvent) { + T item = ((DataRefreshEvent<T>) event).getItem(); + generators.forEach(g -> g.refreshData(item)); + keyMapper.refresh(item, dataProvider::getId); + refresh(item); + } else { + reset(); + } + }); + }); } private void detachDataProviderListener() { diff --git a/server/src/main/java/com/vaadin/data/provider/DataGenerator.java b/server/src/main/java/com/vaadin/data/provider/DataGenerator.java index 5e88f7ff66..c5a5a6b9c9 100644 --- a/server/src/main/java/com/vaadin/data/provider/DataGenerator.java +++ b/server/src/main/java/com/vaadin/data/provider/DataGenerator.java @@ -62,4 +62,14 @@ public interface DataGenerator<T> extends Serializable { */ public default void destroyAllData() { } + + /** + * Informs the {@code DataGenerator} that a data object has been updated. + * This method should update any unneeded information stored for given item. + * + * @param item + * the updated item + */ + public default void refreshData(T item) { + } } diff --git a/server/src/main/java/com/vaadin/data/provider/DataKeyMapper.java b/server/src/main/java/com/vaadin/data/provider/DataKeyMapper.java index 63bd2d92f8..0ec6748a85 100644 --- a/server/src/main/java/com/vaadin/data/provider/DataKeyMapper.java +++ b/server/src/main/java/com/vaadin/data/provider/DataKeyMapper.java @@ -17,6 +17,8 @@ package com.vaadin.data.provider; import java.io.Serializable; +import com.vaadin.data.ValueProvider; + /** * DataKeyMapper to map data objects to key strings. * @@ -59,4 +61,16 @@ public interface DataKeyMapper<T> extends Serializable { * Dropped keys are not reused. */ void removeAll(); + + /** + * Updates any existing mappings of given data object. The equality of two + * data objects is determined by the equality of their identifiers provided + * by the given value provider. + * + * @param dataObject + * the data object to update + * @param identifierGetter + * the function to get an identifier from a data object + */ + void refresh(T dataObject, ValueProvider<T, Object> identifierGetter); } 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 380a1f4809..f9c3c80146 100644 --- a/server/src/main/java/com/vaadin/data/provider/DataProvider.java +++ b/server/src/main/java/com/vaadin/data/provider/DataProvider.java @@ -88,12 +88,39 @@ public interface DataProvider<T, F> extends Serializable { Stream<T> fetch(Query<T, F> query); /** + * Refreshes the given item. This method should be used to inform all + * {@link DataProviderListener DataProviderListeners} that an item has been + * updated or replaced with a new instance. + * + * @param item + * the item to refresh + */ + void refreshItem(T item); + + /** * Refreshes all data based on currently available data in the underlying * provider. */ void refreshAll(); /** + * Gets an identifier for the given item. This identifier is used by the + * framework to determine equality between two items. + * <p> + * Default is to use item itself as its own identifier. If the item has + * {@link Object#equals(Object)} and {@link Object#hashCode()} implemented + * in a way that it can be compared to other items, no changes are required. + * + * @param item + * the item to get identifier for; not {@code null} + * @return the identifier for given item; not {@code null} + */ + public default Object getId(T item) { + Objects.requireNonNull(item, "Cannot provide an id for a null item."); + return item; + } + + /** * Adds a data provider listener. The listener is called when some piece of * data is updated. * <p> @@ -106,7 +133,7 @@ public interface DataProvider<T, F> extends Serializable { * the data change listener, not null * @return a registration for the listener */ - Registration addDataProviderListener(DataProviderListener listener); + Registration addDataProviderListener(DataProviderListener<T> listener); /** * Wraps this data provider to create a data provider that uses a different diff --git a/server/src/main/java/com/vaadin/data/provider/DataProviderListener.java b/server/src/main/java/com/vaadin/data/provider/DataProviderListener.java index 582bdc1d86..da97e34c80 100644 --- a/server/src/main/java/com/vaadin/data/provider/DataProviderListener.java +++ b/server/src/main/java/com/vaadin/data/provider/DataProviderListener.java @@ -23,9 +23,12 @@ import java.io.Serializable; * * @author Vaadin Ltd * @since 8.0 + * + * @param <T> + * the data type */ @FunctionalInterface -public interface DataProviderListener extends Serializable { +public interface DataProviderListener<T> extends Serializable { /** * Invoked when this listener receives a data change event from a data @@ -39,5 +42,5 @@ public interface DataProviderListener extends Serializable { * @param event * the received event, not null */ - void onDataChange(DataChangeEvent event); + void onDataChange(DataChangeEvent<T> event); } diff --git a/server/src/main/java/com/vaadin/data/provider/DataProviderWrapper.java b/server/src/main/java/com/vaadin/data/provider/DataProviderWrapper.java index 1eabb67abe..5195aa7837 100644 --- a/server/src/main/java/com/vaadin/data/provider/DataProviderWrapper.java +++ b/server/src/main/java/com/vaadin/data/provider/DataProviderWrapper.java @@ -65,7 +65,18 @@ public abstract class DataProviderWrapper<T, F, M> } @Override - public Registration addDataProviderListener(DataProviderListener listener) { + public void refreshItem(T item) { + dataProvider.refreshItem(item); + } + + @Override + public Object getId(T item) { + return dataProvider.getId(item); + } + + @Override + public Registration addDataProviderListener( + DataProviderListener<T> listener) { return dataProvider.addDataProviderListener(listener); } diff --git a/server/src/main/java/com/vaadin/server/KeyMapper.java b/server/src/main/java/com/vaadin/server/KeyMapper.java index 471fea3e69..7ed723be66 100644 --- a/server/src/main/java/com/vaadin/server/KeyMapper.java +++ b/server/src/main/java/com/vaadin/server/KeyMapper.java @@ -19,6 +19,7 @@ package com.vaadin.server; import java.io.Serializable; import java.util.HashMap; +import com.vaadin.data.ValueProvider; import com.vaadin.data.provider.DataKeyMapper; /** @@ -113,4 +114,17 @@ public class KeyMapper<V> implements DataKeyMapper<V>, Serializable { public boolean containsKey(String key) { return keyObjectMap.containsKey(key); } + + @Override + public void refresh(V dataObject, + ValueProvider<V, Object> identifierGetter) { + Object id = identifierGetter.apply(dataObject); + objectKeyMap.entrySet().stream() + .filter(e -> identifierGetter.apply(e.getKey()).equals(id)) + .findAny().ifPresent(e -> { + String key = objectKeyMap.remove(e.getKey()); + objectKeyMap.put(dataObject, key); + keyObjectMap.put(key, dataObject); + }); + } } diff --git a/server/src/main/java/com/vaadin/ui/AbstractMultiSelect.java b/server/src/main/java/com/vaadin/ui/AbstractMultiSelect.java index 22df69da5c..9947067288 100644 --- a/server/src/main/java/com/vaadin/ui/AbstractMultiSelect.java +++ b/server/src/main/java/com/vaadin/ui/AbstractMultiSelect.java @@ -15,6 +15,7 @@ */ package com.vaadin.ui; +import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.HashSet; @@ -31,6 +32,7 @@ import com.vaadin.data.HasValue; import com.vaadin.data.SelectionModel; import com.vaadin.data.SelectionModel.Multi; import com.vaadin.data.provider.DataGenerator; +import com.vaadin.data.provider.DataProvider; import com.vaadin.event.selection.MultiSelectionEvent; import com.vaadin.event.selection.MultiSelectionListener; import com.vaadin.server.Resource; @@ -59,7 +61,7 @@ import elemental.json.JsonObject; public abstract class AbstractMultiSelect<T> extends AbstractListing<T> implements MultiSelect<T> { - private Set<T> selection = new LinkedHashSet<>(); + private List<T> selection = new ArrayList<>(); private class MultiSelectServerRpcImpl implements MultiSelectServerRpc { @Override @@ -122,6 +124,11 @@ public abstract class AbstractMultiSelect<T> extends AbstractListing<T> public void destroyAllData() { AbstractMultiSelect.this.deselectAll(); } + + @Override + public void refreshData(T item) { + refreshSelectedItem(item); + } } /** @@ -228,9 +235,9 @@ public abstract class AbstractMultiSelect<T> extends AbstractListing<T> @Override public Registration addValueChangeListener( HasValue.ValueChangeListener<Set<T>> listener) { - return addSelectionListener(event -> listener.valueChange( - new ValueChangeEvent<>(this, event.getOldValue(), - event.isUserOriginated()))); + return addSelectionListener( + event -> listener.valueChange(new ValueChangeEvent<>(this, + event.getOldValue(), event.isUserOriginated()))); } /** @@ -346,12 +353,15 @@ public abstract class AbstractMultiSelect<T> extends AbstractListing<T> return; } - updateSelection(Set::clear, false); + updateSelection(Collection::clear, false); } @Override public boolean isSelected(T item) { - return selection.contains(item); + DataProvider<T, ?> dataProvider = internalGetDataProvider(); + Object id = dataProvider.getId(item); + return selection.stream().map(dataProvider::getId).anyMatch(id::equals); + } /** @@ -469,7 +479,7 @@ public abstract class AbstractMultiSelect<T> extends AbstractListing<T> return item; } - private void updateSelection(SerializableConsumer<Set<T>> handler, + private void updateSelection(SerializableConsumer<Collection<T>> handler, boolean userOriginated) { LinkedHashSet<T> oldSelection = new LinkedHashSet<>(selection); handler.accept(selection); @@ -479,4 +489,15 @@ public abstract class AbstractMultiSelect<T> extends AbstractListing<T> getDataCommunicator().reset(); } + + private final void refreshSelectedItem(T item) { + DataProvider<T, ?> dataProvider = internalGetDataProvider(); + Object id = dataProvider.getId(item); + for (int i = 0; i < selection.size(); ++i) { + if (id.equals(dataProvider.getId(selection.get(i)))) { + selection.set(i, item); + return; + } + } + } } diff --git a/server/src/main/java/com/vaadin/ui/components/grid/MultiSelectionModelImpl.java b/server/src/main/java/com/vaadin/ui/components/grid/MultiSelectionModelImpl.java index 551918ef85..97f79a4655 100644 --- a/server/src/main/java/com/vaadin/ui/components/grid/MultiSelectionModelImpl.java +++ b/server/src/main/java/com/vaadin/ui/components/grid/MultiSelectionModelImpl.java @@ -15,9 +15,12 @@ */ package com.vaadin.ui.components.grid; +import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import java.util.LinkedHashSet; +import java.util.List; import java.util.Objects; import java.util.Set; import java.util.function.Consumer; @@ -119,7 +122,7 @@ public class MultiSelectionModelImpl<T> extends AbstractSelectionModel<T> } } - private Set<T> selection = new LinkedHashSet<>(); + private List<T> selection = new ArrayList<>(); private SelectAllCheckBoxVisibility selectAllCheckBoxVisibility = SelectAllCheckBoxVisibility.DEFAULT; @@ -199,8 +202,20 @@ public class MultiSelectionModelImpl<T> extends AbstractSelectionModel<T> @Override public boolean isSelected(T item) { return isAllSelected() - || com.vaadin.ui.components.grid.MultiSelectionModel.super.isSelected( - item); + || selectionContainsId(getGrid().getDataProvider().getId(item)); + } + + /** + * Returns if the given id belongs to one of the selected items. + * + * @param id + * the id to check for + * @return {@code true} if id is selected, {@code false} if not + */ + protected boolean selectionContainsId(Object id) { + DataProvider<T, ?> dataProvider = getGrid().getDataProvider(); + return selection.stream().map(dataProvider::getId) + .anyMatch(i -> id.equals(i)); } @Override @@ -447,7 +462,7 @@ public class MultiSelectionModelImpl<T> extends AbstractSelectionModel<T> return getState(false).selectionAllowed; } - private void doUpdateSelection(Consumer<Set<T>> handler, + private void doUpdateSelection(Consumer<Collection<T>> handler, boolean userOriginated) { if (getParent() == null) { throw new IllegalStateException( @@ -460,4 +475,16 @@ public class MultiSelectionModelImpl<T> extends AbstractSelectionModel<T> fireEvent(new MultiSelectionEvent<>(getGrid(), asMultiSelect(), oldSelection, userOriginated)); } + + @Override + public void refreshData(T item) { + DataProvider<T, ?> dataProvider = getGrid().getDataProvider(); + Object refreshId = dataProvider.getId(item); + for (int i = 0; i < selection.size(); ++i) { + if (dataProvider.getId(selection.get(i)).equals(refreshId)) { + selection.set(i, item); + return; + } + } + } } diff --git a/server/src/main/java/com/vaadin/ui/components/grid/SingleSelectionModelImpl.java b/server/src/main/java/com/vaadin/ui/components/grid/SingleSelectionModelImpl.java index 42ca337e65..cd985f473a 100644 --- a/server/src/main/java/com/vaadin/ui/components/grid/SingleSelectionModelImpl.java +++ b/server/src/main/java/com/vaadin/ui/components/grid/SingleSelectionModelImpl.java @@ -284,4 +284,18 @@ public class SingleSelectionModelImpl<T> extends AbstractSelectionModel<T> } }; } + + @Override + public void refreshData(T item) { + if (isSelected(item)) { + selectedItem = item; + } + } + + @Override + public boolean isSelected(T item) { + return item != null && selectedItem != null + && getGrid().getDataProvider().getId(selectedItem) + .equals(getGrid().getDataProvider().getId(item)); + } } diff --git a/server/src/test/java/com/vaadin/data/provider/AbstractDataProviderTest.java b/server/src/test/java/com/vaadin/data/provider/AbstractDataProviderTest.java index 5fbaaaeb8a..3cd368944d 100644 --- a/server/src/test/java/com/vaadin/data/provider/AbstractDataProviderTest.java +++ b/server/src/test/java/com/vaadin/data/provider/AbstractDataProviderTest.java @@ -31,6 +31,7 @@ public class AbstractDataProviderTest { private static class TestDataProvider extends AbstractDataProvider<Object, Object> { + @Override public Stream<Object> fetch(Query<Object, Object> t) { return null; @@ -50,7 +51,7 @@ public class AbstractDataProviderTest { @Test public void refreshAll_notifyListeners() { TestDataProvider dataProvider = new TestDataProvider(); - AtomicReference<DataChangeEvent> event = new AtomicReference<>(); + AtomicReference<DataChangeEvent<Object>> event = new AtomicReference<>(); dataProvider.addDataProviderListener(ev -> { Assert.assertNull(event.get()); event.set(ev); @@ -63,7 +64,7 @@ public class AbstractDataProviderTest { @Test public void removeListener_listenerIsNotNotified() { TestDataProvider dataProvider = new TestDataProvider(); - AtomicReference<DataChangeEvent> event = new AtomicReference<>(); + AtomicReference<DataChangeEvent<Object>> event = new AtomicReference<>(); Registration registration = dataProvider .addDataProviderListener(ev -> event.set(ev)); registration.remove(); diff --git a/server/src/test/java/com/vaadin/data/provider/DataCommunicatorTest.java b/server/src/test/java/com/vaadin/data/provider/DataCommunicatorTest.java index 50a3ebc337..1e75d57263 100644 --- a/server/src/test/java/com/vaadin/data/provider/DataCommunicatorTest.java +++ b/server/src/test/java/com/vaadin/data/provider/DataCommunicatorTest.java @@ -16,6 +16,7 @@ package com.vaadin.data.provider; import java.util.Collections; +import java.util.concurrent.Future; import org.junit.Assert; import org.junit.Test; @@ -28,12 +29,16 @@ import com.vaadin.server.VaadinSession; import com.vaadin.shared.Registration; import com.vaadin.ui.UI; +import elemental.json.JsonObject; + /** * @author Vaadin Ltd * */ public class DataCommunicatorTest { + private static final Object TEST_OBJECT = new Object(); + private static class TestUI extends UI { private final VaadinSession session; @@ -50,6 +55,12 @@ public class DataCommunicatorTest { public VaadinSession getSession() { return session; } + + @Override + public Future<Void> access(Runnable runnable) { + runnable.run(); + return null; + } } private static class TestDataProvider extends ListDataProvider<Object> @@ -58,12 +69,12 @@ public class DataCommunicatorTest { private Registration registration; public TestDataProvider() { - super(Collections.singleton(new Object())); + super(Collections.singleton(TEST_OBJECT)); } @Override public Registration addDataProviderListener( - DataProviderListener listener) { + DataProviderListener<Object> listener) { registration = super.addDataProviderListener(listener); return this; } @@ -86,6 +97,21 @@ public class DataCommunicatorTest { } } + private static class TestDataGenerator implements DataGenerator<Object> { + Object refreshed = null; + Object generated = null; + + @Override + public void generateData(Object item, JsonObject jsonObject) { + generated = item; + } + + @Override + public void refreshData(Object item) { + refreshed = item; + } + } + private final MockVaadinSession session = new MockVaadinSession( Mockito.mock(VaadinService.class)); @@ -127,4 +153,42 @@ public class DataCommunicatorTest { Assert.assertFalse(dataProvider.isListenerAdded()); } + @Test + public void refresh_dataProviderListenerCallsRefreshInDataGeneartors() { + session.lock(); + + UI ui = new TestUI(session); + + TestDataCommunicator communicator = new TestDataCommunicator(); + communicator.extend(ui); + + TestDataProvider dataProvider = new TestDataProvider(); + communicator.setDataProvider(dataProvider, null); + + TestDataGenerator generator = new TestDataGenerator(); + communicator.addDataGenerator(generator); + + // Generate initial data. + communicator.beforeClientResponse(true); + Assert.assertEquals("DataGenerator generate was not called", + TEST_OBJECT, generator.generated); + generator.generated = null; + + // Make sure data does not get re-generated + communicator.beforeClientResponse(false); + Assert.assertEquals("DataGenerator generate was called again", null, + generator.generated); + + // Refresh a data object to trigger an update. + dataProvider.refreshItem(TEST_OBJECT); + + Assert.assertEquals("DataGenerator refresh was not called", TEST_OBJECT, + generator.refreshed); + + // Test refreshed data generation + communicator.beforeClientResponse(false); + Assert.assertEquals("DataGenerator generate was not called", + TEST_OBJECT, generator.generated); + } + } diff --git a/server/src/test/java/com/vaadin/data/provider/ReplaceListDataProvider.java b/server/src/test/java/com/vaadin/data/provider/ReplaceListDataProvider.java new file mode 100644 index 0000000000..8d204fbffd --- /dev/null +++ b/server/src/test/java/com/vaadin/data/provider/ReplaceListDataProvider.java @@ -0,0 +1,66 @@ +package com.vaadin.data.provider; + +import java.util.List; +import java.util.stream.Stream; + +/** + * A dummy data provider for testing item replacement and stale elements. + */ +public class ReplaceListDataProvider + extends AbstractDataProvider<StrBean, Void> { + + private final List<StrBean> backend; + + public ReplaceListDataProvider(List<StrBean> items) { + backend = items; + } + + @Override + public void refreshItem(StrBean item) { + if (replaceItem(item)) { + super.refreshItem(item); + } + } + + private boolean replaceItem(StrBean item) { + for (int i = 0; i < backend.size(); ++i) { + if (getId(backend.get(i)).equals(getId(item))) { + if (backend.get(i).equals(item)) { + return false; + } + backend.remove(i); + backend.add(i, item); + return true; + } + } + return false; + } + + @Override + public boolean isInMemory() { + return true; + } + + @Override + public int size(Query<StrBean, Void> t) { + return backend.size(); + } + + @Override + public Stream<StrBean> fetch(Query<StrBean, Void> query) { + return backend.stream().skip(query.getOffset()).limit(query.getLimit()); + } + + public boolean isStale(StrBean item) { + Object id = getId(item); + boolean itemExistsInBackEnd = backend.contains(item); + boolean backEndHasInstanceWithSameId = backend.stream().map(this::getId) + .filter(i -> id.equals(i)).count() == 1; + return !itemExistsInBackEnd && backEndHasInstanceWithSameId; + } + + @Override + public Object getId(StrBean item) { + return item.getId(); + } +} diff --git a/server/src/test/java/com/vaadin/data/provider/ReplaceListDataProviderTest.java b/server/src/test/java/com/vaadin/data/provider/ReplaceListDataProviderTest.java new file mode 100644 index 0000000000..7552691413 --- /dev/null +++ b/server/src/test/java/com/vaadin/data/provider/ReplaceListDataProviderTest.java @@ -0,0 +1,49 @@ +package com.vaadin.data.provider; + +import java.util.ArrayList; +import java.util.Arrays; + +import org.junit.Assert; +import org.junit.Test; + +/** + * Test class that verifies that ReplaceListDataProvider functions the way it's + * meant to. + * + */ +public class ReplaceListDataProviderTest { + + private static final StrBean TEST_OBJECT = new StrBean("Foo", 10, -1); + private ReplaceListDataProvider dataProvider = new ReplaceListDataProvider( + new ArrayList<>(Arrays.asList(TEST_OBJECT))); + + @Test + public void testGetIdOfItem() { + Object id = dataProvider.fetch(new Query<>()).findFirst() + .map(dataProvider::getId).get(); + Assert.assertEquals("DataProvider not using correct identifier getter", + TEST_OBJECT.getId(), id); + } + + @Test + public void testGetIdOfReplacementItem() { + Assert.assertFalse("Test object was stale before making any changes.", + dataProvider.isStale(TEST_OBJECT)); + + dataProvider.refreshItem(new StrBean("Replacement TestObject", 10, -2)); + + StrBean fromDataProvider = dataProvider.fetch(new Query<>()).findFirst() + .get(); + Object id = dataProvider.getId(fromDataProvider); + + Assert.assertNotEquals("DataProvider did not return the replacement", + TEST_OBJECT, fromDataProvider); + + Assert.assertEquals("DataProvider not using correct identifier getter", + TEST_OBJECT.getId(), id); + + Assert.assertTrue("Old test object should be stale", + dataProvider.isStale(TEST_OBJECT)); + } + +} diff --git a/server/src/test/java/com/vaadin/data/provider/StrBean.java b/server/src/test/java/com/vaadin/data/provider/StrBean.java index b596ccdde5..6deb1c1af6 100644 --- a/server/src/test/java/com/vaadin/data/provider/StrBean.java +++ b/server/src/test/java/com/vaadin/data/provider/StrBean.java @@ -6,7 +6,7 @@ import java.util.List; import java.util.Objects; import java.util.Random; -class StrBean implements Serializable { +public class StrBean implements Serializable { private static final String[] values = new String[] { "Foo", "Bar", "Baz" }; diff --git a/server/src/test/java/com/vaadin/tests/data/selection/AbstractStaleSelectionTest.java b/server/src/test/java/com/vaadin/tests/data/selection/AbstractStaleSelectionTest.java new file mode 100644 index 0000000000..45e25cc867 --- /dev/null +++ b/server/src/test/java/com/vaadin/tests/data/selection/AbstractStaleSelectionTest.java @@ -0,0 +1,68 @@ +package com.vaadin.tests.data.selection; + +import java.util.List; +import java.util.concurrent.Future; + +import org.junit.Assert; +import org.junit.Before; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameter; + +import com.vaadin.data.provider.ReplaceListDataProvider; +import com.vaadin.data.provider.StrBean; +import com.vaadin.server.VaadinRequest; +import com.vaadin.server.VaadinSession; +import com.vaadin.tests.util.AlwaysLockedVaadinSession; +import com.vaadin.ui.AbstractListing; +import com.vaadin.ui.UI; + +@RunWith(Parameterized.class) +public abstract class AbstractStaleSelectionTest<S extends AbstractListing<StrBean>> { + + protected ReplaceListDataProvider dataProvider; + protected final List<StrBean> data = StrBean.generateRandomBeans(2); + + @Parameter(0) + public String name; + + @Parameter(1) + public S select; + + @Before + public void setUp() { + dataProvider = new ReplaceListDataProvider(data); + + final VaadinSession application = new AlwaysLockedVaadinSession(null); + final UI uI = new UI() { + @Override + protected void init(VaadinRequest request) { + } + + @Override + public VaadinSession getSession() { + return application; + } + + @Override + public Future<Void> access(Runnable runnable) { + runnable.run(); + return null; + } + }; + uI.setContent(select); + uI.attach(); + select.getDataCommunicator().setDataProvider(dataProvider, null); + } + + protected final void assertIsStale(StrBean bean) { + Assert.assertTrue("Bean with id " + bean.getId() + " should be stale.", + dataProvider.isStale(bean)); + } + + protected final void assertNotStale(StrBean bean) { + Assert.assertFalse( + "Bean with id " + bean.getId() + " should not be stale.", + dataProvider.isStale(bean)); + } +} diff --git a/server/src/test/java/com/vaadin/tests/data/selection/GridStaleElementTest.java b/server/src/test/java/com/vaadin/tests/data/selection/GridStaleElementTest.java new file mode 100644 index 0000000000..64b7ecbc35 --- /dev/null +++ b/server/src/test/java/com/vaadin/tests/data/selection/GridStaleElementTest.java @@ -0,0 +1,116 @@ +package com.vaadin.tests.data.selection; + +import java.util.List; +import java.util.concurrent.Future; + +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; + +import com.vaadin.data.provider.ReplaceListDataProvider; +import com.vaadin.data.provider.StrBean; +import com.vaadin.server.VaadinRequest; +import com.vaadin.server.VaadinSession; +import com.vaadin.tests.util.AlwaysLockedVaadinSession; +import com.vaadin.ui.Grid; +import com.vaadin.ui.Grid.SelectionMode; +import com.vaadin.ui.UI; +import com.vaadin.ui.components.grid.GridSelectionModel; + +public class GridStaleElementTest { + + private Grid<StrBean> grid = new Grid<>(); + private ReplaceListDataProvider dataProvider; + private List<StrBean> data = StrBean.generateRandomBeans(2); + + @Before + public void setUp() { + // Make Grid attached to UI to make DataCommunicator do it's magic. + final VaadinSession application = new AlwaysLockedVaadinSession(null); + final UI uI = new UI() { + @Override + protected void init(VaadinRequest request) { + } + + @Override + public VaadinSession getSession() { + return application; + } + + @Override + public Future<Void> access(Runnable runnable) { + runnable.run(); + return null; + } + }; + uI.setContent(grid); + uI.attach(); + dataProvider = new ReplaceListDataProvider(data); + grid.setDataProvider(dataProvider); + } + + @Test + public void testGridMultiSelectionUpdateOnRefreshItem() { + StrBean toReplace = data.get(0); + assertNotStale(toReplace); + + GridSelectionModel<StrBean> model = grid + .setSelectionMode(SelectionMode.MULTI); + model.select(toReplace); + + StrBean replacement = new StrBean("Replacement bean", toReplace.getId(), + -1); + dataProvider.refreshItem(replacement); + + assertStale(toReplace); + model.getSelectedItems() + .forEach(item -> Assert.assertFalse( + "Selection should not contain stale values", + dataProvider.isStale(item))); + + Object oldId = dataProvider.getId(toReplace); + Assert.assertTrue("Selection did not contain an item with matching Id.", + model.getSelectedItems().stream().map(dataProvider::getId) + .anyMatch(oldId::equals)); + Assert.assertTrue("Stale element is not considered selected.", + model.isSelected(toReplace)); + } + + @Test + public void testGridSingleSelectionUpdateOnRefreshItem() { + StrBean toReplace = data.get(0); + assertNotStale(toReplace); + + GridSelectionModel<StrBean> model = grid + .setSelectionMode(SelectionMode.SINGLE); + model.select(toReplace); + + StrBean replacement = new StrBean("Replacement bean", toReplace.getId(), + -1); + dataProvider.refreshItem(replacement); + + assertStale(toReplace); + model.getSelectedItems() + .forEach(i -> Assert.assertFalse( + "Selection should not contain stale values", + dataProvider.isStale(i))); + + Assert.assertTrue("Selection did not contain an item with matching Id.", + model.getSelectedItems().stream().map(dataProvider::getId) + .filter(i -> dataProvider.getId(toReplace).equals(i)) + .findFirst().isPresent()); + Assert.assertTrue("Stale element is not considered selected.", + model.isSelected(toReplace)); + } + + private void assertNotStale(StrBean bean) { + Assert.assertFalse( + "Bean with id " + bean.getId() + " should not be stale.", + dataProvider.isStale(bean)); + } + + private void assertStale(StrBean bean) { + Assert.assertTrue("Bean with id " + bean.getId() + " should be stale.", + dataProvider.isStale(bean)); + } +} diff --git a/server/src/test/java/com/vaadin/tests/data/selection/StaleMultiSelectionTest.java b/server/src/test/java/com/vaadin/tests/data/selection/StaleMultiSelectionTest.java new file mode 100644 index 0000000000..9769ec3997 --- /dev/null +++ b/server/src/test/java/com/vaadin/tests/data/selection/StaleMultiSelectionTest.java @@ -0,0 +1,54 @@ +package com.vaadin.tests.data.selection; + +import java.util.Collection; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import org.junit.Assert; +import org.junit.Test; +import org.junit.runners.Parameterized.Parameters; + +import com.vaadin.data.provider.StrBean; +import com.vaadin.ui.AbstractMultiSelect; +import com.vaadin.ui.CheckBoxGroup; +import com.vaadin.ui.ListSelect; +import com.vaadin.ui.TwinColSelect; + +public class StaleMultiSelectionTest + extends AbstractStaleSelectionTest<AbstractMultiSelect<StrBean>> { + + @Test + public void testSelectionUpdateOnRefreshItem() { + StrBean toReplace = data.get(0); + assertNotStale(toReplace); + + select.select(toReplace); + + StrBean replacement = new StrBean("Replacement bean", toReplace.getId(), + -1); + dataProvider.refreshItem(replacement); + + assertIsStale(toReplace); + select.getSelectedItems() + .forEach(item -> Assert.assertFalse( + "Selection should not contain stale values", + dataProvider.isStale(item))); + + Object oldId = dataProvider.getId(toReplace); + Assert.assertTrue("Selection did not contain an item with matching Id.", + select.getSelectedItems().stream().map(dataProvider::getId) + .anyMatch(oldId::equals)); + Assert.assertTrue("Stale element is not considered selected.", + select.isSelected(toReplace)); + } + + @Parameters(name = "{0}") + public static Collection<Object[]> getParams() { + return Stream + .of(new ListSelect<>(), new TwinColSelect<>(), + new CheckBoxGroup<>()) + .map(component -> new Object[] { + component.getClass().getSimpleName(), component }) + .collect(Collectors.toList()); + } +} diff --git a/server/src/test/java/com/vaadin/tests/data/selection/StaleSingleSelectionTest.java b/server/src/test/java/com/vaadin/tests/data/selection/StaleSingleSelectionTest.java new file mode 100644 index 0000000000..e8b354de9e --- /dev/null +++ b/server/src/test/java/com/vaadin/tests/data/selection/StaleSingleSelectionTest.java @@ -0,0 +1,48 @@ +package com.vaadin.tests.data.selection; + +import java.util.Collection; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import org.junit.Assert; +import org.junit.Test; +import org.junit.runners.Parameterized.Parameters; + +import com.vaadin.data.provider.StrBean; +import com.vaadin.ui.AbstractSingleSelect; +import com.vaadin.ui.ComboBox; +import com.vaadin.ui.NativeSelect; +import com.vaadin.ui.RadioButtonGroup; + +public class StaleSingleSelectionTest + extends AbstractStaleSelectionTest<AbstractSingleSelect<StrBean>> { + + @Test + public void testGridSingleSelectionUpdateOnRefreshItem() { + StrBean toReplace = data.get(0); + assertNotStale(toReplace); + + select.setValue(toReplace); + + StrBean replacement = new StrBean("Replacement bean", toReplace.getId(), + -1); + dataProvider.refreshItem(replacement); + + assertIsStale(toReplace); + Assert.assertFalse("Selection should not contain stale values", + dataProvider.isStale(select.getValue())); + + Assert.assertEquals("Selected item id did not match original.", + toReplace.getId(), dataProvider.getId(select.getValue())); + } + + @Parameters(name = "{0}") + public static Collection<Object[]> getParams() { + return Stream + .of(new NativeSelect<>(), new ComboBox<>(), + new RadioButtonGroup<>()) + .map(c -> new Object[] { c.getClass().getSimpleName(), c }) + .collect(Collectors.toList()); + } + +} diff --git a/uitest/src/main/java/com/vaadin/tests/data/ReplaceDataProvider.java b/uitest/src/main/java/com/vaadin/tests/data/ReplaceDataProvider.java index 8a33cae95d..8cc8013618 100644 --- a/uitest/src/main/java/com/vaadin/tests/data/ReplaceDataProvider.java +++ b/uitest/src/main/java/com/vaadin/tests/data/ReplaceDataProvider.java @@ -19,16 +19,6 @@ public class ReplaceDataProvider extends AbstractTestUI { this.hash = hash; someField = "a"; } - - @Override - public int hashCode() { - return hash; - } - - @Override - public boolean equals(Object obj) { - return true; - } } @Override |