killbill-aplcache

util: fix Shiro session caching The cache wouldn't be used

3/18/2015 5:14:21 PM

Details

diff --git a/util/src/main/java/org/killbill/billing/util/glue/EhCacheManagerProvider.java b/util/src/main/java/org/killbill/billing/util/glue/EhCacheManagerProvider.java
index 3577271..d8b92a7 100644
--- a/util/src/main/java/org/killbill/billing/util/glue/EhCacheManagerProvider.java
+++ b/util/src/main/java/org/killbill/billing/util/glue/EhCacheManagerProvider.java
@@ -1,7 +1,9 @@
 /*
  * Copyright 2010-2013 Ning, Inc.
+ * Copyright 2014-2015 Groupon, Inc
+ * Copyright 2014-2015 The Billing Project, LLC
  *
- * Ning licenses this file to you under the Apache License, version 2.0
+ * The Billing Project licenses this file to you 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:
  *
@@ -23,16 +25,26 @@ import org.apache.shiro.cache.ehcache.EhCacheManager;
 import org.apache.shiro.mgt.DefaultSecurityManager;
 import org.apache.shiro.mgt.SecurityManager;
 import org.apache.shiro.session.mgt.eis.CachingSessionDAO;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
+import com.codahale.metrics.MetricRegistry;
+import com.codahale.metrics.ehcache.InstrumentedEhcache;
+import net.sf.ehcache.CacheException;
 import net.sf.ehcache.CacheManager;
+import net.sf.ehcache.Ehcache;
 
 public class EhCacheManagerProvider implements Provider<EhCacheManager> {
 
+    private static final Logger logger = LoggerFactory.getLogger(EhCacheManagerProvider.class);
+
+    private final MetricRegistry metricRegistry;
     private final SecurityManager securityManager;
     private final CacheManager ehCacheCacheManager;
 
     @Inject
-    public EhCacheManagerProvider(final SecurityManager securityManager, final CacheManager ehCacheCacheManager) {
+    public EhCacheManagerProvider(final MetricRegistry metricRegistry, final SecurityManager securityManager, final CacheManager ehCacheCacheManager) {
+        this.metricRegistry = metricRegistry;
         this.securityManager = securityManager;
         this.ehCacheCacheManager = ehCacheCacheManager;
     }
@@ -48,6 +60,15 @@ public class EhCacheManagerProvider implements Provider<EhCacheManager> {
         // As a workaround, create the cache manually here
         shiroEhCacheManager.getCache(CachingSessionDAO.ACTIVE_SESSION_CACHE_NAME);
 
+        // Instrument the cache
+        final Ehcache shiroActiveSessionEhcache = ehCacheCacheManager.getEhcache(CachingSessionDAO.ACTIVE_SESSION_CACHE_NAME);
+        final Ehcache decoratedCache = InstrumentedEhcache.instrument(metricRegistry, shiroActiveSessionEhcache);
+        try {
+            ehCacheCacheManager.replaceCacheWithDecoratedCache(shiroActiveSessionEhcache, decoratedCache);
+        } catch (final CacheException e) {
+            logger.warn("Unable to instrument cache {}: {}", shiroActiveSessionEhcache.getName(), e.getMessage());
+        }
+
         if (securityManager instanceof DefaultSecurityManager) {
             ((DefaultSecurityManager) securityManager).setCacheManager(shiroEhCacheManager);
         }
diff --git a/util/src/main/java/org/killbill/billing/util/security/shiro/dao/JDBCSessionDao.java b/util/src/main/java/org/killbill/billing/util/security/shiro/dao/JDBCSessionDao.java
index 7620280..9e6df64 100644
--- a/util/src/main/java/org/killbill/billing/util/security/shiro/dao/JDBCSessionDao.java
+++ b/util/src/main/java/org/killbill/billing/util/security/shiro/dao/JDBCSessionDao.java
@@ -61,7 +61,8 @@ public class JDBCSessionDao extends CachingSessionDAO {
                 return transactional.getLastInsertId();
             }
         });
-        assignSessionId(session, sessionId);
+        // See SessionModelDao#toSimpleSession for why we use toString()
+        assignSessionId(session, sessionId.toString());
         return sessionId;
     }
 
diff --git a/util/src/main/java/org/killbill/billing/util/security/shiro/dao/SessionModelDao.java b/util/src/main/java/org/killbill/billing/util/security/shiro/dao/SessionModelDao.java
index 3b5e3f0..acfd28a 100644
--- a/util/src/main/java/org/killbill/billing/util/security/shiro/dao/SessionModelDao.java
+++ b/util/src/main/java/org/killbill/billing/util/security/shiro/dao/SessionModelDao.java
@@ -42,21 +42,26 @@ public class SessionModelDao {
     public SessionModelDao() { /* For the DAO mapper */ }
 
     public SessionModelDao(final Session session) {
-        this.recordId = (Long) session.getId();
+        this.recordId = session.getId() == null ? null : Long.valueOf(session.getId().toString());
         this.startTimestamp = new DateTime(session.getStartTimestamp(), DateTimeZone.UTC);
         this.lastAccessTime = new DateTime(session.getLastAccessTime(), DateTimeZone.UTC);
         this.timeout = session.getTimeout();
         this.host = session.getHost();
         try {
             this.sessionData = serializeSessionData(session);
-        } catch (IOException e) {
+        } catch (final IOException e) {
             this.sessionData = new byte[]{};
         }
     }
 
     public Session toSimpleSession() throws IOException {
         final SimpleSession simpleSession = new SimpleSession();
-        simpleSession.setId(recordId);
+        if (recordId != null) {
+            // Make sure to use a String here! It will be used as-is as the key in Ehcache.
+            // When retrieving the session, the sessionId will be a String: if a Long is used,
+            // the lookup will trigger a miss. See https://github.com/killbill/killbill/issues/299
+            simpleSession.setId(recordId.toString());
+        }
         simpleSession.setStartTimestamp(startTimestamp.toDate());
         simpleSession.setLastAccessTime(lastAccessTime.toDate());
         simpleSession.setTimeout(timeout);
diff --git a/util/src/test/java/org/killbill/billing/util/security/shiro/dao/TestSessionModelDao.java b/util/src/test/java/org/killbill/billing/util/security/shiro/dao/TestSessionModelDao.java
index 7f9d7db..7bdbb79 100644
--- a/util/src/test/java/org/killbill/billing/util/security/shiro/dao/TestSessionModelDao.java
+++ b/util/src/test/java/org/killbill/billing/util/security/shiro/dao/TestSessionModelDao.java
@@ -1,7 +1,9 @@
 /*
  * Copyright 2010-2013 Ning, Inc.
+ * Copyright 2014-2015 Groupon, Inc
+ * Copyright 2014-2015 The Billing Project, LLC
  *
- * Ning licenses this file to you under the Apache License, version 2.0
+ * The Billing Project licenses this file to you 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:
  *
@@ -21,11 +23,10 @@ import java.util.UUID;
 
 import org.apache.shiro.session.Session;
 import org.apache.shiro.session.mgt.SimpleSession;
+import org.killbill.billing.util.UtilTestSuiteNoDB;
 import org.testng.Assert;
 import org.testng.annotations.Test;
 
-import org.killbill.billing.util.UtilTestSuiteNoDB;
-
 public class TestSessionModelDao extends UtilTestSuiteNoDB {
 
     @Test(groups = "fast")