]> source.dussan.org Git - vaadin-framework.git/commitdiff
Table ignores Container updates while painting (#12258)
authorHenrik Paul <henrik@vaadin.com>
Tue, 20 Aug 2013 12:47:54 +0000 (15:47 +0300)
committerVaadin Code Review <review@vaadin.com>
Mon, 2 Sep 2013 11:14:14 +0000 (11:14 +0000)
Vaadin threw an IllegalStateException if a Container was updated while
Table was being painted. SQLContainer was known to invalidate its cached
size during a Table repaint, resulting in an ItemSetChangeEvent. This fix
has been copied over from how ComboBox handles this situation.

Change-Id: I04af71a5ea3844da245cb9e31ada4a30ff704619

server/src/com/vaadin/ui/Table.java
uitest/src/com/vaadin/tests/components/table/ContainerSizeChangeDuringTablePaint.html [new file with mode: 0644]
uitest/src/com/vaadin/tests/components/table/ContainerSizeChangeDuringTablePaint.java [new file with mode: 0644]

index 3507e6b0a524ac1016a386c8f38f2bab604142f3..bd2b7828decd622d827ca803ab1037e61561b256 100644 (file)
@@ -564,6 +564,8 @@ public class Table extends AbstractSelect implements Action.Container,
 
     private List<Throwable> exceptionsDuringCachePopulation = new ArrayList<Throwable>();
 
+    private boolean isBeingPainted;
+
     /* Table constructors */
 
     /**
@@ -3166,6 +3168,15 @@ public class Table extends AbstractSelect implements Action.Container,
 
     @Override
     public void paintContent(PaintTarget target) throws PaintException {
+        isBeingPainted = true;
+        try {
+            doPaintContent(target);
+        } finally {
+            isBeingPainted = false;
+        }
+    }
+
+    private void doPaintContent(PaintTarget target) throws PaintException {
         /*
          * Body actions - Actions which has the target null and can be invoked
          * by right clicking on the table body.
@@ -4394,6 +4405,10 @@ public class Table extends AbstractSelect implements Action.Container,
 
     @Override
     public void containerItemSetChange(Container.ItemSetChangeEvent event) {
+        if (isBeingPainted) {
+            return;
+        }
+
         super.containerItemSetChange(event);
 
         // super method clears the key map, must inform client about this to
@@ -4416,6 +4431,10 @@ public class Table extends AbstractSelect implements Action.Container,
     @Override
     public void containerPropertySetChange(
             Container.PropertySetChangeEvent event) {
+        if (isBeingPainted) {
+            return;
+        }
+
         disableContentRefreshing();
         super.containerPropertySetChange(event);
 
diff --git a/uitest/src/com/vaadin/tests/components/table/ContainerSizeChangeDuringTablePaint.html b/uitest/src/com/vaadin/tests/components/table/ContainerSizeChangeDuringTablePaint.html
new file mode 100644 (file)
index 0000000..236f184
--- /dev/null
@@ -0,0 +1,47 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
+<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en">
+<head profile="http://selenium-ide.openqa.org/profiles/test-case">
+<meta http-equiv="Content-Type" content="text/html; charset=UTF-8" />
+<link rel="selenium.base" href="http://localhost:8888/" />
+<title>ContainerSizeChangeDuringTablePaint</title>
+</head>
+<body>
+<table cellpadding="1" cellspacing="1" border="1">
+<thead>
+<tr><td rowspan="1" colspan="3">ContainerSizeChangeDuringTablePaint</td></tr>
+</thead><tbody>
+<tr>
+       <td>open</td>
+       <td>/run/com.vaadin.tests.components.table.ContainerSizeChangeDuringTablePaint?restartApplication</td>
+       <td></td>
+</tr>
+<tr>
+       <td>verifyTextPresent</td>
+       <td>Add an item and also trigger an ItemSetChangeEvent in Container during next Table paint</td>
+       <td></td>
+</tr>
+<tr>
+       <td>click</td>
+       <td>vaadin=runcomvaadintestscomponentstableContainerSizeChangeDuringTablePaint::PID_SaddRow/domChild[0]/domChild[0]</td>
+       <td></td>
+</tr>
+<tr>
+       <td>verifyTextPresent</td>
+       <td>Event was fired successfully.</td>
+       <td></td>
+</tr>
+<tr>
+       <td>verifyTextPresent</td>
+       <td>New</td>
+       <td></td>
+</tr>
+<tr>
+       <td>verifyTextPresent</td>
+       <td>Row</td>
+       <td></td>
+</tr>
+
+</tbody></table>
+</body>
+</html>
diff --git a/uitest/src/com/vaadin/tests/components/table/ContainerSizeChangeDuringTablePaint.java b/uitest/src/com/vaadin/tests/components/table/ContainerSizeChangeDuringTablePaint.java
new file mode 100644 (file)
index 0000000..0f38517
--- /dev/null
@@ -0,0 +1,163 @@
+/*
+ * Copyright 2000-2013 Vaadin Ltd.
+ * 
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package com.vaadin.tests.components.table;
+
+import java.util.Iterator;
+
+import com.vaadin.data.Container;
+import com.vaadin.data.Item;
+import com.vaadin.data.util.IndexedContainer;
+import com.vaadin.server.PaintException;
+import com.vaadin.server.PaintTarget;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.ui.Button;
+import com.vaadin.ui.Button.ClickEvent;
+import com.vaadin.ui.Table;
+
+public class ContainerSizeChangeDuringTablePaint extends AbstractTestUI {
+
+    /**
+     * A test {@link Table} that simply acts a hook for when Vaadin starts
+     * painting the Table.
+     */
+    private static class WobblyTable extends Table {
+        /**
+         * A flag for the container to know when it should change the size.
+         */
+        boolean isBeingPainted;
+
+        public WobblyTable(String caption, Container dataSource) {
+            super(caption, dataSource);
+        }
+
+        @Override
+        public void paintContent(PaintTarget target) throws PaintException {
+            isBeingPainted = true;
+            try {
+                super.paintContent(target);
+            } finally {
+                isBeingPainted = false;
+            }
+        }
+    }
+
+    /**
+     * A {@link Container} that can change its size while its
+     * {@link WobblyTable} is being painted.
+     */
+    private static class WobblyContainer extends IndexedContainer {
+        private WobblyTable table = null;
+        private boolean shouldSabotageNextPaint = false;
+
+        public void setWobblyTable(WobblyTable table) {
+            this.table = table;
+        }
+
+        @Override
+        public int size() {
+            if (table != null && table.isBeingPainted
+                    && shouldSabotageNextPaint) {
+                try {
+                    System.out.print("Firing item set change "
+                            + "event during Table paint... ");
+                    fireItemSetChange();
+                    System.out.println("Success!");
+                } finally {
+                    shouldSabotageNextPaint = false;
+                }
+            }
+
+            return super.size();
+        }
+
+        public void sabotageNextPaint() {
+            shouldSabotageNextPaint = true;
+        }
+    }
+
+    private static final Object PROPERTY_1 = new Object();
+    private static final Object PROPERTY_2 = new Object();
+    private static final Object PROPERTY_3 = new Object();
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        final WobblyContainer container = generateContainer();
+        final WobblyTable table = createTable(container);
+        container.setWobblyTable(table);
+
+        addComponent(table);
+        Button button = new Button(
+                "Add an item and also trigger an ItemSetChangeEvent in Container during next Table paint",
+                new Button.ClickListener() {
+                    @Override
+                    public void buttonClick(ClickEvent event) {
+
+                        // we need this to simply trigger a table paint.
+                        addItem(container, "A", "New", "Row");
+                        container.sabotageNextPaint();
+
+                        event.getButton().setCaption(
+                                "Event was fired successfully.");
+                    }
+                });
+        button.setId("addRow");
+        addComponent(button);
+    }
+
+    private static WobblyTable createTable(IndexedContainer container) {
+        WobblyTable t = new WobblyTable(null, container);
+        t.setColumnHeader(PROPERTY_1, "Property 1");
+        t.setColumnHeader(PROPERTY_2, "Property 2");
+        t.setColumnHeader(PROPERTY_3, "Property 3");
+        t.setPageLength(container.size() + 1);
+        return t;
+    }
+
+    private static WobblyContainer generateContainer() {
+        WobblyContainer c = new WobblyContainer();
+        c.addContainerProperty(PROPERTY_1, String.class, null);
+        c.addContainerProperty(PROPERTY_2, String.class, null);
+        c.addContainerProperty(PROPERTY_3, String.class, null);
+        addItem(c, "Hello", "World", "!");
+        return c;
+    }
+
+    @SuppressWarnings("unchecked")
+    private static void addItem(Container c, Object... properties) {
+        Object itemId = c.addItem();
+        Item item = c.getItem(itemId);
+        int i = 0;
+        Iterator<?> propIter = c.getContainerPropertyIds().iterator();
+        while (propIter.hasNext()) {
+            Object propertyId = propIter.next();
+            item.getItemProperty(propertyId).setValue(properties[i]);
+            i++;
+        }
+    }
+
+    @Override
+    protected String getTestDescription() {
+        return "Container changes during the painting cycle should not lead to an IllegalStateException";
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return 12258;
+    }
+
+}