Merge of (#6476) to Vaadin 7.

Table.setVisibleColumns should throw exception from duplicate property
ids.

Change-Id: I1465a99bd0c6241c3b31d88f5480fee99bbdc0ad

1 files changed, 10 insertions, 11 deletions

diff --git a/server/src/com/vaadin/ui/Table.java b/server/src/com/vaadin/ui/Table.java
index 65dea7594b..b035b6da8c 100644
--- a/server/src/com/vaadin/ui/Table.java
+++ b/server/src/com/vaadin/ui/Table.java
@@ -625,10 +625,10 @@ public class Table extends AbstractSelect implements Action.Container,
                     "Can not set visible columns to null value");
         }
 
-        // TODO add error check that no duplicate identifiers exist
+        final LinkedList<Object> newVC = new LinkedList<Object>();
 
-        // Checks that the new visible columns contains no nulls and properties
-        // exist
+        // Checks that the new visible columns contains no nulls, properties
+        // exist and that there are no duplicates before adding them to newVC.
         final Collection<?> properties = getContainerPropertyIds();
         for (int i = 0; i < visibleColumns.length; i++) {
             if (visibleColumns[i] == null) {
@@ -636,18 +636,17 @@ public class Table extends AbstractSelect implements Action.Container,
             } else if (!properties.contains(visibleColumns[i])
                     && !columnGenerators.containsKey(visibleColumns[i])) {
                 throw new IllegalArgumentException(
-                        "Ids must exist in the Container or as a generated column , missing id: "
+                        "Ids must exist in the Container or as a generated column, missing id: "
+                                + visibleColumns[i]);
+            } else if (newVC.contains(visibleColumns[i])) {
+                throw new IllegalArgumentException(
+                        "Ids must be unique, duplicate id: "
                                 + visibleColumns[i]);
+            } else {
+                newVC.add(visibleColumns[i]);
             }
         }
 
-        // If this is called before the constructor is finished, it might be
-        // uninitialized
-        final LinkedList<Object> newVC = new LinkedList<Object>();
-        for (int i = 0; i < visibleColumns.length; i++) {
-            newVC.add(visibleColumns[i]);
-        }
-
         // Removes alignments, icons and headers from hidden columns
         if (this.visibleColumns != null) {
             boolean disabledHere = disableContentRefreshing();