Add @BackgroundMessage annotation (#15373)

Change-Id: Id5367b7b1ef4b7dbabfd58902ac6134222e641ba

12 files changed, 249 insertions, 12 deletions

diff --git a/client-compiler/src/com/vaadin/server/widgetsetutils/metadata/ServerRpcVisitor.java b/client-compiler/src/com/vaadin/server/widgetsetutils/metadata/ServerRpcVisitor.java
index a1852d5b32..7efdf9db81 100644
--- a/client-compiler/src/com/vaadin/server/widgetsetutils/metadata/ServerRpcVisitor.java
+++ b/client-compiler/src/com/vaadin/server/widgetsetutils/metadata/ServerRpcVisitor.java
@@ -24,6 +24,7 @@ import com.google.gwt.core.ext.typeinfo.JClassType;
 import com.google.gwt.core.ext.typeinfo.JMethod;
 import com.google.gwt.core.ext.typeinfo.JType;
 import com.vaadin.client.metadata.TypeDataStore.MethodAttribute;
+import com.vaadin.shared.annotations.BackgroundMessage;
 import com.vaadin.shared.annotations.Delayed;
 
 public class ServerRpcVisitor extends TypeVisitor {
@@ -51,6 +52,11 @@ public class ServerRpcVisitor extends TypeVisitor {
                         }
                     }
 
+                    if (method.getAnnotation(BackgroundMessage.class) != null) {
+                        bundle.setMethodAttribute(type, method,
+                                MethodAttribute.BACKGROUND_MESSAGE);
+                    }
+
                     bundle.setNeedsParamTypes(type, method);
 
                     JType[] parameterTypes = method.getParameterTypes();
diff --git a/client/src/com/vaadin/client/ApplicationConnection.java b/client/src/com/vaadin/client/ApplicationConnection.java
index aa00516feb..c00e5a8dae 100644
--- a/client/src/com/vaadin/client/ApplicationConnection.java
+++ b/client/src/com/vaadin/client/ApplicationConnection.java
@@ -81,6 +81,7 @@ import com.vaadin.client.metadata.NoDataException;
 import com.vaadin.client.metadata.Property;
 import com.vaadin.client.metadata.Type;
 import com.vaadin.client.metadata.TypeData;
+import com.vaadin.client.metadata.TypeDataStore;
 import com.vaadin.client.ui.AbstractComponentConnector;
 import com.vaadin.client.ui.AbstractConnector;
 import com.vaadin.client.ui.FontIcon;
@@ -1275,7 +1276,6 @@ public class ApplicationConnection implements HasHandlers {
         }
         hasActiveRequest = true;
         requestStartTime = new Date();
-        loadingIndicator.trigger();
         eventBus.fireEvent(new RequestStartingEvent(this));
     }
 
@@ -1300,7 +1300,8 @@ public class ApplicationConnection implements HasHandlers {
         Scheduler.get().scheduleDeferred(new Command() {
             @Override
             public void execute() {
-                if (!hasActiveRequest()) {
+                if (!isApplicationRunning()
+                        || !(hasActiveRequest() || deferredSendPending)) {
                     getLoadingIndicator().hide();
 
                     // If on Liferay and session expiration management is in
@@ -2720,8 +2721,8 @@ public class ApplicationConnection implements HasHandlers {
      *
      */
     public void sendPendingVariableChanges() {
-        if (!deferedSendPending) {
-            deferedSendPending = true;
+        if (!deferredSendPending) {
+            deferredSendPending = true;
             Scheduler.get().scheduleFinally(sendPendingCommand);
         }
     }
@@ -2729,11 +2730,11 @@ public class ApplicationConnection implements HasHandlers {
     private final ScheduledCommand sendPendingCommand = new ScheduledCommand() {
         @Override
         public void execute() {
-            deferedSendPending = false;
+            deferredSendPending = false;
             doSendPendingVariableChanges();
         }
     };
-    private boolean deferedSendPending = false;
+    private boolean deferredSendPending = false;
 
     private void doSendPendingVariableChanges() {
         if (isApplicationRunning()) {
@@ -2768,7 +2769,7 @@ public class ApplicationConnection implements HasHandlers {
      */
     private void buildAndSendVariableBurst(
             LinkedHashMap<String, MethodInvocation> pendingInvocations) {
-
+        boolean showLoadingIndicator = false;
         JsonArray reqJson = Json.createArray();
         if (!pendingInvocations.isEmpty()) {
             if (ApplicationConfiguration.isDebugMode()) {
@@ -2791,10 +2792,16 @@ public class ApplicationConnection implements HasHandlers {
                         Method method = type.getMethod(invocation
                                 .getMethodName());
                         parameterTypes = method.getParameterTypes();
+
+                        showLoadingIndicator |= !TypeDataStore
+                                .isBackgroundMessage(method);
                     } catch (NoDataException e) {
                         throw new RuntimeException("No type data for "
                                 + invocation.toString(), e);
                     }
+                } else {
+                    // Always show loading indicator for legacy requests
+                    showLoadingIndicator = true;
                 }
 
                 for (int i = 0; i < invocation.getParameters().length; ++i) {
@@ -2826,6 +2833,9 @@ public class ApplicationConnection implements HasHandlers {
 
             getConfiguration().setWidgetsetVersionSent();
         }
+        if (showLoadingIndicator) {
+            getLoadingIndicator().trigger();
+        }
         makeUidlRequest(reqJson, extraParams);
     }
 
diff --git a/client/src/com/vaadin/client/VLoadingIndicator.java b/client/src/com/vaadin/client/VLoadingIndicator.java
index e873005d3a..6a6eaf71b2 100644
--- a/client/src/com/vaadin/client/VLoadingIndicator.java
+++ b/client/src/com/vaadin/client/VLoadingIndicator.java
@@ -154,6 +154,18 @@ public class VLoadingIndicator {
     }
 
     /**
+     * Triggers displaying of this loading indicator unless it's already visible
+     * or scheduled to be shown after a delay.
+     * 
+     * @since
+     */
+    public void ensureTriggered() {
+        if (!isVisible() && !firstTimer.isRunning()) {
+            trigger();
+        }
+    }
+
+    /**
      * Shows the loading indicator in its standard state and triggers timers for
      * transitioning into the "second" and "third" states.
      */
diff --git a/client/src/com/vaadin/client/connectors/RpcDataSourceConnector.java b/client/src/com/vaadin/client/connectors/RpcDataSourceConnector.java
index c46db08553..74c8dfb02f 100644
--- a/client/src/com/vaadin/client/connectors/RpcDataSourceConnector.java
+++ b/client/src/com/vaadin/client/connectors/RpcDataSourceConnector.java
@@ -104,6 +104,43 @@ public class RpcDataSourceConnector extends AbstractExtensionConnector {
 
             rpcProxy.requestRows(firstRowIndex, numberOfRows,
                     cached.getStart(), cached.length());
+
+            /*
+             * Show the progress indicator if there is a pending data request
+             * and some of the visible rows are being requested. The RPC in
+             * itself will not trigger the indicator since it might just fetch
+             * some rows in the background to fill the cache.
+             * 
+             * The indicator will be hidden by the framework when the response
+             * is received (unless another request is already on its way at that
+             * point).
+             */
+            if (getRequestedAvailability().intersects(
+                    Range.withLength(firstRowIndex, numberOfRows))) {
+                getConnection().getLoadingIndicator().ensureTriggered();
+            }
+        }
+
+        @Override
+        public void ensureAvailability(int firstRowIndex, int numberOfRows) {
+            super.ensureAvailability(firstRowIndex, numberOfRows);
+
+            /*
+             * We trigger the indicator already at this point since the actual
+             * RPC will not be sent right away when waiting for the response to
+             * a previous request.
+             * 
+             * Only triggering here would not be enough since the check that
+             * sets isWaitingForData is deferred. We don't want to trigger the
+             * loading indicator here if we don't know that there is actually a
+             * request going on since some other bug might then cause the
+             * loading indicator to not be hidden.
+             */
+            if (isWaitingForData()
+                    && !Range.withLength(firstRowIndex, numberOfRows)
+                            .isSubsetOf(getCachedRange())) {
+                getConnection().getLoadingIndicator().ensureTriggered();
+            }
         }
 
         @Override
diff --git a/client/src/com/vaadin/client/data/AbstractRemoteDataSource.java b/client/src/com/vaadin/client/data/AbstractRemoteDataSource.java
index ffd1d4d170..0ad1631e19 100644
--- a/client/src/com/vaadin/client/data/AbstractRemoteDataSource.java
+++ b/client/src/com/vaadin/client/data/AbstractRemoteDataSource.java
@@ -262,10 +262,19 @@ public abstract class AbstractRemoteDataSource<T> implements DataSource<T> {
         ensureCoverageCheck();
     }
 
+    /**
+     * Gets the row index range that was requested by the previous call to
+     * {@link #ensureAvailability(int, int)}.
+     * 
+     * @return the requested availability range
+     */
+    public Range getRequestedAvailability() {
+        return requestedAvailability;
+    }
+
     private void checkCacheCoverage() {
-        if (currentRequestCallback != null) {
-            // Anyone clearing currentRequestCallback should run this method
-            // again
+        if (isWaitingForData()) {
+            // Anyone clearing the waiting status should run this method again
             return;
         }
 
@@ -301,6 +310,17 @@ public abstract class AbstractRemoteDataSource<T> implements DataSource<T> {
         Profiler.leave("AbstractRemoteDataSource.checkCacheCoverage");
     }
 
+    /**
+     * Checks whether this data source is currently waiting for more rows to
+     * become available.
+     * 
+     * @return <code>true</code> if waiting for data; otherwise
+     *         <code>false</code>
+     */
+    public boolean isWaitingForData() {
+        return currentRequestCallback != null;
+    }
+
     private void discardStaleCacheEntries() {
         Range[] cacheParition = cached.partitionWith(getMaxCacheRange());
         dropFromCache(cacheParition[0]);
@@ -378,7 +398,7 @@ public abstract class AbstractRemoteDataSource<T> implements DataSource<T> {
 
         Range received = Range.withLength(firstRowIndex, rowData.size());
 
-        if (currentRequestCallback != null) {
+        if (isWaitingForData()) {
             cacheStrategy.onDataArrive(Duration.currentTimeMillis()
                     - currentRequestCallback.requestStart, received.length());
 
diff --git a/client/src/com/vaadin/client/metadata/TypeDataStore.java b/client/src/com/vaadin/client/metadata/TypeDataStore.java
index e3db0ccded..adbf24d411 100644
--- a/client/src/com/vaadin/client/metadata/TypeDataStore.java
+++ b/client/src/com/vaadin/client/metadata/TypeDataStore.java
@@ -29,7 +29,7 @@ import com.vaadin.shared.annotations.NoLayout;
 
 public class TypeDataStore {
     public static enum MethodAttribute {
-        DELAYED, LAST_ONLY, NO_LAYOUT;
+        DELAYED, LAST_ONLY, NO_LAYOUT, BACKGROUND_MESSAGE;
     }
 
     private static final String CONSTRUCTOR_NAME = "!new";
@@ -219,6 +219,10 @@ public class TypeDataStore {
         return hasMethodAttribute(method, MethodAttribute.DELAYED);
     }
 
+    public static boolean isBackgroundMessage(Method method) {
+        return hasMethodAttribute(method, MethodAttribute.BACKGROUND_MESSAGE);
+    }
+
     private static boolean hasMethodAttribute(Method method,
             MethodAttribute attribute) {
         FastStringSet attributes = get().methodAttributes.get(method
diff --git a/shared/src/com/vaadin/shared/annotations/BackgroundMessage.java b/shared/src/com/vaadin/shared/annotations/BackgroundMessage.java
new file mode 100644
index 0000000000..885c0de4d2
--- /dev/null
+++ b/shared/src/com/vaadin/shared/annotations/BackgroundMessage.java
@@ -0,0 +1,35 @@
+/*
+ * Copyright 2000-2014 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.shared.annotations;
+
+import java.lang.annotation.Documented;
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Target;
+
+/**
+ * Annotation used to mark server RPC methods that perform background tasks that
+ * are transparent to the user. The framework will show a loading indicator when
+ * sending requests for RPC methods that are not marked with this annotation.
+ * The loading indicator is hidden once a response is received.
+ * 
+ * @since
+ * @author Vaadin Ltd
+ */
+@Target(ElementType.METHOD)
+@Documented
+public @interface BackgroundMessage {
+    // Just an empty marker annotation
+}
diff --git a/shared/src/com/vaadin/shared/data/DataRequestRpc.java b/shared/src/com/vaadin/shared/data/DataRequestRpc.java
index 773a82fa9a..c2a13d470c 100644
--- a/shared/src/com/vaadin/shared/data/DataRequestRpc.java
+++ b/shared/src/com/vaadin/shared/data/DataRequestRpc.java
@@ -16,6 +16,7 @@
 
 package com.vaadin.shared.data;
 
+import com.vaadin.shared.annotations.BackgroundMessage;
 import com.vaadin.shared.annotations.Delayed;
 import com.vaadin.shared.communication.ServerRpc;
 
@@ -39,6 +40,7 @@ public interface DataRequestRpc extends ServerRpc {
      * @param cacheSize
      *            the number of cached rows
      */
+    @BackgroundMessage
     public void requestRows(int firstRowIndex, int numberOfRows,
             int firstCachedRowIndex, int cacheSize);
 
diff --git a/shared/src/com/vaadin/shared/ui/progressindicator/ProgressIndicatorServerRpc.java b/shared/src/com/vaadin/shared/ui/progressindicator/ProgressIndicatorServerRpc.java
index dd437094c7..54153005e3 100644
--- a/shared/src/com/vaadin/shared/ui/progressindicator/ProgressIndicatorServerRpc.java
+++ b/shared/src/com/vaadin/shared/ui/progressindicator/ProgressIndicatorServerRpc.java
@@ -15,8 +15,10 @@
  */
 package com.vaadin.shared.ui.progressindicator;
 
+import com.vaadin.shared.annotations.BackgroundMessage;
 import com.vaadin.shared.communication.ServerRpc;
 
 public interface ProgressIndicatorServerRpc extends ServerRpc {
+    @BackgroundMessage
     public void poll();
 }
diff --git a/shared/src/com/vaadin/shared/ui/ui/UIServerRpc.java b/shared/src/com/vaadin/shared/ui/ui/UIServerRpc.java
index 8227415e58..20e4dbdf4c 100644
--- a/shared/src/com/vaadin/shared/ui/ui/UIServerRpc.java
+++ b/shared/src/com/vaadin/shared/ui/ui/UIServerRpc.java
@@ -15,6 +15,7 @@
  */
 package com.vaadin.shared.ui.ui;
 
+import com.vaadin.shared.annotations.BackgroundMessage;
 import com.vaadin.shared.annotations.Delayed;
 import com.vaadin.shared.communication.ServerRpc;
 import com.vaadin.shared.ui.ClickRpc;
@@ -27,6 +28,7 @@ public interface UIServerRpc extends ClickRpc, ServerRpc {
     @Delayed(lastOnly = true)
     public void scroll(int scrollTop, int scrollLeft);
 
+    @BackgroundMessage
     @Delayed(lastOnly = true)
     /*
      * @Delayed just to get lastOnly semantics, sendPendingVariableChanges()
diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java
index 20b6a3c418..5912f2b5a5 100644
--- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java
+++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java
@@ -77,6 +77,8 @@ public class GridBasicFeatures extends AbstractComponentTest<Grid> {
     public static final int COLUMNS = 12;
     public static final int ROWS = 1000;
 
+    private int containerDelay = 0;
+
     private IndexedContainer ds;
     private Grid grid;
     private SelectionListener selectionListener = new SelectionListener() {
@@ -101,6 +103,13 @@ public class GridBasicFeatures extends AbstractComponentTest<Grid> {
             public List<Object> getItemIds(int startIndex, int numberOfIds) {
                 log("Requested items " + startIndex + " - "
                         + (startIndex + numberOfIds));
+                if (containerDelay > 0) {
+                    try {
+                        Thread.sleep(containerDelay);
+                    } catch (InterruptedException e) {
+                        e.printStackTrace();
+                    }
+                }
                 return super.getItemIds(startIndex, numberOfIds);
             }
         };
@@ -437,6 +446,19 @@ public class GridBasicFeatures extends AbstractComponentTest<Grid> {
                         c.setFrozenColumnCount(value.intValue());
                     }
                 });
+
+        LinkedHashMap<String, Integer> containerDelayValues = new LinkedHashMap<String, Integer>();
+        for (int delay : new int[] { 0, 500, 2000, 10000 }) {
+            containerDelayValues.put(String.valueOf(delay),
+                    Integer.valueOf(delay));
+        }
+        createSelectAction("Container delay", "State", containerDelayValues,
+                "0", new Command<Grid, Integer>() {
+                    @Override
+                    public void execute(Grid grid, Integer delay, Object data) {
+                        containerDelay = delay.intValue();
+                    }
+                });
     }
 
     protected void createHeaderActions() {
diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/LoadingIndicatorTest.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/LoadingIndicatorTest.java
new file mode 100644
index 0000000000..1f16efdd39
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/LoadingIndicatorTest.java
@@ -0,0 +1,85 @@
+/*
+ * Copyright 2000-2014 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.grid.basicfeatures.server;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.openqa.selenium.By;
+import org.openqa.selenium.WebElement;
+import org.openqa.selenium.support.ui.ExpectedConditions;
+
+import com.vaadin.testbench.elements.GridElement;
+import com.vaadin.tests.components.grid.basicfeatures.GridBasicFeaturesTest;
+
+public class LoadingIndicatorTest extends GridBasicFeaturesTest {
+    @Test
+    public void testLoadingIndicator() throws InterruptedException {
+        setDebug(true);
+        openTestURL();
+
+        selectMenuPath("Component", "State", "Container delay", "2000");
+
+        GridElement gridElement = $(GridElement.class).first();
+
+        Assert.assertFalse(
+                "Loading indicator should not be visible before disabling waitForVaadin",
+                isLoadingIndicatorVisible());
+
+        testBench().disableWaitForVaadin();
+
+        // Scroll to a completely new location
+        gridElement.getCell(200, 1);
+
+        // Wait for loading indicator delay
+        Thread.sleep(500);
+
+        Assert.assertTrue(
+                "Loading indicator should be visible when fetching rows that are visible",
+                isLoadingIndicatorVisible());
+
+        waitUntilNot(ExpectedConditions.visibilityOfElementLocated(By
+                .className("v-loading-indicator")));
+
+        // Scroll so much that more data gets fetched, but not so much that
+        // missing rows are shown
+        gridElement.getCell(230, 1);
+
+        // Wait for potentially triggered loading indicator to become visible
+        Thread.sleep(500);
+
+        Assert.assertFalse(
+                "Loading indicator should not be visible when fetching rows that are not visible",
+                isLoadingIndicatorVisible());
+
+        // Finally verify that there was actually a request going on
+        Thread.sleep(2000);
+
+        String firstLogRow = getLogRow(0);
+        Assert.assertTrue("Last log message was not the fourth message: "
+                + firstLogRow, firstLogRow.startsWith("4. Requested items"));
+    }
+
+    private boolean isLoadingIndicatorVisible() {
+        WebElement loadingIndicator = findElement(By
+                .className("v-loading-indicator"));
+        if (loadingIndicator == null) {
+            return false;
+        } else {
+            return loadingIndicator.isDisplayed();
+        }
+
+    }
+}