From 0b7dff43495ef12dd74f3c424f8862ca3e00066f Mon Sep 17 00:00:00 2001 From: Denis Anisimov Date: Tue, 6 Feb 2018 13:38:36 +0300 Subject: [PATCH 01/14] Send all updatable properties to the client side --- .../client/ExecuteJavaScriptElementUtils.java | 23 +++++++ .../flow/ExecuteJavaScriptProcessor.java | 3 + .../flow/model/UpdatableModelProperties.java | 62 +++++++++++++++++ .../polymertemplate/PolymerTemplate.java | 68 +++++++++++++------ 4 files changed, 137 insertions(+), 19 deletions(-) create mode 100644 flow-client/src/main/java/com/vaadin/client/flow/model/UpdatableModelProperties.java diff --git a/flow-client/src/main/java/com/vaadin/client/ExecuteJavaScriptElementUtils.java b/flow-client/src/main/java/com/vaadin/client/ExecuteJavaScriptElementUtils.java index a25d82f8e17..d12acfc4acb 100644 --- a/flow-client/src/main/java/com/vaadin/client/ExecuteJavaScriptElementUtils.java +++ b/flow-client/src/main/java/com/vaadin/client/ExecuteJavaScriptElementUtils.java @@ -20,6 +20,7 @@ import com.vaadin.client.flow.collection.JsArray; import com.vaadin.client.flow.collection.JsMap; import com.vaadin.client.flow.dom.DomApi; +import com.vaadin.client.flow.model.UpdatableModelProperties; import com.vaadin.client.flow.nodefeature.NodeList; import com.vaadin.client.flow.nodefeature.NodeMap; import com.vaadin.client.flow.reactive.Reactive; @@ -145,6 +146,28 @@ public static void populateModelProperties(StateNode node, } } + /** + * Register the updatable model properties of the {@code node}. + *

+ * Only updates for the properties from the {@code properties} array will be + * sent to the server without explicit synchronization. The + * {@code properties} array includes all properties that are allowed to be + * updated (including sub properties). + * + * @param node + * the node whose updatable properties should be registered + * @param properties + * all updatable model properties + */ + public static void registerUpdatableModelProperties(StateNode node, + JsArray properties) { + if (!properties.isEmpty()) { + UpdatableModelProperties data = new UpdatableModelProperties( + properties); + node.setNodeData(data); + } + } + private static Integer getExistingIdOrUpdate(StateNode parent, int serverSideId, Element existingElement, Integer existingId) { if (existingId == null) { diff --git a/flow-client/src/main/java/com/vaadin/client/flow/ExecuteJavaScriptProcessor.java b/flow-client/src/main/java/com/vaadin/client/flow/ExecuteJavaScriptProcessor.java index 94f02be5999..9eb3cd7c869 100644 --- a/flow-client/src/main/java/com/vaadin/client/flow/ExecuteJavaScriptProcessor.java +++ b/flow-client/src/main/java/com/vaadin/client/flow/ExecuteJavaScriptProcessor.java @@ -205,6 +205,9 @@ private native JsonObject getContextExecutionObject( object.populateModelProperties = function(element, properties){ @com.vaadin.client.ExecuteJavaScriptElementUtils::populateModelProperties(*)(object.getNode(element), properties); }; + object.registerUpdatableModelProperties = function(element, properties){ + @com.vaadin.client.ExecuteJavaScriptElementUtils::registerUpdatableModelProperties(*)(object.getNode(element), properties); + }; return object; }-*/; } diff --git a/flow-client/src/main/java/com/vaadin/client/flow/model/UpdatableModelProperties.java b/flow-client/src/main/java/com/vaadin/client/flow/model/UpdatableModelProperties.java new file mode 100644 index 00000000000..2e17ca464e5 --- /dev/null +++ b/flow-client/src/main/java/com/vaadin/client/flow/model/UpdatableModelProperties.java @@ -0,0 +1,62 @@ +/* + * Copyright 2000-2017 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.client.flow.model; + +import com.vaadin.client.flow.StateNode; +import com.vaadin.client.flow.binding.SimpleElementBindingStrategy; +import com.vaadin.client.flow.collection.JsArray; +import com.vaadin.client.flow.collection.JsCollections; +import com.vaadin.client.flow.collection.JsSet; + +/** + * The storage class for set of updatable model properties. + *

+ * This class is stored inside a {@link StateNode} via + * {@link StateNode#setNodeData(Object)} if there is any data to store at all. + * Once it's stored in the {@link StateNode} the code which sends updates to the + * server side when a polymer property is updated uses this data to detect + * whether server expects the update to be sent(see + * {@link SimpleElementBindingStrategy}). + * + * @author Vaadin Ltd + * + */ +public class UpdatableModelProperties { + + private final JsSet properties = JsCollections.set(); + + /** + * Creates a new instance of storage class based on given + * {@code properties}. + * + * @param properties + * updatable properties array + */ + public UpdatableModelProperties(JsArray properties) { + properties.forEach(this.properties::add); + } + + /** + * Tests whether the {@code property} is updatable. + * + * @param property + * the property to test + * @return {@code true} if property is updatable + */ + public boolean isUpdatableProperty(String property) { + return properties.has(property); + } +} diff --git a/flow-server/src/main/java/com/vaadin/flow/component/polymertemplate/PolymerTemplate.java b/flow-server/src/main/java/com/vaadin/flow/component/polymertemplate/PolymerTemplate.java index d4d4fb6cfe7..832b40f5677 100644 --- a/flow-server/src/main/java/com/vaadin/flow/component/polymertemplate/PolymerTemplate.java +++ b/flow-server/src/main/java/com/vaadin/flow/component/polymertemplate/PolymerTemplate.java @@ -16,13 +16,16 @@ package com.vaadin.flow.component.polymertemplate; import java.lang.reflect.Type; +import java.util.Collections; +import java.util.HashSet; import java.util.List; +import java.util.Map; +import java.util.Map.Entry; import java.util.Set; import java.util.stream.Collectors; import com.vaadin.flow.component.Tag; import com.vaadin.flow.component.dependency.HtmlImport; -import com.vaadin.flow.function.SerializablePredicate; import com.vaadin.flow.internal.StateNode; import com.vaadin.flow.internal.nodefeature.ElementPropertyMap; import com.vaadin.flow.templatemodel.BeanModelType; @@ -181,20 +184,6 @@ private static ModelType getModelTypeForListModel(Type type, return null; } - private JsonArray filterUnsetProperties(List properties) { - JsonArray array = Json.createArray(); - ElementPropertyMap map = getStateNode() - .getFeature(ElementPropertyMap.class); - int i = 0; - for (String property : properties) { - if (!map.hasProperty(property)) { - array.set(i, property); - i++; - } - } - return array; - } - private void initModel(Set twoWayBindingPaths) { // Find metadata, fill initial values and create a proxy getModel(); @@ -202,11 +191,16 @@ private void initModel(Set twoWayBindingPaths) { BeanModelType modelType = TemplateModelProxyHandler .getModelTypeForProxy(model); - SerializablePredicate updateFilter = modelType - .getClientUpdateAllowedProperties(twoWayBindingPaths) - .keySet()::contains; + Map allowedProperties = modelType + .getClientUpdateAllowedProperties(twoWayBindingPaths); + + Set allowedPropertyName = Collections.emptySet(); + if (!allowedProperties.isEmpty()) { + // copy to avoid referencing a map in the filter below + allowedPropertyName = new HashSet<>(allowedProperties.keySet()); + } ElementPropertyMap.getModel(getStateNode()) - .setUpdateFromClientFilter(updateFilter); + .setUpdateFromClientFilter(allowedPropertyName::contains); // remove properties whose values are not StateNode from the property // map and return their names as a list @@ -228,6 +222,42 @@ private void initModel(Set twoWayBindingPaths) { "this.populateModelProperties($0, $1)", getElement(), filterUnsetProperties(propertyNames)))); + getStateNode().runWhenAttached(ui -> ui.getInternals().getStateTree() + .beforeClientResponse(getStateNode(), + () -> ui.getPage().executeJavaScript( + "this.registerUpdatableModelProperties($0, $1)", + getElement(), + filterUpdatableProperties(allowedProperties)))); + } + + private JsonArray filterUnsetProperties(List properties) { + JsonArray array = Json.createArray(); + ElementPropertyMap map = getStateNode() + .getFeature(ElementPropertyMap.class); + int i = 0; + for (String property : properties) { + if (!map.hasProperty(property)) { + array.set(i, property); + i++; + } + } + return array; + } + + /* + * Keep only properties with getter. + */ + private JsonArray filterUpdatableProperties( + Map allowedProperties) { + JsonArray array = Json.createArray(); + int i = 0; + for (Entry entry : allowedProperties.entrySet()) { + if (entry.getValue()) { + array.set(i, entry.getKey()); + i++; + } + } + return array; } private List removeSimpleProperties() { From 27bde1591080622de15c99816143b1486c2415c6 Mon Sep 17 00:00:00 2001 From: Denis Anisimov Date: Tue, 6 Feb 2018 14:04:32 +0300 Subject: [PATCH 02/14] Don't sync property updates for the non updatable properties --- .../binding/SimpleElementBindingStrategy.java | 28 ++++++------------- 1 file changed, 9 insertions(+), 19 deletions(-) diff --git a/flow-client/src/main/java/com/vaadin/client/flow/binding/SimpleElementBindingStrategy.java b/flow-client/src/main/java/com/vaadin/client/flow/binding/SimpleElementBindingStrategy.java index 2f0e54ddb47..af2e13925c4 100644 --- a/flow-client/src/main/java/com/vaadin/client/flow/binding/SimpleElementBindingStrategy.java +++ b/flow-client/src/main/java/com/vaadin/client/flow/binding/SimpleElementBindingStrategy.java @@ -34,6 +34,7 @@ import com.vaadin.client.flow.collection.JsWeakMap; import com.vaadin.client.flow.dom.DomApi; import com.vaadin.client.flow.dom.DomElement.DomTokenList; +import com.vaadin.client.flow.model.UpdatableModelProperties; import com.vaadin.client.flow.nodefeature.ListSpliceEvent; import com.vaadin.client.flow.nodefeature.MapProperty; import com.vaadin.client.flow.nodefeature.NodeList; @@ -265,6 +266,14 @@ private void handlePropertiesChanged( private void handlePropertyChange(String fullPropertyName, Supplier valueProvider, StateNode node) { + UpdatableModelProperties updatableProperties = node + .getNodeData(UpdatableModelProperties.class); + if (updatableProperties == null + || !updatableProperties.isUpdatableProperty(fullPropertyName)) { + // don't do anything is the property/sub-property is not in the + // collection of updatable properties + return; + } // This is not the property value itself, its a parent node of the // property String[] subProperties = fullPropertyName.split("\\."); @@ -279,14 +288,6 @@ private void handlePropertyChange(String fullPropertyName, + "' which isn't defined from server"); return; } - if (containsProperty( - model.getList(NodeFeatures.SYNCHRONIZED_PROPERTIES), - subProperty)) { - Console.debug("Ignoring property change for property '" - + fullPropertyName - + "' which is intended to be synchronized separately"); - return; - } mapProperty = elementProperties.getProperty(subProperty); if (mapProperty.getValue() instanceof StateNode) { @@ -306,17 +307,6 @@ private void handlePropertyChange(String fullPropertyName, mapProperty.syncToServer(valueProvider.get()); } - private boolean containsProperty(NodeList synchronizedProperties, - String property) { - for (int i = 0; i < synchronizedProperties.length(); i++) { - if (Objects.equals(synchronizedProperties.get(i).toString(), - property)) { - return true; - } - } - return false; - } - private EventRemover bindShadowRoot(BindingContext context) { assert context.htmlNode instanceof Element : "Cannot bind shadow root to a Node"; NodeMap map = context.node.getMap(NodeFeatures.SHADOW_ROOT_DATA); From 0a936eff2b3b21d0e4b0ca6fc2c01506f4a0b57c Mon Sep 17 00:00:00 2001 From: Denis Anisimov Date: Tue, 6 Feb 2018 14:13:23 +0300 Subject: [PATCH 03/14] Correct GWT unit test. --- .../vaadin/client/flow/GwtPolymerModelTest.java | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/flow-client/src/test-gwt/java/com/vaadin/client/flow/GwtPolymerModelTest.java b/flow-client/src/test-gwt/java/com/vaadin/client/flow/GwtPolymerModelTest.java index 0df74d27165..a5b0d121175 100644 --- a/flow-client/src/test-gwt/java/com/vaadin/client/flow/GwtPolymerModelTest.java +++ b/flow-client/src/test-gwt/java/com/vaadin/client/flow/GwtPolymerModelTest.java @@ -8,6 +8,8 @@ import com.vaadin.client.Registry; import com.vaadin.client.WidgetUtil; import com.vaadin.client.flow.binding.Binder; +import com.vaadin.client.flow.collection.JsCollections; +import com.vaadin.client.flow.model.UpdatableModelProperties; import com.vaadin.client.flow.nodefeature.NodeList; import com.vaadin.client.flow.reactive.Reactive; import com.vaadin.client.flow.util.NativeFunction; @@ -239,6 +241,9 @@ public void testLatePolymerInit() { String propertyValue = "coffee"; setModelProperty(node, propertyName, propertyValue); + node.setNodeData( + new UpdatableModelProperties(JsCollections.array("black"))); + Binder.bind(node, element); Reactive.flush(); assertEquals( @@ -263,17 +268,13 @@ public void testLatePolymerInit() { 1.0, WidgetUtil.getJsProperty(element, "callbackCallCount")); } - public void testPropertiesWithSpecificSyncAreNotUpdated() { + public void testUpdateModelProperty_propertyIsNotUpdatable_propertyIsNotSync() { emulatePolymerNotLoaded(); addMockMethods(element); String propertyName = "black"; String propertyValue = "coffee"; - NodeList synchronizedProperties = node - .getList(NodeFeatures.SYNCHRONIZED_PROPERTIES); - synchronizedProperties.add(synchronizedProperties.length(), - propertyName); setModelProperty(node, propertyName, propertyValue); Binder.bind(node, element); @@ -285,8 +286,9 @@ public void testPropertiesWithSpecificSyncAreNotUpdated() { emulatePolymerPropertyChange(element, propertyName, "doesNotMatter"); Reactive.flush(); - assertEquals("Expected the property with name " + propertyName - + " not to be updated since it's contained in NodeFeatures.SYNCHRONIZED_PROPERTIES", + assertEquals( + "Expected the property with name " + propertyName + + " not to be updated since it's not updatable", propertyValue, WidgetUtil.getJsProperty(element, propertyName)); } From d0ba9c2dcc12188befcfde82dcf700e7be9a29b8 Mon Sep 17 00:00:00 2001 From: Denis Anisimov Date: Tue, 6 Feb 2018 14:28:22 +0300 Subject: [PATCH 04/14] Correct unit test for polymer template. --- .../flow/component/polymertemplate/PolymerTemplateTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flow-server/src/test/java/com/vaadin/flow/component/polymertemplate/PolymerTemplateTest.java b/flow-server/src/test/java/com/vaadin/flow/component/polymertemplate/PolymerTemplateTest.java index 3463efc25d8..7da6fcc682a 100644 --- a/flow-server/src/test/java/com/vaadin/flow/component/polymertemplate/PolymerTemplateTest.java +++ b/flow-server/src/test/java/com/vaadin/flow/component/polymertemplate/PolymerTemplateTest.java @@ -722,7 +722,7 @@ public void initModel_requestPopulateModel_onlyUnsetPropertiesAreSent() { ui.getInternals().getStateTree().runExecutionsBeforeClientResponse(); - Assert.assertEquals(1, executionOrder.size()); + Assert.assertEquals(2, executionOrder.size()); Assert.assertEquals("this.populateModelProperties($0, $1)", executionOrder.get(0)); From ccfc57469cd1d498eaad19b2e58e395c6b7d3797 Mon Sep 17 00:00:00 2001 From: Denis Anisimov Date: Tue, 6 Feb 2018 16:41:10 +0300 Subject: [PATCH 05/14] Correct testing updatable properties and send updatable properties before anything else. --- .../client/ExecuteJavaScriptElementUtils.java | 26 +++++++--- .../binding/SimpleElementBindingStrategy.java | 50 ++++++++++++++++--- .../GwtExecuteJavaScriptElementUtilsTest.java | 5 ++ .../polymertemplate/PolymerTemplate.java | 15 +++--- .../polymertemplate/PolymerTemplateTest.java | 4 +- .../uitest/ui/template/JsSubTemplate.java | 13 ++++- .../flow/uitest/ui/template/Message.java | 3 +- 7 files changed, 88 insertions(+), 28 deletions(-) diff --git a/flow-client/src/main/java/com/vaadin/client/ExecuteJavaScriptElementUtils.java b/flow-client/src/main/java/com/vaadin/client/ExecuteJavaScriptElementUtils.java index d12acfc4acb..256fe88f778 100644 --- a/flow-client/src/main/java/com/vaadin/client/ExecuteJavaScriptElementUtils.java +++ b/flow-client/src/main/java/com/vaadin/client/ExecuteJavaScriptElementUtils.java @@ -134,15 +134,25 @@ public static void populateModelProperties(StateNode node, return; } for (int i = 0; i < properties.length(); i++) { - String property = properties.get(i); - if (!isPropertyDefined(node.getDomNode(), property)) { - if (!map.hasPropertyValue(property)) { - map.getProperty(property).setValue(null); - } - } else { - map.getProperty(property).syncToServer( - WidgetUtil.getJsProperty(node.getDomNode(), property)); + populateModelProperty(node, map, properties.get(i)); + } + } + + private static void populateModelProperty(StateNode node, NodeMap map, + String property) { + if (!isPropertyDefined(node.getDomNode(), property)) { + if (!map.hasPropertyValue(property)) { + map.getProperty(property).setValue(null); + } + } else { + UpdatableModelProperties updatableProperties = node + .getNodeData(UpdatableModelProperties.class); + if (updatableProperties == null + || !updatableProperties.isUpdatableProperty(property)) { + return; } + map.getProperty(property).syncToServer( + WidgetUtil.getJsProperty(node.getDomNode(), property)); } } diff --git a/flow-client/src/main/java/com/vaadin/client/flow/binding/SimpleElementBindingStrategy.java b/flow-client/src/main/java/com/vaadin/client/flow/binding/SimpleElementBindingStrategy.java index af2e13925c4..b937de018aa 100644 --- a/flow-client/src/main/java/com/vaadin/client/flow/binding/SimpleElementBindingStrategy.java +++ b/flow-client/src/main/java/com/vaadin/client/flow/binding/SimpleElementBindingStrategy.java @@ -65,6 +65,8 @@ */ public class SimpleElementBindingStrategy implements BindingStrategy { + private static final String INITIAL_CHANGE = "isInitialChange"; + private static final String HIDDEN_ATTRIBUTE = "hidden"; private static final String ELEMENT_ATTACH_ERROR_PREFIX = "Element addressed by the "; @@ -236,17 +238,19 @@ private native void hookUpPolymerElement(StateNode node, Element element) /*-{ this.@SimpleElementBindingStrategy::bindInitialModelProperties(*)(node, element); var self = this; - + var originalPropertiesChanged = element._propertiesChanged; if (originalPropertiesChanged) { + var initialChangeWrapper = {}; + initialChangeWrapper.isInitialChange = true; element._propertiesChanged = function (currentProps, changedProps, oldProps) { $entry(function () { - self.@SimpleElementBindingStrategy::handlePropertiesChanged(*)(changedProps, node); + self.@SimpleElementBindingStrategy::handlePropertiesChanged(*)(changedProps, node, initialChangeWrapper); })(); originalPropertiesChanged.apply(this, arguments); }; } - + var originalReady = element.ready; element.ready = function (){ originalReady.apply(this, arguments); @@ -255,13 +259,43 @@ private native void hookUpPolymerElement(StateNode node, Element element) }-*/; private void handlePropertiesChanged( - JavaScriptObject changedPropertyPathsToValues, StateNode node) { + JavaScriptObject changedPropertyPathsToValues, StateNode node, + JavaScriptObject initialChangeWrapper) { String[] keys = WidgetUtil.getKeys(changedPropertyPathsToValues); + + /* + * The initialChangeWrapper object is used above in the + * hookUpPolymerElement method to distinguish the very + * first property change call and any subsequent. + * + * The very first call MAY BE (not necessary) done at the element + * initialization which is done from the server side via + * NodeFeeature mechanism. In this case it's called before + * any JS execution. And we won't have any info about updatable + * properties. So we need to postpone handling to wait when initial JS + * execution happens. So in case of initial change the handling is + * postponed via Reactive.addPostFlushListener(). + */ + + Boolean isInitialChange = (Boolean) WidgetUtil + .getJsProperty(initialChangeWrapper, INITIAL_CHANGE); + for (String propertyName : keys) { - handlePropertyChange(propertyName, () -> WidgetUtil - .getJsProperty(changedPropertyPathsToValues, propertyName), + Runnable handleProperty = () -> handlePropertyChange(propertyName, + () -> WidgetUtil.getJsProperty(changedPropertyPathsToValues, + propertyName), node); + if (isInitialChange) { + Reactive.addPostFlushListener(() -> { + handleProperty.run(); + WidgetUtil.setJsProperty(initialChangeWrapper, + INITIAL_CHANGE, false); + }); + } else { + handleProperty.run(); + } } + } private void handlePropertyChange(String fullPropertyName, @@ -270,10 +304,11 @@ private void handlePropertyChange(String fullPropertyName, .getNodeData(UpdatableModelProperties.class); if (updatableProperties == null || !updatableProperties.isUpdatableProperty(fullPropertyName)) { - // don't do anything is the property/sub-property is not in the + // don't do anything if the property/sub-property is not in the // collection of updatable properties return; } + // This is not the property value itself, its a parent node of the // property String[] subProperties = fullPropertyName.split("\\."); @@ -303,7 +338,6 @@ private void handlePropertyChange(String fullPropertyName, return; } } - mapProperty.syncToServer(valueProvider.get()); } diff --git a/flow-client/src/test-gwt/java/com/vaadin/client/GwtExecuteJavaScriptElementUtilsTest.java b/flow-client/src/test-gwt/java/com/vaadin/client/GwtExecuteJavaScriptElementUtilsTest.java index d840edc75a6..dc82a0ef7aa 100644 --- a/flow-client/src/test-gwt/java/com/vaadin/client/GwtExecuteJavaScriptElementUtilsTest.java +++ b/flow-client/src/test-gwt/java/com/vaadin/client/GwtExecuteJavaScriptElementUtilsTest.java @@ -18,6 +18,7 @@ import com.vaadin.client.flow.StateNode; import com.vaadin.client.flow.StateTree; import com.vaadin.client.flow.collection.JsCollections; +import com.vaadin.client.flow.model.UpdatableModelProperties; import com.vaadin.client.flow.nodefeature.MapProperty; import com.vaadin.client.flow.nodefeature.NodeMap; import com.vaadin.client.flow.reactive.Reactive; @@ -206,6 +207,8 @@ public void testPopulateModelProperties_elementIsNotReadyAndPropertyIsNotDefined public void testPopulateModelProperties_propertyIsDefined_syncToServer() { defineProperty(element, "foo"); + node.setNodeData( + new UpdatableModelProperties(JsCollections.array("foo"))); WidgetUtil.setJsProperty(element, "foo", "bar"); @@ -225,6 +228,8 @@ public void testPopulateModelProperties_elementIsNotReadyAndPropertyIsDefined_sy mockWhenDefined(element); defineProperty(element, "foo"); + node.setNodeData( + new UpdatableModelProperties(JsCollections.array("foo"))); WidgetUtil.setJsProperty(element, "foo", "bar"); diff --git a/flow-server/src/main/java/com/vaadin/flow/component/polymertemplate/PolymerTemplate.java b/flow-server/src/main/java/com/vaadin/flow/component/polymertemplate/PolymerTemplate.java index 832b40f5677..1f10277885e 100644 --- a/flow-server/src/main/java/com/vaadin/flow/component/polymertemplate/PolymerTemplate.java +++ b/flow-server/src/main/java/com/vaadin/flow/component/polymertemplate/PolymerTemplate.java @@ -206,6 +206,15 @@ private void initModel(Set twoWayBindingPaths) { // map and return their names as a list List propertyNames = removeSimpleProperties(); + // This has to be executed BEFORE model population to be able to know + // which properties needs update to the server + getStateNode().runWhenAttached(ui -> ui.getInternals().getStateTree() + .beforeClientResponse(getStateNode(), + () -> ui.getPage().executeJavaScript( + "this.registerUpdatableModelProperties($0, $1)", + getElement(), + filterUpdatableProperties(allowedProperties)))); + /* * Now populate model properties on the client side. Only explicitly set * by the developer properties are in the map at the moment of execution @@ -222,12 +231,6 @@ private void initModel(Set twoWayBindingPaths) { "this.populateModelProperties($0, $1)", getElement(), filterUnsetProperties(propertyNames)))); - getStateNode().runWhenAttached(ui -> ui.getInternals().getStateTree() - .beforeClientResponse(getStateNode(), - () -> ui.getPage().executeJavaScript( - "this.registerUpdatableModelProperties($0, $1)", - getElement(), - filterUpdatableProperties(allowedProperties)))); } private JsonArray filterUnsetProperties(List properties) { diff --git a/flow-server/src/test/java/com/vaadin/flow/component/polymertemplate/PolymerTemplateTest.java b/flow-server/src/test/java/com/vaadin/flow/component/polymertemplate/PolymerTemplateTest.java index 7da6fcc682a..edca192bd59 100644 --- a/flow-server/src/test/java/com/vaadin/flow/component/polymertemplate/PolymerTemplateTest.java +++ b/flow-server/src/test/java/com/vaadin/flow/component/polymertemplate/PolymerTemplateTest.java @@ -724,9 +724,9 @@ public void initModel_requestPopulateModel_onlyUnsetPropertiesAreSent() { Assert.assertEquals(2, executionOrder.size()); Assert.assertEquals("this.populateModelProperties($0, $1)", - executionOrder.get(0)); + executionOrder.get(1)); - Serializable[] params = executionParams.get(0); + Serializable[] params = executionParams.get(1); JsonArray properties = (JsonArray) params[1]; Assert.assertEquals(1, properties.length()); Assert.assertEquals("title", properties.get(0).asString()); diff --git a/flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/template/JsSubTemplate.java b/flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/template/JsSubTemplate.java index 6be89ef31d0..49441f5d732 100644 --- a/flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/template/JsSubTemplate.java +++ b/flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/template/JsSubTemplate.java @@ -19,17 +19,26 @@ import com.vaadin.flow.component.dependency.HtmlImport; import com.vaadin.flow.component.polymertemplate.Id; import com.vaadin.flow.component.polymertemplate.PolymerTemplate; +import com.vaadin.flow.templatemodel.AllowClientUpdates; import com.vaadin.flow.templatemodel.TemplateModel; @Tag("js-sub-template") @HtmlImport("frontend://com/vaadin/flow/uitest/ui/template/JsSubTemplate.html") -public class JsSubTemplate extends PolymerTemplate { +public class JsSubTemplate + extends PolymerTemplate { @Id("js-grand-child") private JsInjectedGrandChild component; + public interface JsSubTemplateModel extends TemplateModel { + @AllowClientUpdates + String getFoo(); + + void setFoo(String value); + } + public JsSubTemplate() { - getElement().setProperty("foo", "bar"); + getModel().setFoo("bar"); } } diff --git a/flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/template/Message.java b/flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/template/Message.java index 18230568a02..03dae705acc 100644 --- a/flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/template/Message.java +++ b/flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/template/Message.java @@ -16,12 +16,11 @@ package com.vaadin.flow.uitest.ui.template; import com.vaadin.flow.templatemodel.AllowClientUpdates; -import com.vaadin.flow.templatemodel.ClientUpdateMode; import com.vaadin.flow.templatemodel.TemplateModel; public interface Message extends TemplateModel { void setText(String text); - @AllowClientUpdates(ClientUpdateMode.ALLOW) + @AllowClientUpdates String getText(); } From d3a6b84b4055667d142e9b127782111a431c47f0 Mon Sep 17 00:00:00 2001 From: Denis Anisimov Date: Tue, 6 Feb 2018 16:59:24 +0300 Subject: [PATCH 06/14] Add flush() to ensure that post listener is called --- .../client/flow/binding/SimpleElementBindingStrategy.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/flow-client/src/main/java/com/vaadin/client/flow/binding/SimpleElementBindingStrategy.java b/flow-client/src/main/java/com/vaadin/client/flow/binding/SimpleElementBindingStrategy.java index b937de018aa..096a9edff8b 100644 --- a/flow-client/src/main/java/com/vaadin/client/flow/binding/SimpleElementBindingStrategy.java +++ b/flow-client/src/main/java/com/vaadin/client/flow/binding/SimpleElementBindingStrategy.java @@ -291,6 +291,11 @@ private void handlePropertiesChanged( WidgetUtil.setJsProperty(initialChangeWrapper, INITIAL_CHANGE, false); }); + // It might be that there will be no automatic flush since this + // change is not caused by the server, in this case let's call + // flush(). Extra flush() won't affect + // anything since it will be inside another flush() + Reactive.flush(); } else { handleProperty.run(); } From 4a9665a275dd83353ab6e0191917230296874bc5 Mon Sep 17 00:00:00 2001 From: Denis Anisimov Date: Tue, 6 Feb 2018 18:12:51 +0300 Subject: [PATCH 07/14] Postpone initial property handling via post flush listener and next browser loop iteration. --- .../com/vaadin/client/flow/StateNode.java | 10 +++ .../binding/SimpleElementBindingStrategy.java | 75 +++++++++++-------- 2 files changed, 53 insertions(+), 32 deletions(-) diff --git a/flow-client/src/main/java/com/vaadin/client/flow/StateNode.java b/flow-client/src/main/java/com/vaadin/client/flow/StateNode.java index d3db609f3c4..61a4bebaeb7 100644 --- a/flow-client/src/main/java/com/vaadin/client/flow/StateNode.java +++ b/flow-client/src/main/java/com/vaadin/client/flow/StateNode.java @@ -317,4 +317,14 @@ public void setNodeData(T object) { public T getNodeData(Class clazz) { return (T) nodeData.get(clazz); } + + /** + * Removes the {@code object} from the stored data. + * + * @param object + * the object to remove + */ + public void clearNodeData(T object) { + nodeData.delete(object.getClass()); + } } diff --git a/flow-client/src/main/java/com/vaadin/client/flow/binding/SimpleElementBindingStrategy.java b/flow-client/src/main/java/com/vaadin/client/flow/binding/SimpleElementBindingStrategy.java index 096a9edff8b..d983f79e47f 100644 --- a/flow-client/src/main/java/com/vaadin/client/flow/binding/SimpleElementBindingStrategy.java +++ b/flow-client/src/main/java/com/vaadin/client/flow/binding/SimpleElementBindingStrategy.java @@ -19,6 +19,7 @@ import java.util.function.Supplier; import com.google.gwt.core.client.JavaScriptObject; +import com.google.gwt.core.client.Scheduler; import com.vaadin.client.Command; import com.vaadin.client.Console; import com.vaadin.client.ExistingElementMap; @@ -139,6 +140,26 @@ private BindingContext(StateNode node, Node htmlNode, } } + private static class InitialPropertyUpdate { + private JsArray commands; + private final StateNode node; + + private InitialPropertyUpdate(StateNode node) { + this.node = node; + } + + private void setCommands(JsArray commands) { + this.commands = commands; + } + + private void execute() { + if (commands != null) { + commands.forEach(Runnable::run); + } + node.clearNodeData(this); + } + } + @Override public Element create(StateNode node) { String tag = getTag(node); @@ -214,6 +235,17 @@ assert hasSameTag(stateNode, htmlNode) : "Element tag name is '" } listeners.push(bindVisibility(listeners, context, computationsCollection, nodeFactory)); + + InitialPropertyUpdate update = new InitialPropertyUpdate(stateNode); + stateNode.setNodeData(update); + /* + * Update command will be executed after all initial Reactive stuff. + * E.g. initial JS (if any) will be executed BEFORE initial update + * command execution + */ + Reactive.addPostFlushListener( + () -> Scheduler.get().scheduleDeferred(() -> stateNode + .getNodeData(InitialPropertyUpdate.class).execute())); } private native void bindPolymerModelProperties(StateNode node, @@ -241,11 +273,9 @@ private native void hookUpPolymerElement(StateNode node, Element element) var originalPropertiesChanged = element._propertiesChanged; if (originalPropertiesChanged) { - var initialChangeWrapper = {}; - initialChangeWrapper.isInitialChange = true; element._propertiesChanged = function (currentProps, changedProps, oldProps) { $entry(function () { - self.@SimpleElementBindingStrategy::handlePropertiesChanged(*)(changedProps, node, initialChangeWrapper); + self.@SimpleElementBindingStrategy::handlePropertiesChanged(*)(changedProps, node); })(); originalPropertiesChanged.apply(this, arguments); }; @@ -259,43 +289,24 @@ private native void hookUpPolymerElement(StateNode node, Element element) }-*/; private void handlePropertiesChanged( - JavaScriptObject changedPropertyPathsToValues, StateNode node, - JavaScriptObject initialChangeWrapper) { + JavaScriptObject changedPropertyPathsToValues, StateNode node) { String[] keys = WidgetUtil.getKeys(changedPropertyPathsToValues); - /* - * The initialChangeWrapper object is used above in the - * hookUpPolymerElement method to distinguish the very - * first property change call and any subsequent. - * - * The very first call MAY BE (not necessary) done at the element - * initialization which is done from the server side via - * NodeFeeature mechanism. In this case it's called before - * any JS execution. And we won't have any info about updatable - * properties. So we need to postpone handling to wait when initial JS - * execution happens. So in case of initial change the handling is - * postponed via Reactive.addPostFlushListener(). - */ - - Boolean isInitialChange = (Boolean) WidgetUtil - .getJsProperty(initialChangeWrapper, INITIAL_CHANGE); + InitialPropertyUpdate initialUpdate = node + .getNodeData(InitialPropertyUpdate.class); + JsArray commands = null; + if (initialUpdate != null) { + commands = JsCollections.array(); + initialUpdate.setCommands(commands); + } for (String propertyName : keys) { Runnable handleProperty = () -> handlePropertyChange(propertyName, () -> WidgetUtil.getJsProperty(changedPropertyPathsToValues, propertyName), node); - if (isInitialChange) { - Reactive.addPostFlushListener(() -> { - handleProperty.run(); - WidgetUtil.setJsProperty(initialChangeWrapper, - INITIAL_CHANGE, false); - }); - // It might be that there will be no automatic flush since this - // change is not caused by the server, in this case let's call - // flush(). Extra flush() won't affect - // anything since it will be inside another flush() - Reactive.flush(); + if (commands != null) { + commands.set(commands.length(), handleProperty); } else { handleProperty.run(); } From 7b1b9bfbbc719af3421a2c6d64c2a8825a233199 Mon Sep 17 00:00:00 2001 From: Denis Anisimov Date: Wed, 7 Feb 2018 08:54:03 +0300 Subject: [PATCH 08/14] Emulate scheduler to be able to handle initial property change --- .../binding/SimpleElementBindingStrategy.java | 48 +++++++++---------- .../com/vaadin/client/CustomScheduler.java | 27 +++++++++++ .../client/GwtDependencyLoaderTest.java | 8 ---- .../client/flow/GwtPolymerModelTest.java | 11 ++++- .../vaadin/client/flow/dom/GwtDomApiTest.java | 12 +++++ 5 files changed, 73 insertions(+), 33 deletions(-) create mode 100644 flow-client/src/test-gwt/java/com/vaadin/client/CustomScheduler.java diff --git a/flow-client/src/main/java/com/vaadin/client/flow/binding/SimpleElementBindingStrategy.java b/flow-client/src/main/java/com/vaadin/client/flow/binding/SimpleElementBindingStrategy.java index d983f79e47f..52cb8136d97 100644 --- a/flow-client/src/main/java/com/vaadin/client/flow/binding/SimpleElementBindingStrategy.java +++ b/flow-client/src/main/java/com/vaadin/client/flow/binding/SimpleElementBindingStrategy.java @@ -141,20 +141,20 @@ private BindingContext(StateNode node, Node htmlNode, } private static class InitialPropertyUpdate { - private JsArray commands; + private Runnable command; private final StateNode node; private InitialPropertyUpdate(StateNode node) { this.node = node; } - private void setCommands(JsArray commands) { - this.commands = commands; + private void setCommand(Runnable command) { + this.command = command; } private void execute() { - if (commands != null) { - commands.forEach(Runnable::run); + if (command != null) { + command.run(); } node.clearNodeData(this); } @@ -236,6 +236,10 @@ assert hasSameTag(stateNode, htmlNode) : "Element tag name is '" listeners.push(bindVisibility(listeners, context, computationsCollection, nodeFactory)); + scheduleInitialExecution(stateNode); + } + + private void scheduleInitialExecution(StateNode stateNode) { InitialPropertyUpdate update = new InitialPropertyUpdate(stateNode); stateNode.setNodeData(update); /* @@ -270,7 +274,7 @@ private native void hookUpPolymerElement(StateNode node, Element element) /*-{ this.@SimpleElementBindingStrategy::bindInitialModelProperties(*)(node, element); var self = this; - + var originalPropertiesChanged = element._propertiesChanged; if (originalPropertiesChanged) { element._propertiesChanged = function (currentProps, changedProps, oldProps) { @@ -280,7 +284,7 @@ private native void hookUpPolymerElement(StateNode node, Element element) originalPropertiesChanged.apply(this, arguments); }; } - + var originalReady = element.ready; element.ready = function (){ originalReady.apply(this, arguments); @@ -292,26 +296,22 @@ private void handlePropertiesChanged( JavaScriptObject changedPropertyPathsToValues, StateNode node) { String[] keys = WidgetUtil.getKeys(changedPropertyPathsToValues); + Runnable runnable = () -> { + for (String propertyName : keys) { + handlePropertyChange(propertyName, + () -> WidgetUtil.getJsProperty( + changedPropertyPathsToValues, propertyName), + node); + } + }; + InitialPropertyUpdate initialUpdate = node .getNodeData(InitialPropertyUpdate.class); - JsArray commands = null; - if (initialUpdate != null) { - commands = JsCollections.array(); - initialUpdate.setCommands(commands); - } - - for (String propertyName : keys) { - Runnable handleProperty = () -> handlePropertyChange(propertyName, - () -> WidgetUtil.getJsProperty(changedPropertyPathsToValues, - propertyName), - node); - if (commands != null) { - commands.set(commands.length(), handleProperty); - } else { - handleProperty.run(); - } + if (initialUpdate == null) { + runnable.run(); + } else { + initialUpdate.setCommand(runnable); } - } private void handlePropertyChange(String fullPropertyName, diff --git a/flow-client/src/test-gwt/java/com/vaadin/client/CustomScheduler.java b/flow-client/src/test-gwt/java/com/vaadin/client/CustomScheduler.java new file mode 100644 index 00000000000..079fdca0470 --- /dev/null +++ b/flow-client/src/test-gwt/java/com/vaadin/client/CustomScheduler.java @@ -0,0 +1,27 @@ +/* + * Copyright 2000-2017 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.client; + +import com.google.gwt.core.client.Scheduler.ScheduledCommand; +import com.google.gwt.core.client.impl.SchedulerImpl; + +public class CustomScheduler extends SchedulerImpl { + + @Override + public void scheduleDeferred(ScheduledCommand cmd) { + cmd.execute(); + } +} \ No newline at end of file diff --git a/flow-client/src/test-gwt/java/com/vaadin/client/GwtDependencyLoaderTest.java b/flow-client/src/test-gwt/java/com/vaadin/client/GwtDependencyLoaderTest.java index b0011fd85b7..11e546a8846 100644 --- a/flow-client/src/test-gwt/java/com/vaadin/client/GwtDependencyLoaderTest.java +++ b/flow-client/src/test-gwt/java/com/vaadin/client/GwtDependencyLoaderTest.java @@ -104,14 +104,6 @@ public void inlineStyleSheet(String styleSheetContents, } } - public static class CustomScheduler extends SchedulerImpl { - - @Override - public void scheduleDeferred(ScheduledCommand cmd) { - cmd.execute(); - } - } - private MockResourceLoader mockResourceLoader; private Registry registry; diff --git a/flow-client/src/test-gwt/java/com/vaadin/client/flow/GwtPolymerModelTest.java b/flow-client/src/test-gwt/java/com/vaadin/client/flow/GwtPolymerModelTest.java index a5b0d121175..37dcb3c79bc 100644 --- a/flow-client/src/test-gwt/java/com/vaadin/client/flow/GwtPolymerModelTest.java +++ b/flow-client/src/test-gwt/java/com/vaadin/client/flow/GwtPolymerModelTest.java @@ -4,6 +4,8 @@ import java.util.List; import java.util.function.Function; +import com.google.gwt.core.client.impl.SchedulerImpl; +import com.vaadin.client.CustomScheduler; import com.vaadin.client.PolymerUtils; import com.vaadin.client.Registry; import com.vaadin.client.WidgetUtil; @@ -44,6 +46,8 @@ protected void gwtSetUp() throws Exception { nextId = node.getId() + 1; element = createHtmlElement(); initPolymer(element); + + initScheduler(new CustomScheduler()); } public void testPropertyAdded() { @@ -299,7 +303,7 @@ private native void addMockMethods(Element element) element._propertiesChanged = function() { element.propertiesChangedCallCount += 1; }; - + element.callbackCallCount = 0; $wnd.customElements = { whenDefined: function() { @@ -497,4 +501,9 @@ private native void initPolymer(Element element) $wnd.Polymer.version="2.0.1"; element.__proto__ = $wnd.Polymer.Element; }-*/; + + private native void initScheduler(SchedulerImpl scheduler) + /*-{ + @com.google.gwt.core.client.impl.SchedulerImpl::INSTANCE = scheduler; + }-*/; } diff --git a/flow-client/src/test-gwt/java/com/vaadin/client/flow/dom/GwtDomApiTest.java b/flow-client/src/test-gwt/java/com/vaadin/client/flow/dom/GwtDomApiTest.java index bd984edb668..2e8dfad04ae 100644 --- a/flow-client/src/test-gwt/java/com/vaadin/client/flow/dom/GwtDomApiTest.java +++ b/flow-client/src/test-gwt/java/com/vaadin/client/flow/dom/GwtDomApiTest.java @@ -1,6 +1,7 @@ package com.vaadin.client.flow.dom; import com.google.gwt.core.client.JavaScriptObject; +import com.google.gwt.core.client.impl.SchedulerImpl; import com.vaadin.client.ClientEngineTestBase; import com.vaadin.client.DependencyLoader; import com.vaadin.client.Registry; @@ -42,6 +43,12 @@ private void initTest() { DomApi.polymerMicroLoaded = false; DomApi.impl = node -> (DomElement) node; + initScheduler(new SchedulerImpl() { + @Override + public void scheduleDeferred(ScheduledCommand cmd) { + } + }); + GwtPolymerApiImplTest.clearPolymer(); verifyPolymerDomApiUsed(false); @@ -88,4 +95,9 @@ private void verifyPolymerMicroLoaded(boolean used) { used, DomApi.polymerMicroLoaded); } + private native void initScheduler(SchedulerImpl scheduler) + /*-{ + @com.google.gwt.core.client.impl.SchedulerImpl::INSTANCE = scheduler; + }-*/; + } From 1eed3a27545ba6c727830e91f931be25fe3790da Mon Sep 17 00:00:00 2001 From: Denis Anisimov Date: Wed, 7 Feb 2018 10:02:09 +0300 Subject: [PATCH 09/14] GWT unit tests for the initial property update --- .../client/flow/GwtPolymerModelTest.java | 57 ++++++++++++++++++- 1 file changed, 56 insertions(+), 1 deletion(-) diff --git a/flow-client/src/test-gwt/java/com/vaadin/client/flow/GwtPolymerModelTest.java b/flow-client/src/test-gwt/java/com/vaadin/client/flow/GwtPolymerModelTest.java index 37dcb3c79bc..2248678c448 100644 --- a/flow-client/src/test-gwt/java/com/vaadin/client/flow/GwtPolymerModelTest.java +++ b/flow-client/src/test-gwt/java/com/vaadin/client/flow/GwtPolymerModelTest.java @@ -272,6 +272,61 @@ public void testLatePolymerInit() { 1.0, WidgetUtil.getJsProperty(element, "callbackCallCount")); } + public void testInitialUpdateModelProperty_propertyIsUpdatable_propertyIsSynced() { + String propertyName = "black"; + String propertyValue = "coffee"; + setModelProperty(node, propertyName, propertyValue); + + node.setNodeData( + new UpdatableModelProperties(JsCollections.array("black"))); + + Binder.bind(node, element); + Reactive.flush(); + assertEquals( + "Expected to have property with name " + propertyName + + " defined after initial binding", + propertyValue, WidgetUtil.getJsProperty(element, propertyName)); + + String newPropertyValue = "bubblegum"; + emulatePolymerPropertyChange(element, propertyName, newPropertyValue); + Reactive.flush(); + assertEquals( + "Expected to have property with name " + propertyName + + " updated from client side", + newPropertyValue, + WidgetUtil.getJsProperty(element, propertyName)); + } + + public void testInitialUpdateModelProperty_propertyIsUpdatableAndSchedulerIsNotExecuted_propertyIsNotSync() { + String propertyName = "black"; + String propertyValue = "coffee"; + setModelProperty(node, propertyName, propertyValue); + + initScheduler(new SchedulerImpl() { + @Override + public void scheduleDeferred(ScheduledCommand cmd) { + } + }); + + node.setNodeData( + new UpdatableModelProperties(JsCollections.array("black"))); + + Binder.bind(node, element); + Reactive.flush(); + assertEquals( + "Expected to have property with name " + propertyName + + " defined after initial binding", + propertyValue, WidgetUtil.getJsProperty(element, propertyName)); + + String newPropertyValue = "bubblegum"; + emulatePolymerPropertyChange(element, propertyName, newPropertyValue); + Reactive.flush(); + assertEquals( + "Expected to have property with name " + propertyName + + " updated from client side", + propertyValue, WidgetUtil.getJsProperty(element, propertyName)); + } + public void testUpdateModelProperty_propertyIsNotUpdatable_propertyIsNotSync() { emulatePolymerNotLoaded(); addMockMethods(element); @@ -303,7 +358,7 @@ private native void addMockMethods(Element element) element._propertiesChanged = function() { element.propertiesChangedCallCount += 1; }; - + element.callbackCallCount = 0; $wnd.customElements = { whenDefined: function() { From 7d342bebadffe23447f84412931c5a8e60b4c2d4 Mon Sep 17 00:00:00 2001 From: Denis Anisimov Date: Wed, 7 Feb 2018 10:09:22 +0300 Subject: [PATCH 10/14] Gwt unit test for JS execution --- .../GwtExecuteJavaScriptElementUtilsTest.java | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/flow-client/src/test-gwt/java/com/vaadin/client/GwtExecuteJavaScriptElementUtilsTest.java b/flow-client/src/test-gwt/java/com/vaadin/client/GwtExecuteJavaScriptElementUtilsTest.java index dc82a0ef7aa..02a41f9cc04 100644 --- a/flow-client/src/test-gwt/java/com/vaadin/client/GwtExecuteJavaScriptElementUtilsTest.java +++ b/flow-client/src/test-gwt/java/com/vaadin/client/GwtExecuteJavaScriptElementUtilsTest.java @@ -222,6 +222,20 @@ public void testPopulateModelProperties_propertyIsDefined_syncToServer() { assertEquals("foo", tree.syncedProperty.getName()); } + public void testPopulateModelProperties_propertyIsDefinedAndNotUpodatable_noSyncToServer() { + defineProperty(element, "foo"); + + WidgetUtil.setJsProperty(element, "foo", "bar"); + + ExecuteJavaScriptElementUtils.populateModelProperties(node, + JsCollections.array("foo")); + + NodeMap map = node.getMap(NodeFeatures.ELEMENT_PROPERTIES); + assertFalse(map.hasPropertyValue("foo")); + + assertNull(tree.syncedProperty); + } + public void testPopulateModelProperties_elementIsNotReadyAndPropertyIsDefined_syncToServerWhenElementBecomesReady() { node.setDomNode(null); From 921ef3d3959970ae13f1e443adfdb0b4d953f3f8 Mon Sep 17 00:00:00 2001 From: Denis Anisimov Date: Wed, 7 Feb 2018 10:25:51 +0300 Subject: [PATCH 11/14] Unit test for reporting updatable properties to the client side. --- .../polymertemplate/PolymerTemplateTest.java | 40 +++++++++++++------ 1 file changed, 28 insertions(+), 12 deletions(-) diff --git a/flow-server/src/test/java/com/vaadin/flow/component/polymertemplate/PolymerTemplateTest.java b/flow-server/src/test/java/com/vaadin/flow/component/polymertemplate/PolymerTemplateTest.java index edca192bd59..7c4f4654547 100644 --- a/flow-server/src/test/java/com/vaadin/flow/component/polymertemplate/PolymerTemplateTest.java +++ b/flow-server/src/test/java/com/vaadin/flow/component/polymertemplate/PolymerTemplateTest.java @@ -26,8 +26,10 @@ import java.lang.reflect.Field; import java.util.ArrayList; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Function; @@ -55,7 +57,6 @@ import com.vaadin.flow.server.VaadinSession; import com.vaadin.flow.server.startup.CustomElementRegistry; import com.vaadin.flow.templatemodel.AllowClientUpdates; -import com.vaadin.flow.templatemodel.ClientUpdateMode; import com.vaadin.flow.templatemodel.TemplateModel; import elemental.json.JsonArray; @@ -102,17 +103,15 @@ public interface ModelClass extends TemplateModel { void setTitle(String title); - @AllowClientUpdates(ClientUpdateMode.ALLOW) + @AllowClientUpdates String getMessage(); - @AllowClientUpdates(ClientUpdateMode.ALLOW) + @AllowClientUpdates String getTitle(); } public interface TestModel extends ModelClass { - @AllowClientUpdates(ClientUpdateMode.ALLOW) - List getList(); - + @AllowClientUpdates void setList(List list); } @@ -732,12 +731,29 @@ public void initModel_requestPopulateModel_onlyUnsetPropertiesAreSent() { Assert.assertEquals("title", properties.get(0).asString()); } - private List convertIntArray(JsonArray array) { - List list = new ArrayList<>(); - for (int i = 0; i < array.length(); i++) { - list.add((int) array.get(i).asNumber()); - } - return list; + @Test + public void initModel_sendUpdatableProperties() { + UI ui = UI.getCurrent(); + InitModelTemplate template = new InitModelTemplate(); + + ui.add(template); + + ui.getInternals().getStateTree().runExecutionsBeforeClientResponse(); + + Assert.assertEquals(2, executionOrder.size()); + Assert.assertEquals("this.registerUpdatableModelProperties($0, $1)", + executionOrder.get(0)); + + Serializable[] params = executionParams.get(0); + JsonArray properties = (JsonArray) params[1]; + Assert.assertEquals(2, properties.length()); + + Set props = new HashSet<>(); + props.add(properties.get(0).asString()); + props.add(properties.get(1).asString()); + // all model properties except 'list' which has no getter + Assert.assertTrue(props.contains("message")); + Assert.assertTrue(props.contains("title")); } private void doParseTemplate_hasIdChild_childIsRegisteredInFeature( From b659bdd693addcf85a97654f7500ea433c68cbf2 Mon Sep 17 00:00:00 2001 From: Denis Anisimov Date: Wed, 7 Feb 2018 11:59:13 +0300 Subject: [PATCH 12/14] UI and IT test for updatable properties --- .../UpdatableModelPropertiesView.java | 82 ++++++++++++++ .../ui/template/UpdatableModelProperties.html | 89 +++++++++++++++ .../template/UpdatableModelPropertiesIT.java | 104 ++++++++++++++++++ 3 files changed, 275 insertions(+) create mode 100644 flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/template/UpdatableModelPropertiesView.java create mode 100644 flow-tests/test-root-context/src/main/webapp/frontend/com/vaadin/flow/uitest/ui/template/UpdatableModelProperties.html create mode 100644 flow-tests/test-root-context/src/test/java/com/vaadin/flow/uitest/ui/template/UpdatableModelPropertiesIT.java diff --git a/flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/template/UpdatableModelPropertiesView.java b/flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/template/UpdatableModelPropertiesView.java new file mode 100644 index 00000000000..a48feb0412b --- /dev/null +++ b/flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/template/UpdatableModelPropertiesView.java @@ -0,0 +1,82 @@ +/* + * Copyright 2000-2017 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.flow.uitest.ui.template; + +import java.util.UUID; + +import com.vaadin.flow.component.ClientDelegate; +import com.vaadin.flow.component.HasComponents; +import com.vaadin.flow.component.Tag; +import com.vaadin.flow.component.dependency.HtmlImport; +import com.vaadin.flow.component.html.Label; +import com.vaadin.flow.component.polymertemplate.EventHandler; +import com.vaadin.flow.component.polymertemplate.PolymerTemplate; +import com.vaadin.flow.router.Route; +import com.vaadin.flow.templatemodel.AllowClientUpdates; +import com.vaadin.flow.templatemodel.ClientUpdateMode; +import com.vaadin.flow.templatemodel.TemplateModel; +import com.vaadin.flow.uitest.servlet.ViewTestLayout; + +@Tag("updatable-model-properties") +@HtmlImport("frontend://com/vaadin/flow/uitest/ui/template/UpdatableModelProperties.html") +@Route(value = "com.vaadin.flow.uitest.ui.template.UpdatableModelPropertiesView", layout = ViewTestLayout.class) +public class UpdatableModelPropertiesView extends + PolymerTemplate + implements HasComponents { + + public interface UpdatablePropertiesModel extends TemplateModel { + + @AllowClientUpdates(ClientUpdateMode.IF_TWO_WAY_BINDING) + String getName(); + + @AllowClientUpdates + String getEmail(); + + @AllowClientUpdates + void setAge(int age); + + @AllowClientUpdates(ClientUpdateMode.DENY) + void setText(String text); + } + + public UpdatableModelPropertiesView() { + setId("template"); + + Label label = new Label(); + label.setId("property-value"); + add(label); + + getElement().addPropertyChangeListener("name", + event -> label.setText(getModel().getName())); + getElement().addPropertyChangeListener("email", + event -> label.setText(getModel().getEmail())); + getElement().addPropertyChangeListener("age", + event -> label.setText(getElement().getProperty("age"))); + getElement().addPropertyChangeListener("text", + event -> label.setText(getElement().getProperty("text"))); + } + + @EventHandler + private void syncAge() { + getElement().synchronizeProperty("age", "age-changed"); + } + + @ClientDelegate + private void updateStatus() { + getElement().setProperty("updateStatus", + "Update Done " + UUID.randomUUID().toString()); + } +} diff --git a/flow-tests/test-root-context/src/main/webapp/frontend/com/vaadin/flow/uitest/ui/template/UpdatableModelProperties.html b/flow-tests/test-root-context/src/main/webapp/frontend/com/vaadin/flow/uitest/ui/template/UpdatableModelProperties.html new file mode 100644 index 00000000000..9f537b22360 --- /dev/null +++ b/flow-tests/test-root-context/src/main/webapp/frontend/com/vaadin/flow/uitest/ui/template/UpdatableModelProperties.html @@ -0,0 +1,89 @@ + + + + + + + + diff --git a/flow-tests/test-root-context/src/test/java/com/vaadin/flow/uitest/ui/template/UpdatableModelPropertiesIT.java b/flow-tests/test-root-context/src/test/java/com/vaadin/flow/uitest/ui/template/UpdatableModelPropertiesIT.java new file mode 100644 index 00000000000..63aa5b19e3d --- /dev/null +++ b/flow-tests/test-root-context/src/test/java/com/vaadin/flow/uitest/ui/template/UpdatableModelPropertiesIT.java @@ -0,0 +1,104 @@ +/* + * Copyright 2000-2017 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.flow.uitest.ui.template; + +import org.junit.Assert; +import org.junit.Test; +import org.openqa.selenium.By; +import org.openqa.selenium.WebElement; + +import com.vaadin.flow.testutil.ChromeBrowserTest; + +public class UpdatableModelPropertiesIT extends ChromeBrowserTest { + + @Test + public void updateName_propertyIsSentToServer() { + open(); + + WebElement name = getElement("name"); + name.click(); + + assertUpdate("foo"); + } + + @Test + public void updateAge_propertyIsNotSentToServerIfIsNotSynced_propertyIsSentWhenSynced() { + open(); + + WebElement age = getElement("age"); + age.click(); + + String value = age.getText(); + + assertNoUpdate(value); + + getElement("syncAge").click(); + + age.click(); + + value = age.getText(); + assertUpdate(value); + } + + @Test + public void updateEmail_propertyIsSentToServer() { + open(); + + WebElement email = getElement("email"); + email.click(); + + assertUpdate(email.getText()); + } + + @Test + public void updateText_propertyIsNotSentToServer() { + open(); + + WebElement text = getElement("text"); + text.click(); + + String value = text.getText(); + + assertNoUpdate(value); + } + + private WebElement getElement(String id) { + WebElement template = findElement(By.id("template")); + return getInShadowRoot(template, By.id(id)); + } + + private void waitUpdate() { + waitUntil(driver -> getElement("updateStatus").getText() + .startsWith("Update Done")); + } + + private void assertUpdate(String expectedValue) { + waitUpdate(); + + WebElement template = findElement(By.id("template")); + WebElement value = template.findElement(By.id("property-value")); + Assert.assertEquals(expectedValue, value.getText()); + } + + private void assertNoUpdate(String unexpectedValue) { + waitUpdate(); + + WebElement template = findElement(By.id("template")); + WebElement value = template.findElement(By.id("property-value")); + Assert.assertNotEquals(unexpectedValue, value.getText()); + } + +} From ce7fdb118a0da931e18c895e765beabc987e562d Mon Sep 17 00:00:00 2001 From: Denis Anisimov Date: Wed, 7 Feb 2018 13:00:28 +0300 Subject: [PATCH 13/14] Correct GWT unit tests and small correction in the populate model code. --- .../client/ExecuteJavaScriptElementUtils.java | 4 +-- .../binding/SimpleElementBindingStrategy.java | 4 +-- .../GwtExecuteJavaScriptElementUtilsTest.java | 7 ++-- .../client/flow/GwtPolymerModelTest.java | 34 ++++++++++++------- 4 files changed, 28 insertions(+), 21 deletions(-) diff --git a/flow-client/src/main/java/com/vaadin/client/ExecuteJavaScriptElementUtils.java b/flow-client/src/main/java/com/vaadin/client/ExecuteJavaScriptElementUtils.java index 256fe88f778..91b50a5e9f4 100644 --- a/flow-client/src/main/java/com/vaadin/client/ExecuteJavaScriptElementUtils.java +++ b/flow-client/src/main/java/com/vaadin/client/ExecuteJavaScriptElementUtils.java @@ -196,7 +196,7 @@ private static Integer getExistingIdOrUpdate(StateNode parent, private static native boolean isPropertyDefined(Node node, String property) /*-{ return !!(node["constructor"] && node["constructor"]["properties"] && - node["constructor"]["properties"][property] && - node["constructor"]["properties"][property]["value"]); + node["constructor"]["properties"][property]) && (typeof( + node["constructor"]["properties"][property]["value"] ) != "undefined"); }-*/; } diff --git a/flow-client/src/main/java/com/vaadin/client/flow/binding/SimpleElementBindingStrategy.java b/flow-client/src/main/java/com/vaadin/client/flow/binding/SimpleElementBindingStrategy.java index 52cb8136d97..bdb85af0d1e 100644 --- a/flow-client/src/main/java/com/vaadin/client/flow/binding/SimpleElementBindingStrategy.java +++ b/flow-client/src/main/java/com/vaadin/client/flow/binding/SimpleElementBindingStrategy.java @@ -274,7 +274,7 @@ private native void hookUpPolymerElement(StateNode node, Element element) /*-{ this.@SimpleElementBindingStrategy::bindInitialModelProperties(*)(node, element); var self = this; - + var originalPropertiesChanged = element._propertiesChanged; if (originalPropertiesChanged) { element._propertiesChanged = function (currentProps, changedProps, oldProps) { @@ -284,7 +284,7 @@ private native void hookUpPolymerElement(StateNode node, Element element) originalPropertiesChanged.apply(this, arguments); }; } - + var originalReady = element.ready; element.ready = function (){ originalReady.apply(this, arguments); diff --git a/flow-client/src/test-gwt/java/com/vaadin/client/GwtExecuteJavaScriptElementUtilsTest.java b/flow-client/src/test-gwt/java/com/vaadin/client/GwtExecuteJavaScriptElementUtilsTest.java index 02a41f9cc04..8f13133f6b4 100644 --- a/flow-client/src/test-gwt/java/com/vaadin/client/GwtExecuteJavaScriptElementUtilsTest.java +++ b/flow-client/src/test-gwt/java/com/vaadin/client/GwtExecuteJavaScriptElementUtilsTest.java @@ -105,7 +105,6 @@ protected void gwtSetUp() throws Exception { node = new StateNode(0, tree); element = Browser.getDocument().createElement("div"); node.setDomNode(element); - } public void testAttachExistingElement_noSibling() { @@ -193,16 +192,16 @@ public void testPopulateModelProperties_elementIsNotReadyAndPropertyIsNotDefined mockWhenDefined(element); ExecuteJavaScriptElementUtils.populateModelProperties(node, - JsCollections.array("foo")); + JsCollections.array("bar")); NodeMap map = node.getMap(NodeFeatures.ELEMENT_PROPERTIES); - assertFalse(map.hasPropertyValue("foo")); + assertFalse(map.hasPropertyValue("bar")); node.setDomNode(element); runWhenDefined(element); Reactive.flush(); - assertTrue(map.hasPropertyValue("foo")); + assertTrue(map.hasPropertyValue("bar")); } public void testPopulateModelProperties_propertyIsDefined_syncToServer() { diff --git a/flow-client/src/test-gwt/java/com/vaadin/client/flow/GwtPolymerModelTest.java b/flow-client/src/test-gwt/java/com/vaadin/client/flow/GwtPolymerModelTest.java index 2248678c448..a463c648f79 100644 --- a/flow-client/src/test-gwt/java/com/vaadin/client/flow/GwtPolymerModelTest.java +++ b/flow-client/src/test-gwt/java/com/vaadin/client/flow/GwtPolymerModelTest.java @@ -253,7 +253,8 @@ public void testLatePolymerInit() { assertEquals( "Expected to have property with name " + propertyName + " defined after initial binding", - propertyValue, WidgetUtil.getJsProperty(element, propertyName)); + propertyValue, WidgetUtil.getJsProperty(element, + PROPERTY_PREFIX + propertyName)); String newPropertyValue = "bubblegum"; emulatePolymerPropertyChange(element, propertyName, newPropertyValue); @@ -261,8 +262,8 @@ public void testLatePolymerInit() { assertEquals( "Expected to have property with name " + propertyName + " updated from client side", - newPropertyValue, - WidgetUtil.getJsProperty(element, propertyName)); + newPropertyValue, WidgetUtil.getJsProperty(element, + PROPERTY_PREFIX + propertyName)); assertEquals("`_propertiesChanged` should be triggered exactly once", 1.0, WidgetUtil.getJsProperty(element, @@ -273,19 +274,21 @@ public void testLatePolymerInit() { } public void testInitialUpdateModelProperty_propertyIsUpdatable_propertyIsSynced() { + addMockMethods(element); String propertyName = "black"; String propertyValue = "coffee"; setModelProperty(node, propertyName, propertyValue); - node.setNodeData( - new UpdatableModelProperties(JsCollections.array("black"))); + node.setNodeData(new UpdatableModelProperties( + JsCollections.array(propertyName))); Binder.bind(node, element); Reactive.flush(); assertEquals( "Expected to have property with name " + propertyName + " defined after initial binding", - propertyValue, WidgetUtil.getJsProperty(element, propertyName)); + propertyValue, WidgetUtil.getJsProperty(element, + PROPERTY_PREFIX + propertyName)); String newPropertyValue = "bubblegum"; emulatePolymerPropertyChange(element, propertyName, newPropertyValue); @@ -293,11 +296,12 @@ public void testInitialUpdateModelProperty_propertyIsUpdatable_propertyIsSynced( assertEquals( "Expected to have property with name " + propertyName + " updated from client side", - newPropertyValue, - WidgetUtil.getJsProperty(element, propertyName)); + newPropertyValue, WidgetUtil.getJsProperty(element, + PROPERTY_PREFIX + propertyName)); } public void testInitialUpdateModelProperty_propertyIsUpdatableAndSchedulerIsNotExecuted_propertyIsNotSync() { + addMockMethods(element); String propertyName = "black"; String propertyValue = "coffee"; setModelProperty(node, propertyName, propertyValue); @@ -316,7 +320,8 @@ public void scheduleDeferred(ScheduledCommand cmd) { assertEquals( "Expected to have property with name " + propertyName + " defined after initial binding", - propertyValue, WidgetUtil.getJsProperty(element, propertyName)); + propertyValue, WidgetUtil.getJsProperty(element, + PROPERTY_PREFIX + propertyName)); String newPropertyValue = "bubblegum"; emulatePolymerPropertyChange(element, propertyName, newPropertyValue); @@ -324,7 +329,8 @@ public void scheduleDeferred(ScheduledCommand cmd) { assertEquals( "Expected to have property with name " + propertyName + " updated from client side", - propertyValue, WidgetUtil.getJsProperty(element, propertyName)); + propertyValue, WidgetUtil.getJsProperty(element, + PROPERTY_PREFIX + propertyName)); } public void testUpdateModelProperty_propertyIsNotUpdatable_propertyIsNotSync() { @@ -341,14 +347,16 @@ public void testUpdateModelProperty_propertyIsNotUpdatable_propertyIsNotSync() { assertEquals( "Expected to have property with name " + propertyName + " defined after initial binding", - propertyValue, WidgetUtil.getJsProperty(element, propertyName)); + propertyValue, WidgetUtil.getJsProperty(element, + PROPERTY_PREFIX + propertyName)); emulatePolymerPropertyChange(element, propertyName, "doesNotMatter"); Reactive.flush(); assertEquals( "Expected the property with name " + propertyName + " not to be updated since it's not updatable", - propertyValue, WidgetUtil.getJsProperty(element, propertyName)); + propertyValue, WidgetUtil.getJsProperty(element, + PROPERTY_PREFIX + propertyName)); } //////////////////////// @@ -358,7 +366,7 @@ private native void addMockMethods(Element element) element._propertiesChanged = function() { element.propertiesChangedCallCount += 1; }; - + element.callbackCallCount = 0; $wnd.customElements = { whenDefined: function() { From c6a652910eae5ced71ef36aa516fe5d54ab6366b Mon Sep 17 00:00:00 2001 From: Denis Anisimov Date: Wed, 7 Feb 2018 13:43:15 +0300 Subject: [PATCH 14/14] Change IT test --- .../com/vaadin/client/ExecuteJavaScriptElementUtils.java | 4 ++-- .../flow/uitest/ui/template/BeanInListingView.java | 9 ++++----- .../vaadin/flow/uitest/ui/template/BeanInListing.html | 2 +- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/flow-client/src/main/java/com/vaadin/client/ExecuteJavaScriptElementUtils.java b/flow-client/src/main/java/com/vaadin/client/ExecuteJavaScriptElementUtils.java index 91b50a5e9f4..bb1758c3a1d 100644 --- a/flow-client/src/main/java/com/vaadin/client/ExecuteJavaScriptElementUtils.java +++ b/flow-client/src/main/java/com/vaadin/client/ExecuteJavaScriptElementUtils.java @@ -196,7 +196,7 @@ private static Integer getExistingIdOrUpdate(StateNode parent, private static native boolean isPropertyDefined(Node node, String property) /*-{ return !!(node["constructor"] && node["constructor"]["properties"] && - node["constructor"]["properties"][property]) && (typeof( - node["constructor"]["properties"][property]["value"] ) != "undefined"); + node["constructor"]["properties"][property]) && + (typeof(node["constructor"]["properties"][property]["value"]) != "undefined"); }-*/; } diff --git a/flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/template/BeanInListingView.java b/flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/template/BeanInListingView.java index 36feb675e58..d2d4bbdd33c 100644 --- a/flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/template/BeanInListingView.java +++ b/flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/template/BeanInListingView.java @@ -23,7 +23,6 @@ import com.vaadin.flow.component.polymertemplate.PolymerTemplate; import com.vaadin.flow.router.Route; import com.vaadin.flow.templatemodel.AllowClientUpdates; -import com.vaadin.flow.templatemodel.ClientUpdateMode; import com.vaadin.flow.templatemodel.TemplateModel; import com.vaadin.flow.uitest.servlet.ViewTestLayout; import com.vaadin.flow.uitest.ui.template.BeanInListingView.ListModel; @@ -37,10 +36,10 @@ public interface ListModel extends TemplateModel { void setUsers(List users); - @AllowClientUpdates(ClientUpdateMode.ALLOW) + @AllowClientUpdates List getUsers(); - @AllowClientUpdates(ClientUpdateMode.ALLOW) + @AllowClientUpdates User getActiveUser(); void setActiveUser(User user); @@ -49,14 +48,14 @@ public interface ListModel extends TemplateModel { void setMessages(List messages); - @AllowClientUpdates(ClientUpdateMode.ALLOW) + @AllowClientUpdates String getActiveMessage(); } public static class User { private String name; - @AllowClientUpdates(ClientUpdateMode.ALLOW) + @AllowClientUpdates public String getName() { return name; } diff --git a/flow-tests/test-root-context/src/main/webapp/frontend/com/vaadin/flow/uitest/ui/template/BeanInListing.html b/flow-tests/test-root-context/src/main/webapp/frontend/com/vaadin/flow/uitest/ui/template/BeanInListing.html index dc11d0db905..7285a961fc7 100644 --- a/flow-tests/test-root-context/src/main/webapp/frontend/com/vaadin/flow/uitest/ui/template/BeanInListing.html +++ b/flow-tests/test-root-context/src/main/webapp/frontend/com/vaadin/flow/uitest/ui/template/BeanInListing.html @@ -42,7 +42,7 @@ }, activeMessage: { type: String, - value: null, + value: '', notify: true } };