diff --git a/spring-session/src/integration-test/java/org/springframework/session/jdbc/AbstractJdbcOperationsSessionRepositoryITests.java b/spring-session/src/integration-test/java/org/springframework/session/jdbc/AbstractJdbcOperationsSessionRepositoryITests.java index 4b9757bc..851e3297 100644 --- a/spring-session/src/integration-test/java/org/springframework/session/jdbc/AbstractJdbcOperationsSessionRepositoryITests.java +++ b/spring-session/src/integration-test/java/org/springframework/session/jdbc/AbstractJdbcOperationsSessionRepositoryITests.java @@ -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(); } diff --git a/spring-session/src/main/java/org/springframework/session/jdbc/JdbcOperationsSessionRepository.java b/spring-session/src/main/java/org/springframework/session/jdbc/JdbcOperationsSessionRepository.java index 129fc05a..a1d38dc4 100644 --- a/spring-session/src/main/java/org/springframework/session/jdbc/JdbcOperationsSessionRepository.java +++ b/spring-session/src/main/java/org/springframework/session/jdbc/JdbcOperationsSessionRepository.java @@ -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() { public Integer doInTransaction(TransactionStatus transactionStatus) { return JdbcOperationsSessionRepository.this.jdbcOperations.update( JdbcOperationsSessionRepository.this.deleteSessionsByLastAccessTimeQuery, - sessionsValidFromTime); + System.currentTimeMillis()); } }); diff --git a/spring-session/src/main/java/org/springframework/session/jdbc/config/annotation/web/http/JdbcHttpSessionConfiguration.java b/spring-session/src/main/java/org/springframework/session/jdbc/config/annotation/web/http/JdbcHttpSessionConfiguration.java index 2eda6dbe..ccda2442 100644 --- a/spring-session/src/main/java/org/springframework/session/jdbc/config/annotation/web/http/JdbcHttpSessionConfiguration.java +++ b/spring-session/src/main/java/org/springframework/session/jdbc/config/annotation/web/http/JdbcHttpSessionConfiguration.java @@ -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)) { diff --git a/spring-session/src/test/java/org/springframework/session/jdbc/JdbcOperationsSessionRepositoryTests.java b/spring-session/src/test/java/org/springframework/session/jdbc/JdbcOperationsSessionRepositoryTests.java index 4832fe9e..bfc8ff1d 100644 --- a/spring-session/src/test/java/org/springframework/session/jdbc/JdbcOperationsSessionRepositoryTests.java +++ b/spring-session/src/test/java/org/springframework/session/jdbc/JdbcOperationsSessionRepositoryTests.java @@ -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 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 argument = ArgumentCaptor.forClass(TransactionDefinition.class);