Use diffstate for JS connectors (#20335)

Change-Id: If2401d724f782ee76f92a6b89c54e51f90218bee

6 files changed, 265 insertions, 7 deletions

diff --git a/server/src/main/java/com/vaadin/server/LegacyCommunicationManager.java b/server/src/main/java/com/vaadin/server/LegacyCommunicationManager.java
index 858f61d0c7..7c438ca999 100644
--- a/server/src/main/java/com/vaadin/server/LegacyCommunicationManager.java
+++ b/server/src/main/java/com/vaadin/server/LegacyCommunicationManager.java
@@ -31,7 +31,9 @@ import java.util.logging.Logger;
 import com.vaadin.server.ClientConnector.ConnectorErrorEvent;
 import com.vaadin.shared.ApplicationConstants;
 import com.vaadin.shared.JavaScriptConnectorState;
+import com.vaadin.shared.JavaScriptExtensionState;
 import com.vaadin.shared.communication.SharedState;
+import com.vaadin.shared.ui.JavaScriptComponentState;
 import com.vaadin.ui.Component;
 import com.vaadin.ui.ConnectorTracker;
 import com.vaadin.ui.HasComponents;
@@ -94,9 +96,8 @@ public class LegacyCommunicationManager implements Serializable {
         ConnectorTracker connectorTracker = uI.getConnectorTracker();
         Class<? extends SharedState> stateType = connector.getStateType();
         JsonValue diffState = connectorTracker.getDiffState(connector);
-        boolean supportsDiffState = !JavaScriptConnectorState.class
-                .isAssignableFrom(stateType);
-        if (diffState == null && supportsDiffState) {
+
+        if (diffState == null) {
             // Use an empty state object as reference for full
             // repaints
             diffState = referenceDiffStates.get(stateType);
@@ -107,15 +108,24 @@ public class LegacyCommunicationManager implements Serializable {
         }
         EncodeResult encodeResult = JsonCodec.encode(state, diffState,
                 stateType, uI.getConnectorTracker());
-        if (supportsDiffState) {
-            connectorTracker.setDiffState(connector,
-                    (JsonObject) encodeResult.getEncodedValue());
-        }
+        connectorTracker.setDiffState(connector,
+                (JsonObject) encodeResult.getEncodedValue());
+
         return (JsonObject) encodeResult.getDiff();
     }
 
     private static JsonValue createReferenceDiffStateState(
             Class<? extends SharedState> stateType) {
+        if (JavaScriptConnectorState.class.isAssignableFrom(stateType)) {
+            /*
+             * For JS state types, we should only include the framework-provided
+             * state fields in the reference diffstate since other fields are
+             * not know by the client and would therefore not get the right
+             * initial value if it would be recorded in the diffstate.
+             */
+            stateType = findJsStateReferenceType(stateType);
+        }
+
         try {
             SharedState referenceState = stateType.newInstance();
             EncodeResult encodeResult = JsonCodec.encode(referenceState, null,
@@ -130,6 +140,35 @@ public class LegacyCommunicationManager implements Serializable {
     }
 
     /**
+     * Finds the highest super class which implements
+     * {@link JavaScriptConnectorState}. In practice, this finds either
+     * {@link JavaScriptComponentState} or {@link JavaScriptExtensionState}.
+     * This is used to find which state properties the client side knows
+     * something about.
+     *
+     * @param stateType
+     *            the state type for which the reference type should be found
+     * @return the found reference type
+     */
+    private static Class<? extends SharedState> findJsStateReferenceType(
+            Class<? extends SharedState> stateType) {
+        assert JavaScriptConnectorState.class.isAssignableFrom(stateType);
+
+        Class<?> type = stateType;
+
+        while (type != null) {
+            Class<?> superclass = type.getSuperclass();
+            if (!JavaScriptConnectorState.class.isAssignableFrom(superclass)) {
+                break;
+            }
+
+            type = superclass;
+        }
+
+        return type.asSubclass(SharedState.class);
+    }
+
+    /**
      * Resolves a dependency URI, registering the URI with this
      * {@code LegacyCommunicationManager} if needed and returns a fully
      * qualified URI.
diff --git a/server/src/test/java/com/vaadin/ui/AbstractJavaScriptComponentTest.java b/server/src/test/java/com/vaadin/ui/AbstractJavaScriptComponentTest.java
new file mode 100644
index 0000000000..0c923823a2
--- /dev/null
+++ b/server/src/test/java/com/vaadin/ui/AbstractJavaScriptComponentTest.java
@@ -0,0 +1,49 @@
+/*
+ * Copyright 2000-2016 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.ui;
+
+import org.junit.Test;
+
+import com.vaadin.shared.ui.JavaScriptComponentState;
+import com.vaadin.tests.util.MockUI;
+
+public class AbstractJavaScriptComponentTest {
+    public static class TestJsComponentState extends JavaScriptComponentState {
+        public String ownField = "foo";
+    }
+
+    public static class TestJsComponent extends AbstractJavaScriptComponent {
+        @Override
+        protected TestJsComponentState getState() {
+            return (TestJsComponentState) super.getState();
+        }
+    }
+
+    @Test
+    public void testComponentStateEncoding() {
+        MockUI ui = new MockUI();
+        TestJsComponent component = new TestJsComponent();
+        ui.setContent(component);
+
+        ComponentTest.assertEncodedStateProperties(component,
+                "Only defaults not known by the client should be sent",
+                "ownField");
+
+        component.setCaption("My caption");
+        ComponentTest.assertEncodedStateProperties(component,
+                "Caption should be the only changed state property", "caption");
+    }
+}
diff --git a/server/src/test/java/com/vaadin/ui/ComponentTest.java b/server/src/test/java/com/vaadin/ui/ComponentTest.java
index cc58dbbf93..09870b866b 100644
--- a/server/src/test/java/com/vaadin/ui/ComponentTest.java
+++ b/server/src/test/java/com/vaadin/ui/ComponentTest.java
@@ -16,11 +16,17 @@
 package com.vaadin.ui;
 
 import java.lang.reflect.Method;
+import java.util.Arrays;
+import java.util.HashSet;
+
+import org.junit.Assert;
 
 import com.vaadin.server.ClientConnector;
 import com.vaadin.server.ServerRpcManager;
 import com.vaadin.shared.communication.ServerRpc;
 
+import elemental.json.JsonObject;
+
 /**
  * Base class for component unit tests, providing helper methods for e.g.
  * invoking RPC and updating diff state.
@@ -87,4 +93,27 @@ public class ComponentTest {
         }
     }
 
+    /**
+     * Asserts the set of properties that would be sent as state changes for the
+     * given connector.
+     *
+     * @param connector
+     *            the connector that has state changes
+     * @param message
+     *            the message to show if the properties are not as expected
+     * @param expectedProperties
+     *            names of the expected properties
+     */
+    public static void assertEncodedStateProperties(ClientConnector connector,
+            String message, String... expectedProperties) {
+        assert connector.isAttached();
+
+        JsonObject encodeState = connector.encodeState();
+
+        // Collect to HashSet so that order doesn't matter
+        Assert.assertEquals(message,
+                new HashSet<>(Arrays.asList(expectedProperties)),
+                new HashSet<>(Arrays.asList(encodeState.keys())));
+    }
+
 }
diff --git a/uitest/src/main/java/com/vaadin/tests/components/javascriptcomponent/StateChangeCounter.java b/uitest/src/main/java/com/vaadin/tests/components/javascriptcomponent/StateChangeCounter.java
new file mode 100644
index 0000000000..a82151c5a7
--- /dev/null
+++ b/uitest/src/main/java/com/vaadin/tests/components/javascriptcomponent/StateChangeCounter.java
@@ -0,0 +1,64 @@
+/*
+ * Copyright 2000-2016 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.javascriptcomponent;
+
+import com.vaadin.annotations.JavaScript;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.shared.ui.JavaScriptComponentState;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.ui.AbstractJavaScriptComponent;
+import com.vaadin.ui.Button;
+
+public class StateChangeCounter extends AbstractTestUI {
+
+    @Override
+    public String getTestDescription() {
+        return "onStateChange should be called only if the state has actually changed";
+    }
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        StateChangeCounterComponent counter = new StateChangeCounterComponent();
+
+        addComponents(new Button("Send RPC", event -> counter.sendRpc()),
+                new Button("Change state", event -> counter.changeState()),
+                new Button("Mark as dirty", event -> counter.markAsDirty()),
+                counter);
+    }
+
+    @JavaScript("StateChangeCounter.js")
+    public static class StateChangeCounterComponent
+            extends AbstractJavaScriptComponent {
+        public void sendRpc() {
+            callFunction("sendRpc");
+        }
+
+        public void changeState() {
+            getState().stateCounter++;
+        }
+
+        @Override
+        protected StateChangeCounterState getState() {
+            return (StateChangeCounterState) super.getState();
+        }
+    }
+
+    public static class StateChangeCounterState
+            extends JavaScriptComponentState {
+        public int stateCounter = 0;
+    }
+
+}
diff --git a/uitest/src/main/resources/com/vaadin/tests/components/javascriptcomponent/StateChangeCounter.js b/uitest/src/main/resources/com/vaadin/tests/components/javascriptcomponent/StateChangeCounter.js
new file mode 100644
index 0000000000..79e2ef79a7
--- /dev/null
+++ b/uitest/src/main/resources/com/vaadin/tests/components/javascriptcomponent/StateChangeCounter.js
@@ -0,0 +1,18 @@
+window.com_vaadin_tests_components_javascriptcomponent_StateChangeCounter_StateChangeCounterComponent = function() {
+	var self = this;
+	
+	var logRow = function(text) {
+		var child = document.createElement("div");
+		child.className="logRow";
+		child.textContent = text;
+		self.getElement().appendChild(child);
+	}
+	
+	this.onStateChange = function() {
+		logRow("State change, counter = " + this.getState().stateCounter);
+	}
+	
+	this.sendRpc = function() {
+		logRow("RPC")
+	}
+}
\ No newline at end of file
diff --git a/uitest/src/test/java/com/vaadin/tests/components/javascriptcomponent/StateChangeCounterTest.java b/uitest/src/test/java/com/vaadin/tests/components/javascriptcomponent/StateChangeCounterTest.java
new file mode 100644
index 0000000000..0dd88ef75a
--- /dev/null
+++ b/uitest/src/test/java/com/vaadin/tests/components/javascriptcomponent/StateChangeCounterTest.java
@@ -0,0 +1,59 @@
+/*
+ * Copyright 2000-2016 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.javascriptcomponent;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Collectors;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.openqa.selenium.WebElement;
+
+import com.vaadin.testbench.By;
+import com.vaadin.testbench.elements.ButtonElement;
+import com.vaadin.tests.tb3.SingleBrowserTest;
+
+public class StateChangeCounterTest extends SingleBrowserTest {
+    @Test
+    public void testStateChanges() {
+        openTestURL();
+
+        // Expecting message from initial state change
+        assertMessages("State change, counter = 0");
+
+        $(ButtonElement.class).caption("Mark as dirty").first().click();
+        // Shouldn't change anything
+        assertMessages("State change, counter = 0");
+
+        $(ButtonElement.class).caption("Send RPC").first().click();
+
+        // Should only add an RPC message, no state change message
+        assertMessages("State change, counter = 0", "RPC");
+
+        $(ButtonElement.class).caption("Change state").first().click();
+
+        // Should add one message, about a new state change
+        assertMessages("State change, counter = 0", "RPC",
+                "State change, counter = 1");
+    }
+
+    private void assertMessages(String... expectedMessages) {
+        List<String> actualMessages = findElements(By.className("logRow"))
+                .stream().map(WebElement::getText).collect(Collectors.toList());
+        Assert.assertEquals(Arrays.asList(expectedMessages), actualMessages);
+    }
+}