Skip to content

Commit

Permalink
Revert "util: optimize session update query"
Browse files Browse the repository at this point in the history
This reverts commit d9b1b00.

Session attributes need to be updated in the database for authentication
to work across nodes.

Signed-off-by: Pierre-Alexandre Meyer <pierre@mouraf.org>
  • Loading branch information
pierre committed May 13, 2016
1 parent aa58819 commit 288c10a
Show file tree
Hide file tree
Showing 4 changed files with 9 additions and 26 deletions.
@@ -1,7 +1,7 @@
/*
* Copyright 2010-2013 Ning, Inc.
* Copyright 2014-2015 Groupon, Inc
* Copyright 2014-2015 The Billing Project, LLC
* Copyright 2014-2016 Groupon, Inc
* Copyright 2014-2016 The Billing Project, LLC
*
* 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
Expand All @@ -25,8 +25,6 @@

import org.apache.shiro.session.Session;
import org.apache.shiro.session.mgt.eis.CachingSessionDAO;
import org.joda.time.DateTime;
import org.joda.time.DateTimeZone;
import org.skife.jdbi.v2.IDBI;
import org.skife.jdbi.v2.Transaction;
import org.skife.jdbi.v2.TransactionStatus;
Expand All @@ -46,10 +44,7 @@ public JDBCSessionDao(final IDBI dbi) {

@Override
protected void doUpdate(final Session 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);
jdbcSessionSqlDao.update(new SessionModelDao(session));
}

@Override
Expand Down
@@ -1,7 +1,7 @@
/*
* Copyright 2010-2013 Ning, Inc.
* Copyright 2014-2015 Groupon, Inc
* Copyright 2014-2015 The Billing Project, LLC
* Copyright 2014-2016 Groupon, Inc
* Copyright 2014-2016 The Billing Project, LLC
*
* 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
Expand All @@ -18,7 +18,6 @@

package org.killbill.billing.util.security.shiro.dao;

import org.joda.time.DateTime;
import org.killbill.billing.util.entity.dao.EntitySqlDaoStringTemplate;
import org.killbill.commons.jdbi.binder.SmartBindBean;
import org.skife.jdbi.v2.sqlobject.Bind;
Expand All @@ -38,9 +37,6 @@ public interface JDBCSessionSqlDao extends Transactional<JDBCSessionSqlDao> {
@SqlUpdate
public void update(@SmartBindBean final SessionModelDao sessionModelDao);

@SqlUpdate
public void updateLastAccessTime(@Bind("lastAccessTime") final DateTime lastAccessTime, @Bind("recordId") final Long sessionId);

@SqlUpdate
public void delete(@SmartBindBean final SessionModelDao sessionModelDao);

Expand Down
Expand Up @@ -40,13 +40,6 @@ where record_id = :recordId
;
>>

updateLastAccessTime() ::= <<
update sessions set
last_access_time = :lastAccessTime
where record_id = :recordId
;
>>

delete() ::= <<
delete from sessions
where record_id = :recordId
Expand Down
Expand Up @@ -22,7 +22,6 @@

import org.apache.shiro.session.Session;
import org.apache.shiro.session.mgt.SimpleSession;
import org.joda.time.DateTime;
import org.killbill.billing.util.UtilTestSuiteWithEmbeddedDB;
import org.testng.Assert;
import org.testng.annotations.Test;
Expand Down Expand Up @@ -57,11 +56,11 @@ public void testCRUD() throws Exception {
Assert.assertEquals(retrievedSession, session);

// Update
final Date lastAccessTime = DateTime.now().withTimeAtStartOfDay().toDate(); // Milliseconds will be truncated
Assert.assertNotEquals(retrievedSession.getLastAccessTime(), lastAccessTime);
session.setLastAccessTime(lastAccessTime);
final String newHost = UUID.randomUUID().toString();
Assert.assertNotEquals(retrievedSession.getHost(), newHost);
session.setHost(newHost);
jdbcSessionDao.doUpdate(session);
Assert.assertEquals(jdbcSessionDao.doReadSession(sessionId).getLastAccessTime().compareTo(lastAccessTime), 0);
Assert.assertEquals(jdbcSessionDao.doReadSession(sessionId).getHost(), newHost);

// Delete
jdbcSessionDao.doDelete(session);
Expand Down

1 comment on commit 288c10a

@sbrossie
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Please sign in to comment.