From 070cff669864d9463dba7228ae9bfc4c30707f48 Mon Sep 17 00:00:00 2001
From: brinn <brinn>
Date: Mon, 13 Aug 2012 06:50:27 +0000
Subject: [PATCH] [BIS-149/SP-237] Display settings for custom web UIs. Add
 DisplaySettingsProvider to avoid different web app having parallel sessions
 overwrite each other's settings.

SVN: 26342
---
 .../web/server/AbstractClientService.java     |   2 -
 .../generic/server/AbstractServer.java        |  23 ++--
 .../generic/server/ComponentNames.java        |   2 +
 .../server/DisplaySettingsProvider.java       | 101 ++++++++++++++++++
 .../v1/GeneralInformationChangingService.java |  29 +++--
 .../shared/basic/dto/DisplaySettings.java     |  22 +++-
 .../source/java/genericApplicationContext.xml |   2 +
 7 files changed, 153 insertions(+), 28 deletions(-)
 create mode 100644 openbis/source/java/ch/systemsx/cisd/openbis/generic/server/DisplaySettingsProvider.java

diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/server/AbstractClientService.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/server/AbstractClientService.java
index 02aba3e588b..5a70b641070 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/server/AbstractClientService.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/server/AbstractClientService.java
@@ -302,14 +302,12 @@ public abstract class AbstractClientService implements IClientService,
         }
     }
 
-    @SuppressWarnings("deprecation")
     private final SessionContext createSessionContext(final SessionContextDTO session)
     {
         final SessionContext sessionContext = new SessionContext();
         sessionContext.setSessionID(session.getSessionToken());
 
         DisplaySettings displaySettings = session.getDisplaySettings();
-        displaySettings.clearCustomWebAppSettings();
         sessionContext.setDisplaySettings(displaySettings);
 
         final User user = new User();
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/AbstractServer.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/AbstractServer.java
index 709b78e5518..134ca1302d1 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/AbstractServer.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/AbstractServer.java
@@ -137,6 +137,9 @@ public abstract class AbstractServer<T> extends AbstractServiceWithLogger<T> imp
     @Resource(name = ComponentNames.SESSION_MANAGER)
     protected ISessionManager<Session> sessionManager;
 
+    @Resource(name = ComponentNames.DISPLAY_SETTINGS_PROVIDER)
+    protected DisplaySettingsProvider displaySettingsProvider;
+
     @Resource(name = ComponentNames.DAO_FACTORY)
     private IDAOFactory daoFactory;
 
@@ -480,6 +483,7 @@ public abstract class AbstractServer<T> extends AbstractServiceWithLogger<T> imp
         if (session.tryGetPerson() == null)
         {
             session.setPerson(person);
+            displaySettingsProvider.addDisplaySettingsForPerson(person);
         }
 
         if (roles.isEmpty())
@@ -619,12 +623,12 @@ public abstract class AbstractServer<T> extends AbstractServiceWithLogger<T> imp
         return person.getUserId().startsWith(ETL_SERVER_USERNAME_PREFIX);
     }
 
-    private static SessionContextDTO asDTO(Session session)
+    private SessionContextDTO asDTO(Session session)
     {
         SessionContextDTO result = new SessionContextDTO();
         PersonPE person = session.tryGetPerson();
         assert person != null : "cannot obtain the person which is logged in";
-        result.setDisplaySettings(person.getDisplaySettings());
+        result.setDisplaySettings(displaySettingsProvider.getRegularDisplaySettings(person));
         SpacePE homeGroup = person.getHomeSpace();
         result.setHomeGroupCode(homeGroup == null ? null : homeGroup.getCode());
         result.setSessionExpirationTime(session.getSessionExpirationTime());
@@ -670,11 +674,11 @@ public abstract class AbstractServer<T> extends AbstractServiceWithLogger<T> imp
     {
         try
         {
-            final Session session = getSessionManager().getSession(sessionToken);
+            final Session session = getSession(sessionToken);
             PersonPE person = session.tryGetPerson();
             if (person != null)
             {
-                if (displaySettings != null)
+                synchronized (displaySettingsProvider)
                 {
                     if (maxEntityVisits >= 0)
                     {
@@ -685,15 +689,9 @@ public abstract class AbstractServer<T> extends AbstractServiceWithLogger<T> imp
                             visits.remove(i);
                         }
                     }
-                    final DisplaySettings oldDisplaySettings = person.getDisplaySettings();
-                    displaySettings.overwriteCustomWebAppSettings(oldDisplaySettings);
-                    person.setDisplaySettings(displaySettings);
-                } else
-                {
-                    // Update serialized form of display settings.
-                    person.setDisplaySettings(person.getDisplaySettings());
+                    displaySettingsProvider.replaceRegularDisplaySettings(person, displaySettings);
+                    getDAOFactory().getPersonDAO().updatePerson(person);
                 }
-                getDAOFactory().getPersonDAO().updatePerson(person);
             }
         } catch (InvalidSessionException e)
         {
@@ -804,6 +802,7 @@ public abstract class AbstractServer<T> extends AbstractServiceWithLogger<T> imp
         }
         HibernateUtils.initialize(person.getAllPersonRoles());
         session.setPerson(person);
+        displaySettingsProvider.addDisplaySettingsForPerson(person);
     }
 
     protected void registerSamples(final Session session,
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/ComponentNames.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/ComponentNames.java
index f6c47d566e7..86e46556c4e 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/ComponentNames.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/ComponentNames.java
@@ -33,6 +33,8 @@ public final class ComponentNames
 
     public final static String SESSION_MANAGER = "session-manager";
 
+    public final static String DISPLAY_SETTINGS_PROVIDER = "display-settings-provider";
+
     public static final String LOG_INTERCEPTOR = "log-interceptor";
 
     public static final String DAO_FACTORY = "dao-factory";
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/DisplaySettingsProvider.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/DisplaySettingsProvider.java
new file mode 100644
index 00000000000..e93b628e167
--- /dev/null
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/DisplaySettingsProvider.java
@@ -0,0 +1,101 @@
+/*
+ * Copyright 2012 ETH Zuerich, CISD
+ *
+ * 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 ch.systemsx.cisd.openbis.generic.server;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import ch.systemsx.cisd.openbis.generic.shared.api.v1.dto.WebAppSettings;
+import ch.systemsx.cisd.openbis.generic.shared.basic.dto.DisplaySettings;
+import ch.systemsx.cisd.openbis.generic.shared.dto.PersonPE;
+
+/**
+ * A provider for user display settings, independent of the session.
+ * 
+ * @author Bernd Rinn
+ */
+public class DisplaySettingsProvider
+{
+    // user id to display settings map
+    private final Map<String, DisplaySettings> displaySettingsMap =
+            new HashMap<String, DisplaySettings>();
+
+    public synchronized void addDisplaySettingsForPerson(PersonPE person)
+    {
+        DisplaySettings settings = displaySettingsMap.get(person.getUserId());
+        if (settings == null)
+        {
+            settings = person.getDisplaySettings();
+            displaySettingsMap.put(person.getUserId(), settings);
+        }
+    }
+
+    @SuppressWarnings("deprecation")
+    public synchronized DisplaySettings getRegularDisplaySettings(PersonPE person)
+    {
+        DisplaySettings settings = displaySettingsMap.get(person.getUserId());
+        if (settings == null)
+        {
+            settings = person.getDisplaySettings();
+            displaySettingsMap.put(person.getUserId(), settings);
+        }
+        settings = settings.clone();
+        settings.clearCustomWebAppSettings();
+        return settings;
+    }
+
+    @SuppressWarnings("deprecation")
+    public synchronized DisplaySettings replaceRegularDisplaySettings(PersonPE person,
+            DisplaySettings settings)
+    {
+        final DisplaySettings oldSettings = displaySettingsMap.get(person.getUserId());
+        if (oldSettings != null)
+        {
+            settings.overwriteCustomWebAppSettings(oldSettings);
+        }
+        displaySettingsMap.put(person.getUserId(), settings);
+        person.setDisplaySettings(settings);
+        return settings;
+    }
+
+    @SuppressWarnings("deprecation")
+    public synchronized WebAppSettings getWebAppSettings(PersonPE person, String webAppId)
+    {
+        DisplaySettings settings = displaySettingsMap.get(person.getUserId());
+        if (settings == null)
+        {
+            settings = person.getDisplaySettings();
+            displaySettingsMap.put(person.getUserId(), settings);
+        }
+        return new WebAppSettings(webAppId, settings.getCustomWebAppSettings(webAppId));
+    }
+
+    @SuppressWarnings("deprecation")
+    public synchronized DisplaySettings replaceWebAppSettings(PersonPE person,
+            WebAppSettings webAppSettings)
+    {
+        DisplaySettings settings = displaySettingsMap.get(person.getUserId());
+        if (settings == null)
+        {
+            settings = person.getDisplaySettings();
+            displaySettingsMap.put(person.getUserId(), settings);
+        }
+        settings.setCustomWebAppSettings(webAppSettings.getWebAppId(), webAppSettings.getSettings());
+        person.setDisplaySettings(settings);
+        return settings;
+    }
+}
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/api/v1/GeneralInformationChangingService.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/api/v1/GeneralInformationChangingService.java
index 072141ab48c..1c0470177a0 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/api/v1/GeneralInformationChangingService.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/api/v1/GeneralInformationChangingService.java
@@ -24,6 +24,7 @@ import org.springframework.stereotype.Component;
 import org.springframework.transaction.annotation.Transactional;
 
 import ch.systemsx.cisd.authentication.ISessionManager;
+import ch.systemsx.cisd.common.exceptions.InvalidSessionException;
 import ch.systemsx.cisd.common.spring.IInvocationLoggerContext;
 import ch.systemsx.cisd.openbis.generic.server.AbstractServer;
 import ch.systemsx.cisd.openbis.generic.server.business.IPropertiesBatchManager;
@@ -36,8 +37,8 @@ import ch.systemsx.cisd.openbis.generic.shared.api.v1.dto.WebAppSettings;
 import ch.systemsx.cisd.openbis.generic.shared.authorization.annotation.RolesAllowed;
 import ch.systemsx.cisd.openbis.generic.shared.basic.TechId;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.DatabaseModificationKind.ObjectKind;
-import ch.systemsx.cisd.openbis.generic.shared.basic.dto.DisplaySettings;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.RoleWithHierarchy;
+import ch.systemsx.cisd.openbis.generic.shared.dto.PersonPE;
 import ch.systemsx.cisd.openbis.generic.shared.dto.Session;
 import ch.systemsx.cisd.openbis.generic.shared.util.EntityHelper;
 
@@ -112,21 +113,31 @@ public class GeneralInformationChangingService extends
     public WebAppSettings getWebAppSettings(String sessionToken, String webAppId)
     {
         final Session session = getSession(sessionToken);
-        return new WebAppSettings(webAppId, session.getPerson().getDisplaySettings()
-                .getCustomWebAppSettings(webAppId));
+        return new WebAppSettings(webAppId, displaySettingsProvider.getWebAppSettings(
+                session.getPerson(), webAppId));
     }
 
     @Override
     @Transactional(readOnly = false)
     @RolesAllowed(RoleWithHierarchy.SPACE_OBSERVER)
-    @SuppressWarnings("deprecation")
     public void setWebAppSettings(String sessionToken, WebAppSettings webAppSettings)
     {
-        final Session session = getSession(sessionToken);
-        final DisplaySettings displaySettings = session.getPerson().getDisplaySettings();
-        displaySettings.setCustomWebAppSettings(webAppSettings.getWebAppId(),
-                webAppSettings.getSettings());
-        saveDisplaySettings(session.getSessionToken(), null, -1);
+        try
+        {
+            final Session session = getSession(sessionToken);
+            PersonPE person = session.tryGetPerson();
+            if (person != null)
+            {
+                synchronized (displaySettingsProvider)
+                {
+                    displaySettingsProvider.replaceWebAppSettings(person, webAppSettings);
+                    getDAOFactory().getPersonDAO().updatePerson(person);
+                }
+            }
+        } catch (InvalidSessionException e)
+        {
+            // ignore the situation when session is not available
+        }
     }
 
     @Override
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/basic/dto/DisplaySettings.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/basic/dto/DisplaySettings.java
index 36ae4826944..1224b4e6558 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/basic/dto/DisplaySettings.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/basic/dto/DisplaySettings.java
@@ -34,7 +34,7 @@ import java.util.Map;
  * 
  * @author Franz-Josef Elmer
  */
-public class DisplaySettings implements Serializable
+public class DisplaySettings implements Serializable, Cloneable
 {
     private static final long serialVersionUID = 1L;
 
@@ -311,7 +311,7 @@ public class DisplaySettings implements Serializable
      * @deprecated Don't use in generic web client - will be overwritten.
      */
     @Deprecated
-    public synchronized Map<String, String> getCustomWebAppSettings(String webAppId)
+    public Map<String, String> getCustomWebAppSettings(String webAppId)
     {
         if (customWebAppDisplaySettings == null)
         {
@@ -330,7 +330,7 @@ public class DisplaySettings implements Serializable
      * @deprecated Don't use in generic web client - will be overwritten.
      */
     @Deprecated
-    public synchronized void setCustomWebAppSettings(String webAppId, Map<String, String> settings)
+    public void setCustomWebAppSettings(String webAppId, Map<String, String> settings)
     {
         if (customWebAppDisplaySettings == null)
         {
@@ -343,7 +343,7 @@ public class DisplaySettings implements Serializable
      * @deprecated Don't use in generic web client - will be overwritten.
      */
     @Deprecated
-    public synchronized void overwriteCustomWebAppSettings(DisplaySettings customDisplaySettings)
+    public void overwriteCustomWebAppSettings(DisplaySettings customDisplaySettings)
     {
         synchronized (customDisplaySettings)
         {
@@ -352,8 +352,20 @@ public class DisplaySettings implements Serializable
     }
 
     @Deprecated
-    public synchronized void clearCustomWebAppSettings()
+    public void clearCustomWebAppSettings()
     {
         customWebAppDisplaySettings = null;
     }
+
+    @Override
+    public DisplaySettings clone()
+    {
+        try
+        {
+            return (DisplaySettings) super.clone();
+        } catch (CloneNotSupportedException ex)
+        {
+            throw new Error(ex);
+        }
+    }
 }
diff --git a/openbis/source/java/genericApplicationContext.xml b/openbis/source/java/genericApplicationContext.xml
index d6c42de0193..cbec61d3330 100644
--- a/openbis/source/java/genericApplicationContext.xml
+++ b/openbis/source/java/genericApplicationContext.xml
@@ -76,6 +76,8 @@
         <constructor-arg value="true" />
     </bean>
     
+    <bean id="display-settings-provider" class="ch.systemsx.cisd.openbis.generic.server.DisplaySettingsProvider" />
+    
     <bean id="remote-host-validator" class="ch.systemsx.cisd.openbis.generic.server.WhiteListBasedRemoteHostValidator">
         <constructor-arg value="${accepted-remote-hosts-for-identity-change}"/>
     </bean>
-- 
GitLab