Make fields with value change listener immediate

Makes fields with value change listener immediate without explicit
call. If immediate value has been explicitly set, it is honoured.
In most cases immediate now works seamlessly and excess server
round trips should be rare as a regression.

“Fixes” #8029 in a more elegant manner

Change-Id: Ic240c78c0a29447809a17de74196d3325a78ec1f

server/src/com/vaadin/ui/AbstractComponent.java

@@ -38,6 +38,7 @@ import com.vaadin.server.VaadinSession;
 import com.vaadin.shared.AbstractComponentState;
 import com.vaadin.shared.ComponentConstants;
 import com.vaadin.shared.ui.ComponentStateUtil;
+import com.vaadin.ui.Field.ValueChangeEvent;
 import com.vaadin.util.ReflectTools;
 
 /**
@@ -97,6 +98,8 @@ public abstract class AbstractComponent extends AbstractClientConnector
 
     private HasComponents parent;
 
+    private Boolean explicitImmediateValue;
+
     /* Constructor */
 
     /**
@@ -361,7 +364,17 @@ public abstract class AbstractComponent extends AbstractClientConnector
     }
 
     public boolean isImmediate() {
-        return getState(false).immediate;
+        if (explicitImmediateValue != null) {
+            return explicitImmediateValue;
+        } else if (hasListeners(ValueChangeEvent.class)) {
+            /*
+             * Automatic immediate for fields that developers are interested
+             * about.
+             */
+            return true;
+        } else {
+            return false;
+        }
     }
 
     /**
@@ -372,6 +385,7 @@ public abstract class AbstractComponent extends AbstractClientConnector
      *            immediate mode after the call.
      */
     public void setImmediate(boolean immediate) {
+        explicitImmediateValue = immediate;
         getState().immediate = immediate;
     }
 
@@ -668,6 +682,8 @@ public abstract class AbstractComponent extends AbstractClientConnector
         } else {
             getState().errorMessage = null;
         }
+
+        getState().immediate = isImmediate();
     }
 
     /* General event framework */

server/src/com/vaadin/ui/AbstractField.java

@@ -1085,6 +1085,8 @@ public abstract class AbstractField<T> extends AbstractComponent implements
     public void addValueChangeListener(Property.ValueChangeListener listener) {
         addListener(AbstractField.ValueChangeEvent.class, listener,
                 VALUE_CHANGE_METHOD);
+        // ensure "automatic immediate handling" works
+        markAsDirty();
     }
 
     /**
@@ -1106,6 +1108,8 @@ public abstract class AbstractField<T> extends AbstractComponent implements
     public void removeValueChangeListener(Property.ValueChangeListener listener) {
         removeListener(AbstractField.ValueChangeEvent.class, listener,
                 VALUE_CHANGE_METHOD);
+        // ensure "automatic immediate handling" works
+        markAsDirty();
     }
 
     /**

uitest/src/com/vaadin/tests/components/textfield/AutomaticImmediate.java

@@ -0,0 +1,147 @@
+/*
+ * 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.textfield;
+
+import com.vaadin.data.Property.ValueChangeEvent;
+import com.vaadin.data.Property.ValueChangeListener;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUIWithLog;
+import com.vaadin.ui.Button;
+import com.vaadin.ui.Button.ClickEvent;
+import com.vaadin.ui.Button.ClickListener;
+import com.vaadin.ui.CheckBox;
+import com.vaadin.ui.TextField;
+
+/**
+ * Test to verify fields become implicitly "immediate" when adding value change
+ * listener to them.
+ * 
+ * @since 7.2
+ * @author Vaadin Ltd
+ */
+public class AutomaticImmediate extends AbstractTestUIWithLog {
+
+    /**
+     * 
+     */
+    static final String BUTTON = "button";
+    /**
+     * 
+     */
+    static final String EXPLICIT_FALSE = "explicit-false";
+    /**
+     * 
+     */
+    static final String FIELD = "field";
+    /**
+     * 
+     */
+    static final String LISTENER_TOGGLE = "listener-toggle";
+
+    /*
+     * (non-Javadoc)
+     * 
+     * @see com.vaadin.tests.components.AbstractTestUI#setup(com.vaadin.server.
+     * VaadinRequest)
+     */
+    @Override
+    protected void setup(VaadinRequest request) {
+
+        final TextField textField = new TextField() {
+
+            /*
+             * (non-Javadoc)
+             * 
+             * @see com.vaadin.ui.AbstractField#fireValueChange(boolean)
+             */
+            @Override
+            protected void fireValueChange(boolean repaintIsNotNeeded) {
+                log("fireValueChange");
+                super.fireValueChange(repaintIsNotNeeded);
+            }
+        };
+        textField.setId(FIELD);
+
+        final ValueChangeListener listener = new ValueChangeListener() {
+
+            @Override
+            public void valueChange(ValueChangeEvent event) {
+                log("Value changed: " + event.getProperty().getValue());
+            }
+        };
+
+        final CheckBox checkBox = new CheckBox("Toggle listener");
+        checkBox.addValueChangeListener(new ValueChangeListener() {
+
+            @Override
+            public void valueChange(ValueChangeEvent event) {
+                if (checkBox.getValue()) {
+                    textField.addValueChangeListener(listener);
+                } else {
+                    textField.removeValueChangeListener(listener);
+                }
+            }
+        });
+        checkBox.setId(LISTENER_TOGGLE);
+
+        Button b = new Button(
+                "setImmediate(false), sets explicitly false and causes server roundtrip",
+                new ClickListener() {
+
+                    @Override
+                    public void buttonClick(ClickEvent event) {
+                        textField.setImmediate(false);
+                    }
+                });
+        b.setId(EXPLICIT_FALSE);
+
+        Button b2 = new Button("Hit server, causes server roundtrip",
+                new ClickListener() {
+
+                    @Override
+                    public void buttonClick(ClickEvent event) {
+                    }
+                });
+        b2.setId(BUTTON);
+
+        addComponent(textField);
+        addComponent(checkBox);
+        addComponent(b);
+        addComponent(b2);
+
+    }
+
+    /*
+     * (non-Javadoc)
+     * 
+     * @see com.vaadin.tests.components.AbstractTestUI#getTestDescription()
+     */
+    @Override
+    protected String getTestDescription() {
+        return "Field should be immediate automatically if it has value change listener";
+    }
+
+    /*
+     * (non-Javadoc)
+     * 
+     * @see com.vaadin.tests.components.AbstractTestUI#getTicketNumber()
+     */
+    @Override
+    protected Integer getTicketNumber() {
+        return 8029;
+    }
+
+}

uitest/src/com/vaadin/tests/components/textfield/AutomaticImmediateTest.java

@@ -0,0 +1,109 @@
+/*
+ * 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.textfield;
+
+import org.apache.commons.lang.RandomStringUtils;
+import org.junit.Assert;
+import org.junit.Test;
+import org.openqa.selenium.By;
+import org.openqa.selenium.Keys;
+import org.openqa.selenium.WebElement;
+
+import com.vaadin.tests.tb3.MultiBrowserTest;
+
+public class AutomaticImmediateTest extends MultiBrowserTest {
+
+    /*
+     * (non-Javadoc)
+     * 
+     * @see com.vaadin.tests.tb3.AbstractTB3Test#getUIClass()
+     */
+    @Override
+    protected Class<?> getUIClass() {
+        return AutomaticImmediate.class;
+    }
+
+    @Test
+    public void test() {
+        openTestURL();
+
+        WebElement field = getDriver().findElement(
+                By.id(AutomaticImmediate.FIELD));
+
+        WebElement toggle = getDriver().findElement(
+                By.xpath("//input[@type = 'checkbox']"));
+
+        WebElement explicitFalseButton = getDriver().findElement(
+                By.id(AutomaticImmediate.EXPLICIT_FALSE));
+
+        WebElement hitServerButton = getDriver().findElement(
+                By.id(AutomaticImmediate.BUTTON));
+
+        String string = getRandomString();
+        field.sendKeys(string + Keys.ENTER);
+
+        // Non immediate, just the initial server side valuechange
+        assertLastLog("1. fireValueChange");
+
+        hitServerButton.click();
+
+        // No value change, but value sent to server
+        assertLastLog("2. fireValueChange");
+
+        // listener on -> immediate on
+        toggle.click();
+
+        string = getRandomString();
+        String delSequence = "" + Keys.BACK_SPACE + Keys.BACK_SPACE;
+        field.sendKeys(delSequence + string + Keys.ENTER);
+        assertLastLog("4. Value changed: " + string);
+
+        // listener off -> immediate off
+        String lastvalue = string;
+        toggle.click();
+        string = getRandomString();
+        field.sendKeys(delSequence + string + Keys.ENTER);
+        // No new value change should happen...
+        assertLastLog("4. Value changed: " + lastvalue);
+        hitServerButton.click();
+        // ... but server should receive value with roundtrip
+        assertLastLog("5. fireValueChange");
+
+        // explicitly non immediate, but with listener
+        explicitFalseButton.click();
+        toggle.click();
+
+        string = getRandomString();
+        field.sendKeys(delSequence + string + Keys.ENTER);
+        // non immediate, no change...
+        assertLastLog("5. fireValueChange");
+        // ... until server round trip
+        hitServerButton.click();
+        assertLastLog("7. Value changed: " + string);
+
+    }
+
+    private String getRandomString() {
+        String string = RandomStringUtils.randomAlphanumeric(2);
+        return string;
+    }
+
+    private void assertLastLog(String string) {
+        String text = getDriver().findElement(By.id("Log_row_0")).getText();
+        Assert.assertEquals(string, text);
+    }
+
+}