Release version 1.3.3.RELEASE

Previously, SessionResultSetExtractor used JdbcSession.setAttribute which had a side effect of freshly loaded session potentially having a non-empty delta and/or changed flag set. This commit optimizes session retrieval to invoke setAttribute directly on the delegate, therefore preventing unnecessary modifications of delta and change flags.

Closes gh-1053

.gitignore

@@ -10,5 +10,5 @@ target
 out
 .springBeans
 *.rdb
-!eclispe/.checkstyle
-.checkstyle
+.checkstyle
+!etc/eclipse/.checkstyle

etc/eclipse/org.eclipse.jdt.ui.prefs

@@ -59,7 +59,7 @@ cleanup_profile=_Spring Boot Cleanup Conventions
 cleanup_settings_version=2
 eclipse.preferences.version=1
 editor_save_participant_org.eclipse.jdt.ui.postsavelistener.cleanup=true
-formatter_profile=_Spring Boot Java Conventions
+formatter_profile=Spring Session Java Conventions
 formatter_settings_version=12
 org.eclipse.jdt.ui.exception.name=e
 org.eclipse.jdt.ui.gettersetter.use.is=true

gradle.properties

@@ -4,7 +4,7 @@ jacksonVersion=2.6.5
 jspApiVersion=2.0
 servletApiVersion=3.0.1
 jstlelVersion=1.2.5
-version=1.3.2.RELEASE
+version=1.3.3.RELEASE
 springDataRedisVersion=1.7.10.RELEASE
 html5ShivVersion=3.7.3
 commonsLoggingVersion=1.2

gradle/java.gradle

@@ -78,7 +78,7 @@ task integrationTest(type: Test, dependsOn: jar) {
 check.dependsOn integrationTest
 
 checkstyle {
-	configFile = rootProject.file('config/checkstyle/checkstyle.xml')
+	configFile = rootProject.file('etc/checkstyle/checkstyle.xml')
 	configProperties.configDir = configFile.parentFile
 	toolVersion = '6.16.1'
 }

spring-session/src/integration-test/java/org/springframework/session/jdbc/AbstractJdbcOperationsSessionRepositoryITests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2014-2016 the original author or authors.
+ * Copyright 2014-2018 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -34,10 +34,8 @@ import org.springframework.security.core.Authentication;
 import org.springframework.security.core.authority.AuthorityUtils;
 import org.springframework.security.core.context.SecurityContext;
 import org.springframework.security.core.context.SecurityContextHolder;
-import org.springframework.session.ExpiringSession;
 import org.springframework.session.FindByIndexNameSessionRepository;
 import org.springframework.session.MapSession;
-import org.springframework.session.Session;
 import org.springframework.session.jdbc.config.annotation.web.http.EnableJdbcHttpSession;
 import org.springframework.test.context.ContextConfiguration;
 import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
@@ -82,6 +80,19 @@ public abstract class AbstractJdbcOperationsSessionRepositoryITests {
 				AuthorityUtils.createAuthorityList("ROLE_USER")));
 	}
 
+	@Test
+	public void saveWhenNoAttributesThenCanBeFound() {
+		JdbcOperationsSessionRepository.JdbcSession toSave = this.repository
+				.createSession();
+
+		this.repository.save(toSave);
+		JdbcOperationsSessionRepository.JdbcSession session = this.repository.getSession(toSave.getId());
+
+		assertThat(session).isNotNull();
+		assertThat(session.isChanged()).isFalse();
+		assertThat(session.getDelta()).isEmpty();
+	}
+
 	@Test
 	public void saves() throws InterruptedException {
 		String username = "saves-" + System.currentTimeMillis();
@@ -100,9 +111,11 @@ public abstract class AbstractJdbcOperationsSessionRepositoryITests {
 
 		this.repository.save(toSave);
 
-		Session session = this.repository.getSession(toSave.getId());
+		JdbcOperationsSessionRepository.JdbcSession session = this.repository.getSession(toSave.getId());
 
 		assertThat(session.getId()).isEqualTo(toSave.getId());
+		assertThat(session.isChanged()).isFalse();
+		assertThat(session.getDelta()).isEmpty();
 		assertThat(session.getAttributeNames()).isEqualTo(toSave.getAttributeNames());
 		assertThat(session.getAttribute(expectedAttributeName))
 				.isEqualTo(toSave.getAttribute(expectedAttributeName));
@@ -135,7 +148,9 @@ public abstract class AbstractJdbcOperationsSessionRepositoryITests {
 		this.repository.save(toSave);
 		toSave = this.repository.getSession(toSave.getId());
 
-		Session session = this.repository.getSession(toSave.getId());
+		JdbcOperationsSessionRepository.JdbcSession session = this.repository.getSession(toSave.getId());
+		assertThat(session.isChanged()).isFalse();
+		assertThat(session.getDelta()).isEmpty();
 		assertThat(session.getAttributeNames().size()).isEqualTo(2);
 		assertThat(session.getAttribute("a")).isEqualTo("b");
 		assertThat(session.getAttribute("1")).isEqualTo("2");
@@ -156,9 +171,11 @@ public abstract class AbstractJdbcOperationsSessionRepositoryITests {
 		toSave.setLastAccessedTime(lastAccessedTime);
 		this.repository.save(toSave);
 
-		ExpiringSession session = this.repository.getSession(toSave.getId());
+		JdbcOperationsSessionRepository.JdbcSession session = this.repository.getSession(toSave.getId());
 
 		assertThat(session).isNotNull();
+		assertThat(session.isChanged()).isFalse();
+		assertThat(session.getDelta()).isEmpty();
 		assertThat(session.isExpired()).isFalse();
 		assertThat(session.getLastAccessedTime()).isEqualTo(lastAccessedTime);
 	}
@@ -225,6 +242,10 @@ public abstract class AbstractJdbcOperationsSessionRepositoryITests {
 
 		assertThat(findByPrincipalName).hasSize(1);
 		assertThat(findByPrincipalName.keySet()).containsOnly(toSave.getId());
+		for (JdbcOperationsSessionRepository.JdbcSession session : findByPrincipalName.values()) {
+			assertThat(session.isChanged()).isFalse();
+			assertThat(session.getDelta()).isEmpty();
+		}
 	}
 
 	@Test
@@ -247,6 +268,10 @@ public abstract class AbstractJdbcOperationsSessionRepositoryITests {
 
 		assertThat(findByPrincipalName).hasSize(1);
 		assertThat(findByPrincipalName.keySet()).containsOnly(toSave.getId());
+		for (JdbcOperationsSessionRepository.JdbcSession session : findByPrincipalName.values()) {
+			assertThat(session.isChanged()).isFalse();
+			assertThat(session.getDelta()).isEmpty();
+		}
 	}
 
 	@Test
@@ -289,6 +314,10 @@ public abstract class AbstractJdbcOperationsSessionRepositoryITests {
 
 		assertThat(findByPrincipalName).hasSize(1);
 		assertThat(findByPrincipalName.keySet()).containsOnly(toSave.getId());
+		for (JdbcOperationsSessionRepository.JdbcSession session : findByPrincipalName.values()) {
+			assertThat(session.isChanged()).isFalse();
+			assertThat(session.getDelta()).isEmpty();
+		}
 	}
 
 	@Test
@@ -336,6 +365,10 @@ public abstract class AbstractJdbcOperationsSessionRepositoryITests {
 
 		assertThat(findByPrincipalName).hasSize(1);
 		assertThat(findByPrincipalName.keySet()).containsOnly(toSave.getId());
+		for (JdbcOperationsSessionRepository.JdbcSession session : findByPrincipalName.values()) {
+			assertThat(session.isChanged()).isFalse();
+			assertThat(session.getDelta()).isEmpty();
+		}
 	}
 
 	@Test
@@ -395,6 +428,10 @@ public abstract class AbstractJdbcOperationsSessionRepositoryITests {
 
 		assertThat(findByPrincipalName).hasSize(1);
 		assertThat(findByPrincipalName.keySet()).containsOnly(toSave.getId());
+		for (JdbcOperationsSessionRepository.JdbcSession session : findByPrincipalName.values()) {
+			assertThat(session.isChanged()).isFalse();
+			assertThat(session.getDelta()).isEmpty();
+		}
 	}
 
 	@Test
@@ -416,6 +453,10 @@ public abstract class AbstractJdbcOperationsSessionRepositoryITests {
 
 		assertThat(findByPrincipalName).hasSize(1);
 		assertThat(findByPrincipalName.keySet()).containsOnly(toSave.getId());
+		for (JdbcOperationsSessionRepository.JdbcSession session : findByPrincipalName.values()) {
+			assertThat(session.isChanged()).isFalse();
+			assertThat(session.getDelta()).isEmpty();
+		}
 	}
 
 	@Test
@@ -455,6 +496,10 @@ public abstract class AbstractJdbcOperationsSessionRepositoryITests {
 
 		assertThat(findByPrincipalName).hasSize(1);
 		assertThat(findByPrincipalName.keySet()).containsOnly(toSave.getId());
+		for (JdbcOperationsSessionRepository.JdbcSession session : findByPrincipalName.values()) {
+			assertThat(session.isChanged()).isFalse();
+			assertThat(session.getDelta()).isEmpty();
+		}
 	}
 
 	@Test
@@ -499,6 +544,10 @@ public abstract class AbstractJdbcOperationsSessionRepositoryITests {
 
 		assertThat(findByPrincipalName).hasSize(1);
 		assertThat(findByPrincipalName.keySet()).containsOnly(toSave.getId());
+		for (JdbcOperationsSessionRepository.JdbcSession session : findByPrincipalName.values()) {
+			assertThat(session.isChanged()).isFalse();
+			assertThat(session.getDelta()).isEmpty();
+		}
 	}
 
 	@Test

spring-session/src/main/java/org/springframework/session/data/redis/RedisOperationsSessionRepository.java

@@ -521,6 +521,7 @@ public class RedisOperationsSessionRepository implements
 			if (session == null) {
 				logger.warn("Unable to publish SessionDestroyedEvent for session "
 						+ sessionId);
+				return;
 			}
 
 			if (logger.isDebugEnabled()) {
@@ -535,8 +536,6 @@ public class RedisOperationsSessionRepository implements
 			else {
 				handleExpired(session);
 			}
-
-			return;
 		}
 	}
 

spring-session/src/main/java/org/springframework/session/jdbc/JdbcOperationsSessionRepository.java

@@ -188,8 +188,7 @@ public class JdbcOperationsSessionRepository implements
 
 	private final TransactionOperations transactionOperations;
 
-	private final ResultSetExtractor<List<ExpiringSession>> extractor =
-			new ExpiringSessionResultSetExtractor();
+	private final ResultSetExtractor<List<JdbcSession>> extractor = new SessionResultSetExtractor();
 
 	/**
 	 * The name of database table used by Spring Session to store sessions.
@@ -484,10 +483,10 @@ public class JdbcOperationsSessionRepository implements
 	}
 
 	public JdbcSession getSession(final String id) {
-		final ExpiringSession session = this.transactionOperations.execute(new TransactionCallback<ExpiringSession>() {
+		final JdbcSession session = this.transactionOperations.execute(new TransactionCallback<JdbcSession>() {
 
-			public ExpiringSession doInTransaction(TransactionStatus status) {
-				List<ExpiringSession> sessions = JdbcOperationsSessionRepository.this.jdbcOperations.query(
+			public JdbcSession doInTransaction(TransactionStatus status) {
+				List<JdbcSession> sessions = JdbcOperationsSessionRepository.this.jdbcOperations.query(
 						JdbcOperationsSessionRepository.this.getSessionQuery,
 						new PreparedStatementSetter() {
 
@@ -511,7 +510,7 @@ public class JdbcOperationsSessionRepository implements
 				delete(id);
 			}
 			else {
-				return new JdbcSession(session);
+				return session;
 			}
 		}
 		return null;
@@ -534,9 +533,9 @@ public class JdbcOperationsSessionRepository implements
 			return Collections.emptyMap();
 		}
 
-		List<ExpiringSession> sessions = this.transactionOperations.execute(new TransactionCallback<List<ExpiringSession>>() {
+		List<JdbcSession> sessions = this.transactionOperations.execute(new TransactionCallback<List<JdbcSession>>() {
 
-			public List<ExpiringSession> doInTransaction(TransactionStatus status) {
+			public List<JdbcSession> doInTransaction(TransactionStatus status) {
 				return JdbcOperationsSessionRepository.this.jdbcOperations.query(
 						JdbcOperationsSessionRepository.this.listSessionsByPrincipalNameQuery,
 						new PreparedStatementSetter() {
@@ -555,8 +554,8 @@ public class JdbcOperationsSessionRepository implements
 		Map<String, JdbcSession> sessionMap = new HashMap<String, JdbcSession>(
 				sessions.size());
 
-		for (ExpiringSession session : sessions) {
-			sessionMap.put(session.getId(), new JdbcSession(session));
+		for (JdbcSession session : sessions) {
+			sessionMap.put(session.getId(), session);
 		}
 
 		return sessionMap;
@@ -764,33 +763,33 @@ public class JdbcOperationsSessionRepository implements
 
 	}
 
-	private class ExpiringSessionResultSetExtractor
-			implements ResultSetExtractor<List<ExpiringSession>> {
+	private class SessionResultSetExtractor implements ResultSetExtractor<List<JdbcSession>> {
 
-		public List<ExpiringSession> extractData(ResultSet rs) throws SQLException, DataAccessException {
-			List<ExpiringSession> sessions = new ArrayList<ExpiringSession>();
+		public List<JdbcSession> extractData(ResultSet rs) throws SQLException, DataAccessException {
+			List<JdbcSession> sessions = new ArrayList<JdbcSession>();
 			while (rs.next()) {
 				String id = rs.getString("SESSION_ID");
-				MapSession session;
+				JdbcSession session;
 				if (sessions.size() > 0 && getLast(sessions).getId().equals(id)) {
-					session = (MapSession) getLast(sessions);
+					session = getLast(sessions);
 				}
 				else {
-					session = new MapSession(id);
-					session.setCreationTime(rs.getLong("CREATION_TIME"));
-					session.setLastAccessedTime(rs.getLong("LAST_ACCESS_TIME"));
-					session.setMaxInactiveIntervalInSeconds(rs.getInt("MAX_INACTIVE_INTERVAL"));
+					MapSession delegate = new MapSession(id);
+					delegate.setCreationTime(rs.getLong("CREATION_TIME"));
+					delegate.setLastAccessedTime(rs.getLong("LAST_ACCESS_TIME"));
+					delegate.setMaxInactiveIntervalInSeconds(rs.getInt("MAX_INACTIVE_INTERVAL"));
+					session = new JdbcSession(delegate);
 				}
 				String attributeName = rs.getString("ATTRIBUTE_NAME");
 				if (attributeName != null) {
-					session.setAttribute(attributeName, deserialize(rs, "ATTRIBUTE_BYTES"));
+					session.delegate.setAttribute(attributeName, deserialize(rs, "ATTRIBUTE_BYTES"));
 				}
 				sessions.add(session);
 			}
 			return sessions;
 		}
 
-		private ExpiringSession getLast(List<ExpiringSession> sessions) {
+		private JdbcSession getLast(List<JdbcSession> sessions) {
 			return sessions.get(sessions.size() - 1);
 		}
 

spring-session/src/test/java/org/springframework/session/data/redis/RedisOperationsSessionRepositoryTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2014-2016 the original author or authors.
+ * Copyright 2014-2018 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -59,6 +59,7 @@ import static org.mockito.BDDMockito.given;
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyLong;
 import static org.mockito.Matchers.anyString;
+import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.atLeastOnce;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.reset;
@@ -529,6 +530,104 @@ public class RedisOperationsSessionRepositoryTests {
 		verify(this.defaultSerializer).deserialize(body);
 	}
 
+	@Test
+	public void onMessageDeletedSessionFound() throws Exception {
+		String deletedId = "deleted-id";
+		given(this.redisOperations.boundHashOps(getKey(deletedId)))
+				.willReturn(this.boundHashOperations);
+		Map map = map(RedisOperationsSessionRepository.MAX_INACTIVE_ATTR, 0,
+				RedisOperationsSessionRepository.LAST_ACCESSED_ATTR,
+				System.currentTimeMillis() - TimeUnit.MINUTES.toMillis(5));
+		given(this.boundHashOperations.entries()).willReturn(map);
+
+		String channel = "__keyevent@0__:del";
+		String body = "spring:session:sessions:expires:" + deletedId;
+		DefaultMessage message = new DefaultMessage(channel.getBytes("UTF-8"), body.getBytes("UTF-8"));
+
+		this.redisRepository.setApplicationEventPublisher(this.publisher);
+		this.redisRepository.onMessage(message, "".getBytes("UTF-8"));
+
+		verify(this.redisOperations).boundHashOps(eq(getKey(deletedId)));
+		verify(this.boundHashOperations).entries();
+		verify(this.publisher).publishEvent(this.event.capture());
+		assertThat(this.event.getValue().getSessionId()).isEqualTo(deletedId);
+		verifyZeroInteractions(this.defaultSerializer);
+		verifyZeroInteractions(this.publisher);
+		verifyZeroInteractions(this.redisOperations);
+		verifyZeroInteractions(this.boundHashOperations);
+	}
+
+	@Test
+	public void onMessageDeletedSessionNotFound() throws Exception {
+		String deletedId = "deleted-id";
+		given(this.redisOperations.boundHashOps(getKey(deletedId)))
+				.willReturn(this.boundHashOperations);
+		given(this.boundHashOperations.entries()).willReturn(map());
+
+		String channel = "__keyevent@0__:del";
+		String body = "spring:session:sessions:expires:" + deletedId;
+		DefaultMessage message = new DefaultMessage(channel.getBytes("UTF-8"), body.getBytes("UTF-8"));
+
+		this.redisRepository.setApplicationEventPublisher(this.publisher);
+		this.redisRepository.onMessage(message, "".getBytes("UTF-8"));
+
+		verify(this.redisOperations).boundHashOps(eq(getKey(deletedId)));
+		verify(this.boundHashOperations).entries();
+		verifyZeroInteractions(this.defaultSerializer);
+		verifyZeroInteractions(this.publisher);
+		verifyZeroInteractions(this.redisOperations);
+		verifyZeroInteractions(this.boundHashOperations);
+	}
+
+	@Test
+	public void onMessageExpiredSessionFound() throws Exception {
+		String expiredId = "expired-id";
+		given(this.redisOperations.boundHashOps(getKey(expiredId)))
+				.willReturn(this.boundHashOperations);
+		Map map = map(RedisOperationsSessionRepository.MAX_INACTIVE_ATTR, 1,
+				RedisOperationsSessionRepository.LAST_ACCESSED_ATTR,
+				System.currentTimeMillis() - TimeUnit.MINUTES.toMillis(5));
+		given(this.boundHashOperations.entries()).willReturn(map);
+
+		String channel = "__keyevent@0__:expired";
+		String body = "spring:session:sessions:expires:" + expiredId;
+		DefaultMessage message = new DefaultMessage(channel.getBytes("UTF-8"), body.getBytes("UTF-8"));
+
+		this.redisRepository.setApplicationEventPublisher(this.publisher);
+		this.redisRepository.onMessage(message, "".getBytes("UTF-8"));
+
+		verify(this.redisOperations).boundHashOps(eq(getKey(expiredId)));
+		verify(this.boundHashOperations).entries();
+		verify(this.publisher).publishEvent(this.event.capture());
+		assertThat(this.event.getValue().getSessionId()).isEqualTo(expiredId);
+		verifyZeroInteractions(this.defaultSerializer);
+		verifyZeroInteractions(this.publisher);
+		verifyZeroInteractions(this.redisOperations);
+		verifyZeroInteractions(this.boundHashOperations);
+	}
+
+	@Test
+	public void onMessageExpiredSessionNotFound() throws Exception {
+		String expiredId = "expired-id";
+		given(this.redisOperations.boundHashOps(getKey(expiredId)))
+				.willReturn(this.boundHashOperations);
+		given(this.boundHashOperations.entries()).willReturn(map());
+
+		String channel = "__keyevent@0__:expired";
+		String body = "spring:session:sessions:expires:" + expiredId;
+		DefaultMessage message = new DefaultMessage(channel.getBytes("UTF-8"), body.getBytes("UTF-8"));
+
+		this.redisRepository.setApplicationEventPublisher(this.publisher);
+		this.redisRepository.onMessage(message, "".getBytes("UTF-8"));
+
+		verify(this.redisOperations).boundHashOps(eq(getKey(expiredId)));
+		verify(this.boundHashOperations).entries();
+		verifyZeroInteractions(this.defaultSerializer);
+		verifyZeroInteractions(this.publisher);
+		verifyZeroInteractions(this.redisOperations);
+		verifyZeroInteractions(this.boundHashOperations);
+	}
+
 	@Test
 	public void resolvePrincipalIndex() {
 		PrincipalNameResolver resolver = RedisOperationsSessionRepository.PRINCIPAL_NAME_RESOLVER;

spring-session/src/test/java/org/springframework/session/jdbc/JdbcOperationsSessionRepositoryTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2014-2016 the original author or authors.
+ * Copyright 2014-2018 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -412,7 +412,7 @@ public class JdbcOperationsSessionRepositoryTests {
 
 	@Test
 	public void getSessionExpired() {
-		MapSession expired = new MapSession();
+		JdbcOperationsSessionRepository.JdbcSession expired = this.repository.new JdbcSession();
 		expired.setLastAccessedTime(System.currentTimeMillis() -
 				(MapSession.DEFAULT_MAX_INACTIVE_INTERVAL_SECONDS * 1000 + 1000));
 		given(this.jdbcOperations.query(isA(String.class),
@@ -432,7 +432,8 @@ public class JdbcOperationsSessionRepositoryTests {
 
 	@Test
 	public void getSessionFound() {
-		MapSession saved = new MapSession();
+		JdbcOperationsSessionRepository.JdbcSession saved = this.repository.new JdbcSession(
+				new MapSession());
 		saved.setAttribute("savedName", "savedValue");
 		given(this.jdbcOperations.query(isA(String.class),
 				isA(PreparedStatementSetter.class), isA(ResultSetExtractor.class)))
@@ -493,11 +494,12 @@ public class JdbcOperationsSessionRepositoryTests {
 		String principal = "username";
 		Authentication authentication = new UsernamePasswordAuthenticationToken(principal,
 				"notused", AuthorityUtils.createAuthorityList("ROLE_USER"));
-		List<MapSession> saved = new ArrayList<MapSession>(2);
-		MapSession saved1 = new MapSession();
+		List<JdbcOperationsSessionRepository.JdbcSession> saved =
+				new ArrayList<JdbcOperationsSessionRepository.JdbcSession>(2);
+		JdbcOperationsSessionRepository.JdbcSession saved1 = this.repository.new JdbcSession();
 		saved1.setAttribute(SPRING_SECURITY_CONTEXT, authentication);
 		saved.add(saved1);
-		MapSession saved2 = new MapSession();
+		JdbcOperationsSessionRepository.JdbcSession saved2 = this.repository.new JdbcSession();
 		saved2.setAttribute(SPRING_SECURITY_CONTEXT, authentication);
 		saved.add(saved2);
 		given(this.jdbcOperations.query(isA(String.class),