Move session store/load/remove logic to service from session (#9782, #18998)

The session storage logic when implemented in VaadinSession was and must be
partly static. This makes it impossible to override the storage logic.
To be able to support storage customizations for portals (storing using
APPLICATION_SCOPE), the logic was moved to VaadinService.

For portals, all storage operations now use APPLICATION_SCOPE. Previously
only the store operation was done using APPLICATION_SCOPE, leading to
some portals not being able to load the session at all (the default
scope for reading attributes in portals is PORTLET_SCOPE).

Change-Id: I3d112608d98c883a457e650c0a25bf10c81acc78

7 files changed, 177 insertions, 73 deletions

diff --git a/server/src/com/vaadin/server/GAEVaadinServlet.java b/server/src/com/vaadin/server/GAEVaadinServlet.java
index 6f5c15ebdd..2429fde746 100644
--- a/server/src/com/vaadin/server/GAEVaadinServlet.java
+++ b/server/src/com/vaadin/server/GAEVaadinServlet.java
@@ -349,9 +349,8 @@ public class GAEVaadinServlet extends VaadinServlet {
             ObjectInputStream ois;
             try {
                 ois = new ObjectInputStream(bais);
-                VaadinSession applicationContext = (VaadinSession) ois
-                        .readObject();
-                applicationContext.storeInSession(getService(),
+                VaadinSession vaadinSession = (VaadinSession) ois.readObject();
+                getService().storeSession(vaadinSession,
                         new WrappedHttpSession(session));
             } catch (IOException e) {
                 getLogger().log(
@@ -396,8 +395,7 @@ public class GAEVaadinServlet extends VaadinServlet {
         if (wrappedSession == null) {
             return;
         }
-        VaadinSession serviceSession = VaadinSession.getForSession(
-                getService(), wrappedSession);
+        VaadinSession serviceSession = getService().loadSession(wrappedSession);
         if (serviceSession == null) {
             return;
         }
@@ -408,7 +406,7 @@ public class GAEVaadinServlet extends VaadinServlet {
          */
         serviceSession.setAttribute(
                 VaadinService.PRESERVE_UNBOUND_SESSION_ATTRIBUTE, Boolean.TRUE);
-        serviceSession.removeFromSession(getService());
+        getService().removeSession(serviceSession.getSession());
 
         // Remove preservation marker
         serviceSession.setAttribute(
diff --git a/server/src/com/vaadin/server/VaadinPortletService.java b/server/src/com/vaadin/server/VaadinPortletService.java
index 878b8964b6..5f22dd7d7c 100644
--- a/server/src/com/vaadin/server/VaadinPortletService.java
+++ b/server/src/com/vaadin/server/VaadinPortletService.java
@@ -28,6 +28,7 @@ import java.util.logging.Logger;
 import javax.portlet.EventRequest;
 import javax.portlet.PortletContext;
 import javax.portlet.PortletRequest;
+import javax.portlet.PortletSession;
 import javax.portlet.RenderRequest;
 
 import com.vaadin.server.VaadinPortlet.RequestType;
@@ -344,4 +345,29 @@ public class VaadinPortletService extends VaadinService {
         getLogger().finest("A user session has expired");
     }
 
+    private WrappedPortletSession getWrappedPortletSession(
+            WrappedSession wrappedSession) {
+        return (WrappedPortletSession) wrappedSession;
+    }
+
+    @Override
+    protected void writeToHttpSession(WrappedSession wrappedSession,
+            VaadinSession session) {
+        getWrappedPortletSession(wrappedSession).setAttribute(
+                getSessionAttributeName(), session,
+                PortletSession.APPLICATION_SCOPE);
+    }
+
+    @Override
+    protected VaadinSession readFromHttpSession(WrappedSession wrappedSession) {
+        return (VaadinSession) getWrappedPortletSession(wrappedSession)
+                .getAttribute(getSessionAttributeName(),
+                        PortletSession.APPLICATION_SCOPE);
+    }
+
+    @Override
+    protected void removeFromHttpSession(WrappedSession wrappedSession) {
+        getWrappedPortletSession(wrappedSession).removeAttribute(
+                getSessionAttributeName(), PortletSession.APPLICATION_SCOPE);
+    }
 }
diff --git a/server/src/com/vaadin/server/VaadinPortletSession.java b/server/src/com/vaadin/server/VaadinPortletSession.java
index 512e91a82f..11c0db23a8 100644
--- a/server/src/com/vaadin/server/VaadinPortletSession.java
+++ b/server/src/com/vaadin/server/VaadinPortletSession.java
@@ -317,14 +317,4 @@ public class VaadinPortletSession extends VaadinSession {
         }
     }
 
-    @Override
-    public void storeInSession(VaadinService service, WrappedSession session) {
-        assert hasLock(service, session);
-        ((WrappedPortletSession) session).setAttribute(
-                getSessionAttributeName(service), this,
-                PortletSession.APPLICATION_SCOPE);
-
-        refreshLock();
-    }
-
 }
diff --git a/server/src/com/vaadin/server/VaadinService.java b/server/src/com/vaadin/server/VaadinService.java
index bff71fedee..8a6be63dd6 100644
--- a/server/src/com/vaadin/server/VaadinService.java
+++ b/server/src/com/vaadin/server/VaadinService.java
@@ -756,7 +756,7 @@ public abstract class VaadinService implements Serializable {
 
         VaadinSession.setCurrent(session);
 
-        session.storeInSession(this, request.getWrappedSession());
+        storeSession(session, request.getWrappedSession());
 
         // Initial WebBrowser data comes from the request
         session.getBrowser().updateRequestDetails(request);
@@ -824,7 +824,7 @@ public abstract class VaadinService implements Serializable {
         }
 
         if (session != null) {
-            vaadinSession.removeFromSession(this);
+            removeSession(session);
         }
     }
 
@@ -834,8 +834,7 @@ public abstract class VaadinService implements Serializable {
         final WrappedSession session = getWrappedSession(request,
                 allowSessionCreation);
 
-        VaadinSession vaadinSession = VaadinSession
-                .getForSession(this, session);
+        VaadinSession vaadinSession = loadSession(session);
 
         if (vaadinSession == null) {
             return null;
@@ -987,8 +986,7 @@ public abstract class VaadinService implements Serializable {
      */
     public UI findUI(VaadinRequest request) {
         // getForSession asserts that the lock is held
-        VaadinSession session = VaadinSession.getForSession(this,
-                request.getWrappedSession());
+        VaadinSession session = loadSession(request.getWrappedSession());
 
         // Get UI id from the request
         String uiIdString = request.getParameter(UIConstants.UI_ID_PARAMETER);
@@ -1075,7 +1073,7 @@ public abstract class VaadinService implements Serializable {
                 service.setSessionLock(newSession,
                         serviceSession.getLockInstance());
 
-                serviceSession.storeInSession(service, newSession);
+                service.storeSession(serviceSession, newSession);
                 serviceSession.setAttribute(PRESERVE_UNBOUND_SESSION_ATTRIBUTE,
                         null);
             }
@@ -1159,7 +1157,7 @@ public abstract class VaadinService implements Serializable {
                  * already been removed from the HttpSession and we do not have
                  * to do it again
                  */
-                session.removeFromSession(this);
+                removeSession(session.getSession());
             }
 
             /*
@@ -1883,4 +1881,113 @@ public abstract class VaadinService implements Serializable {
             throw e;
         }
     }
+
+    /**
+     * Called when the VaadinSession should be stored.
+     * <p>
+     * By default stores the VaadinSession in the underlying HTTP session.
+     * 
+     * @since
+     * @param session
+     *            the VaadinSession to store
+     * @param wrappedSession
+     *            the underlying HTTP session
+     */
+    protected void storeSession(VaadinSession session,
+            WrappedSession wrappedSession) {
+        assert VaadinSession.hasLock(this, wrappedSession);
+        writeToHttpSession(wrappedSession, session);
+        session.refreshTransients(wrappedSession, this);
+    }
+
+    /**
+     * Performs the actual write of the VaadinSession to the underlying HTTP
+     * session after sanity checks have been performed.
+     * <p>
+     * Called by {@link #storeSession(VaadinSession, WrappedSession)}
+     * 
+     * @since
+     * @param wrappedSession
+     *            the underlying HTTP session
+     * @param session
+     *            the VaadinSession to store
+     */
+    protected void writeToHttpSession(WrappedSession wrappedSession,
+            VaadinSession session) {
+        wrappedSession.setAttribute(getSessionAttributeName(), session);
+    }
+
+    /**
+     * Called when the VaadinSession should be loaded from the underlying HTTP
+     * session
+     * 
+     * @since
+     * @param wrappedSession
+     *            the underlying HTTP session
+     * @return the VaadinSession in the HTTP session or null if not found
+     */
+    protected VaadinSession loadSession(WrappedSession wrappedSession) {
+        assert VaadinSession.hasLock(this, wrappedSession);
+
+        VaadinSession vaadinSession = readFromHttpSession(wrappedSession);
+        if (vaadinSession == null) {
+            return null;
+        }
+        vaadinSession.refreshTransients(wrappedSession, this);
+        return vaadinSession;
+    }
+
+    /**
+     * Performs the actual read of the VaadinSession from the underlying HTTP
+     * session after sanity checks have been performed.
+     * <p>
+     * Called by {@link #loadSession(WrappedSession)}.
+     * 
+     * @param wrappedSession
+     *            the underlying HTTP session
+     * @since
+     * @return the VaadinSession or null if no session was found
+     */
+    protected VaadinSession readFromHttpSession(WrappedSession wrappedSession) {
+        return (VaadinSession) wrappedSession
+                .getAttribute(getSessionAttributeName());
+    }
+
+    /**
+     * Called when the VaadinSession should be removed from the underlying HTTP
+     * session
+     * 
+     * @since
+     * @param wrappedSession
+     *            the underlying HTTP session
+     */
+    public void removeSession(WrappedSession wrappedSession) {
+        assert VaadinSession.hasLock(this, wrappedSession);
+        removeFromHttpSession(wrappedSession);
+    }
+
+    /**
+     * Performs the actual removal of the VaadinSession from the underlying HTTP
+     * session after sanity checks have been performed
+     * 
+     * @since
+     * @param wrappedSession
+     *            the underlying HTTP session
+     */
+    protected void removeFromHttpSession(WrappedSession wrappedSession) {
+        wrappedSession.removeAttribute(getSessionAttributeName());
+
+    }
+
+    /**
+     * Returns the name used for storing the VaadinSession in the underlying
+     * HTTP session
+     * 
+     * @since
+     * @return the attribute name used for storing the VaadinSession
+     */
+    protected String getSessionAttributeName() {
+        return VaadinSession.class.getName() + "." + getServiceName();
+    }
+
 }
diff --git a/server/src/com/vaadin/server/VaadinSession.java b/server/src/com/vaadin/server/VaadinSession.java
index 8706cd0821..dac4f40f5a 100644
--- a/server/src/com/vaadin/server/VaadinSession.java
+++ b/server/src/com/vaadin/server/VaadinSession.java
@@ -437,24 +437,13 @@ public class VaadinSession implements HttpSessionBindingListener, Serializable {
      *            The wrapped HTTP session for the user
      * @return A VaadinSession instance for the service, session combination or
      *         null if none was found.
-     * @deprecated As of 7.0. Should be moved to a separate session storage
-     *             class some day.
+     * @deprecated as of 7.6, call
+     *             {@link VaadinService#loadSession(WrappedSession)} instead
      */
     @Deprecated
     public static VaadinSession getForSession(VaadinService service,
             WrappedSession underlyingSession) {
-        assert hasLock(service, underlyingSession);
-
-        VaadinSession vaadinSession = (VaadinSession) underlyingSession
-                .getAttribute(getSessionAttributeName(service));
-        if (vaadinSession == null) {
-            return null;
-        }
-
-        vaadinSession.session = underlyingSession;
-        vaadinSession.service = service;
-        vaadinSession.refreshLock();
-        return vaadinSession;
+        return service.loadSession(underlyingSession);
     }
 
     /**
@@ -488,26 +477,12 @@ public class VaadinSession implements HttpSessionBindingListener, Serializable {
      *
      * @param service
      *            The service this session is associated with
-     * @deprecated As of 7.0. Should be moved to a separate session storage
-     *             class some day.
+     * @deprecated as of 7.6, call
+     *             {@link VaadinService#removeSession(WrappedSession)} instead
      */
     @Deprecated
     public void removeFromSession(VaadinService service) {
-        assert hasLock();
-        session.removeAttribute(getSessionAttributeName(service));
-    }
-
-    /**
-     * Retrieves the name of the attribute used for storing a VaadinSession for
-     * the given service.
-     *
-     * @param service
-     *            The service associated with the sessio
-     * @return The attribute name used for storing the session
-     * @since
-     */
-    protected static String getSessionAttributeName(VaadinService service) {
-        return VaadinSession.class.getName() + "." + service.getServiceName();
+        service.removeSession(session);
     }
 
     /**
@@ -517,30 +492,19 @@ public class VaadinSession implements HttpSessionBindingListener, Serializable {
      *            The service this session is associated with
      * @param session
      *            The HTTP session this VaadinSession should be stored in
-     * @deprecated As of 7.0. Should be moved to a separate session storage
-     *             class some day.
+     * @deprecated as of 7.6, call
+     *             {@link VaadinService#storeSession(VaadinSession, WrappedSession)}
+     *             instead
      */
     @Deprecated
     public void storeInSession(VaadinService service, WrappedSession session) {
-        assert hasLock(service, session);
-        session.setAttribute(getSessionAttributeName(service), this);
-
-        /*
-         * GAEVaadinServlet passes newly deserialized sessions here, which means
-         * that these transient fields need to be populated to avoid NPE from
-         * refreshLock().
-         */
-        this.service = service;
-        this.session = session;
-        refreshLock();
+        service.storeSession(this, session);
     }
 
     /**
      * Updates the transient session lock from VaadinService.
-     * 
-     * @since
      */
-    protected void refreshLock() {
+    private void refreshLock() {
         assert lock == null || lock == service.getSessionLock(session) : "Cannot change the lock from one instance to another";
         assert hasLock(service, session);
         lock = service.getSessionLock(session);
@@ -1470,4 +1434,23 @@ public class VaadinSession implements HttpSessionBindingListener, Serializable {
         }
     }
 
+    /**
+     * Refreshes the transient fields of the session to ensure they are up to
+     * date.
+     * <p>
+     * Called internally by the framework.
+     * 
+     * @since
+     * @param wrappedSession
+     *            the session this VaadinSession is stored in
+     * @param vaadinService
+     *            the service associated with this VaadinSession
+     */
+    public void refreshTransients(WrappedSession wrappedSession,
+            VaadinService vaadinService) {
+        session = wrappedSession;
+        service = vaadinService;
+        refreshLock();
+    }
+
 }
diff --git a/server/tests/src/com/vaadin/server/VaadinPortletServiceTests.java b/server/tests/src/com/vaadin/server/VaadinPortletServiceTests.java
index 0e094bfabb..38f3b85043 100644
--- a/server/tests/src/com/vaadin/server/VaadinPortletServiceTests.java
+++ b/server/tests/src/com/vaadin/server/VaadinPortletServiceTests.java
@@ -200,7 +200,7 @@ public class VaadinPortletServiceTests {
             when(mockLock.isHeldByCurrentThread()).thenReturn(true);
 
             WrappedSession emptyWrappedSession = Mockito
-                    .mock(WrappedSession.class);
+                    .mock(WrappedPortletSession.class);
             when(emptyWrappedSession.getAttribute("null.lock")).thenReturn(
                     mockLock);
             VaadinRequest requestWithUIIDSet = Mockito
diff --git a/server/tests/src/com/vaadin/server/VaadinSessionTest.java b/server/tests/src/com/vaadin/server/VaadinSessionTest.java
index fa7e892066..4944d37856 100644
--- a/server/tests/src/com/vaadin/server/VaadinSessionTest.java
+++ b/server/tests/src/com/vaadin/server/VaadinSessionTest.java
@@ -93,7 +93,7 @@ public class VaadinSessionTest {
         };
 
         session = new VaadinSession(mockService);
-        session.storeInSession(mockService, mockWrappedSession);
+        mockService.storeSession(session, mockWrappedSession);
 
         ui = new UI() {
             Page page = new Page(this, getState(false).pageState) {