killbill-uncached

Revert "util: revisit optimization of session update query" This

6/7/2015 10:26:54 AM

Changes

util/src/main/java/org/killbill/billing/util/security/shiro/dao/SessionUtils.java 74(+0 -74)

util/src/test/java/org/killbill/billing/util/security/shiro/dao/TestSessionUtils.java 176(+0 -176)

Details

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 01a8729..0b0188b 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
@@ -24,8 +24,6 @@ import java.io.Serializable;
 import javax.inject.Inject;
 
 import org.apache.shiro.session.Session;
-import org.apache.shiro.session.UnknownSessionException;
-import org.apache.shiro.session.mgt.SimpleSession;
 import org.apache.shiro.session.mgt.eis.CachingSessionDAO;
 import org.joda.time.DateTime;
 import org.joda.time.DateTimeZone;
@@ -48,25 +46,10 @@ public class JDBCSessionDao extends CachingSessionDAO {
 
     @Override
     protected void doUpdate(final Session session) {
-        // The look-up should be cheap (most likely cached)
-        final Session previousSession = readSession(session.getId());
-
-        if (SessionUtils.sameSession(previousSession, session)) {
-            // Only the last access time attribute was updated.
-            // Avoid writing the state to disk for each request: we don't care so much about precision in the database,
-            // we just want to make sure the session doesn't timeout too early.
-            // Note also that in the case of a single node (or distributed cache), the timeout computation
-            // will be correct (because the cache value is correct).
-            // See https://github.com/killbill/killbill/issues/326
-            if (!SessionUtils.accessedRecently(previousSession, session)) {
-                final DateTime lastAccessTime = new DateTime(session.getLastAccessTime(), DateTimeZone.UTC);
-                final Long sessionId = Long.valueOf(session.getId().toString());
-                jdbcSessionSqlDao.updateLastAccessTime(lastAccessTime, sessionId);
-            }
-        } else {
-            // Various fields were changed, update the full row
-            jdbcSessionSqlDao.update(new SessionModelDao(session));
-        }
+        // Assume only the last access time attribute was updated (see https://github.com/killbill/killbill/issues/326)
+        final DateTime lastAccessTime = new DateTime(session.getLastAccessTime(), DateTimeZone.UTC);
+        final Long sessionId = Long.valueOf(session.getId().toString());
+        jdbcSessionSqlDao.updateLastAccessTime(lastAccessTime, sessionId);
     }
 
     @Override
@@ -89,28 +72,6 @@ public class JDBCSessionDao extends CachingSessionDAO {
     }
 
     @Override
-    public Session readSession(final Serializable sessionId) throws UnknownSessionException {
-        final Session session = super.readSession(sessionId);
-
-        // Clone the session to avoid making changes to the existing one in the cache.
-        // This is required for the lookup in doUpdate to work
-        final SimpleSession clonedSession = new SimpleSession();
-        clonedSession.setId(session.getId());
-        clonedSession.setStartTimestamp(session.getStartTimestamp());
-        clonedSession.setLastAccessTime(session.getLastAccessTime());
-        clonedSession.setTimeout(session.getTimeout());
-        clonedSession.setHost(session.getHost());
-        clonedSession.setAttributes(SessionUtils.getSessionAttributes(session));
-
-        if (session instanceof SimpleSession) {
-            clonedSession.setStopTimestamp(((SimpleSession) session).getStopTimestamp());
-            clonedSession.setExpired(((SimpleSession) session).isExpired());
-        }
-
-        return clonedSession;
-    }
-
-    @Override
     protected Session doReadSession(final Serializable sessionId) {
         // Shiro should not pass us a null sessionId, but be safe...
         if (sessionId == null) {
diff --git a/util/src/test/java/org/killbill/billing/util/security/shiro/dao/TestJDBCSessionDao.java b/util/src/test/java/org/killbill/billing/util/security/shiro/dao/TestJDBCSessionDao.java
index b943b2d..df97ca8 100644
--- a/util/src/test/java/org/killbill/billing/util/security/shiro/dao/TestJDBCSessionDao.java
+++ b/util/src/test/java/org/killbill/billing/util/security/shiro/dao/TestJDBCSessionDao.java
@@ -56,15 +56,8 @@ public class TestJDBCSessionDao extends UtilTestSuiteWithEmbeddedDB {
         final Session retrievedSession = jdbcSessionDao.doReadSession(sessionId);
         Assert.assertEquals(retrievedSession, session);
 
-        // Update too soon, the database state won't be updated
-        Date lastAccessTime = new Date(retrievedSession.getLastAccessTime().getTime() + 1000);
-        Assert.assertNotEquals(retrievedSession.getLastAccessTime(), lastAccessTime);
-        session.setLastAccessTime(lastAccessTime);
-        jdbcSessionDao.doUpdate(session);
-        Assert.assertEquals(jdbcSessionDao.doReadSession(sessionId).getLastAccessTime().compareTo(retrievedSession.getLastAccessTime()), 0);
-
-        // Actual database update
-        lastAccessTime = new Date(retrievedSession.getLastAccessTime().getTime() + 100000);
+        // Update
+        final Date lastAccessTime = DateTime.now().withTimeAtStartOfDay().toDate(); // Milliseconds will be truncated
         Assert.assertNotEquals(retrievedSession.getLastAccessTime(), lastAccessTime);
         session.setLastAccessTime(lastAccessTime);
         jdbcSessionDao.doUpdate(session);
@@ -77,9 +70,8 @@ public class TestJDBCSessionDao extends UtilTestSuiteWithEmbeddedDB {
 
     private SimpleSession createSession() {
         final SimpleSession simpleSession = new SimpleSession();
-        // Truncate milliseconds for MySQL
-        simpleSession.setStartTimestamp(DateTime.now().withTimeAtStartOfDay().minusSeconds(5).toDate());
-        simpleSession.setLastAccessTime(DateTime.now().withTimeAtStartOfDay().toDate());
+        simpleSession.setStartTimestamp(new Date(System.currentTimeMillis() - 5000));
+        simpleSession.setLastAccessTime(new Date(System.currentTimeMillis()));
         simpleSession.setTimeout(493934L);
         simpleSession.setHost(UUID.randomUUID().toString());
         simpleSession.setAttribute(UUID.randomUUID().toString(), Short.MIN_VALUE);