Fix custom expire for JdbcOperationsSessionRepository.cleanUpExpiredSessions

Currently, JdbcOperationsSessionRepository#cleanUpExpiredSessions only considers
the repository defined max inactive interval which causes incorrect cleanup of
sessions that define custom inactive interval. This commit fixes the problem by
delegating calculation of deletion interval to the underlying SQL DELETE statement.

Fixes gh-580
This commit is contained in:
Vedran Pavic
2016-09-08 00:04:36 +02:00
committed by Rob Winch
parent 26eca5b448
commit d3379029bb
4 changed files with 57 additions and 51 deletions

View File

@@ -18,6 +18,7 @@ package org.springframework.session.jdbc;
import java.util.Map;
import java.util.UUID;
import java.util.concurrent.TimeUnit;
import javax.sql.DataSource;
@@ -501,9 +502,7 @@ public abstract class AbstractJdbcOperationsSessionRepositoryITests {
}
@Test
public void cleanupCleansUpInactiveSessions() {
this.repository.setDefaultMaxInactiveInterval(1800); // 30 minutes
public void cleanupInactiveSessionsUsingRepositoryDefinedInterval() {
JdbcOperationsSessionRepository.JdbcSession session = this.repository
.createSession();
@@ -513,32 +512,50 @@ public abstract class AbstractJdbcOperationsSessionRepositoryITests {
this.repository.cleanUpExpiredSessions();
// verify its not cleaned up immediately after creation
assertThat(this.repository.getSession(session.getId())).isNotNull();
long now = System.currentTimeMillis();
long tenMinutesInMilliseconds = 1000 * 60 * 10;
long thirtyMinutesInMilliseconds = 1000 * 60 * 30;
// set the last accessed time to 10 minutes in the past, session should not get
// cleaned up because it has not been inactive for 30 minutes yet
long tenMinutesInThePast = now - tenMinutesInMilliseconds;
session.setLastAccessedTime(tenMinutesInThePast);
session.setLastAccessedTime(now - TimeUnit.MINUTES.toMillis(10));
this.repository.save(session);
this.repository.cleanUpExpiredSessions();
// session should still exist
assertThat(this.repository.getSession(session.getId())).isNotNull();
// set the last accessed time to 30 minutes in the past, session should get
// cleaned up as it has been inactive for 30 minutes
long thirtyMinutesInThePast = now - thirtyMinutesInMilliseconds;
session.setLastAccessedTime(thirtyMinutesInThePast);
session.setLastAccessedTime(now - TimeUnit.MINUTES.toMillis(30));
this.repository.save(session);
this.repository.cleanUpExpiredSessions();
assertThat(this.repository.getSession(session.getId())).isNull();
}
// gh-580
@Test
public void cleanupInactiveSessionsUsingSessionDefinedInterval() {
JdbcOperationsSessionRepository.JdbcSession session = this.repository
.createSession();
session.setMaxInactiveIntervalInSeconds((int) TimeUnit.MINUTES.toSeconds(45));
this.repository.save(session);
assertThat(this.repository.getSession(session.getId())).isNotNull();
this.repository.cleanUpExpiredSessions();
// session should have been cleaned up
assertThat(this.repository.getSession(session.getId())).isNotNull();
long now = System.currentTimeMillis();
session.setLastAccessedTime(now - TimeUnit.MINUTES.toMillis(40));
this.repository.save(session);
this.repository.cleanUpExpiredSessions();
assertThat(this.repository.getSession(session.getId())).isNotNull();
session.setLastAccessedTime(now - TimeUnit.MINUTES.toMillis(50));
this.repository.save(session);
this.repository.cleanUpExpiredSessions();
assertThat(this.repository.getSession(session.getId())).isNull();
}

View File

@@ -21,7 +21,6 @@ import java.sql.ResultSet;
import java.sql.SQLException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Date;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@@ -177,7 +176,7 @@ public class JdbcOperationsSessionRepository implements
private static final String DELETE_SESSIONS_BY_LAST_ACCESS_TIME_QUERY =
"DELETE FROM %TABLE_NAME% " +
"WHERE LAST_ACCESS_TIME < ?";
"WHERE LAST_ACCESS_TIME < ? - MAX_INACTIVE_INTERVAL * 1000";
private static final Log logger = LogFactory
.getLog(JdbcOperationsSessionRepository.class);
@@ -564,24 +563,12 @@ public class JdbcOperationsSessionRepository implements
@Scheduled(cron = "0 * * * * *")
public void cleanUpExpiredSessions() {
long now = System.currentTimeMillis();
long maxInactiveIntervalSeconds = (this.defaultMaxInactiveInterval != null)
? this.defaultMaxInactiveInterval
: MapSession.DEFAULT_MAX_INACTIVE_INTERVAL_SECONDS;
final long sessionsValidFromTime = now - (maxInactiveIntervalSeconds * 1000);
if (logger.isDebugEnabled()) {
logger.debug(
"Cleaning up sessions older than " + new Date(sessionsValidFromTime));
}
int deletedCount = this.transactionOperations.execute(new TransactionCallback<Integer>() {
public Integer doInTransaction(TransactionStatus transactionStatus) {
return JdbcOperationsSessionRepository.this.jdbcOperations.update(
JdbcOperationsSessionRepository.this.deleteSessionsByLastAccessTimeQuery,
sessionsValidFromTime);
System.currentTimeMillis());
}
});

View File

@@ -15,6 +15,7 @@
*/
package org.springframework.session.jdbc.config.annotation.web.http;
import java.util.Arrays;
import java.util.Map;
import javax.sql.DataSource;
@@ -33,6 +34,8 @@ import org.springframework.core.serializer.support.SerializingConverter;
import org.springframework.core.type.AnnotationMetadata;
import org.springframework.jdbc.core.JdbcOperations;
import org.springframework.jdbc.core.JdbcTemplate;
import org.springframework.jdbc.support.JdbcUtils;
import org.springframework.jdbc.support.MetaDataAccessException;
import org.springframework.jdbc.support.lob.LobHandler;
import org.springframework.scheduling.annotation.EnableScheduling;
import org.springframework.session.config.annotation.web.http.SpringHttpSessionConfiguration;
@@ -80,14 +83,19 @@ public class JdbcHttpSessionConfiguration extends SpringHttpSessionConfiguration
@Bean
public JdbcOperationsSessionRepository sessionRepository(
@Qualifier("springSessionJdbcOperations") JdbcOperations jdbcOperations,
@Qualifier("springSessionJdbcOperations") JdbcTemplate jdbcTemplate,
PlatformTransactionManager transactionManager) {
JdbcOperationsSessionRepository sessionRepository =
new JdbcOperationsSessionRepository(jdbcOperations, transactionManager);
new JdbcOperationsSessionRepository(jdbcTemplate, transactionManager);
String tableName = getTableName();
if (StringUtils.hasText(tableName)) {
sessionRepository.setTableName(tableName);
}
String databaseName = getDatabaseName(jdbcTemplate.getDataSource());
if (Arrays.asList("Apache Derby", "H2").contains(databaseName)) {
sessionRepository.setDeleteSessionsByLastAccessTimeQuery("DELETE FROM " + tableName +
" WHERE LAST_ACCESS_TIME < ? - CAST(MAX_INACTIVE_INTERVAL AS BIGINT) * 1000");
}
sessionRepository
.setDefaultMaxInactiveInterval(this.maxInactiveIntervalInSeconds);
if (this.lobHandler != null) {
@@ -150,6 +158,17 @@ public class JdbcHttpSessionConfiguration extends SpringHttpSessionConfiguration
this.maxInactiveIntervalInSeconds = maxInactiveIntervalInSeconds;
}
private String getDatabaseName(DataSource dataSource) {
try {
String databaseProductName = JdbcUtils.extractDatabaseMetaData(dataSource,
"getDatabaseProductName").toString();
return JdbcUtils.commonDatabaseName(databaseProductName);
}
catch (MetaDataAccessException e) {
return null;
}
}
private String getTableName() {
String systemProperty = System.getProperty("spring.session.jdbc.tableName", "");
if (StringUtils.hasText(systemProperty)) {

View File

@@ -22,7 +22,6 @@ import java.util.Map;
import javax.sql.DataSource;
import org.assertj.core.data.Offset;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
@@ -524,22 +523,6 @@ public class JdbcOperationsSessionRepositoryTests {
verify(this.jdbcOperations, times(1)).update(startsWith("DELETE"), anyLong());
}
// gh-564
@Test
public void cleanupExpiredSessionsNoOverflow() {
long now = System.currentTimeMillis();
long toDelete = now - (new Long(Integer.MAX_VALUE) * 1000L);
this.repository.setDefaultMaxInactiveInterval(Integer.MAX_VALUE);
this.repository.cleanUpExpiredSessions();
ArgumentCaptor<Long> time = ArgumentCaptor.forClass(Long.class);
assertPropagationRequiresNew();
verify(this.jdbcOperations, times(1)).update(startsWith("DELETE"),
time.capture());
assertThat(time.getValue()).isCloseTo(toDelete, Offset.offset(5L));
}
private void assertPropagationRequiresNew() {
ArgumentCaptor<TransactionDefinition> argument =
ArgumentCaptor.forClass(TransactionDefinition.class);