* 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 messagetags/8.2.0.alpha3
@@ -127,7 +127,12 @@ | |||
<artifactId>slf4j-log4j12</artifactId> | |||
<scope>test</scope> | |||
</dependency> | |||
<dependency> | |||
<groupId>org.reflections</groupId> | |||
<artifactId>reflections</artifactId> | |||
<version>0.9.11</version> | |||
<scope>test</scope> | |||
</dependency> | |||
<!-- For manual testing with PostgreSQL (see SQLTestConstants) --> | |||
<!-- <dependency><groupId>postgresql</groupId><artifactId>postgresql</artifactId><version>9.1-901.jdbc3</version></dependency> --> | |||
</dependencies> |
@@ -15,6 +15,7 @@ | |||
*/ | |||
package com.vaadin.data; | |||
import java.util.ArrayList; | |||
import java.util.Arrays; | |||
import java.util.Collection; | |||
import java.util.stream.Collectors; | |||
@@ -100,7 +101,7 @@ public interface HasItems<T> extends Component { | |||
* the data items to display | |||
*/ | |||
public default void setItems(@SuppressWarnings("unchecked") T... items) { | |||
setItems(Arrays.asList(items)); | |||
setItems(new ArrayList<>(Arrays.asList(items))); | |||
} | |||
/** |
@@ -16,6 +16,7 @@ | |||
package com.vaadin.data.provider; | |||
import java.io.Serializable; | |||
import java.util.ArrayList; | |||
import java.util.Arrays; | |||
import java.util.Collection; | |||
import java.util.Objects; | |||
@@ -266,7 +267,7 @@ public interface DataProvider<T, F> extends Serializable { | |||
*/ | |||
@SafeVarargs | |||
public static <T> ListDataProvider<T> ofItems(T... items) { | |||
return new ListDataProvider<>(Arrays.asList(items)); | |||
return new ListDataProvider<>(new ArrayList<>(Arrays.asList(items))); | |||
} | |||
/** |
@@ -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"); | |||
} | |||
} | |||
} |
@@ -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; | |||
@@ -22,6 +24,20 @@ public class ListDataProviderTest | |||
return DataProvider.ofCollection(data); | |||
} | |||
@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(); |