ContainerSizeChangeDuringTablePaint

From a24d391349354579a83ab2f6b50e9552fbe2566a Mon Sep 17 00:00:00 2001 From: Henrik Paul Date: Tue, 20 Aug 2013 15:47:54 +0300 Subject: [PATCH] Table ignores Container updates while painting (#12258) 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 | 19 ++ .../ContainerSizeChangeDuringTablePaint.html | 47 +++++ .../ContainerSizeChangeDuringTablePaint.java | 163 ++++++++++++++++++ 3 files changed, 229 insertions(+) create mode 100644 uitest/src/com/vaadin/tests/components/table/ContainerSizeChangeDuringTablePaint.html create mode 100644 uitest/src/com/vaadin/tests/components/table/ContainerSizeChangeDuringTablePaint.java diff --git a/server/src/com/vaadin/ui/Table.java b/server/src/com/vaadin/ui/Table.java index 3507e6b0a5..bd2b7828de 100644 --- a/server/src/com/vaadin/ui/Table.java +++ b/server/src/com/vaadin/ui/Table.java @@ -564,6 +564,8 @@ public class Table extends AbstractSelect implements Action.Container, private List exceptionsDuringCachePopulation = new ArrayList(); + 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 index 0000000000..236f184a41 --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/table/ContainerSizeChangeDuringTablePaint.html @@ -0,0 +1,47 @@ + + + + + + +ContainerSizeChangeDuringTablePaint + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +

ContainerSizeChangeDuringTablePaint
open	/run/com.vaadin.tests.components.table.ContainerSizeChangeDuringTablePaint?restartApplication
verifyTextPresent	Add an item and also trigger an ItemSetChangeEvent in Container during next Table paint
click	vaadin=runcomvaadintestscomponentstableContainerSizeChangeDuringTablePaint::PID_SaddRow/domChild[0]/domChild[0]
verifyTextPresent	Event was fired successfully.
verifyTextPresent	New
verifyTextPresent	Row

+ + 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 index 0000000000..0f385176bf --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/table/ContainerSizeChangeDuringTablePaint.java @@ -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; + } + +} -- 2.39.5