From c79b10369c47a76ff4e636b355b7e862ae7effd2 Mon Sep 17 00:00:00 2001
From: pkupczyk <pkupczyk>
Date: Fri, 21 Aug 2015 14:21:44 +0000
Subject: [PATCH] SSDM-2294 : V3 AS API - paging and sorting of search results:
 - remove cached search results on logout - make cache thread safe - log cache
 operations

SVN: 34508
---
 .../v3/cache/SearchCacheEventListener.java    |  97 ++++++++++
 .../SearchCacheEventListenerFactory.java      |  36 ++++
 .../method/AbstractSearchMethodExecutor.java  | 173 +++++++++++++++---
 .../openbis/generic/shared/dto/Session.java   |   9 -
 .../api/v3/dto/search/SearchResult.java       |   5 +-
 5 files changed, 284 insertions(+), 36 deletions(-)
 create mode 100644 openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/cache/SearchCacheEventListener.java
 create mode 100644 openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/cache/SearchCacheEventListenerFactory.java

diff --git a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/cache/SearchCacheEventListener.java b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/cache/SearchCacheEventListener.java
new file mode 100644
index 00000000000..4ea05afca16
--- /dev/null
+++ b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/cache/SearchCacheEventListener.java
@@ -0,0 +1,97 @@
+/*
+ * Copyright 2015 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.ethz.sis.openbis.generic.server.api.v3.cache;
+
+import net.sf.ehcache.CacheException;
+import net.sf.ehcache.Ehcache;
+import net.sf.ehcache.Element;
+import net.sf.ehcache.event.CacheEventListener;
+
+import org.apache.log4j.Logger;
+
+import ch.ethz.sis.openbis.generic.server.api.v3.executor.method.AbstractSearchMethodExecutor.CacheEntry;
+import ch.systemsx.cisd.common.logging.LogCategory;
+import ch.systemsx.cisd.common.logging.LogFactory;
+
+/**
+ * @author pkupczyk
+ */
+public class SearchCacheEventListener implements CacheEventListener
+{
+
+    private final Logger operationLog = LogFactory.getLogger(LogCategory.OPERATION, getClass());
+
+    @Override
+    public void dispose()
+    {
+        operationLog.info("Disposing cache listener");
+    }
+
+    @Override
+    public void notifyElementEvicted(Ehcache cache, Element element)
+    {
+        logOperation(element, "has been evicted from the cache");
+    }
+
+    @Override
+    public void notifyElementExpired(Ehcache cache, Element element)
+    {
+        logOperation(element, "has expired");
+    }
+
+    @Override
+    public void notifyElementPut(Ehcache cache, Element element) throws CacheException
+    {
+        logOperation(element, "has been put to the cache");
+    }
+
+    @Override
+    public void notifyElementRemoved(Ehcache cache, Element element) throws CacheException
+    {
+        logOperation(element, "has been removed from the cache");
+    }
+
+    @Override
+    public void notifyElementUpdated(Ehcache cache, Element element) throws CacheException
+    {
+        logOperation(element, "has been updated");
+    }
+
+    @Override
+    public void notifyRemoveAll(Ehcache cache)
+    {
+        operationLog.info("All search results have been removed from the cache.");
+    }
+
+    @SuppressWarnings("rawtypes")
+    private void logOperation(Element element, String operation)
+    {
+        CacheEntry entry = (CacheEntry) element.getObjectValue();
+        if (entry != null)
+        {
+            operationLog.info("Cache entry " + entry.hashCode() + " that contains search result with "
+                    + (entry.getObjects() != null ? entry.getObjects().size() : 0) + " objects " + operation + ".");
+        }
+    }
+
+    @Override
+    public Object clone()
+    {
+        return new SearchCacheEventListener();
+    }
+
+}
diff --git a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/cache/SearchCacheEventListenerFactory.java b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/cache/SearchCacheEventListenerFactory.java
new file mode 100644
index 00000000000..3dd5a72a08f
--- /dev/null
+++ b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/cache/SearchCacheEventListenerFactory.java
@@ -0,0 +1,36 @@
+/*
+ * Copyright 2015 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.ethz.sis.openbis.generic.server.api.v3.cache;
+
+import java.util.Properties;
+
+import net.sf.ehcache.event.CacheEventListener;
+import net.sf.ehcache.event.CacheEventListenerFactory;
+
+/**
+ * @author pkupczyk
+ */
+public class SearchCacheEventListenerFactory extends CacheEventListenerFactory
+{
+
+    @Override
+    public CacheEventListener createCacheEventListener(Properties arg0)
+    {
+        return new SearchCacheEventListener();
+    }
+
+}
diff --git a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/method/AbstractSearchMethodExecutor.java b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/method/AbstractSearchMethodExecutor.java
index 02b8a2b9f2e..dfcd8f01801 100644
--- a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/method/AbstractSearchMethodExecutor.java
+++ b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/method/AbstractSearchMethodExecutor.java
@@ -16,12 +16,14 @@
 
 package ch.ethz.sis.openbis.generic.server.api.v3.executor.method;
 
+import java.io.Serializable;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 
+import org.apache.log4j.Logger;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.cache.Cache;
 import org.springframework.cache.Cache.ValueWrapper;
@@ -37,6 +39,9 @@ import ch.ethz.sis.openbis.generic.shared.api.v3.dto.fetchoptions.FetchOptionsMa
 import ch.ethz.sis.openbis.generic.shared.api.v3.dto.fetchoptions.sort.SortAndPage;
 import ch.ethz.sis.openbis.generic.shared.api.v3.dto.search.AbstractObjectSearchCriterion;
 import ch.ethz.sis.openbis.generic.shared.api.v3.dto.search.SearchResult;
+import ch.systemsx.cisd.common.logging.LogCategory;
+import ch.systemsx.cisd.common.logging.LogFactory;
+import ch.systemsx.cisd.openbis.generic.shared.dto.Session;
 
 /**
  * @author pkupczyk
@@ -45,6 +50,8 @@ public abstract class AbstractSearchMethodExecutor<OBJECT, OBJECT_PE, CRITERION
         extends AbstractMethodExecutor implements ISearchMethodExecutor<OBJECT, CRITERION, FETCH_OPTIONS>
 {
 
+    private final Logger operationLog = LogFactory.getLogger(LogCategory.OPERATION, getClass());
+
     @Autowired
     private CacheManager cacheManager;
 
@@ -75,37 +82,18 @@ public abstract class AbstractSearchMethodExecutor<OBJECT, OBJECT_PE, CRITERION
             });
     }
 
-    @SuppressWarnings("unchecked")
     private Collection<OBJECT> searchAndTranslate(IOperationContext context, CRITERION criterion, FETCH_OPTIONS fetchOptions)
     {
+        operationLog.info("Cache mode: " + fetchOptions.getCacheMode());
+
         if (CacheMode.NO_CACHE.equals(fetchOptions.getCacheMode()))
         {
             return doSearchAndTranslate(context, criterion, fetchOptions);
         } else if (CacheMode.CACHE.equals(fetchOptions.getCacheMode()) || CacheMode.RELOAD_AND_CACHE.equals(fetchOptions.getCacheMode()))
         {
-            Cache cache = cacheManager.getCache("searchCache");
-            CacheKey key = new CacheKey(context.getSession().getSessionToken(), criterion, fetchOptions);
-            Collection<OBJECT> results = null;
-
-            if (CacheMode.RELOAD_AND_CACHE.equals(fetchOptions.getCacheMode()))
-            {
-                cache.evict(key);
-            } else
-            {
-                ValueWrapper wrapper = cache.get(key);
-                if (wrapper != null)
-                {
-                    results = (Collection<OBJECT>) wrapper.get();
-                }
-            }
-
-            if (results == null)
-            {
-                results = doSearchAndTranslate(context, criterion, fetchOptions);
-                cache.put(key, results);
-            }
-
-            return results;
+            CacheEntry entry = getCacheEntry(context, criterion, fetchOptions);
+            populateCacheEntry(context, criterion, fetchOptions, entry);
+            return entry.getObjects();
         } else
         {
             throw new IllegalArgumentException("Unsupported cache mode: " + fetchOptions.getCacheMode());
@@ -114,10 +102,19 @@ public abstract class AbstractSearchMethodExecutor<OBJECT, OBJECT_PE, CRITERION
 
     private Collection<OBJECT> doSearchAndTranslate(IOperationContext context, CRITERION criterion, FETCH_OPTIONS fetchOptions)
     {
+        operationLog.info("Searching...");
+
         List<OBJECT_PE> ids = getSearchExecutor().search(context, criterion);
+
+        operationLog.info("Found " + ids.size() + " object id(s).");
+
         TranslationContext translationContext = new TranslationContext(context.getSession());
         Map<OBJECT_PE, OBJECT> idToObjectMap = getTranslator().translate(translationContext, ids, fetchOptions);
-        return idToObjectMap.values();
+        Collection<OBJECT> objects = idToObjectMap.values();
+
+        operationLog.info("Translated " + objects.size() + " object(s).");
+
+        return objects;
     }
 
     private List<OBJECT> sortAndPage(IOperationContext context, Collection<OBJECT> results, FETCH_OPTIONS fetchOptions)
@@ -130,11 +127,98 @@ public abstract class AbstractSearchMethodExecutor<OBJECT, OBJECT_PE, CRITERION
         SortAndPage sap = new SortAndPage();
         Collection<OBJECT> objects = sap.sortAndPage(results, fetchOptions);
 
+        operationLog.info("Return " + objects.size() + " object(s) after sorting and paging.");
+
         return new ArrayList<OBJECT>(objects);
     }
 
-    private class CacheKey
+    private Cache getCache()
+    {
+        return cacheManager.getCache("searchCache");
+    }
+
+    @SuppressWarnings("unchecked")
+    private CacheEntry getCacheEntry(IOperationContext context, CRITERION criterion, FETCH_OPTIONS fetchOptions)
     {
+        Cache cache = getCache();
+        CacheKey key = new CacheKey(context.getSession().getSessionToken(), criterion, fetchOptions);
+        CacheEntry entry = null;
+
+        if (operationLog.isDebugEnabled())
+        {
+            operationLog.debug("Will try to lock on session " + context.getSession().hashCode());
+        }
+
+        synchronized (context.getSession())
+        {
+            if (operationLog.isDebugEnabled())
+            {
+                operationLog.debug("Locked on session " + context.getSession().hashCode());
+            }
+
+            if (CacheMode.RELOAD_AND_CACHE.equals(fetchOptions.getCacheMode()))
+            {
+                cache.evict(key);
+            }
+
+            ValueWrapper wrapper = cache.get(key);
+
+            if (wrapper == null)
+            {
+                entry = new CacheEntry();
+                cache.put(key, entry);
+                context.getSession().addCleanupListener(new CacheCleanupListener(key));
+            } else
+            {
+                entry = (CacheEntry) wrapper.get();
+            }
+
+            if (operationLog.isDebugEnabled())
+            {
+                operationLog.debug("Released lock on session " + context.getSession().hashCode());
+            }
+        }
+
+        return entry;
+    }
+
+    private void populateCacheEntry(IOperationContext context, CRITERION criterion, FETCH_OPTIONS fetchOptions, CacheEntry entry)
+    {
+        if (operationLog.isDebugEnabled())
+        {
+            operationLog.debug("Will try to lock on cache entry " + entry.hashCode());
+        }
+
+        synchronized (entry)
+        {
+            if (operationLog.isDebugEnabled())
+            {
+                operationLog.debug("Locked on cache entry " + entry.hashCode());
+            }
+
+            if (entry.getObjects() == null)
+            {
+                entry.setObjects(doSearchAndTranslate(context, criterion, fetchOptions));
+
+                operationLog.info("Updated cache entry " + entry.hashCode() + " to contain search result with " + entry.getObjects().size()
+                        + " object(s).");
+            } else
+            {
+                operationLog.info("Found cache entry " + entry.hashCode() + " that contains search result with " + entry.getObjects().size()
+                        + " object(s).");
+            }
+
+            if (operationLog.isDebugEnabled())
+            {
+                operationLog.debug("Released lock on cache entry " + entry.hashCode());
+            }
+        }
+    }
+
+    public class CacheKey implements Serializable
+    {
+
+        private static final long serialVersionUID = 1L;
 
         private String sessionToken;
 
@@ -178,4 +262,41 @@ public abstract class AbstractSearchMethodExecutor<OBJECT, OBJECT_PE, CRITERION
         }
     }
 
+    public class CacheEntry implements Serializable
+    {
+        private static final long serialVersionUID = 1L;
+
+        private Collection<OBJECT> objects;
+
+        public Collection<OBJECT> getObjects()
+        {
+            return objects;
+        }
+
+        public void setObjects(Collection<OBJECT> objects)
+        {
+            this.objects = objects;
+        }
+
+    }
+
+    private class CacheCleanupListener implements Session.ISessionCleaner
+    {
+
+        private CacheKey key;
+
+        public CacheCleanupListener(CacheKey key)
+        {
+            this.key = key;
+        }
+
+        @Override
+        public void cleanup()
+        {
+            operationLog.info("Clean up cached search result on logout.");
+            getCache().evict(key);
+        }
+
+    }
+
 }
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/dto/Session.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/dto/Session.java
index 32a493af338..82e75bd099c 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/dto/Session.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/dto/Session.java
@@ -16,9 +16,7 @@
 
 package ch.systemsx.cisd.openbis.generic.shared.dto;
 
-import java.util.HashMap;
 import java.util.LinkedHashSet;
-import java.util.Map;
 import java.util.Set;
 
 import org.apache.commons.lang.time.DateFormatUtils;
@@ -63,8 +61,6 @@ public final class Session extends BasicSession implements IAuthSession
 
     private final Set<ISessionCleaner> cleanupListeners = new LinkedHashSet<ISessionCleaner>();
 
-    private Map<String, Object> attributes = new HashMap<String, Object>();
-
     @Deprecated
     public Session()
     {
@@ -184,11 +180,6 @@ public final class Session extends BasicSession implements IAuthSession
         cleanupListeners.remove(sessionCleaner);
     }
 
-    public Map<String, Object> getAttributes()
-    {
-        return attributes;
-    }
-
     //
     // Object
     //
diff --git a/openbis_api/source/java/ch/ethz/sis/openbis/generic/shared/api/v3/dto/search/SearchResult.java b/openbis_api/source/java/ch/ethz/sis/openbis/generic/shared/api/v3/dto/search/SearchResult.java
index 2f5a66919a6..c3a921e6d72 100644
--- a/openbis_api/source/java/ch/ethz/sis/openbis/generic/shared/api/v3/dto/search/SearchResult.java
+++ b/openbis_api/source/java/ch/ethz/sis/openbis/generic/shared/api/v3/dto/search/SearchResult.java
@@ -16,14 +16,17 @@
 
 package ch.ethz.sis.openbis.generic.shared.api.v3.dto.search;
 
+import java.io.Serializable;
 import java.util.List;
 
 /**
  * @author pkupczyk
  */
-public class SearchResult<OBJECT>
+public class SearchResult<OBJECT> implements Serializable
 {
 
+    private static final long serialVersionUID = 1L;
+
     private List<OBJECT> objects;
 
     private int totalCount;
-- 
GitLab