From c7ca048dcc60b331c1b96bf4d3404a4abb18192e Mon Sep 17 00:00:00 2001 From: Artur Signell Date: Wed, 12 Oct 2016 21:59:08 +0300 Subject: [PATCH] Check for duplicate property ids when setting Grid columns or column order (#20386) Change-Id: I76be83642f0e56e55b0c0e502ac6769de1ee8af0 --- server/src/main/java/com/vaadin/ui/Grid.java | 11 +++++ .../component/grid/GridColumnsTest.java | 18 ++++++++ .../com/vaadin/shared/util/SharedUtil.java | 41 +++++++++++++++++++ .../vaadin/shared/util/SharedUtilTest.java | 32 +++++++++++++++ 4 files changed, 102 insertions(+) diff --git a/server/src/main/java/com/vaadin/ui/Grid.java b/server/src/main/java/com/vaadin/ui/Grid.java index 8ca1a3de11..e7eed7725f 100644 --- a/server/src/main/java/com/vaadin/ui/Grid.java +++ b/server/src/main/java/com/vaadin/ui/Grid.java @@ -104,6 +104,7 @@ import com.vaadin.shared.ui.grid.selection.MultiSelectionModelState; import com.vaadin.shared.ui.grid.selection.SingleSelectionModelServerRpc; import com.vaadin.shared.ui.grid.selection.SingleSelectionModelState; import com.vaadin.shared.util.SharedUtil; +import com.vaadin.ui.Grid.SelectionModel; import com.vaadin.ui.declarative.DesignAttributeHandler; import com.vaadin.ui.declarative.DesignContext; import com.vaadin.ui.declarative.DesignException; @@ -5305,6 +5306,11 @@ public class Grid extends AbstractFocusable implements SelectionNotifier, * properties in the desired column order */ public void setColumns(Object... propertyIds) { + if (SharedUtil.containsDuplicates(propertyIds)) { + throw new IllegalArgumentException( + "The propertyIds array contains duplicates: " + + SharedUtil.getDuplicates(propertyIds)); + } Set removePids = new HashSet(columns.keySet()); removePids.removeAll(Arrays.asList(propertyIds)); for (Object removePid : removePids) { @@ -5327,6 +5333,11 @@ public class Grid extends AbstractFocusable implements SelectionNotifier, * properties in the order columns should be */ public void setColumnOrder(Object... propertyIds) { + if (SharedUtil.containsDuplicates(propertyIds)) { + throw new IllegalArgumentException( + "The propertyIds array contains duplicates: " + + SharedUtil.getDuplicates(propertyIds)); + } List columnOrder = new ArrayList(); for (Object propertyId : propertyIds) { if (columns.containsKey(propertyId)) { diff --git a/server/src/test/java/com/vaadin/tests/server/component/grid/GridColumnsTest.java b/server/src/test/java/com/vaadin/tests/server/component/grid/GridColumnsTest.java index bf321f7ee9..24bd8b7f2a 100644 --- a/server/src/test/java/com/vaadin/tests/server/component/grid/GridColumnsTest.java +++ b/server/src/test/java/com/vaadin/tests/server/component/grid/GridColumnsTest.java @@ -410,4 +410,22 @@ public class GridColumnsTest { assertThat(firstColumn.getHeaderCaption(), is("Column0")); } + + @Test(expected = IllegalStateException.class) + public void addColumnManyTimes() { + grid.removeAllColumns(); + grid.addColumn("column0"); + grid.addColumn("column0"); + } + + @Test(expected = IllegalArgumentException.class) + public void setColumnDuplicates() { + grid.removeAllColumns(); + grid.setColumns("column0", "column0"); + } + + @Test(expected = IllegalArgumentException.class) + public void setColumnOrderDuplicates() { + grid.setColumnOrder("column0", "column0"); + } } diff --git a/shared/src/main/java/com/vaadin/shared/util/SharedUtil.java b/shared/src/main/java/com/vaadin/shared/util/SharedUtil.java index 271e8d0e5e..141da10c35 100644 --- a/shared/src/main/java/com/vaadin/shared/util/SharedUtil.java +++ b/shared/src/main/java/com/vaadin/shared/util/SharedUtil.java @@ -16,6 +16,9 @@ package com.vaadin.shared.util; import java.io.Serializable; +import java.util.Arrays; +import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.Locale; /** @@ -145,6 +148,9 @@ public class SharedUtil implements Serializable { * @return The constructed string of words and separators */ public static String join(String[] parts, String separator) { + if (parts.length == 0) { + return ""; + } StringBuilder sb = new StringBuilder(); for (int i = 0; i < parts.length; i++) { sb.append(parts[i]); @@ -266,4 +272,39 @@ public class SharedUtil implements Serializable { return join(parts, ""); } + /** + * Checks if the given array contains duplicates (according to + * {@link Object#equals(Object)}. + * + * @param values + * the array to check for duplicates + * @return true if the array contains duplicates, + * false otherwise + */ + public static boolean containsDuplicates(Object[] values) { + int uniqueCount = new HashSet(Arrays.asList(values)).size(); + return uniqueCount != values.length; + } + + /** + * Return duplicate values in the given array in the format + * "duplicateValue1, duplicateValue2". + * + * @param values + * the values to check for duplicates + * @return a comma separated string of duplicates or an empty string if no + * duplicates were found + */ + public static String getDuplicates(Object[] values) { + HashSet set = new HashSet(); + LinkedHashSet duplicates = new LinkedHashSet(); + for (Object o : values) { + if (!set.add(o)) { + duplicates.add(String.valueOf(o)); + } + + } + return join(duplicates.toArray(new String[duplicates.size()]), ", "); + } + } diff --git a/shared/src/test/java/com/vaadin/shared/util/SharedUtilTest.java b/shared/src/test/java/com/vaadin/shared/util/SharedUtilTest.java index 081f594a97..6d8cab4fea 100644 --- a/shared/src/test/java/com/vaadin/shared/util/SharedUtilTest.java +++ b/shared/src/test/java/com/vaadin/shared/util/SharedUtilTest.java @@ -118,4 +118,36 @@ public class SharedUtilTest { } } + @Test + public void duplicatesInArray() { + Assert.assertTrue( + SharedUtil.containsDuplicates(new Object[] { "a", "a" })); + Assert.assertTrue( + SharedUtil.containsDuplicates(new Object[] { "a", "b", "a" })); + Assert.assertTrue(SharedUtil + .containsDuplicates(new Object[] { "a", "b", "a", "b" })); + Assert.assertTrue( + SharedUtil.containsDuplicates(new Object[] { 1, "b", 1 })); + + Assert.assertFalse(SharedUtil.containsDuplicates(new Object[] {})); + Assert.assertFalse(SharedUtil.containsDuplicates(new Object[] { "a" })); + Assert.assertFalse( + SharedUtil.containsDuplicates(new Object[] { "a", "b" })); + Assert.assertFalse( + SharedUtil.containsDuplicates(new Object[] { "1", 1 })); + } + + @Test + public void getDuplicates() { + Assert.assertEquals("", SharedUtil.getDuplicates(new Object[] { "a" })); + Assert.assertEquals("a", + SharedUtil.getDuplicates(new Object[] { "a", "a" })); + Assert.assertEquals("a, b", + SharedUtil.getDuplicates(new Object[] { "a", "b", "a", "b" })); + Assert.assertEquals("a, b, c", SharedUtil + .getDuplicates(new Object[] { "c", "a", "b", "a", "b", "c" })); + Assert.assertEquals("1.2", + SharedUtil.getDuplicates(new Object[] { 1.2, "a", 1.2 })); + } + } -- 2.39.5