Move VaadinSession.runPendingAccessTasks to VaadinService (#11964)

Change-Id: Idb893baec693d0aaa3ccba1d3f61a62922e0a1ce

5 files changed, 137 insertions, 65 deletions

diff --git a/server/src/com/vaadin/server/VaadinService.java b/server/src/com/vaadin/server/VaadinService.java
index f9b17537df..a040c72175 100644
--- a/server/src/com/vaadin/server/VaadinService.java
+++ b/server/src/com/vaadin/server/VaadinService.java
@@ -33,6 +33,9 @@ import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Locale;
+import java.util.Map;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
 import java.util.concurrent.locks.Lock;
 import java.util.concurrent.locks.ReentrantLock;
 import java.util.logging.Level;
@@ -47,6 +50,7 @@ import org.json.JSONObject;
 
 import com.vaadin.annotations.PreserveOnRefresh;
 import com.vaadin.event.EventRouter;
+import com.vaadin.server.VaadinSession.FutureAccess;
 import com.vaadin.server.communication.FileUploadHandler;
 import com.vaadin.server.communication.HeartbeatHandler;
 import com.vaadin.server.communication.PublishedFileHandler;
@@ -1609,4 +1613,88 @@ public abstract class VaadinService implements Serializable {
         return true;
     }
 
+    /**
+     * Implementation for {@link VaadinSession#access(Runnable)}. This method is
+     * implemented here instead of in {@link VaadinSession} to enable overriding
+     * the implementation without using a custom subclass of VaadinSession.
+     * 
+     * @since 7.1
+     * @see VaadinSession#access(Runnable)
+     * 
+     * @param session
+     *            the vaadin session to access
+     * @param runnable
+     *            the runnable to run with the session locked
+     * 
+     * @return a future that can be used to check for task completion and to
+     *         cancel the task
+     */
+    public Future<Void> accessSession(VaadinSession session, Runnable runnable) {
+        FutureAccess future = new FutureAccess(session, runnable);
+        session.getPendingAccessQueue().add(future);
+
+        /*
+         * If no thread is currently holding the lock, pending changes for UIs
+         * with automatic push would not be processed and pushed until the next
+         * time there is a request or someone does an explicit push call.
+         * 
+         * To remedy this, we try to get the lock at this point. If the lock is
+         * currently held by another thread, we just back out as the queue will
+         * get purged once it is released. If the lock is held by the current
+         * thread, we just release it knowing that the queue gets purged once
+         * the lock is ultimately released. If the lock is not held by any
+         * thread and we acquire it, we just release it again to purge the queue
+         * right away.
+         */
+        try {
+            // tryLock() would be shorter, but it does not guarantee fairness
+            if (session.getLockInstance().tryLock(0, TimeUnit.SECONDS)) {
+                // unlock triggers runPendingAccessTasks
+                session.unlock();
+            }
+        } catch (InterruptedException e) {
+            // Just ignore
+        }
+
+        return future;
+    }
+
+    /**
+     * Purges the queue of pending access invocations enqueued with
+     * {@link VaadinSession#access(Runnable)}.
+     * <p>
+     * This method is automatically run by the framework at appropriate
+     * situations and is not intended to be used by application developers.
+     * 
+     * @param session
+     *            the vaadin session to purge the queue for
+     * @since 7.1
+     */
+    public void runPendingAccessTasks(VaadinSession session) {
+        assert session.hasLock();
+
+        if (session.getPendingAccessQueue().isEmpty()) {
+            return;
+        }
+
+        Map<Class<?>, CurrentInstance> oldInstances = CurrentInstance
+                .getInstances(false);
+
+        FutureAccess pendingAccess;
+        try {
+            while ((pendingAccess = session.getPendingAccessQueue().poll()) != null) {
+                if (!pendingAccess.isCancelled()) {
+                    CurrentInstance.clearAll();
+                    CurrentInstance.restoreInstances(pendingAccess
+                            .getCurrentInstances());
+                    CurrentInstance.setCurrent(session);
+                    pendingAccess.run();
+                }
+            }
+        } finally {
+            CurrentInstance.clearAll();
+            CurrentInstance.restoreInstances(oldInstances);
+        }
+    }
+
 }
diff --git a/server/src/com/vaadin/server/VaadinSession.java b/server/src/com/vaadin/server/VaadinSession.java
index 9ef3d33195..17b696b17a 100644
--- a/server/src/com/vaadin/server/VaadinSession.java
+++ b/server/src/com/vaadin/server/VaadinSession.java
@@ -25,12 +25,12 @@ import java.util.LinkedList;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
+import java.util.Queue;
 import java.util.UUID;
 import java.util.concurrent.ConcurrentLinkedQueue;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Future;
 import java.util.concurrent.FutureTask;
-import java.util.concurrent.TimeUnit;
 import java.util.concurrent.locks.Lock;
 import java.util.concurrent.locks.ReentrantLock;
 import java.util.logging.Logger;
@@ -67,16 +67,37 @@ import com.vaadin.util.ReflectTools;
 @SuppressWarnings("serial")
 public class VaadinSession implements HttpSessionBindingListener, Serializable {
 
-    private class FutureAccess extends FutureTask<Void> {
+    /**
+     * Encapsulates a {@link Runnable} submitted using
+     * {@link VaadinSession#access(Runnable)}. This class is used internally by
+     * the framework and is not intended to be directly used by application
+     * developers.
+     * 
+     * @since 7.1
+     * @author Vaadin Ltd
+     */
+    public static class FutureAccess extends FutureTask<Void> {
         /**
          * Snapshot of all non-inheritable current instances at the time this
          * object was created.
          */
         private final Map<Class<?>, CurrentInstance> instances = CurrentInstance
                 .getInstances(true);
+        private final VaadinSession session;
 
-        public FutureAccess(Runnable arg0) {
-            super(arg0, null);
+        /**
+         * Creates an instance for the given runnable
+         * 
+         * @param session
+         *            the session to which the task belongs
+         * 
+         * @param runnable
+         *            the runnable to run when this task is purged from the
+         *            queue
+         */
+        public FutureAccess(VaadinSession session, Runnable runnable) {
+            super(runnable, null);
+            this.session = session;
         }
 
         @Override
@@ -91,9 +112,21 @@ public class VaadinSession implements HttpSessionBindingListener, Serializable {
              * run, the check is always done as the deterministic behavior makes
              * it easier to detect potential problems.
              */
-            VaadinService.verifyNoOtherSessionLocked(VaadinSession.this);
+            VaadinService.verifyNoOtherSessionLocked(session);
             return super.get();
         }
+
+        /**
+         * Gets the current instance values that should be used when running
+         * this task.
+         * 
+         * @see CurrentInstance#restoreInstances(Map)
+         * 
+         * @return a map of current instances.
+         */
+        public Map<Class<?>, CurrentInstance> getCurrentInstances() {
+            return instances;
+        }
     }
 
     /**
@@ -866,7 +899,7 @@ public class VaadinSession implements HttpSessionBindingListener, Serializable {
              * released by this unlock() invocation.
              */
             if (((ReentrantLock) getLockInstance()).getHoldCount() == 1) {
-                runPendingAccessTasks();
+                getService().runPendingAccessTasks(this);
 
                 for (UI ui : getUIs()) {
                     if (ui.getPushMode() == PushMode.AUTOMATIC) {
@@ -1205,68 +1238,18 @@ public class VaadinSession implements HttpSessionBindingListener, Serializable {
      *         cancel the task
      */
     public Future<Void> access(Runnable runnable) {
-        FutureAccess future = new FutureAccess(runnable);
-        pendingAccessQueue.add(future);
-
-        /*
-         * If no thread is currently holding the lock, pending changes for UIs
-         * with automatic push would not be processed and pushed until the next
-         * time there is a request or someone does an explicit push call.
-         * 
-         * To remedy this, we try to get the lock at this point. If the lock is
-         * currently held by another thread, we just back out as the queue will
-         * get purged once it is released. If the lock is held by the current
-         * thread, we just release it knowing that the queue gets purged once
-         * the lock is ultimately released. If the lock is not held by any
-         * thread and we acquire it, we just release it again to purge the queue
-         * right away.
-         */
-        try {
-            // tryLock() would be shorter, but it does not guarantee fairness
-            if (getLockInstance().tryLock(0, TimeUnit.SECONDS)) {
-                // unlock triggers runPendingAccessTasks
-                unlock();
-            }
-        } catch (InterruptedException e) {
-            // Just ignore
-        }
-
-        return future;
+        return getService().accessSession(this, runnable);
     }
 
     /**
-     * Purges the queue of pending access invocations enqueued with
-     * {@link #access(Runnable)}.
-     * <p>
-     * This method is automatically run by the framework at appropriate
-     * situations and is not intended to be used by application developers.
+     * Gets the queue of tasks submitted using {@link #access(Runnable)}.
      * 
      * @since 7.1
+     * 
+     * @return the pending access queue
      */
-    public void runPendingAccessTasks() {
-        assert hasLock();
-
-        if (pendingAccessQueue.isEmpty()) {
-            return;
-        }
-
-        Map<Class<?>, CurrentInstance> oldInstances = CurrentInstance
-                .getInstances(false);
-
-        FutureAccess pendingAccess;
-        try {
-            while ((pendingAccess = pendingAccessQueue.poll()) != null) {
-                if (!pendingAccess.isCancelled()) {
-                    CurrentInstance.clearAll();
-                    CurrentInstance.restoreInstances(pendingAccess.instances);
-                    CurrentInstance.setCurrent(this);
-                    pendingAccess.run();
-                }
-            }
-        } finally {
-            CurrentInstance.clearAll();
-            CurrentInstance.restoreInstances(oldInstances);
-        }
+    public Queue<FutureAccess> getPendingAccessQueue() {
+        return pendingAccessQueue;
     }
 
     /**
diff --git a/server/src/com/vaadin/server/communication/UidlWriter.java b/server/src/com/vaadin/server/communication/UidlWriter.java
index f715569424..9c736d8dd9 100644
--- a/server/src/com/vaadin/server/communication/UidlWriter.java
+++ b/server/src/com/vaadin/server/communication/UidlWriter.java
@@ -78,7 +78,7 @@ public class UidlWriter implements Serializable {
 
         // Purge pending access calls as they might produce additional changes
         // to write out
-        session.runPendingAccessTasks();
+        session.getService().runPendingAccessTasks(session);
 
         ArrayList<ClientConnector> dirtyVisibleConnectors = ui
                 .getConnectorTracker().getDirtyVisibleConnectors();
diff --git a/server/src/com/vaadin/ui/UI.java b/server/src/com/vaadin/ui/UI.java
index d1d8dd1df1..d4ac156787 100644
--- a/server/src/com/vaadin/ui/UI.java
+++ b/server/src/com/vaadin/ui/UI.java
@@ -1274,7 +1274,7 @@ public abstract class UI extends AbstractSingleComponentContainer implements
              * dirty when the push would otherwise be ignored because there are
              * no changes to push.
              */
-            session.runPendingAccessTasks();
+            session.getService().runPendingAccessTasks(session);
 
             if (!getConnectorTracker().hasDirtyConnectors()) {
                 // Do not push if there is nothing to push
diff --git a/uitest/src/com/vaadin/tests/components/ui/UiAccess.java b/uitest/src/com/vaadin/tests/components/ui/UiAccess.java
index 297a985778..2bc91fa7b4 100644
--- a/uitest/src/com/vaadin/tests/components/ui/UiAccess.java
+++ b/uitest/src/com/vaadin/tests/components/ui/UiAccess.java
@@ -273,7 +273,8 @@ public class UiAccess extends AbstractTestUIWithLog {
                                 new CurrentInstanceTestType(
                                         "Set before run pending"));
 
-                        getSession().runPendingAccessTasks();
+                        getSession().getService().runPendingAccessTasks(
+                                getSession());
 
                         log.log("has request after access? "
                                 + (VaadinService.getCurrentRequest() != null));