From 8044cec886000ea03d4de54191f5521d73ebf6b4 Mon Sep 17 00:00:00 2001
From: Anna Koskinen <anna@vaadin.com>
Date: Wed, 2 Jan 2013 16:57:52 +0200
Subject: [PATCH] Merge of (#6476) to Vaadin 7.

Table.setVisibleColumns should throw exception from duplicate property
ids.

Change-Id: I1465a99bd0c6241c3b31d88f5480fee99bbdc0ad
---
 server/src/com/vaadin/ui/Table.java           | 21 +++++++++----------
 .../component/table/TableVisibleColumns.java  |  7 +------
 2 files changed, 11 insertions(+), 17 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();
diff --git a/server/tests/src/com/vaadin/tests/server/component/table/TableVisibleColumns.java b/server/tests/src/com/vaadin/tests/server/component/table/TableVisibleColumns.java
index be312044db..ee3eefba05 100644
--- a/server/tests/src/com/vaadin/tests/server/component/table/TableVisibleColumns.java
+++ b/server/tests/src/com/vaadin/tests/server/component/table/TableVisibleColumns.java
@@ -54,15 +54,10 @@ public class TableVisibleColumns {
         try {
             t.setVisibleColumns(new Object[] { "Property 0", "Property 1",
                     "Property 2", "Property 1" });
-            // FIXME: Multiple properties in the Object array should be detected
-            // (#6476)
-            // junit.framework.Assert.fail("IllegalArgumentException expected");
         } catch (IllegalArgumentException e) {
             // OK, expected
         }
-        // FIXME: Multiple properties in the Object array should be detected
-        // (#6476)
-        // assertArrayEquals(defaultColumns3, t.getVisibleColumns());
+        assertArrayEquals(defaultColumns3, t.getVisibleColumns());
     }
 
     @Test
-- 
2.39.5