From 24b43d902c71ea469e2105f3a1c8be2f84610f04 Mon Sep 17 00:00:00 2001 From: Denis Anisimov Date: Mon, 19 Sep 2016 16:26:32 +0300 Subject: Data should be updated when it's set for disabled components. Fixes vaadin/framework8-issues#286 Change-Id: I0d6cf49addfd558d43671ad2953dee54529392cd --- .../client/data/AbstractRemoteDataSource.java | 11 +++-- .../server/communication/ServerRpcHandler.java | 14 +++++- .../com/vaadin/server/data/DataCommunicator.java | 53 +++++++++++++++++++++- .../checkboxgroup/DisabledCheckBoxGroup.java | 36 +++++++++++++++ .../radiobuttongroup/DisabledRadioButtonGroup.java | 36 +++++++++++++++ .../checkboxgroup/CheckBoxGroupTest.java | 23 ++++++++++ .../checkboxgroup/DisabledCheckBoxGroupTest.java | 43 ++++++++++++++++++ .../components/grid/InitiallyDisabledGridTest.java | 7 --- .../DisabledRadioButtonGroupTest.java | 43 ++++++++++++++++++ .../radiobuttongroup/RadioButtonGroupTest.java | 23 ++++++++++ .../java/com/vaadin/tests/data/DummyDataTest.java | 22 ++++++++- 11 files changed, 296 insertions(+), 15 deletions(-) create mode 100644 uitest/src/main/java/com/vaadin/tests/components/checkboxgroup/DisabledCheckBoxGroup.java create mode 100644 uitest/src/main/java/com/vaadin/tests/components/radiobuttongroup/DisabledRadioButtonGroup.java create mode 100644 uitest/src/test/java/com/vaadin/tests/components/checkboxgroup/DisabledCheckBoxGroupTest.java create mode 100644 uitest/src/test/java/com/vaadin/tests/components/radiobuttongroup/DisabledRadioButtonGroupTest.java diff --git a/client/src/main/java/com/vaadin/client/data/AbstractRemoteDataSource.java b/client/src/main/java/com/vaadin/client/data/AbstractRemoteDataSource.java index 9dd1a504ab..4ca363ea3c 100644 --- a/client/src/main/java/com/vaadin/client/data/AbstractRemoteDataSource.java +++ b/client/src/main/java/com/vaadin/client/data/AbstractRemoteDataSource.java @@ -320,6 +320,7 @@ public abstract class AbstractRemoteDataSource implements DataSource { * @return true if waiting for data; otherwise * false */ + @Override public boolean isWaitingForData() { return currentRequestCallback != null; } @@ -462,7 +463,7 @@ public abstract class AbstractRemoteDataSource implements DataSource { currentRequestCallback = null; } - Range maxCacheRange = getMaxCacheRange(); + Range maxCacheRange = getMaxCacheRange(received); Range[] partition = received.partitionWith(maxCacheRange); @@ -698,9 +699,13 @@ public abstract class AbstractRemoteDataSource implements DataSource { } private Range getMaxCacheRange() { + return getMaxCacheRange(getRequestedAvailability()); + } + + private Range getMaxCacheRange(Range range) { Range availableDataRange = getAvailableRangeForCache(); - Range maxCacheRange = cacheStrategy.getMaxCacheRange( - requestedAvailability, cached, availableDataRange); + Range maxCacheRange = cacheStrategy.getMaxCacheRange(range, cached, + availableDataRange); assert maxCacheRange.isSubsetOf(availableDataRange); diff --git a/server/src/main/java/com/vaadin/server/communication/ServerRpcHandler.java b/server/src/main/java/com/vaadin/server/communication/ServerRpcHandler.java index 5e5fd8a81e..134fd569ff 100644 --- a/server/src/main/java/com/vaadin/server/communication/ServerRpcHandler.java +++ b/server/src/main/java/com/vaadin/server/communication/ServerRpcHandler.java @@ -46,6 +46,7 @@ import com.vaadin.shared.communication.LegacyChangeVariablesInvocation; import com.vaadin.shared.communication.MethodInvocation; import com.vaadin.shared.communication.ServerRpc; import com.vaadin.shared.communication.UidlValue; +import com.vaadin.shared.data.DataRequestRpc; import com.vaadin.ui.Component; import com.vaadin.ui.ConnectorTracker; import com.vaadin.ui.UI; @@ -354,9 +355,9 @@ public class ServerRpcHandler implements Serializable { if (connector == null) { getLogger().log(Level.WARNING, "Received RPC call for unknown connector with id {0} (tried to invoke {1}.{2})", - new Object[]{invocation.getConnectorId(), + new Object[] { invocation.getConnectorId(), invocation.getInterfaceName(), - invocation.getMethodName()}); + invocation.getMethodName() }); continue; } @@ -377,6 +378,15 @@ public class ServerRpcHandler implements Serializable { // Silently ignore this continue; } + } else if (invocation instanceof ServerRpcMethodInvocation) { + ServerRpcMethodInvocation rpc = (ServerRpcMethodInvocation) invocation; + // special case for data communicator requesting more + // data + if (DataRequestRpc.class.getName() + .equals(rpc.getInterfaceClass().getName())) { + handleInvocation(ui, connector, rpc); + } + continue; } // Connector is disabled, log a warning and move to the next diff --git a/server/src/main/java/com/vaadin/server/data/DataCommunicator.java b/server/src/main/java/com/vaadin/server/data/DataCommunicator.java index 4709230d82..fbafff6845 100644 --- a/server/src/main/java/com/vaadin/server/data/DataCommunicator.java +++ b/server/src/main/java/com/vaadin/server/data/DataCommunicator.java @@ -184,7 +184,8 @@ public class DataCommunicator extends AbstractExtension { private boolean reset = false; private final Set updatedData = new HashSet<>(); - private Range pushRows = Range.withLength(0, 40); + private int minPushSize = 40; + private Range pushRows = Range.withLength(0, minPushSize); private Comparator inMemorySorting; private SerializablePredicate inMemoryFilter; @@ -473,12 +474,62 @@ public class DataCommunicator extends AbstractExtension { Objects.requireNonNull(dataProvider, "data provider cannot be null"); this.dataProvider = dataProvider; detachDataProviderListener(); + /* + * This introduces behavior which influence on the client-server + * communication: now the very first response to the client will always + * contain some data. If data provider has been set already then {@code + * pushRows} is empty at this point. So without the next line the very + * first response will be without data. And the client will request more + * data in the next request after the response. The next line allows to + * send some data (in the {@code pushRows} range) to the client even in + * the very first response. This is necessary for disabled component + * (and theoretically allows to the client doesn't request more data in + * a happy path). + */ + pushRows = Range.between(0, getMinPushSize()); if (isAttached()) { attachDataProviderListener(); } reset(); } + /** + * Set minimum size of data which will be sent to the client when data + * source is set. + *

+ * Server doesn't send all data from data source to the client. It sends + * some initial chunk of data (whose size is determined as minimum between + * {@code size} parameter of this method and data size). Client decides + * whether it is able to show more data and request server to send more data + * (next chunk). + *

+ * When component is disabled then client cannot communicate to the server + * side (by design, because of security reasons). It means that client will + * get only initial chunk of data whose size is set here. + * + * @param size + * the size of initial data to send to the client + */ + public void setMinPushSize(int size) { + if (size < 0) { + throw new IllegalArgumentException("Value cannot be negative"); + } + minPushSize = size; + } + + /** + * Get minimum size of data which will be sent to the client when data + * source is set. + * + * @see #setMinPushSize(int) + * + * @return current minimum push size of initial data chunk which is sent to + * the client when data source is set + */ + public int getMinPushSize() { + return minPushSize; + } + private void attachDataProviderListener() { dataProviderUpdateRegistration = getDataProvider() .addDataProviderListener( diff --git a/uitest/src/main/java/com/vaadin/tests/components/checkboxgroup/DisabledCheckBoxGroup.java b/uitest/src/main/java/com/vaadin/tests/components/checkboxgroup/DisabledCheckBoxGroup.java new file mode 100644 index 0000000000..6af172fa38 --- /dev/null +++ b/uitest/src/main/java/com/vaadin/tests/components/checkboxgroup/DisabledCheckBoxGroup.java @@ -0,0 +1,36 @@ +/* + * 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.checkboxgroup; + +import com.vaadin.server.VaadinRequest; +import com.vaadin.tests.components.AbstractTestUI; +import com.vaadin.ui.CheckBoxGroup; + +/** + * @author Vaadin Ltd + * + */ +public class DisabledCheckBoxGroup extends AbstractTestUI { + + @Override + protected void setup(VaadinRequest request) { + CheckBoxGroup group = new CheckBoxGroup<>(); + group.setEnabled(false); + group.setItems("a", "b", "c"); + addComponent(group); + } + +} diff --git a/uitest/src/main/java/com/vaadin/tests/components/radiobuttongroup/DisabledRadioButtonGroup.java b/uitest/src/main/java/com/vaadin/tests/components/radiobuttongroup/DisabledRadioButtonGroup.java new file mode 100644 index 0000000000..255fb074d9 --- /dev/null +++ b/uitest/src/main/java/com/vaadin/tests/components/radiobuttongroup/DisabledRadioButtonGroup.java @@ -0,0 +1,36 @@ +/* + * 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.radiobuttongroup; + +import com.vaadin.server.VaadinRequest; +import com.vaadin.tests.components.AbstractTestUI; +import com.vaadin.ui.RadioButtonGroup; + +/** + * @author Vaadin Ltd + * + */ +public class DisabledRadioButtonGroup extends AbstractTestUI { + + @Override + protected void setup(VaadinRequest request) { + RadioButtonGroup group = new RadioButtonGroup<>(); + group.setEnabled(false); + group.setItems("a", "b", "c"); + addComponent(group); + } + +} diff --git a/uitest/src/test/java/com/vaadin/tests/components/checkboxgroup/CheckBoxGroupTest.java b/uitest/src/test/java/com/vaadin/tests/components/checkboxgroup/CheckBoxGroupTest.java index 65c12e2687..db630511f1 100644 --- a/uitest/src/test/java/com/vaadin/tests/components/checkboxgroup/CheckBoxGroupTest.java +++ b/uitest/src/test/java/com/vaadin/tests/components/checkboxgroup/CheckBoxGroupTest.java @@ -59,12 +59,35 @@ public class CheckBoxGroupTest extends MultiBrowserTest { assertItems(5); } + @Test + public void disabled_reduceItemCount_containsCorrectItems() { + selectMenuPath("Component", "State", "Enabled"); + selectMenuPath("Component", "Data provider", "Items", "5"); + assertItems(5); + } + @Test public void initialItems_increaseItemCount_containsCorrectItems() { selectMenuPath("Component", "Data provider", "Items", "100"); assertItems(100); } + @Test + public void disabled_increaseItemCountWithinPushRows_containsCorrectItems() { + selectMenuPath("Component", "Data provider", "Items", "5"); + selectMenuPath("Component", "State", "Enabled"); + selectMenuPath("Component", "Data provider", "Items", "20"); + assertItems(20); + } + + @Test + public void disabled_increaseItemCountBeyondPushRows_containsCorrectItems() { + selectMenuPath("Component", "Data provider", "Items", "5"); + selectMenuPath("Component", "State", "Enabled"); + selectMenuPath("Component", "Data provider", "Items", "100"); + assertItems(100); + } + @Test public void clickToSelect() { selectMenuPath("Component", "Listeners", "Selection listener"); diff --git a/uitest/src/test/java/com/vaadin/tests/components/checkboxgroup/DisabledCheckBoxGroupTest.java b/uitest/src/test/java/com/vaadin/tests/components/checkboxgroup/DisabledCheckBoxGroupTest.java new file mode 100644 index 0000000000..00d6d81cca --- /dev/null +++ b/uitest/src/test/java/com/vaadin/tests/components/checkboxgroup/DisabledCheckBoxGroupTest.java @@ -0,0 +1,43 @@ +/* + * 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.checkboxgroup; + +import java.util.List; + +import org.junit.Assert; +import org.junit.Test; + +import com.vaadin.testbench.customelements.CheckBoxGroupElement; +import com.vaadin.tests.tb3.MultiBrowserTest; + +/** + * @author Vaadin Ltd + * + */ +public class DisabledCheckBoxGroupTest extends MultiBrowserTest { + + @Test + public void initialDataInDisabledCheckBoxGroup() { + openTestURL(); + List options = $(CheckBoxGroupElement.class).first() + .getOptions(); + Assert.assertEquals(3, options.size()); + Assert.assertEquals("a", options.get(0)); + Assert.assertEquals("b", options.get(1)); + Assert.assertEquals("c", options.get(2)); + } + +} diff --git a/uitest/src/test/java/com/vaadin/tests/components/grid/InitiallyDisabledGridTest.java b/uitest/src/test/java/com/vaadin/tests/components/grid/InitiallyDisabledGridTest.java index e57b8d3efe..8e15183435 100644 --- a/uitest/src/test/java/com/vaadin/tests/components/grid/InitiallyDisabledGridTest.java +++ b/uitest/src/test/java/com/vaadin/tests/components/grid/InitiallyDisabledGridTest.java @@ -3,7 +3,6 @@ package com.vaadin.tests.components.grid; import java.util.List; import org.junit.Assert; -import org.junit.Ignore; import org.junit.Test; import org.openqa.selenium.WebElement; @@ -27,12 +26,6 @@ public class InitiallyDisabledGridTest extends SingleBrowserTest { } @Test - @Ignore - /* - * The test fails at the moment because of issues related (or exactly the - * same) as https://github.com/vaadin/framework8-issues/issues/286. It - * should be enabled once it's fixed. - */ public void worksWhenEnabled() { openTestURL(); $(ButtonElement.class).first().click(); diff --git a/uitest/src/test/java/com/vaadin/tests/components/radiobuttongroup/DisabledRadioButtonGroupTest.java b/uitest/src/test/java/com/vaadin/tests/components/radiobuttongroup/DisabledRadioButtonGroupTest.java new file mode 100644 index 0000000000..b54737dd72 --- /dev/null +++ b/uitest/src/test/java/com/vaadin/tests/components/radiobuttongroup/DisabledRadioButtonGroupTest.java @@ -0,0 +1,43 @@ +/* + * 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.radiobuttongroup; + +import java.util.List; + +import org.junit.Assert; +import org.junit.Test; + +import com.vaadin.testbench.customelements.RadioButtonGroupElement; +import com.vaadin.tests.tb3.MultiBrowserTest; + +/** + * @author Vaadin Ltd + * + */ +public class DisabledRadioButtonGroupTest extends MultiBrowserTest { + + @Test + public void initialDataInDisabledCheckBoxGroup() { + openTestURL(); + List options = $(RadioButtonGroupElement.class).first() + .getOptions(); + Assert.assertEquals(3, options.size()); + Assert.assertEquals("a", options.get(0)); + Assert.assertEquals("b", options.get(1)); + Assert.assertEquals("c", options.get(2)); + } + +} diff --git a/uitest/src/test/java/com/vaadin/tests/components/radiobuttongroup/RadioButtonGroupTest.java b/uitest/src/test/java/com/vaadin/tests/components/radiobuttongroup/RadioButtonGroupTest.java index 0b204aaaeb..c0e0b9cd63 100644 --- a/uitest/src/test/java/com/vaadin/tests/components/radiobuttongroup/RadioButtonGroupTest.java +++ b/uitest/src/test/java/com/vaadin/tests/components/radiobuttongroup/RadioButtonGroupTest.java @@ -56,12 +56,35 @@ public class RadioButtonGroupTest extends MultiBrowserTest { assertItems(5); } + @Test + public void disabled_reduceItemCount_containsCorrectItems() { + selectMenuPath("Component", "State", "Enabled"); + selectMenuPath("Component", "Data provider", "Items", "5"); + assertItems(5); + } + @Test public void initialItems_increaseItemCount_containsCorrectItems() { selectMenuPath("Component", "Data provider", "Items", "100"); assertItems(100); } + @Test + public void disabled_increaseItemCountWithinPushRows_containsCorrectItems() { + selectMenuPath("Component", "Data provider", "Items", "5"); + selectMenuPath("Component", "State", "Enabled"); + selectMenuPath("Component", "Data provider", "Items", "20"); + assertItems(20); + } + + @Test + public void disabled_increaseItemCountBeyondPushRows_containsCorrectItems() { + selectMenuPath("Component", "Data provider", "Items", "5"); + selectMenuPath("Component", "State", "Enabled"); + selectMenuPath("Component", "Data provider", "Items", "100"); + assertItems(100); + } + @Test public void clickToSelect() { selectMenuPath("Component", "Listeners", "Selection listener"); diff --git a/uitest/src/test/java/com/vaadin/tests/data/DummyDataTest.java b/uitest/src/test/java/com/vaadin/tests/data/DummyDataTest.java index b05c64e713..e6eabd8967 100644 --- a/uitest/src/test/java/com/vaadin/tests/data/DummyDataTest.java +++ b/uitest/src/test/java/com/vaadin/tests/data/DummyDataTest.java @@ -60,8 +60,26 @@ public class DummyDataTest extends SingleBrowserTest { public void testDataProviderChangeOnlyOneRequest() { // Change to a new logging data provider $(ButtonElement.class).get(1).click(); - assertEquals("DataProvider change should only cause 1 request", - "3. Backend request #0", getLogRow(0)); + /* + * There are two requests between the server and the client. + * + * But current implementation sends some data in both requests: + * + * - the first roundtrip contains data for initial range (normally + * 0..40) + * + * - the second roundtrip initiated by the client sends remaining data ( + * from 41 to the whole size()) + * + * This differs from the previous behavior: when data provider is + * updated (it doesn't apply for the initially set data provider) no + * data is sent to the client. So this first roundtrip is useless. And + * only the second rountrip is used to send the whole data. + */ + assertEquals("DataProvider change should cause 2 requests", + "3. Backend request #0", getLogRow(1)); + assertEquals("DataProvider change should cause 2 request", + "4. Backend request #1", getLogRow(0)); } @Test -- cgit v1.2.3