HasItems.setItems(T... items) should allow edits (#10290)

* HasItems.setItems(T... items) should allow edits

Arrays.asList() creates a immutable Arrays.ArrayList, preventing users from doing dataProvider.getItems() and updating the returned collection. This makes it impossible to keep the same data provider, update it and keep the filters & sorting, and then just call dataProvider.refreshAll() to get changes visible. We should not require users to create a new data provider in this case.
This is the same for DataProvider.ofItems(T... items)

* fix missing whitespace from test error message

2 files changed, 82 insertions, 0 deletions

diff --git a/server/src/test/java/com/vaadin/data/HasItemsTest.java b/server/src/test/java/com/vaadin/data/HasItemsTest.java
new file mode 100644
index 0000000000..6bbdb46f4b
--- /dev/null
+++ b/server/src/test/java/com/vaadin/data/HasItemsTest.java
@@ -0,0 +1,66 @@
+package com.vaadin.data;
+
+import java.lang.reflect.Modifier;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.reflections.Reflections;
+
+import com.vaadin.data.provider.DataProvider;
+import com.vaadin.data.provider.ListDataProvider;
+import com.vaadin.ui.Tree;
+import com.vaadin.ui.TreeGrid;
+
+public class HasItemsTest {
+
+    private static ArrayList<Class<?>> whiteList = new ArrayList<>();
+    {
+        // these create a hierarchical data provider, which is not using
+        // ArrayList or Arrays.ArrayList in the end
+        whiteList.add(TreeGrid.class);
+        whiteList.add(Tree.class);
+    }
+
+    @Test
+    public void setItemsVarargsConstructor_createsListDataProvider_itIsEditable()
+            throws InstantiationException, IllegalAccessException {
+        Set<Class<? extends HasItems>> subTypesOf = new Reflections(
+                "com.vaadin.ui").getSubTypesOf(HasItems.class).stream().filter(
+                        clazz -> !Modifier.isAbstract(clazz.getModifiers()))
+                        .filter(clazz -> Stream.of(clazz.getConstructors())
+                                .anyMatch(constuctor -> constuctor
+                                        .getParameterCount() == 0))
+                        .filter(clazz -> !whiteList.contains(clazz))
+                        .collect(Collectors.toSet());
+
+        for (Class<? extends HasItems> hasItemsType : subTypesOf) {
+            HasItems hasItems = hasItemsType.newInstance();
+
+            hasItems.setItems("0", "1");
+
+            DataProvider dataProvider = hasItems.getDataProvider();
+
+            Assert.assertTrue(
+                    hasItemsType.getSimpleName()
+                            + " setItems method with varargs parameters of does not create a list data provider",
+                    dataProvider instanceof ListDataProvider);
+
+            ListDataProvider listDataProvider = (ListDataProvider) dataProvider;
+
+            Assert.assertTrue(
+                    hasItemsType.getSimpleName()
+                            + " does not have setItems method with varargs parameters of does not create an ArrayList backed list data provider",
+                    listDataProvider.getItems() instanceof ArrayList);
+
+            List list = (List) listDataProvider.getItems();
+            // previously the following would explode since Arrays.ArrayList
+            // does not support it
+            list.add(0, "2");
+        }
+    }
+}
diff --git a/server/src/test/java/com/vaadin/data/provider/ListDataProviderTest.java b/server/src/test/java/com/vaadin/data/provider/ListDataProviderTest.java
index 8b1792689a..1e367e4e91 100644
--- a/server/src/test/java/com/vaadin/data/provider/ListDataProviderTest.java
+++ b/server/src/test/java/com/vaadin/data/provider/ListDataProviderTest.java
@@ -4,11 +4,13 @@ import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.fail;
 
+import java.util.ArrayList;
 import java.util.Comparator;
 import java.util.List;
 import java.util.Locale;
 import java.util.stream.Collectors;
 
+import org.junit.Assert;
 import org.junit.Test;
 
 import com.vaadin.server.SerializableComparator;
@@ -23,6 +25,20 @@ public class ListDataProviderTest
     }
 
     @Test
+    public void dataProvider_ofItems_shouldCreateAnEditableDataProvider() {
+        ListDataProvider<String> dataProvider = DataProvider.ofItems("0", "1");
+
+        Assert.assertTrue(
+                "DataProvider.ofItems should create a list data provider backed an ArrayList allowing edits",
+                dataProvider.getItems() instanceof ArrayList);
+
+        List<String> list = (List<String>) dataProvider.getItems();
+        // previously the following would explode since Arrays.ArrayList does
+        // not support it
+        list.add(0, "2");
+    }
+
+    @Test
     public void setSortByProperty_ascending() {
         ListDataProvider<StrBean> dataProvider = getDataProvider();